Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes validation and guard-rails around “Allowed Login Providers” when SSO is enabled, ensuring SSO is treated as a valid login method even though it is not represented inside allowedProviders, and preventing teams from accidentally locking themselves out by disabling/deleting SSO when it’s the only available login method.
Changes:
- Frontend: treat SSO as satisfying “at least one login provider” validation; prevent disabling/deleting SSO when it’s the only login method.
- Backend: enforce the same rules server-side (login config updates, SSO settings updates/deletes, invitation verification, session revocation).
- Tests: add DB-layer regression coverage for the new SSO-aware guards and session handling.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/features/teams/src/lib/TeamDashboard/TeamLoginConfiguration.tsx | Updates form validation so empty allowedProviders is valid when SSO is active; adjusts SSO help text. |
| libs/features/teams/src/lib/TeamDashboard/TeamDashboard.tsx | Computes and passes ssoIsOnlyLoginProvider down to the SSO configuration UI. |
| libs/features/teams/src/lib/TeamDashboard/sso-configuration/TeamSSOConfiguration.tsx | Disables SSO toggle and blocks delete when SSO is the only allowed login provider; adds explanatory help/toast. |
| apps/docs/scripts/serve-csp.mjs | Removes the CSP-serving helper script (currently still referenced by docs package scripts). |
| apps/api/src/app/services/team.service.ts | Makes invite verification SSO-aware when determining whether the current session provider is acceptable. |
| apps/api/src/app/db/team.db.ts | Adds SSO-aware guards for empty allowedProviders, session revocation, invite verification shaping, and prevents disabling/deleting SSO when it would lock out the team. |
| apps/api/src/app/db/tests/team.db.login-config.spec.ts | Adds regression tests covering SSO-aware login-config guards and SSO session validity rules. |
Comments suppressed due to low confidence (1)
apps/docs/scripts/serve-csp.mjs:1
- This file is removed, but
apps/docs/package.jsonstill defines the scriptserve:cspasnode scripts/serve-csp.mjs, which will now fail with a missing-file error. Either restore this helper script or remove/update theserve:cspscript entry so the docs workspace scripts remain runnable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
apps/api/src/app/services/team.service.ts:380
- When
allowedProvidersis empty (valid when SSO is the only login method), this warning renders an empty provider list ("...login using: ."), which is confusing. It also doesn’t clearly distinguish which linked identities are disallowed vs which login methods are allowed. Build the message from the disallowed linked identities, and include the allowed login methods list (including SSO when active) to keep the prompt actionable.
if (Array.from(providers).some((provider) => !allowedProviders.has(provider) && provider !== activeSsoProvider)) {
teamInviteVerification.linkedIdentities.isValid = false;
teamInviteVerification.session.message = `You have linked identities that are not allowed on this team. You do not need to take any action, but after joining, you will no longer be able to login using: ${loginConfig.allowedProviders
.map((provider) => LoginConfigurationIdentityDisplayNames[provider])
.join(', ')}.`;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/api/src/app/services/team.service.ts:380
- This block flags
linkedIdentities.isValid = falsebut writes the message intosession.message, which can overwrite the current-session validation message. It should setlinkedIdentities.messageinstead. Also, the message currently renders the allowed providers list rather than the specific disallowed linked providers, which can mislead users about what will stop working after they join.
if (Array.from(providers).some((provider) => !allowedProviders.has(provider) && provider !== activeSsoProvider)) {
teamInviteVerification.linkedIdentities.isValid = false;
teamInviteVerification.session.message = `You have linked identities that are not allowed on this team. You do not need to take any action, but after joining, you will no longer be able to login using: ${loginConfig.allowedProviders
.map((provider) => LoginConfigurationIdentityDisplayNames[provider])
.join(', ')}.`;
The merge of PR #1756 into main left duplicated '@xmldom/xmldom@0.8.13' entries in pnpm-lock.yaml, breaking pnpm install with ERR_PNPM_BROKEN_LOCKFILE. Removed the identical duplicate blocks; verified with pnpm install --frozen-lockfile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
apps/api/src/app/services/team.service.ts:381
- This warning says the user will no longer be able to log in using the allowed providers list. It should list the linked identity providers that are not allowed (excluding the active SSO provider), otherwise the message is misleading—especially when SSO is active and
allowedProvidersmay be empty.
if (Array.from(providers).some((provider) => !allowedProviders.has(provider) && provider !== activeSsoProvider)) {
teamInviteVerification.linkedIdentities.isValid = false;
teamInviteVerification.session.message = `You have linked identities that are not allowed on this team. You do not need to take any action, but after joining, you will no longer be able to login using: ${loginConfig.allowedProviders
.map((provider) => LoginConfigurationIdentityDisplayNames[provider])
.join(', ')}.`;
}
The changes ensure that when SSO is enabled, it is recognized as a valid login provider, allowing other providers to be disabled without triggering validation errors. Additionally, SSO cannot be disabled if it is the only login provider selected, preventing configuration issues.
Fixes #1771