From 1fc3e7250eaa4d42a3a1aa3cd1c8ad001a6a6d6d Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Thu, 5 Dec 2024 13:35:26 +0100 Subject: [PATCH 1/3] chore: add acceptance checkbox to pr template --- .github/pull_request_template.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0ea84c5c8a..b8733b5e06 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -7,6 +7,7 @@ - [ ] All open todos and follow ups are defined in a new ticket and justified - [ ] Deviations from the acceptance criteria and design are agreed with the PO and documented. - [ ] Vitest unit tests ensure that components produce expected outputs on different inputs. -- [ ] Cypress integration tests ensure that login app pages work as expected. The ZITADEL API is mocked. +- [ ] Cypress integration tests ensure that login app pages work as expected on good and bad user inputs, ZITADEL responses or IDP redirects. The ZITADEL API is mocked, IDP redirects are simulated. +- [ ] Playwright acceptances tests ensure that the happy paths of common user journeys work as expected. - [ ] No debug or dead code - [ ] My code has no repetitions From 4610ae8764c122ce82acecfa4e2fec2b768cd730 Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Thu, 5 Dec 2024 13:43:16 +0100 Subject: [PATCH 2/3] simulated idp redirects --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index b8733b5e06..138d4919af 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,6 +8,6 @@ - [ ] Deviations from the acceptance criteria and design are agreed with the PO and documented. - [ ] Vitest unit tests ensure that components produce expected outputs on different inputs. - [ ] Cypress integration tests ensure that login app pages work as expected on good and bad user inputs, ZITADEL responses or IDP redirects. The ZITADEL API is mocked, IDP redirects are simulated. -- [ ] Playwright acceptances tests ensure that the happy paths of common user journeys work as expected. +- [ ] Playwright acceptances tests ensure that the happy paths of common user journeys work as expected. The ZITADEL API is not mocked but IDP redirects are simulated. - [ ] No debug or dead code - [ ] My code has no repetitions From 72df8b285e471daf56957fed9fc5d42e6cd7eb6b Mon Sep 17 00:00:00 2001 From: surya-sanity Date: Fri, 13 Dec 2024 10:51:30 +0530 Subject: [PATCH 3/3] refactor: SignInWithIDP optimization. --- .../login/src/components/sign-in-with-idp.tsx | 230 +++++------------- 1 file changed, 63 insertions(+), 167 deletions(-) diff --git a/apps/login/src/components/sign-in-with-idp.tsx b/apps/login/src/components/sign-in-with-idp.tsx index a10680eae5..fd675b62b8 100644 --- a/apps/login/src/components/sign-in-with-idp.tsx +++ b/apps/login/src/components/sign-in-with-idp.tsx @@ -7,8 +7,9 @@ import { IdentityProviderType, } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb"; import { useRouter } from "next/navigation"; -import { ReactNode, useState } from "react"; +import { ReactNode, useCallback, useState } from "react"; import { Alert } from "./alert"; +import { SignInWithIdentityProviderProps } from "./idps/base-button"; import { SignInWithApple } from "./idps/sign-in-with-apple"; import { SignInWithAzureAd } from "./idps/sign-in-with-azure-ad"; import { SignInWithGeneric } from "./idps/sign-in-with-generic"; @@ -29,184 +30,79 @@ export function SignInWithIdp({ authRequestId, organization, linkOnly, -}: SignInWithIDPProps) { - const [loading, setLoading] = useState(false); - const [error, setError] = useState(""); +}: Readonly) { + const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); const router = useRouter(); - async function startFlow(idpId: string, provider: string) { - setLoading(true); + const startFlow = useCallback( + async (idpId: string, provider: string) => { + setLoading(true); + const params = new URLSearchParams(); + if (linkOnly) params.set("link", "true"); + if (authRequestId) params.set("authRequestId", authRequestId); + if (organization) params.set("organization", organization); - const params = new URLSearchParams(); + try { + const response = await startIDPFlow({ + idpId, + successUrl: `/idp/${provider}/success?` + params.toString(), + failureUrl: `/idp/${provider}/failure?` + params.toString(), + }); - if (linkOnly) { - params.set("link", "true"); - } + if (response && "error" in response && response?.error) { + setError(response.error); + return; + } - if (authRequestId) { - params.set("authRequestId", authRequestId); - } - - if (organization) { - params.set("organization", organization); - } - - const response = await startIDPFlow({ - idpId, - successUrl: `/idp/${provider}/success?` + new URLSearchParams(params), - failureUrl: `/idp/${provider}/failure?` + new URLSearchParams(params), - }) - .catch(() => { + if (response && "redirect" in response && response?.redirect) { + return router.push(response.redirect); + } + } catch { setError("Could not start IDP flow"); - return; - }) - .finally(() => { + } finally { setLoading(false); - }); + } + }, + [authRequestId, organization, linkOnly, router], + ); - if (response && "error" in response && response?.error) { - setError(response.error); - return; - } + const renderIDPButton = (idp: IdentityProvider) => { + const { id, name, type } = idp; + const onClick = () => startFlow(id, idpTypeToSlug(type)); + /* - TODO: Implement after https://github.com/zitadel/zitadel/issues/8981 */ - if (response && "redirect" in response && response?.redirect) { - return router.push(response.redirect); - } - } + // .filter((idp) => + // linkOnly ? idp.config?.options.isLinkingAllowed : true, + // ) + const components: Partial< + Record< + IdentityProviderType, + (props: SignInWithIdentityProviderProps) => ReactNode + > + > = { + [IdentityProviderType.APPLE]: SignInWithApple, + [IdentityProviderType.OAUTH]: SignInWithGeneric, + [IdentityProviderType.OIDC]: SignInWithGeneric, + [IdentityProviderType.GITHUB]: SignInWithGithub, + [IdentityProviderType.GITHUB_ES]: SignInWithGithub, + [IdentityProviderType.AZURE_AD]: SignInWithAzureAd, + [IdentityProviderType.GOOGLE]: (props) => ( + + ), + [IdentityProviderType.GITLAB]: SignInWithGitlab, + [IdentityProviderType.GITLAB_SELF_HOSTED]: SignInWithGitlab, + }; + + const Component = components[type]; + return Component ? ( + + ) : null; + }; return (
- {identityProviders && - identityProviders - /* - TODO: Implement after https://github.com/zitadel/zitadel/issues/8981 */ - - // .filter((idp) => - // linkOnly ? idp.config?.options.isLinkingAllowed : true, - // ) - .map((idp, i) => { - switch (idp.type) { - case IdentityProviderType.APPLE: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.APPLE), - ) - } - > - ); - case IdentityProviderType.OAUTH: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.OAUTH), - ) - } - > - ); - case IdentityProviderType.OIDC: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.OIDC), - ) - } - > - ); - case IdentityProviderType.GITHUB: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.GITHUB), - ) - } - > - ); - case IdentityProviderType.GITHUB_ES: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.GITHUB_ES), - ) - } - > - ); - case IdentityProviderType.AZURE_AD: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.AZURE_AD), - ) - } - > - ); - case IdentityProviderType.GOOGLE: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.GOOGLE), - ) - } - > - ); - case IdentityProviderType.GITLAB: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.GITLAB), - ) - } - > - ); - case IdentityProviderType.GITLAB_SELF_HOSTED: - return ( - - startFlow( - idp.id, - idpTypeToSlug(IdentityProviderType.GITLAB_SELF_HOSTED), - ) - } - > - ); - default: - return null; - } - })} + {identityProviders?.map(renderIDPButton)} {error && (
{error}