diff --git a/src/frontend/src/lib/components/ui/IdentityAvatar.svelte b/src/frontend/src/lib/components/ui/IdentityAvatar.svelte index 9454e5ee6b..7366293b9f 100644 --- a/src/frontend/src/lib/components/ui/IdentityAvatar.svelte +++ b/src/frontend/src/lib/components/ui/IdentityAvatar.svelte @@ -17,13 +17,15 @@ identity.authMethod.openid.metadata !== undefined ? openIdLogo( identity.authMethod.openid.iss, - // `sub` and `aud` not currently tracked on `LastUsedIdentity`; - // see #3795. Falls back to issuer-only matching in findConfig — - // correct for direct providers, imprecise for SSO until that's - // fixed. - identity.authMethod.openid.sub, + // `aud` not currently tracked on `LastUsedIdentity` (see + // #3795) — issuer-only fallback in `findConfig` is correct + // for direct providers, imprecise for SSO until that's + // fixed. `ssoDomain` is unknown here for the same reason, + // so SSO-linked last-used identities render with the + // underlying IdP's logo. undefined, identity.authMethod.openid.metadata, + undefined, ) : undefined, ); diff --git a/src/frontend/src/lib/components/ui/ManageIdentities.svelte b/src/frontend/src/lib/components/ui/ManageIdentities.svelte index d897983b3e..7bbaf00dba 100644 --- a/src/frontend/src/lib/components/ui/ManageIdentities.svelte +++ b/src/frontend/src/lib/components/ui/ManageIdentities.svelte @@ -44,10 +44,14 @@ identity.authMethod.openid.metadata !== undefined ? openIdName( identity.authMethod.openid.iss, - identity.authMethod.openid.sub, - // `aud` not tracked on `LastUsedIdentity`; see #3795. + // `aud`, `ssoName`, `ssoDomain` not tracked on + // `LastUsedIdentity`; see #3795. Fall through to issuer-only + // `findConfig` — correct for direct providers, imprecise for + // SSO until that's fixed. undefined, identity.authMethod.openid.metadata, + undefined, + undefined, ) : undefined}
diff --git a/src/frontend/src/lib/components/utils/error.ts b/src/frontend/src/lib/components/utils/error.ts index 729601d734..3aefca6baf 100644 --- a/src/frontend/src/lib/components/utils/error.ts +++ b/src/frontend/src/lib/components/utils/error.ts @@ -11,11 +11,7 @@ import type { OpenIdCredentialAddError, OpenIdCredentialRemoveError, } from "$lib/generated/internet_identity_types"; -import { - OAuthProviderError, - OpenIdCredentialAlreadyLinkedHereError, - isOpenIdCancelError, -} from "$lib/utils/openID"; +import { OAuthProviderError, isOpenIdCancelError } from "$lib/utils/openID"; import { AuthenticationV2Events, authenticationV2Funnel, @@ -32,30 +28,23 @@ export const handleError = (error: unknown) => { return; } - // Specialization of the generic `OpenIdCredentialAlreadyRegistered` error - // below: the credential is already on THIS identity, not some other one. - if (error instanceof OpenIdCredentialAlreadyLinkedHereError) { - toaster.error({ - title: "This account is already linked to this identity", - }); - return; - } - // OAuth provider returned `error=…` in the callback fragment (RFC 6749 // §4.1.2.1 / 4.2.2.1). Surface the provider's own description so a - // misconfigured SSO app (e.g. Okta set to `response_types=[code]` only) + // misconfigured OAuth/OpenID app (e.g. an Okta SSO set to + // `response_types=[code]` only, or a botched direct-Google config) // doesn't look like an II bug. The SSO view's `mapSubmitError` gives // more specific guidance when the error hits inside `SignInWithSso`; // this branch covers callers (direct-OpenID entry points) that route - // through `handleError` instead. + // through `handleError` instead. Wording is provider-agnostic here + // because this path handles both SSO and direct providers. if (error instanceof OAuthProviderError) { toaster.error({ - title: `SSO provider returned "${error.error}"`, + title: `OAuth provider returned "${error.error}"`, description: error.errorDescription !== undefined && error.errorDescription.length > 0 ? error.errorDescription - : "Ask your SSO admin to check the app's OAuth configuration.", + : "Ask your administrator to check the provider's OAuth configuration.", }); return; } diff --git a/src/frontend/src/lib/components/wizards/addAccessMethod/AddAccessMethodWizard.svelte b/src/frontend/src/lib/components/wizards/addAccessMethod/AddAccessMethodWizard.svelte index 656605a21a..f2c5fda593 100644 --- a/src/frontend/src/lib/components/wizards/addAccessMethod/AddAccessMethodWizard.svelte +++ b/src/frontend/src/lib/components/wizards/addAccessMethod/AddAccessMethodWizard.svelte @@ -100,6 +100,7 @@ {/if} diff --git a/src/frontend/src/lib/components/wizards/addAccessMethod/views/AddAccessMethod.svelte b/src/frontend/src/lib/components/wizards/addAccessMethod/views/AddAccessMethod.svelte index ab1106ba98..3c24052b20 100644 --- a/src/frontend/src/lib/components/wizards/addAccessMethod/views/AddAccessMethod.svelte +++ b/src/frontend/src/lib/components/wizards/addAccessMethod/views/AddAccessMethod.svelte @@ -125,7 +125,7 @@ disabled={authenticatingProviderId !== undefined} size="xl" class="flex-1" - aria-label={$t`Sign in with SSO`} + aria-label={$t`Continue with SSO`} > diff --git a/src/frontend/src/lib/components/wizards/auth/views/PickAuthenticationMethod.svelte b/src/frontend/src/lib/components/wizards/auth/views/PickAuthenticationMethod.svelte index 2fc32546d4..2f983c8f4f 100644 --- a/src/frontend/src/lib/components/wizards/auth/views/PickAuthenticationMethod.svelte +++ b/src/frontend/src/lib/components/wizards/auth/views/PickAuthenticationMethod.svelte @@ -98,7 +98,7 @@ disabled={authenticatingProviderId !== undefined} size="xl" class="flex-1" - aria-label={$t`Sign in with SSO`} + aria-label={$t`Continue with SSO`} > diff --git a/src/frontend/src/lib/components/wizards/auth/views/SignInWithSso.svelte b/src/frontend/src/lib/components/wizards/auth/views/SignInWithSso.svelte index 3b3152d86e..3872624a7e 100644 --- a/src/frontend/src/lib/components/wizards/auth/views/SignInWithSso.svelte +++ b/src/frontend/src/lib/components/wizards/auth/views/SignInWithSso.svelte @@ -13,6 +13,7 @@ } from "$lib/utils/ssoDiscovery"; import type { SsoDiscoveryResult } from "$lib/utils/ssoDiscovery"; import { OAuthProviderError } from "$lib/utils/openID"; + import type { OpenIdCredential } from "$lib/generated/internet_identity_types"; import { t } from "$lib/stores/locale.store"; interface Props { @@ -20,9 +21,19 @@ result: SsoDiscoveryResult, ) => Promise; goBack: () => void; + /** + * Credentials already linked to the signed-in identity. When present, + * the Continue button is disabled and an inline hint is shown if the + * discovered SSO's `(iss, aud)` matches an existing credential — + * mirroring how the add-access-method UI disables already-linked + * direct providers (Google / Apple / Microsoft) in + * `AddAccessMethod.svelte`. Leave this `undefined` in the sign-in + * flow, where reusing an already-linked credential is the point. + */ + openIdCredentials?: OpenIdCredential[]; } - const { continueWithSso, goBack }: Props = $props(); + const { continueWithSso, goBack, openIdCredentials }: Props = $props(); /** * Debounce delay before kicking off the (network-heavy) two-hop lookup. @@ -44,33 +55,63 @@ */ let preparedResult = $state(); + /** + * True when `preparedResult`'s `(issuer, client_id)` already matches a + * credential in `openIdCredentials` — i.e. this SSO domain is already + * linked to the signed-in identity. Keeps the add-access-method Continue + * button disabled with an inline hint, matching the direct-provider + * behaviour in `AddAccessMethod.svelte`. Always `false` in the sign-in + * flow (where `openIdCredentials` is left undefined). + */ + const isAlreadyLinked = $derived.by(() => { + if (preparedResult === undefined || openIdCredentials === undefined) { + return false; + } + const { discovery, clientId } = preparedResult; + return openIdCredentials.some( + (c) => c.iss === discovery.issuer && c.aud === clientId, + ); + }); + let debounceTimer: ReturnType | undefined; - const mapSubmitError = (e: unknown, domainInput: string): string => { + /** + * Map caught errors to user-actionable copy, or `undefined` for + * "nothing useful to tell the user" — the caller will fall back to a + * generic message. Raw errors are always logged to the console so + * engineers have the full stack while end users see concise copy. + * + * Only branches that the user (or their SSO admin) can act on live + * here; provider-misconfiguration details (hostname mismatch, HTTPS, + * malformed discovery document) are dropped to the generic path — + * they're dev-facing and the console log is the right audience. + */ + const mapSubmitError = ( + e: unknown, + domainInput: string, + ): string | undefined => { if (e instanceof DomainNotConfiguredError) { if (e.reason === "http-error" && e.httpStatus !== undefined) { - return $t`${domainInput} didn't serve /.well-known/ii-openid-configuration (HTTP ${String(e.httpStatus)}). The domain owner needs to publish it for II to sign you in.`; + return $t`${domainInput} didn't publish /.well-known/ii-openid-configuration (HTTP ${String(e.httpStatus)}).`; } if (e.reason === "network") { - return $t`Couldn't reach ${domainInput}. Check the spelling and your network, then try again.`; - } - if (e.detail !== undefined && e.detail.length > 0) { - return $t`${domainInput}'s /.well-known/ii-openid-configuration is malformed: ${e.detail}`; + return $t`Couldn't reach ${domainInput}. Check the spelling and your network.`; } return $t`${domainInput}'s /.well-known/ii-openid-configuration is malformed.`; } if (e instanceof OAuthProviderError) { - // `unsupported_response_type` is the signature of an SSO app that - // only allows the plain authorization-code flow. II needs the - // hybrid flow (id_token + code) because it verifies JWTs canister- - // side with no token-endpoint exchange. Spell out the fix so the - // SSO admin can act on it directly. + // `unsupported_response_type` = the SSO app is code-only; II needs + // the hybrid flow because it verifies JWTs canister-side with no + // token-endpoint exchange. Spell out the fix so the SSO admin can + // act on it directly. if (e.error === "unsupported_response_type") { - return $t`${domainInput}'s SSO app doesn't allow the hybrid OAuth flow II requires. Ask the SSO admin to enable response_type "id_token code" (e.g. in Okta, set the app to Single-Page Application with "Implicit (hybrid)" grant enabled).`; + return $t`${domainInput}'s SSO app doesn't allow the hybrid OAuth flow II requires. Ask the SSO admin to enable response_type "id_token code".`; } if (e.error === "access_denied") { - return $t`${domainInput}'s SSO denied the sign-in. Try again, and check with your SSO admin if the problem persists.`; + return $t`${domainInput}'s SSO denied the sign-in.`; } + // Everything else: show the provider's own words if any, else the + // error code. Useful because these bubble straight from the IdP. if (e.errorDescription !== undefined && e.errorDescription.length > 0) { return $t`${domainInput}'s SSO returned "${e.error}": ${e.errorDescription}`; } @@ -81,30 +122,28 @@ if (msg.toLowerCase().includes("canary allowlist")) { return $t`SSO is not available for "${domainInput}" yet. Ask an II admin to register this domain.`; } - if (msg.includes("Provider issuer hostname mismatch")) { - return $t`SSO provider misconfigured: issuer doesn't match the configured hostname. (${msg})`; - } - if (msg.includes("Provider authorization endpoint hostname mismatch")) { - return $t`SSO provider misconfigured: authorization endpoint points to a different host than the issuer. (${msg})`; - } - if (msg.includes("Provider issuer must use HTTPS")) { - return $t`SSO provider misconfigured: issuer URL is not HTTPS. (${msg})`; - } - if (msg.includes("Provider authorization endpoint must use HTTPS")) { - return $t`SSO provider misconfigured: authorization endpoint is not HTTPS. (${msg})`; - } - if (msg.startsWith("Provider discovery:")) { - return $t`SSO provider's discovery document is malformed: ${msg}`; - } if (msg.startsWith("Rate limited:")) { - return $t`Too many recent attempts for ${domainInput}. Wait a few minutes and try again.`; + return $t`Too many recent attempts for ${domainInput}. Wait a few minutes.`; } if (msg === "Too many concurrent SSO discovery requests") { - return $t`Several SSO sign-ins are in flight already. Wait a moment and try again.`; + return $t`Several SSO sign-ins are in flight already. Wait a moment.`; } - return msg; } - return $t`SSO sign-in failed. Please try again.`; + return undefined; + }; + + /** + * Set `error` from a thrown exception: always `console.error` the raw + * error (so engineers can inspect the stack / non-Error values), then + * surface a user-actionable message if we have one, or a generic + * fallback. + */ + const setErrorFrom = (e: unknown, domainInput: string) => { + // eslint-disable-next-line no-console + console.error("SSO sign-in failed", e); + error = + mapSubmitError(e, domainInput) ?? + $t`SSO sign-in for ${domainInput} failed. Please try again.`; }; const invalidatePrepared = () => { @@ -160,7 +199,7 @@ } } catch (e) { if (matchesCurrent()) { - error = mapSubmitError(e, trimmed); + setErrorFrom(e, trimmed); } } finally { if (matchesCurrent()) { @@ -187,7 +226,7 @@ // handler. await continueWithSso(preparedResult); } catch (e) { - error = mapSubmitError(e, domain.trim().toLowerCase()); + setErrorFrom(e, domain.trim().toLowerCase()); } finally { isSubmitting = false; } @@ -207,7 +246,7 @@
-

{$t`Sign In With SSO`}

+

{$t`Continue with SSO`}

{$t`Enter your company domain`}

@@ -236,11 +275,19 @@ {error} aria-label={$t`Company domain`} /> + {#if isAlreadyLinked} +

+ {$t`This SSO is already linked to your identity.`} +

+ {/if}