-
Notifications
You must be signed in to change notification settings - Fork 476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make recipe enabled booleans better #979
Comments
Using backend sdk with CDI 4.0 with new tenant creation using 4.0When the backend SDK asks for tenant config, we will return the enabled booleans based on:
Using backend sdk with CDI 4.0 with new tenant creation using 5.1 (new to be CDI)NOTE: We have to tell users that the new CDI will be breaking in how you create tenants. Instead of passing enabled booleans, you need to pass in firstFactors array ONLY.
Using backend sdk with CDI 5.0 with new tenant creation using 5.0
Using backend sdk with CDI 5.0 with new tenant creation using 5.1 (new to be CDI)NOTE: We have to tell users that the new CDI will be breaking in how you create tenants. Instead of passing enabled booleans, you need to pass in firstFactors array ONLY.
Using backend sdk with CDI 5.1 (new CDI to be) with new tenant creation using 5.1 (new CDI to be)
Other changes:
|
Database state for tenantIntroduce new state {
emailPasswordEnabled: boolean,
passwordlessEnabled: boolean,
thirdPartyEnabled: boolean,
firstFactors?: null | string[], // null, empty, non-empty
requiredSecondaryFactors: string[], // null, non-empty
providers: null | Provider[] // null, empty, non-empty
} Recipe enabled check in coreisEmailPasswordEnabled = emailPasswordEnabled || (firstFactors != null && firstFactors.includes('emailpassword')) || (requiredSecondaryFactors != null && requiredSecondaryFactors.includes('emailpassword'))
isThirdPartyEnabled = thirdPartyEnabled || (firstFactors != null && firstFactors.includes('thirdparty')) || (requiredSecondaryFactors != null && requiredSecondaryFactors.includes('thirdparty'))
isPasswordlessEnabled = passwordlessEnabled || (firstFactors != null && firstFactors.includesOneOf('otp-email', 'otp-phone', 'link-email', 'link-phone')) || (requiredSecondaryFactors != null && requiredSecondaryFactors.includesOneOf('otp-email', 'otp-phone', 'link-email', 'link-phone')) In the above conditions, we have three parts with OR conditions:
Note: v2 means new endpoint for create/update and get/list tenants New v2 endpoints for create/update CUD/app/tenant and list cud/apps/tenants and get tenant
Input for create/update tenant{
tenantId: string;
firstFactors?: null | string[], // allows empty array
requiredSecondaryFactors: null | string[], // does not allow empty array
coreConfig: any,
} Output of a tenant in get tenant or list tenants{
tenantId: string;
firstFactors?: string[]; // undefined or empty or non-empty
requiredSecondaryFactors?: string[]; // undefined or non-empty
thirdParty: {
providers?: Provider[]; // undefined or empty or non-empty
};
coreConfig: any;
} Using Backend CDI 4.0, Create/Update tenant using core CDI 4.0
Using Backend CDI 4.0, Create/Update tenant using core CDI 5.0
Edge cases:
Using Backend CDI 4.0, Create/Update tenant using core CDI 5.1 (v2)
Using Backend CDI 5.0, Create/Update tenant using core CDI 5.0
Using Backend CDI 5.0, Create/Update tenant using core CDI 5.1 (v2)
Using Backend CDI 5.1 (v2), Create/Update tenant using core CDI 5.1 (v2)
Edge cases
DB schema changes
Migrating for 5.0 -> 5.1
From dashboard point of viewFor migrated tenant from 4.0firstFactors
thirdParty providers
For migrated tenant from 5.0firstFactors
thirdParty providers
Tenants on 5.1
Use cases
|
database states enabled booleans firstFactors requiredSecondaryFactors providers create or update in v4.0create public tenantIntention
effect on enabledBoolean
create non-public tenantIntention
effect on enabledBoolean
{ emailPasswordEnabled: true }Intention
effect on enabledBoolean
{ emailPasswordEnabled: false }Intention
effect on enabledBoolean
similarly for thirdparty and passwordless create or update in v5.0create public tenant (same as v4)Intention
effect on enabledBoolean
create non-public tenant (same as v4)Intention
effect on enabledBoolean
{ emailPasswordEnabled: true }Intention
effect on enabledBoolean
{ emailPasswordEnabled: false }Intention
effect on enabledBoolean
{ firstFactors: null }Intention
effect on enabledBoolean
{ firstFactors: ["otp-phone"] }Intention
effect on enabledBoolean
{ emailPasswordEnabled: true, firstFactors: null, requiredSecondaryFactors: null }
{ requiredSecondaryFactors: null }
{ requiredSecondaryFactors : [...]}
create or update in v5.1 (v2 API){ firstFactors: null }
{ firstFactors: ['emailpassword']}
{ firstFactors: [] }
{ requiredSecondaryFactors: ['emailpassword'] }
|
馃悰 Bug Report
Changes to the core:
When a new tenant is created, we set all enabled booleans to true, and firstFactor array as. When a new tenant is created, we set all enabled boolean to true, and firstFactor undefined for public AND non public tenants.EDIT: for non public tenants, we will set it to[]
by default because:Modifying the static list wont affect existing tenant config by default (other than the public tenant).Via the UI there is no way to make a tenant have undefined as firstFactors, so it's weird that it starts of as undefined and then there is no way for the UI to go back to that state.firstFactors array being empty vs not being set needs to be distinguished. firstFactor would be undefined for static config to take into affect.For older CDI ->if firstFactor.length > 0, then we only set those enabled flagsif firstFactor.length === 0, then we have all as falseif firstFactor is undefined, then we return the booleans as they are (needs to be communicated as a change to developers).Backend SDK changes:
Docs changes
Migration of tenant config:
We considered doing a migration wherein we move the current tenant config -> enable all boolean + firstFactors array. But decided not to do cause such few people are using mfa at the moment anyway.Things to note:
Useful informations
(Write what happened. Add screenshots, stacktraces, videos, anything that can help)
TODO
Initialising new recipe will enable login methods for all tenants by default - think about thisWhat providers should you see now? statically defined ones or the last provider that was deleted?
It should start from no providers
The text was updated successfully, but these errors were encountered: