Enforce system wide sso providers #9058
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR consolidates authentication provider configuration at the system level by introducing system-wide SSO providers in the client configuration.
- Added
authProvidersfield inpackages/twenty-server/src/engine/core-modules/client-config/client-config.entity.tsto centralize authentication methods configuration - Updated
packages/twenty-front/src/modules/client-config/components/ClientConfigProviderEffect.tsxto manage auth provider states including Google, Microsoft, password and SSO options - Simplified domain logic in
packages/twenty-front/src/pages/auth/SignInUp.tsxby removing special case handling for default domain with multi-workspace - Potential issue: Duplicate SSO enabled check in
client-config.resolver.tsbetweenauthProvidersobject andisSSOEnabledfield could lead to inconsistencies
7 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| authProviders { | ||
| password | ||
| microsoft |
There was a problem hiding this comment.
logic: google, password, and microsoft fields are missing type definitions - they should likely be boolean flags
| sso { | ||
| id | ||
| name | ||
| type | ||
| status | ||
| issuer | ||
| } |
There was a problem hiding this comment.
logic: sso field appears to be an array type but is not marked as such in the query, which could cause runtime type mismatches
| setAuthProviders({ | ||
| google: data?.clientConfig.authProviders.google, | ||
| microsoft: data?.clientConfig.authProviders.microsoft, | ||
| password: data?.clientConfig.authProviders.password, | ||
| magicLink: false, | ||
| sso: data?.clientConfig.authProviders.sso, | ||
| }); |
There was a problem hiding this comment.
logic: hardcoding magicLink to false here could cause issues if magicLink auth is supported in the future - consider getting this value from the config instead
| sso: this.environmentService.get('AUTH_SSO_ENABLED'), | ||
| }, | ||
| isSSOEnabled: this.environmentService.get('AUTH_SSO_ENABLED'), |
There was a problem hiding this comment.
logic: Duplicate SSO configuration. authProviders.sso and isSSOEnabled could become inconsistent if environment variables change. Consider removing isSSOEnabled and using authProviders.sso throughout the codebase.
| @@ -22,6 +22,13 @@ export class ClientConfigResolver { | |||
| 'BILLING_FREE_TRIAL_DURATION_IN_DAYS', | |||
| ), | |||
| }, | |||
| authProviders: { | |||
| google: this.environmentService.get('AUTH_GOOGLE_ENABLED'), | |||
| magicLink: false, | |||
There was a problem hiding this comment.
style: magicLink is hardcoded to false rather than using an environment variable like other auth providers. Consider using AUTH_MAGIC_LINK_ENABLED for consistency.
We have recently introduced the possibility to specify workspace specific auth providers.
I'm: