feat(auth): Adds ability to enable MFA on a Google Cloud Identity Platform tenant #930
Conversation
This includes the following capabilities: - Ability to enable disable MFA on a tenant. - Configure the MFA supported type. - Configure the test phone number / code pairs on the tenant.
src/utils/index.ts
Outdated
| */ | ||
| export function generateUpdateMask(obj: {[key: string]: any}): string[] { | ||
| export function generateUpdateMask( | ||
| obj: any, maxPaths: {[key: string]: boolean} = {}, currentPath = '' |
There was a problem hiding this comment.
Can we retain the type of obj as {[key: string]: any}?
There was a problem hiding this comment.
I changed it to Set<string>. I think that is the best data structure here as it is more efficient than array to check for existence.
| * @param options The options object to convert to a server request. | ||
| * @return The resulting server request. | ||
| */ | ||
| public static buildServerRequest(options: MultiFactorConfig): MultiFactorAuthServerConfig { |
There was a problem hiding this comment.
This follows the same pattern as other config classes here. Personally, I think this is an adequate place for it. If you don't agree, I think we should then refactor the other config classes here in a separate PR to be consistent.
src/auth/auth-config.ts
Outdated
| public toJSON(): object { | ||
| return { | ||
| state: this.state, | ||
| factorIds: this.factorIds.concat(), |
There was a problem hiding this comment.
It was added to ensure that modification of the returned reference would not update class property. Anyway not a big deal. I removed it.
src/utils/index.ts
Outdated
| */ | ||
| export function generateUpdateMask(obj: {[key: string]: any}): string[] { | ||
| export function generateUpdateMask( | ||
| obj: any, terminalPaths: Set<string> = new Set(), root = '' |
There was a problem hiding this comment.
Set is a bit okverkill here. Let's use a regular array and terminalPaths.indexOf() to check membership. This array will be very short in practice, so performance shouldn't be an issue.
There was a problem hiding this comment.
I think Set would have been ok. It is pretty lightweight and readily available to us as we are using ES6. But since the array won't grow too large, I switched to string[].
| * @param options The options object to convert to a server request. | ||
| * @return The resulting server request. | ||
| */ | ||
| public static buildServerRequest(options: MultiFactorConfig): MultiFactorAuthServerConfig { |
There was a problem hiding this comment.
Ok. Let's do that refactor in a future PR.
bojeil-google
changed the title
This includes the following capabilities:
RELEASE NOTE: Added the ability to enable / disable multi-factor authentication with SMS on a Google Cloud Identity Platform tenant. Existing APIs like
createTenant()andupdateTenant()now support configuring multi-factor authentication and test phone number / code pairs on a specified tenant.