From 1939b9c3f95e36e7f31fa694138706c409f550b0 Mon Sep 17 00:00:00 2001 From: Brian Manifold Date: Wed, 14 Feb 2024 13:42:39 -0500 Subject: [PATCH] Update Okta IDP adapter in portal (#3647) Why: * After reviewing the Okta docs closer, in order for an OAuth token to have Okta API scopes attached to it, the Okta org authorization server must be used, not a custom authorization server (which includes the 'default' authorization server). This means that the OAuth Authorization URI that was previously being asked for in the Okta Adapter form won't work for IDP sync to Firezone. This commit updates the form to accept the Okta Account Domain (i.e. `.okta.com`) --- .../lib/domain/auth/adapters/okta/settings.ex | 2 +- .../auth/adapters/okta/settings/changeset.ex | 2 +- .../test/domain/auth/adapters/okta_test.exs | 8 ++-- .../apps/domain/test/support/fixtures/auth.ex | 8 ++-- .../identity_providers/okta/components.ex | 12 +++--- .../settings/identity_providers/okta/edit.ex | 37 ++++++++++++------- .../settings/identity_providers/okta/new.ex | 37 ++++++++++++------- .../identity_providers/okta/edit_test.exs | 20 ++++------ .../identity_providers/okta/new_test.exs | 13 +++---- 9 files changed, 78 insertions(+), 61 deletions(-) diff --git a/elixir/apps/domain/lib/domain/auth/adapters/okta/settings.ex b/elixir/apps/domain/lib/domain/auth/adapters/okta/settings.ex index 4b9268e78..f8bd716aa 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/okta/settings.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/okta/settings.ex @@ -15,7 +15,7 @@ defmodule Domain.Auth.Adapters.Okta.Settings do field :client_id, :string field :client_secret, :string field :discovery_document_uri, :string - field :oauth_uri, :string + field :okta_account_domain, :string field :api_base_url, :string end diff --git a/elixir/apps/domain/lib/domain/auth/adapters/okta/settings/changeset.ex b/elixir/apps/domain/lib/domain/auth/adapters/okta/settings/changeset.ex index 664421bca..43dcb71e6 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/okta/settings/changeset.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/okta/settings/changeset.ex @@ -7,7 +7,7 @@ defmodule Domain.Auth.Adapters.Okta.Settings.Changeset do response_type client_id client_secret discovery_document_uri - oauth_uri + okta_account_domain api_base_url]a def changeset(%Settings{} = settings, attrs) do diff --git a/elixir/apps/domain/test/domain/auth/adapters/okta_test.exs b/elixir/apps/domain/test/domain/auth/adapters/okta_test.exs index bd86a58a7..4b2787d88 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/okta_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/okta_test.exs @@ -58,7 +58,7 @@ defmodule Domain.Auth.Adapters.OktaTest do client_secret: ["can't be blank"], discovery_document_uri: ["can't be blank"], api_base_url: ["can't be blank"], - oauth_uri: ["can't be blank"] + okta_account_domain: ["can't be blank"] } } end @@ -67,7 +67,7 @@ defmodule Domain.Auth.Adapters.OktaTest do account = Fixtures.Accounts.create_account() bypass = Domain.Mocks.OpenIDConnect.discovery_document_server() discovery_document_url = "http://localhost:#{bypass.port}/.well-known/openid-configuration" - oauth_url = "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server" + okta_account_domain = "http://localhost:#{bypass.port}" api_base_url = "http://localhost:#{bypass.port}" attrs = @@ -77,7 +77,7 @@ defmodule Domain.Auth.Adapters.OktaTest do client_id: "client_id", client_secret: "client_secret", discovery_document_uri: discovery_document_url, - oauth_uri: oauth_url, + okta_account_domain: okta_account_domain, api_base_url: api_base_url } ) @@ -107,7 +107,7 @@ defmodule Domain.Auth.Adapters.OktaTest do "client_id" => "client_id", "client_secret" => "client_secret", "discovery_document_uri" => discovery_document_url, - "oauth_uri" => oauth_url, + "okta_account_domain" => okta_account_domain, "api_base_url" => api_base_url } end diff --git a/elixir/apps/domain/test/support/fixtures/auth.ex b/elixir/apps/domain/test/support/fixtures/auth.ex index 240e5389b..53859e45f 100644 --- a/elixir/apps/domain/test/support/fixtures/auth.ex +++ b/elixir/apps/domain/test/support/fixtures/auth.ex @@ -121,13 +121,13 @@ defmodule Domain.Fixtures.Auth do def start_and_create_okta_provider(attrs \\ %{}) do bypass = Domain.Mocks.OpenIDConnect.discovery_document_server() + api_base_url = "http://localhost:#{bypass.port}" adapter_config = openid_connect_adapter_config( - api_base_url: "http://localhost:#{bypass.port}", - oauth_uri: "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server", - discovery_document_uri: - "http://localhost:#{bypass.port}/.well-known/openid-configuration", + api_base_url: api_base_url, + okta_account_domain: api_base_url, + discovery_document_uri: "#{api_base_url}/.well-known/openid-configuration", scope: Domain.Auth.Adapters.Okta.Settings.scope() |> Enum.join(" ") ) diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/components.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/components.ex index 3d567153b..92bdde271 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/components.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/components.ex @@ -83,14 +83,14 @@ defmodule Web.Settings.IdentityProviders.Okta.Components do
<.input - label="OAuth Authorization Server URI" + label="Okta Account Domain" autocomplete="off" - field={adapter_config_form[:oauth_uri]} - placeholder="https://.okta.com/.well-known/oauth-authorization-server" + field={adapter_config_form[:okta_account_domain]} + placeholder=".okta.com" required />

- The Metadata URI of the Authorization Server for your Okta Application. + Your Okta account domain.

@@ -102,7 +102,7 @@ defmodule Web.Settings.IdentityProviders.Okta.Components do placeholder=".well-known/openid-configuration URL" />

- The OIDC Configuration URI. This field is derived from the value in the OAuth Authorization Server URI field. + The OIDC Configuration URI. This field is derived from the value in the Okta Account Domain field.

@@ -123,7 +123,7 @@ defmodule Web.Settings.IdentityProviders.Okta.Components do |> Enum.join("\n") end - def visible?(value) do + defp visible?(value) do case value do nil -> false "" -> false diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/edit.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/edit.ex index c2be08402..5027c0f8c 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/edit.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/edit.ex @@ -44,7 +44,7 @@ defmodule Web.Settings.IdentityProviders.Okta.Edit do def handle_event("change", %{"provider" => attrs}, socket) do attrs = attrs - |> put_discovery_document_uri() + |> Map.update("adapter_config", %{}, &put_discovery_document_uri/1) changeset = Auth.change_provider(socket.assigns.provider, attrs) @@ -56,6 +56,7 @@ defmodule Web.Settings.IdentityProviders.Okta.Edit do def handle_event("submit", %{"provider" => attrs}, socket) do attrs = attrs + |> Map.update("adapter_config", %{}, &put_discovery_document_uri/1) |> Map.update("adapter_config", %{}, &put_api_base_url/1) with {:ok, provider} <- @@ -74,22 +75,32 @@ defmodule Web.Settings.IdentityProviders.Okta.Edit do end defp put_api_base_url(adapter_config) do - uri = URI.parse(adapter_config["discovery_document_uri"]) - Map.put(adapter_config, "api_base_url", "#{uri.scheme}://#{uri.host}") + api_base_url = create_api_base_url(adapter_config["okta_account_domain"]) + + Map.put(adapter_config, "api_base_url", api_base_url) end - defp put_discovery_document_uri(attrs) do - config = attrs["adapter_config"] + defp put_discovery_document_uri(adapter_config) do + api_base_url = create_api_base_url(adapter_config["okta_account_domain"]) - oidc_uri = - String.replace_suffix( - config["oauth_uri"], - "oauth-authorization-server", - "openid-configuration" - ) + Map.put( + adapter_config, + "discovery_document_uri", + "#{api_base_url}/.well-known/openid-configuration" + ) + end - config = Map.put(config, "discovery_document_uri", oidc_uri) + # This is done for easier testing. Production should only use 'https' and Okta domains, + # but in dev and test there are times when putting an explicit URI is useful. + if Mix.env() in [:dev, :test] do + defp create_api_base_url(okta_account_domain) do + uri = URI.parse(okta_account_domain) - Map.put(attrs, "adapter_config", config) + if uri.scheme, do: okta_account_domain, else: "https://#{okta_account_domain}" + end + else + defp create_api_base_url(okta_account_domain) do + "https://#{okta_account_domain}" + end end end diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/new.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/new.ex index 2540ee5b8..29e8b7f85 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/new.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/new.ex @@ -54,7 +54,7 @@ defmodule Web.Settings.IdentityProviders.Okta.New do attrs = attrs |> Map.put("adapter", :okta) - |> put_discovery_document_uri() + |> Map.update("adapter_config", %{}, &put_discovery_document_uri/1) changeset = Auth.new_provider(socket.assigns.account, attrs) @@ -66,6 +66,7 @@ defmodule Web.Settings.IdentityProviders.Okta.New do def handle_event("submit", %{"provider" => attrs}, socket) do attrs = attrs + |> Map.update("adapter_config", %{}, &put_discovery_document_uri/1) |> Map.update("adapter_config", %{}, &put_api_base_url/1) |> Map.put("id", socket.assigns.id) |> Map.put("adapter", :okta) @@ -95,22 +96,32 @@ defmodule Web.Settings.IdentityProviders.Okta.New do end defp put_api_base_url(adapter_config) do - uri = URI.parse(adapter_config["discovery_document_uri"]) - Map.put(adapter_config, "api_base_url", "#{uri.scheme}://#{uri.host}") + api_base_url = create_api_base_url(adapter_config["okta_account_domain"]) + + Map.put(adapter_config, "api_base_url", api_base_url) end - defp put_discovery_document_uri(attrs) do - config = attrs["adapter_config"] + defp put_discovery_document_uri(adapter_config) do + api_base_url = create_api_base_url(adapter_config["okta_account_domain"]) - oidc_uri = - String.replace_suffix( - config["oauth_uri"], - "oauth-authorization-server", - "openid-configuration" - ) + Map.put( + adapter_config, + "discovery_document_uri", + "#{api_base_url}/.well-known/openid-configuration" + ) + end - config = Map.put(config, "discovery_document_uri", oidc_uri) + # This is done for easier testing. Production should only use 'https' and Okta domains, + # but in dev and test there are times when putting an explicit URI is useful. + if Mix.env() in [:dev, :test] do + defp create_api_base_url(okta_account_domain) do + uri = URI.parse(okta_account_domain) - Map.put(attrs, "adapter_config", config) + if uri.scheme, do: okta_account_domain, else: "https://#{okta_account_domain}" + end + else + defp create_api_base_url(okta_account_domain) do + "https://#{okta_account_domain}" + end end end diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/okta/edit_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/okta/edit_test.exs index f71ac33da..461c4304f 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/okta/edit_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/okta/edit_test.exs @@ -55,7 +55,7 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.EditTest do "provider[adapter_config][client_id]", "provider[adapter_config][client_secret]", "provider[adapter_config][discovery_document_uri]", - "provider[adapter_config][oauth_uri]", + "provider[adapter_config][okta_account_domain]", "provider[name]" ] end @@ -67,11 +67,10 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.EditTest do conn: conn } do bypass = Domain.Mocks.OpenIDConnect.discovery_document_server() + api_base_url = "http://localhost:#{bypass.port}" adapter_config_attrs = - Fixtures.Auth.openid_connect_adapter_config( - oauth_uri: "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server" - ) + Fixtures.Auth.openid_connect_adapter_config(okta_account_domain: api_base_url) adapter_config_attrs = Map.drop(adapter_config_attrs, [ @@ -100,8 +99,7 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.EditTest do |> render_submit(%{ provider: %{ adapter_config: %{ - "discovery_document_uri" => - "http://localhost:#{bypass.port}/.well-known/openid-configuration" + "discovery_document_uri" => "#{api_base_url}/.well-known/openid-configuration" } } }) @@ -119,11 +117,10 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.EditTest do assert provider.adapter_config["client_id"] == adapter_config_attrs["client_id"] assert provider.adapter_config["client_secret"] == adapter_config_attrs["client_secret"] - assert provider.adapter_config["oauth_uri"] == - "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server" + assert provider.adapter_config["okta_account_domain"] == api_base_url assert provider.adapter_config["discovery_document_uri"] == - "http://localhost:#{bypass.port}/.well-known/openid-configuration" + "#{api_base_url}/.well-known/openid-configuration" end test "renders changeset errors on invalid attrs", %{ @@ -133,11 +130,10 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.EditTest do conn: conn } do bypass = Domain.Mocks.OpenIDConnect.discovery_document_server() + api_base_url = "http://localhost:#{bypass.port}" adapter_config_attrs = - Fixtures.Auth.openid_connect_adapter_config( - oauth_uri: "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server" - ) + Fixtures.Auth.openid_connect_adapter_config(okta_account_domain: api_base_url) adapter_config_attrs = Map.drop(adapter_config_attrs, [ diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/okta/new_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/okta/new_test.exs index 26898d2ce..ca26d4809 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/okta/new_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/okta/new_test.exs @@ -46,7 +46,7 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.NewTest do "provider[adapter_config][_persistent_id]", "provider[adapter_config][client_id]", "provider[adapter_config][client_secret]", - "provider[adapter_config][oauth_uri]", + "provider[adapter_config][okta_account_domain]", "provider[name]" ] end @@ -57,11 +57,10 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.NewTest do conn: conn } do bypass = Domain.Mocks.OpenIDConnect.discovery_document_server() + api_base_url = "http://localhost:#{bypass.port}" adapter_config_attrs = - Fixtures.Auth.openid_connect_adapter_config( - oauth_uri: "http://localhost:#{bypass.port}/.well-known/oauth-authorization-server" - ) + Fixtures.Auth.openid_connect_adapter_config(okta_account_domain: api_base_url) adapter_config_attrs = Map.drop(adapter_config_attrs, [ @@ -90,8 +89,7 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.NewTest do |> render_submit(%{ provider: %{ adapter_config: %{ - "discovery_document_uri" => - "http://localhost:#{bypass.port}/.well-known/openid-configuration" + "discovery_document_uri" => "#{api_base_url}/.well-known/openid-configuration" } } }) @@ -162,7 +160,8 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.NewTest do assert form_validation_errors(form) == %{ "provider[name]" => ["should be at most 255 character(s)"], "provider[adapter_config][client_id]" => ["can't be blank"], - "provider[adapter_config][oauth_uri]" => ["can't be blank"] + "provider[adapter_config][okta_account_domain]" => ["can't be blank"], + "provider[adapter_config][discovery_document_uri]" => ["is not a valid URL"] } end) end