From d4a6a31cbd07d15e02b9c93de9b4ed04190000e4 Mon Sep 17 00:00:00 2001 From: Xabier Arbulu Insausti Date: Mon, 23 Sep 2024 14:43:40 +0200 Subject: [PATCH] SAML runtime operations (#2999) * Add certificates settings in settings sti table * Add new /api/public_keys route to get uploaded keys * Add release task to initialize saml * Add saml runtime options * Update variable name * Rename certs settings to add SSO prefix to make it obvious * Add tests to the new certificates code * Add saml release task tests --- config/runtime.exs | 60 +++++++++- lib/trento/release.ex | 105 ++++++++++++++++++ lib/trento/settings.ex | 8 ++ .../settings/sso_certificates_settings.ex | 36 ++++++ .../controllers/v1/settings_controller.ex | 14 +++ lib/trento_web/openapi/v1/schema/platform.ex | 21 ++++ lib/trento_web/router.ex | 5 + lib/trento_web/views/v1/settings_view.ex | 8 ++ ...1940_add_sso_certificates_settings_sti.exs | 13 +++ test/support/factory.ex | 15 ++- test/trento/release_test.exs | 78 ++++++++++++- test/trento/settings_test.exs | 8 ++ .../v1/settings_controller_test.exs | 21 ++++ 13 files changed, 384 insertions(+), 8 deletions(-) create mode 100644 lib/trento/settings/sso_certificates_settings.ex create mode 100644 priv/repo/migrations/20240918151940_add_sso_certificates_settings_sti.exs diff --git a/config/runtime.exs b/config/runtime.exs index c14bc8f71b..c36cf6c6cf 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -177,9 +177,12 @@ if config_env() in [:prod, :demo] do enable_oidc = System.get_env("ENABLE_OIDC", "false") == "true" enable_oauth2 = System.get_env("ENABLE_OAUTH2", "false") == "true" + enable_saml = System.get_env("ENABLE_SAML", "false") == "true" - if enable_oauth2 and enable_oidc do - raise("Cannot start Trento with OIDC and OAUTH2 integrations both enabled.") + if Enum.count([enable_oidc, enable_oauth2, enable_saml], fn enabled -> enabled end) > 1 do + raise( + "Cannot start Trento with multiple SSO options enabled. Use one among: OIDC, OAUTH2 and SAML." + ) end config :trento, :oidc, @@ -247,4 +250,57 @@ if config_env() in [:prod, :demo] do ] ] end + + if enable_saml do + saml_dir = System.get_env("SAML_SP_DIR", "/etc/trento/trento-web/saml") + + config :trento, :saml, + enabled: true, + callback_url: "/auth/saml_callback", + idp_id: + System.get_env("SAML_IDP_ID") || + raise("environment variable SAML_IDP_ID is missing") + + config :trento, :pow_assent, + providers: [ + saml_local: [ + strategy: TrentoWeb.Auth.AssentSamlStrategy + ] + ] + + config :samly, Samly.Provider, + idp_id_from: :path_segment, + service_providers: [ + %{ + id: + System.get_env("SAML_SP_ID") || + raise("environment variable SAML_SP_ID is missing"), + entity_id: System.get_env("SAML_SP_ENTITY_ID", ""), + certfile: Path.join([saml_dir, "cert", "saml.pem"]), + keyfile: Path.join([saml_dir, "cert", "saml_key.pem"]), + contact_name: System.get_env("SAML_SP_CONTACT_NAME", "Trento SP Admin"), + contact_email: System.get_env("SAML_SP_CONTACT_EMAIL", "admin@trento.suse.com"), + org_name: System.get_env("SAML_SP_ORG_NAME", "Trento SP"), + org_displayname: System.get_env("SAML_SP_ORG_DISPLAYNAME", "SAML SP build with Trento"), + org_url: System.get_env("SAML_SP_ORG_URL", "https://www.trento-project.io/") + } + ], + identity_providers: [ + %{ + id: System.get_env("SAML_IDP_ID"), + sp_id: System.get_env("SAML_SP_ID"), + base_url: "https://#{System.get_env("TRENTO_WEB_ORIGIN")}/sso", + metadata_file: Path.join([saml_dir, "metadata.xml"]), + sign_requests: System.get_env("SAML_SIGN_REQUESTS", "true") == "true", + sign_metadata: System.get_env("SAML_SIGN_METADATA", "true") == "true", + signed_assertion_in_resp: System.get_env("SAML_SIGNED_ASSERTION", "true") == "true", + signed_envelopes_in_resp: System.get_env("SAML_SIGNED_ENVELOPES", "true") == "true", + nameid_format: + System.get_env( + "SAML_IDP_NAMEID_FORMAT", + "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" + ) + } + ] + end end diff --git a/lib/trento/release.ex b/lib/trento/release.ex index b0d682c8da..5db70ff981 100644 --- a/lib/trento/release.ex +++ b/lib/trento/release.ex @@ -4,11 +4,18 @@ defmodule Trento.Release do installed. """ + require Logger + + alias Mix.Tasks.Phx.Gen.Cert + alias Trento.ActivityLog.Settings, as: ActivityLogSettings alias Trento.Settings.ApiKeySettings + alias Trento.Settings.SSOCertificatesSettings @app :trento + @saml_certificate_name "Trento SAML SP" + def init do migrate() init_event_store() @@ -16,6 +23,7 @@ defmodule Trento.Release do init_admin_user() init_default_api_key() init_default_activity_log_retention_time() + maybe_init_saml(System.get_env("ENABLE_SAML", "false") == "true") end def migrate do @@ -123,6 +131,70 @@ defmodule Trento.Release do ) end + def maybe_init_saml(false), do: :ok + + def maybe_init_saml(true) do + load_app() + Enum.each([:postgrex, :ecto, :httpoison], &Application.ensure_all_started/1) + Trento.Repo.start_link() + Trento.Vault.start_link() + + trento_origin = + System.get_env("TRENTO_WEB_ORIGIN") || + raise """ + environment variable TRENTO_WEB_ORIGIN is missing. + For example: yourdomain.example.com + """ + + saml_dir = System.get_env("SAML_SP_DIR", "/etc/trento/trento-web/saml") + + certificates_settings = + Trento.Repo.one(SSOCertificatesSettings.base_query()) + + {key_file, cert_file} = + case certificates_settings do + nil -> + {key, cert} = + create_certificates_content( + @saml_certificate_name, + [trento_origin] + ) + + %SSOCertificatesSettings{} + |> SSOCertificatesSettings.changeset(%{ + name: @saml_certificate_name, + key_file: key, + certificate_file: cert + }) + |> Trento.Repo.insert!() + + {key, cert} + + %SSOCertificatesSettings{key_file: key, certificate_file: cert} -> + {key, cert} + end + + File.mkdir_p!(Path.join([saml_dir, "cert"])) + File.write!(Path.join([saml_dir, "cert", "saml_key.pem"]), key_file) + File.write!(Path.join([saml_dir, "cert", "saml.pem"]), cert_file) + + Logger.info(IO.ANSI.green() <> "Created certificate content:\n\n#{cert_file}") + + # Create metadata.xml file + case get_saml_metadata_file(System.get_env()) do + {:ok, content} -> + File.write!(Path.join([saml_dir, "metadata.xml"]), content) + + {:error, :request_failure} -> + raise "Error querying the provided SAML_METADATA_URL endpoint" + + {:error, :metadata_is_missing} -> + raise "One of SAML_METADATA_URL or SAML_METADATA_CONTENT must be provided" + end + + :ok + end + defp repos do Application.fetch_env!(@app, :ecto_repos) end @@ -130,4 +202,37 @@ defmodule Trento.Release do defp load_app do Application.load(@app) end + + defp create_certificates_content(name, hostnames) do + {certificate, private_key} = Cert.certificate_and_key(2048, name, hostnames) + + keyfile_content = + :public_key.pem_encode([:public_key.pem_entry_encode(:RSAPrivateKey, private_key)]) + + certfile_content = :public_key.pem_encode([{:Certificate, certificate, :not_encrypted}]) + + {keyfile_content, certfile_content} + end + + defp get_saml_metadata_file(%{"SAML_METADATA_URL" => metadata_url}) when metadata_url != "" do + case HTTPoison.get(metadata_url) do + {:ok, %HTTPoison.Response{status_code: 200, body: body}} -> + {:ok, body} + + {:ok, _} -> + {:error, :request_failure} + + {:error, _} -> + {:error, :request_failure} + end + end + + defp get_saml_metadata_file(%{"SAML_METADATA_CONTENT" => metadata_content}) + when metadata_content != "" do + {:ok, metadata_content} + end + + defp get_saml_metadata_file(_) do + {:error, :metadata_is_missing} + end end diff --git a/lib/trento/settings.ex b/lib/trento/settings.ex index 0c36b1548a..914d2b15cf 100644 --- a/lib/trento/settings.ex +++ b/lib/trento/settings.ex @@ -13,6 +13,7 @@ defmodule Trento.Settings do alias Trento.Settings.{ ApiKeySettings, InstallationSettings, + SSOCertificatesSettings, SuseManagerSettings } @@ -144,6 +145,13 @@ defmodule Trento.Settings do :ok end + # Certificates settings + + @spec get_sso_certificates() :: [SSOCertificatesSettings.t()] + def get_sso_certificates do + Repo.one(SSOCertificatesSettings.base_query()) + end + defp ensure_no_suse_manager_settings_configured do case Repo.one(SuseManagerSettings.base_query()) do nil -> diff --git a/lib/trento/settings/sso_certificates_settings.ex b/lib/trento/settings/sso_certificates_settings.ex new file mode 100644 index 0000000000..4123923ef3 --- /dev/null +++ b/lib/trento/settings/sso_certificates_settings.ex @@ -0,0 +1,36 @@ +defmodule Trento.Settings.SSOCertificatesSettings do + @moduledoc """ + SSOCertificatesSettings is the STI projection containing SSL certificates + """ + + use Ecto.Schema + use Trento.Support.Ecto.STI, sti_identifier: :sso_certificates_settings + + import Ecto.Changeset + + alias Trento.Support.Ecto.EncryptedBinary + + @type t :: %__MODULE__{} + + @derive {Jason.Encoder, except: [:__meta__, :__struct__]} + @primary_key {:id, :binary_id, autogenerate: true} + schema "settings" do + field :name, :string, source: :sso_certificates_settings_name + field :key_file, EncryptedBinary, source: :sso_certificates_settings_key_file + field :certificate_file, EncryptedBinary, source: :sso_certificates_settings_certificate_file + + timestamps(type: :utc_datetime_usec) + sti_fields() + end + + @spec changeset(t() | Ecto.Changeset.t(), map) :: Ecto.Changeset.t() + def changeset(certificates_settings, attrs) do + certificates_settings + |> cast(attrs, __MODULE__.__schema__(:fields)) + |> validate_required([:name, :key_file, :certificate_file]) + # TODO: move suse_manager_settings.ex certificates function to some support module + # |> validate_cert_and_key + |> sti_changes() + |> unique_constraint(:type) + end +end diff --git a/lib/trento_web/controllers/v1/settings_controller.ex b/lib/trento_web/controllers/v1/settings_controller.ex index ee09ba846d..a5a09b1950 100644 --- a/lib/trento_web/controllers/v1/settings_controller.ex +++ b/lib/trento_web/controllers/v1/settings_controller.ex @@ -249,6 +249,20 @@ defmodule TrentoWeb.V1.SettingsController do end end + operation :get_public_keys, + summary: "Get uploaded public keys", + tags: ["Platform"], + description: "Get uploaded public keys", + responses: [ + ok: {"Uploaded public keys", "application/json", Schema.Platform.PublicKeys} + ] + + @spec get_public_keys(Plug.Conn.t(), any) :: Plug.Conn.t() + def get_public_keys(conn, _) do + certificates = Settings.get_sso_certificates() + render(conn, "public_keys.json", %{public_keys: [certificates]}) + end + def get_policy_resource(conn) do case Phoenix.Controller.action_name(conn) do :update_api_key_settings -> Trento.Settings.ApiKeySettings diff --git a/lib/trento_web/openapi/v1/schema/platform.ex b/lib/trento_web/openapi/v1/schema/platform.ex index 6bb4234af4..8bddcdfc22 100644 --- a/lib/trento_web/openapi/v1/schema/platform.ex +++ b/lib/trento_web/openapi/v1/schema/platform.ex @@ -248,4 +248,25 @@ defmodule TrentoWeb.OpenApi.V1.Schema.Platform do struct?: false ) end + + defmodule PublicKeys do + @moduledoc false + + OpenApiSpex.schema( + %{ + title: "PublicKeys", + description: "Uploaded public keys", + type: :array, + items: %Schema{ + title: "PublicKey", + type: :object, + properties: %{ + name: %Schema{type: :string, description: "Name"}, + content: %Schema{type: :string, description: "Public key content"} + } + } + }, + struct?: false + ) + end end diff --git a/lib/trento_web/router.ex b/lib/trento_web/router.ex index 0a4c5ba7cf..c5c6712ffe 100644 --- a/lib/trento_web/router.ex +++ b/lib/trento_web/router.ex @@ -85,6 +85,11 @@ defmodule TrentoWeb.Router do get "/readyz", HealthController, :ready end + scope "/api", TrentoWeb.V1 do + pipe_through [:api, :api_v1] + get "/public_keys", SettingsController, :get_public_keys + end + scope "/api" do pipe_through [:api, :protected_api] diff --git a/lib/trento_web/views/v1/settings_view.ex b/lib/trento_web/views/v1/settings_view.ex index a1403c7305..17237c3640 100644 --- a/lib/trento_web/views/v1/settings_view.ex +++ b/lib/trento_web/views/v1/settings_view.ex @@ -52,4 +52,12 @@ defmodule TrentoWeb.V1.SettingsView do ca_uploaded_at: ca_uploaded_at } end + + def render("public_keys.json", %{public_keys: public_keys}) do + render_many(public_keys, __MODULE__, "public_key.json", as: :public_key) + end + + def render("public_key.json", %{public_key: %{name: name, certificate_file: cert_file}}) do + %{name: name, content: cert_file} + end end diff --git a/priv/repo/migrations/20240918151940_add_sso_certificates_settings_sti.exs b/priv/repo/migrations/20240918151940_add_sso_certificates_settings_sti.exs new file mode 100644 index 0000000000..1372b0d5ad --- /dev/null +++ b/priv/repo/migrations/20240918151940_add_sso_certificates_settings_sti.exs @@ -0,0 +1,13 @@ +defmodule Trento.Repo.Migrations.AddSSOCertificatesSettingsSti do + use Ecto.Migration + + def change do + alter table(:settings) do + add :sso_certificates_settings_name, :string + add :sso_certificates_settings_key_file, :binary + add :sso_certificates_settings_certificate_file, :binary + end + + create unique_index(:settings, [:sso_certificates_settings_name]) + end +end diff --git a/test/support/factory.ex b/test/support/factory.ex index 5c77638311..cee4cc1b55 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -128,11 +128,11 @@ defmodule Trento.Factory do DiscoveryEvent } - alias Trento.Settings.SuseManagerSettings - alias Trento.Settings.{ ApiKeySettings, - InstallationSettings + InstallationSettings, + SSOCertificatesSettings, + SuseManagerSettings } alias Trento.ActivityLog.ActivityLog, as: ActivityLogEntry @@ -967,6 +967,15 @@ defmodule Trento.Factory do } end + def sso_certificates_settings_factory do + %SSOCertificatesSettings{ + type: :sso_certificates_settings, + name: Faker.StarWars.planet(), + certificate_file: Faker.Lorem.paragraph(), + key_file: Faker.Lorem.paragraph() + } + end + def insert_software_updates_settings(attrs \\ []) do insert( :software_updates_settings, diff --git a/test/trento/release_test.exs b/test/trento/release_test.exs index ae763196eb..5fbdc79ef9 100644 --- a/test/trento/release_test.exs +++ b/test/trento/release_test.exs @@ -1,18 +1,21 @@ defmodule Trento.ReleaseTest do @moduledoc false - use ExUnit.Case + use ExUnit.Case, async: false use Trento.DataCase import Trento.Factory require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit + alias Trento.Release + alias Trento.ActivityLog.RetentionTime alias Trento.ActivityLog.Settings, as: ActivityLogSettings + alias Trento.Settings.SSOCertificatesSettings describe "Activity Log settings initiation" do test "should init default activity log retention time" do - Trento.Release.init_default_activity_log_retention_time() + Release.init_default_activity_log_retention_time() assert %ActivityLogSettings{ retention_time: %RetentionTime{ @@ -33,7 +36,7 @@ defmodule Trento.ReleaseTest do } ) - Trento.Release.init_default_activity_log_retention_time() + Release.init_default_activity_log_retention_time() assert %ActivityLogSettings{ retention_time: %RetentionTime{ @@ -43,4 +46,73 @@ defmodule Trento.ReleaseTest do } = Trento.Repo.one(ActivityLogSettings.base_query()) end end + + describe "SAML initialization" do + setup do + temp_dir = Path.join([System.tmp_dir!(), ".trento_test"]) + File.mkdir_p!(temp_dir) + previous_env = System.get_env() + + on_exit(fn -> + System.put_env(previous_env) + File.rm_rf!(temp_dir) + end) + + {:ok, temp_dir: temp_dir} + end + + test "should not initialize SAML if it is not enabled" do + assert :ok = Release.maybe_init_saml(false) + end + + test "should fail if TRENTO_WEB_ORIGIN variable is not set" do + System.delete_env("TRENTO_WEB_ORIGIN") + + assert_raise RuntimeError, + """ + environment variable TRENTO_WEB_ORIGIN is missing. + For example: yourdomain.example.com + """, + fn -> Release.maybe_init_saml(true) end + end + + test "should create and store SSO certificates if they are not present", %{temp_dir: temp_dir} do + System.put_env("TRENTO_WEB_ORIGIN", "localhost") + System.put_env("SAML_SP_DIR", temp_dir) + System.put_env("SAML_METADATA_CONTENT", "some xml") + + assert :ok = Release.maybe_init_saml(true) + + %{key_file: key, certificate_file: cert} = + Trento.Repo.one(SSOCertificatesSettings.base_query()) + + assert cert == File.read!(Path.join([temp_dir, "cert", "saml.pem"])) + assert key == File.read!(Path.join([temp_dir, "cert", "saml_key.pem"])) + assert "some xml" == File.read!(Path.join([temp_dir, "metadata.xml"])) + end + + test "should reuse existing SSO certificates if they are present", %{temp_dir: temp_dir} do + System.put_env("TRENTO_WEB_ORIGIN", "localhost") + System.put_env("SAML_SP_DIR", temp_dir) + System.put_env("SAML_METADATA_CONTENT", "some xml") + + %{key_file: key, certificate_file: cert} = insert(:sso_certificates_settings) + + assert :ok = Release.maybe_init_saml(true) + + assert cert == File.read!(Path.join([temp_dir, "cert", "saml.pem"])) + assert key == File.read!(Path.join([temp_dir, "cert", "saml_key.pem"])) + assert "some xml" == File.read!(Path.join([temp_dir, "metadata.xml"])) + end + + test "should fail if none of metadata obtaining options are used", %{temp_dir: temp_dir} do + System.put_env("TRENTO_WEB_ORIGIN", "localhost") + System.put_env("SAML_SP_DIR", temp_dir) + System.delete_env("SAML_METADATA_CONTENT") + + assert_raise RuntimeError, + "One of SAML_METADATA_URL or SAML_METADATA_CONTENT must be provided", + fn -> Release.maybe_init_saml(true) end + end + end end diff --git a/test/trento/settings_test.exs b/test/trento/settings_test.exs index 5b6c23ed88..73feac7013 100644 --- a/test/trento/settings_test.exs +++ b/test/trento/settings_test.exs @@ -626,4 +626,12 @@ defmodule Trento.SettingsTest do end) end end + + describe "sso_certificates_settings" do + test "should return SSO certificates" do + certificates = insert(:sso_certificates_settings) + + assert certificates == Settings.get_sso_certificates() + end + end end diff --git a/test/trento_web/controllers/v1/settings_controller_test.exs b/test/trento_web/controllers/v1/settings_controller_test.exs index 33f9074c60..4a82d45d28 100644 --- a/test/trento_web/controllers/v1/settings_controller_test.exs +++ b/test/trento_web/controllers/v1/settings_controller_test.exs @@ -637,6 +637,27 @@ defmodule TrentoWeb.V1.SettingsControllerTest do end end + describe "SSOCertificatesSettings" do + setup do + %{api_spec: ApiSpec.spec()} + end + + test "should return uploaded certificates public content in the public_keys route", %{ + conn: conn, + api_spec: api_spec + } do + %{name: name, certificate_file: cert} = insert(:sso_certificates_settings) + + resp = + conn + |> get("/api/public_keys") + |> json_response(200) + |> assert_schema("PublicKeys", api_spec) + + assert [%{name: name, content: cert}] == resp + end + end + describe "forbidden response" do test "should return forbidden if the user does not have the permission to update the api key", %{conn: conn, api_spec: api_spec} do