Make google auth optional on server side #508
Conversation
charlesBochet
force-pushed
the
oauth-optional
branch
from
63c34b6
to
5b4bb10
Compare
| clientID: environmentService.getAuthGoogleClientId(), | ||
| clientSecret: environmentService.getAuthGoogleClientSecret(), | ||
| callbackURL: environmentService.getAuthGoogleCallbackUrl(), | ||
| clientID: isAuthGoogleEnabled |
There was a problem hiding this comment.
@magrinj @emilienchvt not very proud of these disabled but super() has to be called.
Another solution would be to not register the strategy using DynamicModules
There was a problem hiding this comment.
Not extremely better, but disabledClientId, disabledClientSecret and disabledCallbackUrl might be less generic
| } | ||
|
|
||
| export function validate(config: Record<string, unknown>) { | ||
| const validatedConfig = plainToClass(EnvironmentVariables, config, { | ||
| enableImplicitConversion: true, | ||
| enableImplicitConversion: false, |
There was a problem hiding this comment.
I have disabled this because it was casting ENV_VARIABLE=false to TRUE...
There was a problem hiding this comment.
see my other comment below
There was a problem hiding this comment.
Indeed, maybe some ideas here: typestack/class-transformer#550
The workarounds described do not look too different from yours
There was a problem hiding this comment.
nice link did not find it
There was a problem hiding this comment.
I prefer to use Transform instead of disabling this, because it's going to stop inferring types on all other env variables
|
|
||
| return { | ||
| bucketName, | ||
| bucketName: bucketName ?? '', |
There was a problem hiding this comment.
I'm only moving the problem to here. I believe the S3Module should not be instanciated if S3 is not configured in env variables. I would encapsulated LocalDriver and S3Driver behind one common FileStorage Interface.
To be discussed and treated in another PR!
There was a problem hiding this comment.
Should we track this as a new task ?
There was a problem hiding this comment.
Yes, I'll discuss it with Jeremy tomorrow and create a task based on discussion!
There was a problem hiding this comment.
Created here: #510 we can discuss it tomorrow
| @@ -38,34 +39,44 @@ export class EnvironmentVariables { | |||
| @IsUrl({ require_tld: false }) | |||
| FRONT_AUTH_CALLBACK_URL: string; | |||
|
|
|||
| @IsString() | |||
| @Transform(({ value }) => value.toLowerCase() === 'true') | |||
There was a problem hiding this comment.
Ideally, I would like the environment validation to throw if the provided value is not either 'true'/'TRUE'/'1' or 'false'/'FALSE'/'0' and then convert it to a boolean. I'm unsure how to do that properly.
I'm thinking about a pre-process of environment variables maybe a little bit more sophisticated than @Transform or enableImplicitConversion
In this PR, I'm making the following changes:
AUTH_GOOGLE_ENABLEDboolean to .env. If set to true,AUTH_GOOGLE_CLIENT_ID,AUTH_GOOGLE_CLIENT_SECRETandAUTH_GOOGLE_CALLBACK_URLwill be required. If missing or set to any other value, these won't be requiredAUTH_GOOGLE_ENABLEDis set to false,GoogleStrategyis set with placeholder values for credentials and google auth controller routes are disabled through aGoogleProviderEnabledGuardAdditionally:
STORAGE_TYPE=local,STORAGE_LOCAL_PATHis required / ifSTORAGE_TYPE=S3,STORAGE_S3_REGIONandSTORAGE_S3_NAMEare required