From 3ba7962c230ef9b360e71430d559ca80d98c84d6 Mon Sep 17 00:00:00 2001 From: Brian Manifold Date: Tue, 14 May 2024 15:48:36 -0400 Subject: [PATCH] refactor(portal): Update IDP creation flow (#4984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: * The new flow for creating an identity provider in Firezone allows the user to not have to worry what features their plan has enabled. It will allow the user to select which identity provider they use and will take them to the appropriate form depending on the features they have enabled on their plan. ## Screenshots ### Selecting an identity provider Screenshot 2024-05-14 at 11 53 17 AM ### New OIDC form when a custom provider is selected but IDP sync is not enabled for account Screenshot 2024-05-14 at 11 54 58 AM --- elixir/apps/domain/lib/domain/auth.ex | 3 +- elixir/apps/domain/test/domain/auth_test.exs | 16 +- .../live/settings/identity_providers/new.ex | 174 +++++++----------- .../openid_connect/components.ex | 48 ++++- .../identity_providers/openid_connect/new.ex | 7 +- .../settings/identity_providers/new_test.exs | 51 ++++- .../openid_connect/new_test.exs | 32 +++- 7 files changed, 207 insertions(+), 124 deletions(-) diff --git a/elixir/apps/domain/lib/domain/auth.ex b/elixir/apps/domain/lib/domain/auth.ex index 5d4149301..9433a7193 100644 --- a/elixir/apps/domain/lib/domain/auth.ex +++ b/elixir/apps/domain/lib/domain/auth.ex @@ -110,7 +110,8 @@ defmodule Domain.Auth do |> Enum.map(fn adapter -> capabilities = Adapters.fetch_capabilities!(adapter) requires_idp_sync_feature? = capabilities[:default_provisioner] == :custom - {adapter, enabled: idp_sync_enabled? or not requires_idp_sync_feature?} + enabled_for_account? = idp_sync_enabled? or not requires_idp_sync_feature? + {adapter, enabled: enabled_for_account?, sync: requires_idp_sync_feature?} end) end diff --git a/elixir/apps/domain/test/domain/auth_test.exs b/elixir/apps/domain/test/domain/auth_test.exs index 407259f2f..c25a03c5b 100644 --- a/elixir/apps/domain/test/domain/auth_test.exs +++ b/elixir/apps/domain/test/domain/auth_test.exs @@ -11,19 +11,19 @@ defmodule Domain.AuthTest do account = Fixtures.Accounts.create_account(features: %{idp_sync: true}) assert Enum.sort(all_user_provisioned_provider_adapters!(account)) == [ - google_workspace: [enabled: true], - microsoft_entra: [enabled: true], - okta: [enabled: true], - openid_connect: [enabled: true] + google_workspace: [enabled: true, sync: true], + microsoft_entra: [enabled: true, sync: true], + okta: [enabled: true, sync: true], + openid_connect: [enabled: true, sync: false] ] account = Fixtures.Accounts.create_account(features: %{idp_sync: false}) assert Enum.sort(all_user_provisioned_provider_adapters!(account)) == [ - google_workspace: [enabled: false], - microsoft_entra: [enabled: false], - okta: [enabled: false], - openid_connect: [enabled: true] + google_workspace: [enabled: false, sync: true], + microsoft_entra: [enabled: false, sync: true], + okta: [enabled: false, sync: true], + openid_connect: [enabled: true, sync: false] ] end end diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex index 6f0210514..ad7f3aec6 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex @@ -18,6 +18,10 @@ defmodule Web.Settings.IdentityProviders.New do {:noreply, push_navigate(socket, to: next)} end + def handle_event("next_step", %{"next" => next}, socket) do + {:noreply, push_navigate(socket, to: next_step_path(next, socket.assigns.account))} + end + def render(assigns) do ~H""" <.breadcrumbs account={@account}> @@ -32,135 +36,95 @@ defmodule Web.Settings.IdentityProviders.New do <:title> Add a new Identity Provider + <:help> + Set up SSO authentication using your own identity provider. Directory sync + also available for certain providers.
Learn more about + <.website_link href="/kb/authenticate/oidc">SSO authentication + and + <.website_link href="/kb/authenticate/directory-sync">directory sync + in our docs. + <:content> -
-

Choose type

- <.form id="identity-provider-type-form" for={%{}} phx-submit="submit"> -
-
- Identity Provider Type - - <.adapter - :for={{adapter, opts} <- @adapters} - adapter={adapter} - opts={opts} - account={@account} - /> -
+
+
+
+ <%= for {adapter, opts} <- @adapters, opts[:sync] == true do %> + <.provider_card adapter={adapter} opts={opts} account={@account} /> + <% end %> + <%= for {adapter, opts} <- @adapters, opts[:sync] == false do %> + <.provider_card adapter={adapter} opts={opts} account={@account} /> + <% end %>
- <.submit_button> - Next: Configure - - +
""" end - def adapter(%{adapter: :google_workspace} = assigns) do - ~H""" - <.adapter_item - adapter={@adapter} - account={@account} - opts={@opts} - name="Google Workspace" - description="Authenticate users and synchronize users and groups with a custom Google Workspace connector." - /> - """ - end - - def adapter(%{adapter: :microsoft_entra} = assigns) do - ~H""" - <.adapter_item - adapter={@adapter} - account={@account} - opts={@opts} - name="Microsoft Entra" - description="Authenticate users and synchronize users and groups with a custom Microsoft Entra ID connector." - /> - """ - end - - def adapter(%{adapter: :okta} = assigns) do - ~H""" - <.adapter_item - adapter={@adapter} - account={@account} - opts={@opts} - name="Okta" - description="Authenticate users and synchronize users and groups with a custom Okta connector." - /> - """ - end - - def adapter(%{adapter: :openid_connect} = assigns) do - ~H""" - <.adapter_item - adapter={@adapter} - account={@account} - opts={@opts} - name="OpenID Connect" - description="Authenticate users with a universal OpenID Connect adapter and manager users and groups manually." - /> - """ - end - attr :adapter, :any attr :account, :any attr :opts, :any - attr :name, :string - attr :description, :string - def adapter_item(assigns) do + def provider_card(assigns) do ~H""" -
-
- - <.provider_icon adapter={@adapter} class="w-8 h-8 ml-4" /> - - - <%= if @opts[:enabled] == false do %> - <.link navigate={~p"/#{@account}/settings/billing"} class="ml-2 text-sm text-primary-500"> - <.badge class="ml-2" type="primary" title="Feature available on a higher pricing plan"> - <.icon name="hero-lock-closed" class="w-3.5 h-3.5 mr-1" /> UPGRADE TO UNLOCK - - - <% end %> +
+
+ <.provider_icon adapter={@adapter} class="w-10 h-10 inline-block mr-2" /> + <%= pretty_print_provider(@adapter) %> +
+ +
+ <.icon name="hero-arrow-path" class="w-5 h-5 text-neutral-400" />
-

- <%= @description %> -

""" end - def next_step_path(:openid_connect, account) do + def next_step_path("openid_connect", account) do ~p"/#{account}/settings/identity_providers/openid_connect/new" end - def next_step_path(:google_workspace, account) do - ~p"/#{account}/settings/identity_providers/google_workspace/new" + def next_step_path("google_workspace" = provider, account) do + if Domain.Accounts.idp_sync_enabled?(account) do + ~p"/#{account}/settings/identity_providers/google_workspace/new" + else + ~p"/#{account}/settings/identity_providers/openid_connect/new?provider=#{provider}" + end end - def next_step_path(:microsoft_entra, account) do - ~p"/#{account}/settings/identity_providers/microsoft_entra/new" + def next_step_path("microsoft_entra" = provider, account) do + if Domain.Accounts.idp_sync_enabled?(account) do + ~p"/#{account}/settings/identity_providers/microsoft_entra/new" + else + ~p"/#{account}/settings/identity_providers/openid_connect/new?provider=#{provider}" + end end - def next_step_path(:okta, account) do - ~p"/#{account}/settings/identity_providers/okta/new" + def next_step_path("okta" = provider, account) do + if Domain.Accounts.idp_sync_enabled?(account) do + ~p"/#{account}/settings/identity_providers/okta/new" + else + ~p"/#{account}/settings/identity_providers/openid_connect/new?provider=#{provider}" + end + end + + def pretty_print_provider(adapter) do + case adapter do + :openid_connect -> "OpenID Connect" + :google_workspace -> "Google Workspace" + :microsoft_entra -> "Microsoft EntraID" + :okta -> "Okta" + end end end diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/components.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/components.ex index 8974a1f98..25d6bba6e 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/components.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/components.ex @@ -1,6 +1,11 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Components do use Web, :component_library + attr :id, :string + attr :account, :any + attr :form, :any + attr :show_sync_msg, :boolean, default: false + def provider_form(assigns) do ~H"""
@@ -95,12 +100,47 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.Components do
- - <.submit_button> - Connect Identity Provider - + <.step :if={@show_sync_msg}> + <:title> + Step 3. Enable Directory Sync <.icon name="hero-arrow-path" class="w-5 h-5 ml-2" /> + + <.link + navigate={~p"/#{@account}/settings/billing"} + class="text-sm text-primary-500 ml-2" + > + <.badge type="primary" title="Feature available on a higher pricing plan"> + <.icon name="hero-lock-closed" class="w-3.5 h-3.5 mr-1" /> UPGRADE TO UNLOCK + + + + + <:content> + Directory sync is not enabled on your current plan. +
+

+ Ensure the following scopes are added to the OAuth application: +

+ <.code_block + id="scope-fake" + class="w-full text-xs mb-4 whitespace-pre-line rounded" + phx-no-format + ><%= "placeholder\nscopes\nfor\nsync" %> +

+ Placeholder for any additional instructions here: +

+ <.code_block + id="placeholder-instructions" + class="w-full text-xs mb-4 whitespace-pre-line rounded" + phx-no-format + >placeholder instructions here +
+ + + <.submit_button> + Connect Identity Provider +
""" diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/new.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/new.ex index ebd9c4adc..29144fe63 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/new.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/openid_connect/new.ex @@ -3,7 +3,7 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.New do import Web.Settings.IdentityProviders.OpenIDConnect.Components alias Domain.Auth - def mount(_params, _session, socket) do + def mount(params, _session, socket) do id = Ecto.UUID.generate() account = socket.assigns.account @@ -17,7 +17,8 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.New do assign(socket, id: id, form: to_form(changeset), - page_title: "New Identity Provider" + page_title: "New Identity Provider", + provider: params["provider"] ) {:ok, socket, temporary_assigns: [form: %Phoenix.HTML.Form{}]} @@ -41,7 +42,7 @@ defmodule Web.Settings.IdentityProviders.OpenIDConnect.New do Add a new OpenID Connect Identity Provider <:content> - <.provider_form account={@account} id={@id} form={@form} /> + <.provider_form account={@account} id={@id} form={@form} show_sync_msg={!!@provider} /> """ diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/new_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/new_test.exs index ca24ff5ad..3e8ae7366 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/new_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/new_test.exs @@ -44,8 +44,6 @@ defmodule Web.Live.Settings.IdentityProviders.NewTest do assert has_element?(lv, "#idp-option-google_workspace") assert html =~ "Google Workspace" - assert html =~ "Feature available on a higher pricing plan" - assert html =~ "UPGRADE TO UNLOCK" assert has_element?(lv, "#idp-option-microsoft_entra") assert html =~ "Microsoft Entra" @@ -56,4 +54,53 @@ defmodule Web.Live.Settings.IdentityProviders.NewTest do assert has_element?(lv, "#idp-option-openid_connect") assert html =~ "OpenID Connect" end + + test "next step for non-idp-sync plans is OIDC form", %{ + account: account, + identity: identity, + conn: conn + } do + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/settings/identity_providers/new") + + lv + |> element("#idp-option-google_workspace") + |> render_click() + + assert_redirect( + lv, + ~p"/#{account}/settings/identity_providers/openid_connect/new?provider=google_workspace" + ) + end + + test "next step for idp-sync plans is to custom adapter form", %{ + account: account, + identity: identity, + conn: conn + } do + Domain.Config.feature_flag_override(:idp_sync, true) + + {:ok, account} = + Domain.Accounts.update_account(account, %{ + features: %{ + idp_sync: true + } + }) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/settings/identity_providers/new") + + lv + |> element("#idp-option-google_workspace") + |> render_click() + + assert_redirect( + lv, + ~p"/#{account}/settings/identity_providers/google_workspace/new" + ) + end end diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/new_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/new_test.exs index e7fb4a7fb..6f94677a1 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/new_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/openid_connect/new_test.exs @@ -35,7 +35,7 @@ defmodule Web.Live.Settings.IdentityProviders.OpenIDConnect.NewTest do identity: identity, conn: conn } do - {:ok, lv, _html} = + {:ok, lv, html} = conn |> authorize_conn(identity) |> live(~p"/#{account}/settings/identity_providers/openid_connect/new") @@ -51,6 +51,36 @@ defmodule Web.Live.Settings.IdentityProviders.OpenIDConnect.NewTest do "provider[adapter_config][scope]", "provider[name]" ] + + refute html =~ "Enable Directory Sync" + end + + test "renders provider creation form with blurred sync step", %{ + account: account, + identity: identity, + conn: conn + } do + {:ok, lv, html} = + conn + |> authorize_conn(identity) + |> live( + ~p"/#{account}/settings/identity_providers/openid_connect/new?provider=google_workspace" + ) + + form = form(lv, "form") + + assert find_inputs(form) == [ + "provider[adapter_config][_persistent_id]", + "provider[adapter_config][client_id]", + "provider[adapter_config][client_secret]", + "provider[adapter_config][discovery_document_uri]", + "provider[adapter_config][response_type]", + "provider[adapter_config][scope]", + "provider[name]" + ] + + assert html =~ "Enable Directory Sync" + assert html =~ "UPGRADE TO UNLOCK" end test "creates a new provider on valid attrs", %{