Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TOTP / MFA login discussions #554

Closed
rishabhpoddar opened this issue Jan 12, 2023 · 34 comments
Closed

TOTP / MFA login discussions #554

rishabhpoddar opened this issue Jan 12, 2023 · 34 comments
Labels
enhancement New feature or request

Comments

@rishabhpoddar
Copy link
Member

rishabhpoddar commented Jan 12, 2023

Google doc link: https://docs.google.com/document/d/1qyd4XmepLJXNC4uURcak-7JbWP0UuzOPSsjBInVAuNk/edit

TODO:

  • Consider seperately mentioning templates vars used in qr (issuerName, secret, provisioner, etc)
  • Fetch user identifier based on first factor?
  • Give Rishabh all the touch points between account linking and mfa recipe
  • ~~Think if there are any session changes required for TOTP verify code and verify device API? ~~
  • How to prevent the attack in which the attacker finishes the fist factor and then for second factor (which is totp), they call the create new device API and finish the flow with their new device - even though the user already has an existing device. One idea is to add a claim validator to the create device API that all MFA claims should be completed when this API is called. What other solution is there? Some edge cases with this solution:
    • How will this solution affect the sign up process?
    • What if the user signs up with first factor, logs out, and then signs in - that time we still want to allow them to set the factor even though it's during sign in.
    • We decided to prevent this by checking if all factors are completed in the createDevicePOST implementation (claim assertion), if there already exists one device. But we should discuss this with the team (TODO)
  • We need to get rid of TOTP_NOT_ENABLED_ERROR from the core entirely:
    • verifyCode should return INVALID_TOTP_ERROR
    • verifyDevice should return UNKNOWN_DEVICE_ERROR
    • updateDevice should return UNKNOWN_DEVICE_ERROR
    • removeDevice should return { status: "OK"; didDeviceExist: false }
    • listDevices should return an empty array
  • Need to have a separate API for MFA account linking which is enabled if the MFA feature is enabled and account linking is not. This API will only allow account linking for the purpose of MFA. So you need to enforce that, somehow.
    • One idea is that we disallow linking of accounts across the same recipe ID.
  • Devs should be able to set MFA settings for tenants via an API call (similar to how they configure third party providers for tenants via an API call). This should be as flexible as possible.
    • Maybe we can have enableFactorForTenant(tenantId, factorId…) just like we have enableFactorForUser. This is not necessary.. it's just an idea
  • Figure out changes in core, backend sdk and frontend sdk related to this: TOTP / MFA login discussions #554 (comment)
  • Check how often do we end up in the MFA chooser screen in auth-react and that has just one choice. This can happen if the next array has > 1 element, such that all but one are not setup and can’t be setup by the user.
    • If this is often, then we need to rethink about the "choose another factor" button in the MFA screen
  • Contact support UI on the frontend (this UI will be a part of the session recipe):
    • You are on a factor page which is not setup, and you are not allowed to setup
    • Redirection to an unknown factor id
    • First factor returned from the backend is an empty array
  • Other MFA UI related todos:
    • logout button on the chooser screen needs change
    • select a second factor copy in the chooser screen needs change
    • New logos for passwordless-factor id (separate for both)
    • Add a back button to the OTP MFA screen which takes users back (regardless of if that causes a redirection back to this screen). Note that this back button should only been shown if the next array has 0 items (indicating that the user has fully logged in, cause if they havent logged in, the back button would do nothing)
    • Add a "choose another factor" button on the MFA screen if next array has more than 1 item
    • Add a logout button to the MFA screen.
  • Think about 2FA migration. for example migration of TOTP should not need re-setup of authenticator app
  • remove automatic addition of factorId into defaultRequiredUserIdsForUser and provide an easy override instead
  • Change type of userContext to Record<string, any>
  • MFA tests
  • do not return totp in getAllFactorIds
  • validateForMultifactorAuthBeforeFactorCompletion update impl to get all initialized recipe factors directly without calling the function getAllAvailableFactorIds
  • remove recipe function - getAllAvailableFactorIds
  • add an example in getMFARequirementsForAuth as a comment - while completing second factor and the user has first factor + 2 other factors setup, we want the completed second factor to be part of oneOf, otherwise it would result in 3FA. we would like this to be a opt in 2FA by default
  • in docs mention getMFARequirementsForAuth must be overridden for 3FA
  • think about migration of older sessions with all cases
  • should refetch for MFA claim should return true for older sessions and then fetchValue should rebuild the next array (move the impl from mfa info endpoint)
  • should throw will never happen error in isAllowedToSetupFactor where mfa claim value is missing
  • remove tenant id checks in determining factors setup for user
  • we are not considering recipeEnabled for tenant in get mfa info endpoint
  • we should not check tenantId when return in getFactorsSetupForUser (because factors are app wide)
  • create a list of contact support cases
  • check what can be removed from checking the dynamic login methods on the frontend
  • getDefaultRequiredFactorsForUser make it appwide
  • remove functions duplicated for accountlinking in MFA (in core)
  • remove functions duplicated for accountlinking in MFA in the SDK call accountlinking recipe functions directly
  • remove validateForMultifactorAuthBeforeFactorCompletion & createOrUpdateSessionForMultifactorAuthAfterFactorCompletion from recipe interface and make them helper functions
  • use the overwriteSession flag in session to determine whether to call createNewSession or not instead of handling it inside
  • in createOrUpdateSessionForMultifactorAuthAfterFactorCompletion implementation, recurse fetching sessionUser if we get RECIPE_USER_ID_ALREADY_LINKED_WITH_PRIMARY_USER_ID_ERROR while calling createPrimaryUser. also do this for INPUT_USER_IS_NOT_A_PRIMARY_USER while linking accounts
  • add contact support error when RECIPE_USER_ID_ALREADY_LINKED_WITH_ANOTHER_PRIMARY_USER_ID_ERROR happens while linkingAccounts.
  • Check all new status codes and if they display right
  • Update phone-password example to use MFA
  • Update create-supertokens-app (maybe?)
  • Add an example that allows users to select an email address to use if they have multiple ones (later)
  • Re-check style customization
  • Re-check Changelog
  • Check if we need to remove the completed in another tab screen from OTP MFA (merge issue)
  • Update tests after BE behaviour changes (to default requirements) are finalized
  • Multitenancy
    • createOrUpdate tenant,app, cud
      • totpEnabled boolean
      • firstFactors & defaultMFARequirements
    • getTenant API should return firstFactors and defaultMFARequirements
  • MFA related
    • new account linking APIs required for MFA
    • remove factor enabled related changes
    • paid user stats
      • MFA related
      • TOTP related
      • TODO in inmemory
  • MFA license checks in
    • createOrUpdate tenant, app, cud if firstFactors or defaultMFARequirements are set & non empty
    • new account linking APIs
    • if totpEnabled is true in tenant/app/cud creation
    • in the TOTP APIs
  • remove TOTP feature and related checks
  • Auth recipes
    • Sign in / up APIs should return if the corresponding factor was present in firstFactors config for that tenant
  • Add tests with 1 million users in the core
  • New structure for firstFactors in tenant creation
  • Remove plugin interface for MFA
  • Add API to import TOTP device
  • Add created_at for devices and probably add clean up of unverified devices later on
  • Add core version to logging
  • See the unnecessary changes in core and revert them. eg. isValidFirstFactor in signInUp APIs
  • In docs, add a link to storybooks (https://6571be2867f75556541fde98-xieqfaxuuo.chromatic.com/?path=/story/auth-page--playground).
  • Migration docs should take content from: https://supertokens.slack.com/archives/C051BN9QJUX/p1702061857434489
  • Core 1 million user test
  • IsFactorSetup should return false if fake emails are used in third party (otp-email) - based on fake email format
  • https://supertokens.slack.com/archives/C051BN9QJUX/p1702562017159699
  • https://supertokens.slack.com/archives/C051BN9QJUX/p1702556911972539
  • Add feature in license key in backend API
  • supertokens.com Dashboard checklist doesn't make too much sense anymore
  • Allow MFA to be enabled via the supertokens.com dashboard.
  • add mfa example in create-supertokens-app CLI.
  • https://supertokens.slack.com/archives/C051BN9QJUX/p1701777966569869
  • All checklist items here: MFA docs docs#763 (comment)
  • https://supertokens.slack.com/archives/C051BN9QJUX/p1702814144389259
  • Add MFA examples to angular and vue
  • add these validations in core - TOTP / MFA login discussions #554 (comment)
  • finalize on MFA flows considering overwrite session flag
  • Update getLoginMethods to use firstFactors info. ensure it is tested for all possibilities
  • core changes
    • change defaultRequiredFactordIds to requiredSecondaryFactors
    • remove totpEnabled
    • for known factor IDs in firstFactors and requiredSecondaryFactors array, we should only allow them in the array if the factor is enabled in the core for that tenant, else we should throw a 400 error.
    • firstFactors & requiredSecondaryFactors cannot be an empty array, instead, we can tell users to set it to null to remove it.
    • update list tenant and get tenant to return first factor and secondary factors (undefined if empty)
  • Change https://github.com/supertokens/supertokens-auth-react/tree/master/examples/with-emailverification-with-otp to use passwordless otp instead of doing the custom hack
    • Need to manual test after BE has been updated
  • Update auth react SDK for deciding if we should ask for email or not in the factor screen to be:
(emails.length > 0 && query param has setup (is missing || is false)) -> call createCode
(emails.length === 0 || query param has setup=true) -> show enter email screen
  • On the pre built UI sdk, need to make sure that the redirect to path is respected along with its query params.
    • Need to run the test after BE has been updated
  • Auto init user metadata recipe in the backend sdk
  • Remove totpEnabled from tenant config in the core
  • Validation check for tenantConfig when creating / updating tenant. For example, if firstFactors or requiredSecondaryFactors contains email-otp, then passwordless must be enabled.
  • Please tell me points of failures:
    • I do MFA.init with no license key in the core
    • I do MFA.init with just account linking license key
    • I do Accountlinking.init with MFA license key license key in the core
    • I do Accountlinking.init with no license key in the core
  • Backend should throw if firstFactors is an empty array
  • Login methods api should return firstFactors along with other recipe stuff - first factors should be from tenant config if defined else static config (filter based on enabled recipes for the tenant) if defined, else return undefined
  • Frontent changes for login method based on this - https://supertokens.slack.com/archives/C051BN9QJUX/p1702990830888489
  • In backend, add tests for recursion points of MFA
    • especially when a new user created becomes a different primary user, we should return 011 and not 010
  • Change allOf to allIfInAnyOrder in the backend SDK.
  • Create an example for step up auth in the settings page before a factor setup is allowed.
  • Add access-denied special factor id + change the way n array is computed to take into account the claims that cannot be currently setup. We decided not to do this cause of: https://supertokens.slack.com/archives/C051BN9QJUX/p1703693273706539?thread_ts=1703668285.242759&cid=C051BN9QJUX
  • Sign in up APIs can return 403 (from isAllowedToSetup validator), check for it's impact on the frontend SDK
  • Update FDI for 403 errors
  • given the new set of changes, please tell me the number of requests to the core per sign in / up API call with different stuff enabled (like account linking enabled vs disabled, mfa enabled vs disabled, both enabled vs disabled)
  • INVALID_FIRST_FACTOR - add support case
  • CANNOT_LINK_FACTOR_ACCOUNT - make it a separate support case
  • how to determine factorId for validation in createCodePOST?
    • We'll do this by adding a factorIds parameter to createCodePOST
  • Move "account linking to existing session" to the account linking recipe https://supertokens.slack.com/archives/C051BN9QJUX/p1704732840982139
  • Modify example app of account linking where we link third party / password accounts post sign in to just call the pre built APIs, and not have to create manual APIs.
  • Frontend UI for the below should have the back button instead of the choose another factor button:
  • SOMETHING_WENT_WRONG_ERROR in access denied screen should mention that we need to reload.
  • add a test case : discord -> email-otp -> emailverification. Here, the user will have to use their own email for email-otp and not the discord email (in case it's fake), and the email verification claim should pass cause it treats fake emails as verified anyway
  • CHeck the number of core api calls for sign in / sign up with mfa / account linking enabled / disabled.
  • Add events for totp recipe.
  • Add a prop called isFactorFactor in the SUCCESS event for all auth recipes on the frontend.
  • Change the title on this screen: https://6571be2867f75556541fde98-xieqfaxuuo.chromatic.com/?path=/story/totp-mfa--device-setup-with-single-next-option from “Enable TOTP” to “Add a TOTP device”. Cause this screen is reused when creating a new TOTP device.
  • Update http://localhost:3000/docs/session/common-customizations/sessions/jwt-signing-key-rotation to remove the current warning, and instead say that updating will cause a spike in refresh api calls.
  • Update docs for claim validators to have the new access token payload arg in fetchValue function.
  • Ensure next js test doesn't run on node 16.
  • Also add parallel tests in CICD to test with node 16 and node 18 in parallel. Node 18 should run the next js tests.
  • Change the frontend to treat thirdparty factor id as a custom factor.
  • in sign up API in core for emailpassword, we need to mark email as verified if it's a fake email. We also need to prevent sign in for first factor in emailpassword if it's a fake email.
  • Changing how fake emails are generated and checked for in all SDKs.
  • In core, update consume code API to accept createNewRecipeUser optional boolean (true by default)
  • Change account linking example app in auth-react repo to remove the manual apis for adding new third party to an existing user.
  • Include configured firstFactors and requiredSecondaryFactors in multitenancy stats
  • See how many core calls will happen from older session without MFA claim and when that is used during a MGAClaim validator.
  • Make fake email as verified in core for email password sign up
  • Make sure SDK is backwards compatible with older FDI version
  • Add proper description to with-multifactorauth-recovery-codes example app
  • Change the backup code demo app to take the user to the generate backup code screen rigth after sign up or when they have already used their recovery code and created a new device.
  • remove factor id input from create code API of passwordless recipe and also from the frontend functions
  • we should do session account linking in sign in recipe function as well
  • backwards compatibility needs to be there for frontend and backend sdks
  • We should always attempt account linking in sign in recipe functions, similar to how we do in sign up recipe functions
  • We will create a new recipe function called verifyCredentials in emailpassword based recipes and also expose that. That will only call the core to verify credentials and not do account linking. Also, this function will be called by the signIn recipe function internally (as opposed to it calling the core)
  • Remove the shouldAttemptAccountLinkingIfAllowed input from everywhere.
  • in email password recipe in core, if the email is a fake one, we mark it as verified in the sign up api
  • remove the verifyCode after it is implemented in the core
  • remove active user in account linking
  • see all side efffects of GET APIS in core and the effect of caching of it from the backend sdk
  • Core needs to merge with 8.0 branch, and make sure it enforces public tenant in certain APIs ONLY if the CDI version is >= 5.0
  • add index on usermetadata table or fixing the populate useridmapping or create the missing index for useridmapping table
  • changelog and db migration script for core, plugins
@rishabhpoddar rishabhpoddar added the enhancement New feature or request label Jan 12, 2023
@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 12, 2023

SQL Table schema

CREATE TABLE IF NOT EXISTS totp_user_devices (
   user_id VARCHAR(128) NOT NULL,
   device_name VARCHAR(256) NOT NULL,
   secret_key VARCHAR(256) NOT NULL,
   period INTEGER NOT NULL,
   skew INTEGER NOT NULL,
   verified BOOLEAN NOT NULL DEFAULT FALSE,
   PRIMARY KEY (user_id, device_name)
);
 
 
-- This table needs to be cleared regularly (once an hour).
-- This table also contains codes that the user entered which did not succeed so that we can implement brute force protection
CREATE TABLE IF NOT EXISTS totp_used_codes (
   user_id VARCHAR(128) NOT NULL,
   code VARCHAR(6) NOT NULL,
   is_valid_code BOOLEAN NOT NULL,
   expiry_time BIGINT NOT NULL, -- 90 seconds in the future
   PRIMARY KEY (user_id, code)
   FOREIGN KEY (user_id) REFERENCES totp_user_devices(user_id) ON DELETE CASCADE
);
  • if there are no verified devices and the allowUnverifiedDevice boolean is false in the verify API, we do not insert the tried code into the totp_used_codes table cause we didn't reach a point of even verifying the code.

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 12, 2023

Core APIs spec

POST /totp/device

Input body

{
 "deviceName": "My Authy Account",
 "userId": "UUIID of first factor (but it can be any string)",
 "skew": "integer",
 "period": "integer"
}

Output body

[{
 "status": "OK",
 "secret": "<32char-base32-random-secret>",
},
{
 "status": "DEVICE_ALREADY_EXISTS_ERROR"
}]

POST /totp/verify

Input body

{
 "userId": "UUIID of first factor (but it can be any string)",
 "totp": "123456",
 "allowUnverifiedDevice": "bool"
}

Output body

{
 "status": "OK | INVALID_TOTP_ERROR | TOTP_NOT_ENABLED_ERROR | LIMIT_REACHED_ERROR"
}
  • If there are no verified devices, we still return INVALID_TOTP_ERROR

POST /totp/device/verify

Input body

{
 "userId": "UUIID of first factor (but it can be any string)",
 "deviceName": "My Authy Account",
 "totp": "123456"
}

Output body

[{
 "status": "OK",
 "deviceWasAlreadyVerified": "bool",
},
{
 "status": "INVALID_TOTP_ERROR | TOTP_NOT_ENABLED_ERROR | UNKNOWN_DEVICE_ERROR"
}]

POST /totp/device/remove

Input body

{
 "userId": "UUIID of first factor (but it can be any string)",
 "deviceName": "str"
}

Output body

{
 "status": "OK",
 "didDeviceExist": "bool",
} | {
 "status": "TOTP_NOT_ENABLED_ERROR"
}  

PUT /totp/device

Input body

{
 "userId": "UUIID of first factor (but it can be any string)",
 "existingDeviceName": "str",
 "newDeviceName": "str",
}

Output body

{
 "status": "OK | TOTP_NOT_ENABLED_ERROR | DEVICE_ALREADY_EXISTS_ERROR | UNKNOWN_DEVICE_ERROR"
}

GET /totp/device/list

Input body

{
 "userId": "UUIID of first factor (but it can be any string)"
}

Output body

[{
 "status": "OK",
 "devices": [ 
   {"name": "<device-1-name>", "verified": "bool"},
   {"name": "<device-2-name>", "verified": "bool"},
  ]
}, {
 "status": "TOTP_NOT_ENABLED_ERROR"
}]

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 12, 2023

Core API logic flow

POST /totp/device

  • Validate input
    • verify that deviceName and userId are strings and that they are not empty
    • verify that period and skew are integers and 30 <= period < 90 and 0 <= skew <= 2
  • Generate secretKey for user. It should be a random 32char base32 string.
  • Query to insert userId and deviceId into totp_user_devices table.
  • If query fails because of unique constraints on (user_id, device_name), respond with status: DEVICE_ALREADY_EXISTS_ERROR
  • If succeeds, return status: OK
INSERT INTO totp_user_devices (user_id, device_name, secret_key period, skew, verified) 
VALUES (${userId}, ${device_name}, ${secretKey}, ${period}, ${skew}, False);

Notes:

  • Don't change period unless really required. It will make the auth incompatible with most TOTP auth apps (Authy, Google Auth, etc)

POST /totp/verify

  • Validate input
    • verify that userId is string, totp is integer, and allowUnverifiedDevice is bool.
  • Query the database to get all the devices and their secrets for the user.
  • If no device is found, return status: TOTP_NOT_ENABLED_ERROR.
  • If allowUnverifiedDevice is False, remove devices where verified is False. If no device remains after this step, return status: INVALID_TOTP_ERROR
  • Check totp_used_codes and count attempts (agg. sum across all devices for given user id) within last period + (period * skew * 2) seconds. If there are more than 5 (subject to change) entries with is_valid_code False, return status: LIMIT_REACHED_ERROR
  • Use some library to implement TOTP verification algorithm.
  • Run the TOTP verification algorithm for all the devices. Compare against multiple TOTPs based on skew/period.
  • Return status: OK if any of the devices succeed. Otherwise, status: INVALID_TOTP_ERROR
SELECT device_name, secret_key, period, skew, verified FROM totp_user_devices
WHERE user_id = ${userId};

Notes:

  • The default values are a period of 30 and a skew of 1. It is highly recommended you do not change these unless you wish to set skew to 0 (more security but might cause poor user experience)

POST /totp/device/verify

  • Validate input
    • verify that userId & deviceName are strings and totp is integer.
  • Query the database to all the devices for the user.
  • If no device is found, return status: TOTP_NOT_ENABLED_ERROR.
  • If devices are found but none matches the provided device name return status: UNKNOWN_DEVICE_ERROR
  • If device is found and is already verified return {status: OK, deviceWasAlreadyVerified: True}
  • If device is found and is not verified yet, use totp algorithm to check the code and return {status: OK, deviceWasAlreadyVerified: False} if it succeeds.
  • If verification fails, return status: INVALID_TOTP_ERROR
-- Fetch secret of given (user, device)
SELECT device_name, secret_key, period, skew, verified FROM totp_user_devices
WHERE user_id = ${userId} AND device_name = ${deviceName};

-- Mark the device as verified
UPDATE totp_user_devices
SET verified = True
WHERE user_id = ${userId} AND device_name = ${deviceName}

POST /totp/device/remove

  • Validate input
    • Verify that userId & deviceName (Note that SDKs will make seperate query to check that provided TOTP is valid)
  • Query the database to see all the devices that exists for the user.
  • If no device is found, return status: TOTP_NOT_ENABLED_ERROR.
  • If devices are found but doesn't match the provided deviceName, return {status: OK, didDeviceExist: False}
  • If device is matched, delete it and return {status: OK, didDeviceExist: True}

few points:

  • This API doesn't care whether the device is verified or not.
  • Deleting all the devices would mean that TOTP gets disabled.
-- Fetch device_names for given user.
SELECT device_name FROM totp_user_devices
WHERE user_id = ${userId};

-- Delete the device
DELETE totp_user_devices
WHERE user_id = ${userId} AND device_name = ${deviceName};

PUT /totp/device

  • Validate input
    • Verify that userId, existingDeviceName & newDeviceName are string
  • Query the database to see all the devices that exists for the user.
  • If no device is found, return status: TOTP_NOT_ENABLED_ERROR.
  • If devices are found but doesn't match the provided oldDeviceName, return status: UNKNOWN_DEVICE_ERROR
  • If device is matched, update its name. If succeeds, return status: OK
  • If update fails because of UNIQUE constraint on (user_id, device_name) pair. return status: DEVICE_ALREADY_EXISTS_ERROR
-- Fetch device_names for given user.
SELECT device_name FROM totp_user_devices
WHERE user_id = ${userId};

-- Delete the device
UPDATE totp_user_devices
SET device_name = ${newDeviceName}
WHERE user_id = ${userId} AND device_name = ${oldDeviceName};

Notes:

  • Not allowing users to modify skew/period for the device. They need to delete and create a new device.

GET /totp/device/list

  • Validate input
    • Verify that userId is a string
  • Query the database to list all the devices that exists for the user.
  • If no device is found, return status: TOTP_NOT_ENABLED_ERROR.
  • Otherwise return {status: OK, devices: [{name: device1Name, verified: bool}, ...]}
-- Fetch all devices for given user.
SELECT device_name, verified FROM totp_user_devices
WHERE user_id = ${userId};

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 12, 2023

Backend SDK API interface (node js)

export type TOTPAPIInterface = {
    // Skew and Period values are taken from the config not the user.
    createNewDevicePOST?: ((input: { session: SessionContainer; deviceName: string}) => Promise<
        { status: "OK", secret: string } | { status: "DEVICE_ALREADY_EXISTS_ERROR" }
    >);
   // allowUnverifiedDevice is taken from config not the user
    verifyTotpPOST?: ((input: { session: SessionContainer; totp: string }) => Promise<
        { status: "OK" | "INVALID_TOTP_ERROR" | "TOTP_NOT_ENABLED_ERROR" | "LIMIT_REACHED_ERROR" }
    >);
    verifyDevicePOST?: ((input: { session: SessionContainer; deviceName: string; totp: string }) => Promise<
        { status: "OK"; "deviceWasAlreadyVerified": boolean } | { status: "UNKNOWN_DEVICE_ERROR" | "INVALID_TOTP_ERROR" | "TOTP_NOT_ENABLED_ERROR" }
    >);
    removeTOTPDevicePOST?: ((input: { session: SessionContainer; deviceName: string }) => Promise<
        { status: "OK", "didDeviceExist": boolean } | { status: "TOTP_NOT_ENABLED_ERROR" }
    >);
  updateTOTPDevicePUT?: ((input: { session: SessionContainer; existingDeviceName: string; newDeviceName: string; userContext: any }) => Promise<
        { status: "OK" } | { status: "TOTP_NOT_ENABLED_ERROR" | "UNKNOWN_DEVICE_ERROR" }
    >);
    listTOTPDevicesGET?: ((input: { session: SessionContainer; }) => Promise<
        { status: "OK", devices: { name: string, verified: boolean }[] } | { status: "TOTP_NOT_ENABLED_ERROR" }
    >);
}

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 12, 2023

Backend SDK recipe interface (node js)

export type TOTPRecipeInterface = {
    createNewDevice(input: { userId: string; deviceName: string; userContext: any }): Promise<
        { status: "OK", secret: string } | { status: "DEVICE_ALREADY_EXISTS_ERROR" }
    >;
    verifyTotp(input: { userId: string; totp: string; userContext: any }): Promise<
        { status: "OK" } | { status: "INVALID_TOTP_ERROR" | "TOTP_NOT_ENABLED_ERROR" }
    >;
    verifyDevice(input: { userId: string; deviceName: string; totp: string; userContext: any }): Promise<
        { status: "OK"; "deviceWasAlreadyVerified": boolean } | { status: "UNKNOWN_DEVICE_ERROR" | "INVALID_TOTP_ERROR" | "TOTP_NOT_ENABLED_ERROR" }
    >;
    removeDevice(input: { userId: string; deviceName: string; userContext: any }): Promise<
        { status: "OK", "didDeviceExist": boolean } | { status: "TOTP_NOT_ENABLED_ERROR" }
    >;
    updateDevice(input: { userId: string; existingDeviceName: string; newDeviceName: string; userContext: any }): Promise<
        { status: "OK" } | { status: "TOTP_NOT_ENABLED_ERROR" | "DEVICE_ALREADY_EXISTS_ERROR" | "UNKNOWN_DEVICE_ERROR" }
    >;
    listDevices(input: { userId: string; userContext: any }): Promise<
        { status: "OK", devices: { name: string, verified: boolean }[] } | { status: "TOTP_NOT_ENABLED_ERROR" }
    >;   
}

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 16, 2023

TODO

  • Generate secretKey for user. It should be a random 32char base32 string.
    • Confirm 32 is fine.
  • Confirm that 90 seconds should be the time window for a code or something else?
  • Change verify API to also check for brute force issue.
  • /totp/device/verify also needs to account for device already verified state
  • modify /totp/device/remove to not return unknown_device_error cause we already have a boolean in the OK status for this
  • update POST /totp/device/update path and logic
  • update GET /totp/devices path
  • Change UNKNOWN_USER_ID_ERROR to TOTP_NOT_ENABLED_ERROR everywhere
  • What is the removing device flow for other auth providers?
    • We can keep it simple by default where only a session is required, but we want to enable users to be able to customise it to add:
      • totp verification before removing device (how can users use override to achieve this?)
      • step up auth using session claims? (how will the user tell our API's session verification that this needs to be added to the check?)
  • Make sure all the names / paths across all the comments are consistent

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 16, 2023

TODO for 2fa in general

  • Account for other 2fa types:
    • Email OTP / magic link (passwordless recipe)
    • SMS OTP / magic link (passwordless recipe)
    • TOTP (totp recipe)
    • other custom method that we don't support
  • Allowing developers to specify the 2fa that the app needs
  • Allowing developers to specify 2fa that each user has enabled
  • How this ties into 2fa session claim?

How is a factor defined?

  • It has a unique string representing a type of factor. We have a few inbuilt factors:

    • "totp"
    • "thirdparty"
    • "emailpassword"
    • "passwordless"
    • We also allow users to define their own method..? TODO
  • requireFreshSince integer (decided to make it so that user can customise their way through this)

    • This represents the amount of time after which the factor needs to be re asked. For example, if the user has finished totp in second factor now, and then totp is specified in the 4th factor as well, it won't be asked later as long as it's already done within 3600 seconds.
  • name (string)

    • Used by the frontend to display the name of this factor when showing options.
    • This is optional in the input that the user gives in the factors function, but normalised version is not optional.
recipeList: [
   MultiFactorAuth.init({
      factors: [
            {id: "thirdparty", order: 1}, {id: "emailpassword", order: 1}, 
            {id: "totp", order: 2}, {id: "passwordless", order: 2},
            {id: "totp", order: 3},
      ]
   })
]
  • If there are multiple factors on the same level, it means that any one of them can work
  • In the above example, if the user has finished totp in the second factor, then the 3rd factor will not ask totp to the user.
  • The UI for the first factor should pick the thirdpartyemailpassword recipe.
  • During sign up, the second factor will show options to the user (setup totp or email otp), and whichever they pick, will be enabled for them. Then later on in sign in, we will directly show them the enabled one. If the user has later on somehow enabled both the factors (for the second factor), then the sign in flow will ask the user for an option to pick which factor.
  • Constraints on the array:
    • totp cannot be first factor

Defining a custom factor

recipeList: [
   Passwordless.init({
      flowType: "email_or_phone"
   }),
   MultiFactorAuth.init({
      factors: [
            {id: "passwordless", order: 1},
            {id: "biometric", name: "Biometric", order: 2},
      ]
   })
]
  • The only thing multi factor recipe will care about is that the id biometric is there in the session claim for 2fa.

Reusing a current recipe multiple times (decided not to do this now)

recipeList: [
   Passwordless.init({
      flowType: "email_or_phone"
   }),
   Passwordless.init({
      flowType: "phone",
      rid: "custom"
   }),
   MultiFactorAuth.init({
      factors: [
            {id: "passwordless", order: 1},
            {id: "passwordless", name: "SMS OTP", rid: "custom", order: 2},
       ]
   })
]
  • The idea here is that we allow each factor to point to a rid which will enable users to specify the same recipe multiple times in the recipe list (as shown above). Here we have two Passwordless recipes, one with email_or_phone (which will be on the first factor) and one with only phone which points to the second factor (custom rid)

Recipe function to get factors

There needs to be a recipe function which determines what the next factor to apply is. This is overridable by the user:

getFactors: (session?: SessionContainer, userContext) => {
   if (session !== undefined) {
      // first query core to get factors based on user id (SELECT * FROM mfa_factor WHERE user_id = <user-id>;)
      // if found, update the result to have the first factor from calling config.factors and return that.
   }
   return config.factors;
}

2fa session claim structure

Need to capture:

  • How many factors are completed
  • How many factors are not completed
  • For each factor:
    • id of the factor
    • if factor is completed or not
    • time it was completed was last updated.
    • order of this factor
{
   "st-mfa": {
      "v": {
         id: "...",
         c: "..." // "c" for completed with a boolean value
         t: "...", // time this factor was completed
         o: number // factor order
      }[]
   }
}
  • Alt: Make v {'factor_id': {c: bool, t: int, o: int}} (i.e. dict instead of list). This makes it easier for devs to access a factor's attrs.

fetch_value implementation for st-mfa claim:

  • Will call the getFactors function.
  • Transform the data structure to the claim type.

2fa session claim validators

  • hasCompletedAllFactors(toCheckFactorIds?: string[])
    • this is to be added to the global validators as well.
    • TODO: What happens if I pass in an id which is not configured for the user at all.
  • hasCompletedFactorWithinTime(factorId: string, time: number)

Step up auth

  • The dev can use hasCompletedFactorWithinTime validator to achieve this. If they want the user to always reauth, then they should put a low number.

How this ties in with email verification check

By default, we can make it so that the emailverification.isVerified validator is checked after the hasCompletedAllFactors validator - by having the isVerified validator AFTER the hasCompletedAllFactors in the global validator array. This would mean that email verification happens after all factors are completed.

If the user wants to change this order, then can shuffle the global claim validator array themselves. But is this a good way of doing it? TODO

Alt: We can use the factors config of MFA recipe to allow the user to decide the order of email verification.

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 19, 2023

Different user flows (TODO):

Register the user:

  • Let's take the example of TPEP recipe, its signup_post func calls emailpassword.signup_post func.
  • Now emailpassword.recipe_implementation.sign_up calls create_new_session so the new session will contain the MFAClaim (value fetched based on getFactors func). Here's how the claim structure would look like:
{
    "st-mfa": {
        "v": [{
            id: "emailpassword",
            c: True,
            t: 123,
            o: 1
        },
        {
            id: "thirdparty",
            c: False,
            t: 0,
            o: 1
        },
        {
            id: "totp",
            c: False,
            t: 0,
            o: 2,
        },
        ]
    }
}
  • Now, 2nd factor(s) are present in the MFA claim but the c value is False (i.e. not completed). Now we query the core to see if user(id) has any TOTP registration attached or not. If not, we redirect to the TOTP device registration page. If TOTP device verification is required, users need to do that first and then only the TOTP entry of the MFA claim will be marked as completed.
  • Similarly, if there are more factors, we can redirect to respective pages one by one for each layer. If multiple factors are valid for a level/order allow any of them.

User adds a MFA (say TOTP) after registration:

First factor:

  • Already completed. They are signed in.

Second factor:

  1. User clicks something that's equivalent of a "Enable TOTP" button.
  2. Frontend calls POST /auth/mfa/factor API of Backend SDK with totp: True. It checks if TOTP is allowed by the dev based on MFA getFactors().
  3. If allowed, SDK calls core API POST /mfa/factor with user_id and factor order info (here order: 2). Core adds an entry for the user in mfa_factor table.
  4. Backend refetches the mfaClaim using fetchAndSetClaim.
  5. Frontend gets back OK response and then it now asks the backend to query core for whether the user(id) has a TOTP device.
  6. Assuming no existing TOTP devices for the user, core replies to the backend that no device is registered for the user(id). So now the backend will create a new request to the core to register a TOTP device.
  7. This attaches user(id) with a new TOTP device entry and returns a URI that gets converted to QR code by the frontend.
  8. Assume TOTP device verification is required and user enters that via the input box. Backend will verify the TOTP using POST /totp/device/verify API call to the core. If it succeeds, backend updates the TOTP entry of the MFA claim value to be c: True and assigns t the time of completion.

Logging in a user:

First factor:

  1. User enters valid email and password. Server replies back with session tokens that contain MFA claim.
  2. access_token_payload contains all the factors based on getFactors(). Only emailpassword entry has c: True. However, entry with id: totp will have c: False.

2nd factor:

  1. 2nd factor has only one option in this example i.e. TOTP. So we query the core to find if a device is attached to the user(id).
  2. If so, redirect the user to TOTP verification page. User enters TOTP. If it's correct. Mark the TOTP factor as c: True and set its t value.
  3. If not, prompt the user to register a TOTP device first (provided TOTP factor isn't optional)

Validating if a certain factor has been completed:

Use mfaClaim.validators.hasCompletedAllFactors(["totp"])

Accessing any protected API:

  1. mfaClaim.validators.hasCompletedAllFactors if called without any arg ensures that all atleast one factor from each order/level has been completed. This is added to global claim validators list. So, using verifySession will enforce this on any custom API.

Disabling a factor (say TOTP):

  1. User clicks something that's equivalent of a "Disable TOTP factor" button.
  2. Frontend calls backend API PUT /auth/mfa/factor with totp: False
  3. First the backend calls core API PUT /mfa/factor with totp: False.
    • If the provided factor (here TOTP) is the only factor in its order/level, decrement order of all factors in the higher orders and remove the factor itself from mfa_factor table
    • Otherwise if there are multiple in the same order/level, just remove the (TOTP) factor entry from mfa_factor table.
  4. Refetch MFA claim and update the session using fetch_and_set_claim.
  5. Note that TOTP devices corresponding to the user haven't been deleted here. TODO: Can this cause any issues?

Re-enabling a factor (say TOTP):

  1. User clicks something that's equivalent of a "Enable TOTP" button.
  2. Frontend calls backend API PUT /auth/mfa/factor with totp: True
  3. Backend checks the factor config to extract the order/level of the factor (here TOTP is should have order 2)
  4. Rest of the steps are same as User adds a MFA (say TOTP) after registration (starting from point 3)

@KShivendu
Copy link
Contributor

KShivendu commented Jan 25, 2023

SQL Table Schema:

CREATE TABLE IF NOT EXISTS mfa_factor (
    user_id VARCHAR(128) NOT NULL, -- Assume this is final user ID after account linking feature is completed.
    order INTEGER NOT NULL, -- Factor order. This is the position of the factor in the user's list of factors. There can be multiple factors at the same position.
    id VARCHAR(128) NOT NULL, -- Type of factor. Can be "totp", "email-otp", "sms-otp", etc. Alternatively can be called as factor_flow or factor_name
    PRIMARY KEY (user_id, order, id)
);
  • The reason for order and type in primary key is it if user wants to have multiple modes in same factor order. Example: TOTP as well as email-otp in 2nd factor.
  • This table should only store second factor onwards (cause we don't have the user ID before the first factor is completed). We can communicate this by throwing an error from the core if someone inserts into this table with order = 1.

@KShivendu
Copy link
Contributor

KShivendu commented Jan 25, 2023

Core API spec

  • Should we reuse usermetadata recipe/tables to store all the factor metadata?

POST /mfa/factor

{
 "user_id": "user_id",
 "factors": [
   [
    // [{"thirdpartyemailpassword": "tpep-user-id"}], // What will happen when account linking is enabled? // No benefit of providing first factor
    [{"totp": "enabled", "email-otp": "disabled"}],  // Just store that factor has been enabled/disabled/configured.
   ],
  ]
}
{
 "status": "OK"
}

GET /mfa/factor/list

Request body:

{
 "user_id": "<user-id-after-account-linking>"
}

Response body:

{
 "status": "OK",
 "factors": [
    [{"thirdpartyemailpassword": "enabled"}] 
    [{"totp": "enabled"}, {"email-otp": "enabled"}, ]
   ],
}

PUT /mfa/factor

Request body:

{
 "status": "OK",
 "factors": [
   {"totp": False}
  ]
}

Response body:

{
  "status": "OK | UNKNOWN_FACTOR_ERROR",
  "unknown_factor": "<factor>"
}
  • Updates enable column of a factor if it's already there. Otherwise, raises error.

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jan 27, 2023

Changes to passwordless recipe

  • Expose an API which will help the frontend know if it's been configured or not for this user ID.

Reuse passwordless.recipe_impl.get_user_by_id(). Create an API for that.

Changes to emailpassword

  • If this is used in step up auth, and if there is no emailpassword user for the logged in user, we will ask the user to create a password.

Reuse signup/email/exists API. Create new API. But will we ask for email as well? (along with password). If no, will anything break in DB/core?

Changes to thirdparty

  • If using this in step up auth, and if there is no thirdparty user, then you ask them to choose an account (and then that is linked)

Changes to totp

  • If not configured, we will ask the user to setup.

TODO:

  • We need to make this configurable -> can be possibly done via overrides in some way??
  • Think about this whole flow.

@KShivendu
Copy link
Contributor

KShivendu commented Feb 8, 2023

TODO:

  • Think if we really need validate_factors since it's kinda redundant for now. See if there's a real usecase for this.
  • Inserting factors dynamically using based on previous factor.

During sign in: when emailpassword factor is completed, we want to show the email verification screen in case the second factor is going to be TOTP. If the second factor is going to be passwordless, then we want to mark the email verification claim as completed automatically.
During sign up: we want to do emailpassword -> email verification -> totp -> access

  • TOTP don't ask next 30 days on this device

Cases:

1:

During sign in: when emailpassword factor is completed, we want to show the email verification screen if the second factor is going to be TOTP. If the second factor is going to be passwordless, then we want to mark the email verification claim as completed automatically.
During sign up: we want to do emailpassword -> email verification -> totp -> access

mfa.init(
  factors=[
   Factor("emailpassword", 1),
   Factor("totp", 2), Factor("pless", 2),
  ]
)

def completed_factor(session, factor, context):
    if factor == Factor("totp", 2):
        redirect("/email-verification-screen")
    if factor == Factor("pless", 2):
        EVClaim.complete()

def get_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    # all_factors = self.get_all_factors()
    if Factor("totp", 2) in completed_factors:
        redirect("/email-verification-screen")
    if Factor("passwordless", 2) in completed_factors:
        EVClaim.complete()

    return oi_get_factors(session, completed_factors, context)

# Only the factors that remain in get_factors() will be allowed in the access token payload. 

Alt:

mfa.init(factors=["emailpassword", "totp"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if Factor("emailpassword", 1) in completed_factors:
        return [Factor("totp", 2), Factor("pless", 2)] # When any of them is completed, we will remove the other one
    if Factor("totp", 2) in completed_factors:
        redirect("/email-verification-screen") # TODO: How will it redirect?
    if Factor("pless", 2) in completed_factors:
        EVClaim.complete()
        return []
    
    return oi_next_factors()

2:

User logs in -> email password -> totp -> checked on remember this device -> access
User logs in second time -> email password -> access (cause they clicked on rememeber this device)

mfa.init(factors=["emailpassword", "totp"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if completed_factors == [Factor("emailpassword", 1)] and context["flow"] == "login":
        device_exist = core.query()
        if device_exist:
           return []
    
    return oi_next_factors(session, completed_factors, context)

3:

Sign up: email password -> present a choice of totp or passwordless -> if totp is selected, then do that and email verification. If passwordless is selected, then do passwordless only.
Sign in: email password -> if totp is enabled, do totp, else do passwordless.

mfa.init(factors=["emailpassword", "passwordless"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if completed_factors == [Factor("emailpassword", 1)] and context["flow"] == "login":
          return [Factor("totp", 2), Factor("passwordless", 2)]
   if Factor("totp", 2) in completed_factors:
          redirect("/email-verification-redirect")
   return oi_next_factors(session, completed_factors, context)

4:

Sign up: email password -> passwordless -> step up auth (if totp is enabled, then use totp, else ask the user for their password)
Sign in: email password => if totp is enabled, use that, else give access

// TODO: INCOMPLETE 
mfa.init(factors=["emailpassword", "passwordless"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if Factor("passwordless", 2) in completed_factors and context["flow"] == "login":
          is_totp_enabled = core.query()
          if is_totp_enabled:
              return [Factor("totp", 3)]
          else:
              return [Factor("emailpassword", 3)]
   return oi_next_factors(session, completed_factors, context)

5:

Sign up: email password -> passwordless -> step up auth (if totp is enabled, then use totp, else ask the user for their password)
Sign in: email password => if totp is enabled, use that, else force user to enable totp and then give access

6:

Sign in: email password -> passwordless -> if passwordless was done using a new IP address then ask for TOTP else give access.

mfa.init(factors=["emailpassword", "passwordless"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if Factor("passwordless", 2) in completed_factors and context["flow"] == "login":
          is_different_ip = check_request_ip()
          if is_different_ip:
              return [Factor("totp", 3)]

   return oi_next_factors(session, completed_factors, context)

7:

sign in: Email password -> if logged in after 30 days -> do passwordless -> totp -> access. If sign in before 30 days -> totp -> passwordless

mfa.init(factors=["emailpassword"])

def next_factors(session, completed_factors: List[Factor], context) -> List[Factor]:
    if completed_factors == [Factor("emailpassword", 1)] and context["flow"] == "login":
          days_elasped_since_login = db.query()
          if days_elasped_since_login > 30:
              return [Factor("passwordless", 3), [Factor("totp", 3)]]
          else:
              return [Factor("passwordless", 3), [Factor("totp", 3)]]

   return oi_next_factors(session, completed_factors, context)

@KShivendu
Copy link
Contributor

KShivendu commented Feb 8, 2023

Including step up auth factors in config.factors and mark them as completed ?

@KShivendu
Copy link
Contributor

KShivendu commented Mar 6, 2023

factor[]:
[{id: string, order: number}]

This can come from:

  • config
  • user ID (DB)

db:

-- userId -> factor[]
CREATE TABLE mfa_factors(
   user_id VARCHAR(32),
   factor jsonb,
   PRIMARY KEY (user_id)
);
function nextFactor(session?, completedFactors?, factors, userContext) => string[]
  • session will be optional during first factor auth
  • completedFactors is from the session if the session is available
  • factors is from the static config, unless there is a mapping in the db from session.getUserId() to factor[]
  • origibal impl of this function will simply get the next factor from the factors array based on order and the last order in the completedFactors array

Structure of claim in access token:

    "st-mfa": {
        "v": {
            "id": string,
            "t": number,
            "completed": boolean,
            "order": number,
        }[]
    }

mfaclaim:

  • fetchValue
    • call nextFactor
  • completeFactorWithId
    • mark all the factors with id completed
  • completeFactorWithIdAndOrder
    • mark the factor with ID and order as completed
  • isCompletedWithId
  • isCompletedWithIdAndOrder

validators:

  • hasCompletedFactors (factor[]?)
    • if input is empty, it will check for all factors being completed
    • in this case, the function will call next_factor and see if that returns empty, then completed, else not completed.
  • hasCompletedFactorsWithinTime(factor[]?, time)
    • returns success if all factors in the input have been completed within time

————————
Problems:

  • nextFactor needs the factors for the user, but that requires a db call. So doing that on each API call is bad
  • Function body of validators is not clear.

@KShivendu
Copy link
Contributor

KShivendu commented Mar 23, 2023

Alternatives considered:

Alt 1

What if we dump all the factors from the DB/config in the first factor login itself. FE can just follow that order.

Edge cases:

  • Step-up auth: If step-up auth happens, hasCompletedFactorsWithinTime throws an error so FE redirects to the factor completion page.
  • Existing session: If I just updated factors in session 2, existing session 1 will not get affected. I can continue accessing routes that only have hasCompletedFactors with no args. However, if a factor is passed as arg, I must have completed it. Furthermore, if devs want they can just revoke previous sessions whenever factors are updated.
  • Dynamic factors: One can change the factor by overriding next_factors behavior. Say I want to ask for TOTP only if 30 days have passed since the last login.

Pros:

  • Very few DB calls.

Cons:

Soln to Problems:

  • Set up an expiry and do DB calls only once.
  • Store factors stored in DB in the session payload during auth. Later on just remove them?

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Oct 5, 2023

About first factor login:

Tenant creation:

  • During tenant creation, we will specify which will be the first factors:
{
    "tenantId": "customer1",
    "thirdPartyEnabled": true,
    "emailPasswordEnabled": true,
    "firstFactors": ['f1', 'f2', ...],
    "defaultRequiredFactorsForUser": ['f1', 'f2', ...],
  • So the core will check that for each fN in the firstFactors list, if it's a known fN (for example emailpassword, or thirdparty), then that must also be enabled for this tenant. Otherwise send a 400 error. The list of known factors is NOT limited the recipe id - for ex, there is otp-phone which is fulfilled by the passwordless recipe, and in fact, there is no passwordless as a factor at all (see comment below). For unknown fN (like biometric), we just let it be as part of the first_factors and don't do anything with it.
  • firstFactors is optional argument, and by default it will be null in the db, which is != empty array. An empty array should just be treated as no first factors which is useless, but it's fine.
  • We also add an totpEnabled boolean which is similar to enabled booleans. We check for this in the create device, verify device and verify code APIs for the TOTP recipe, on a tenant level (even though some of these apis are app specific).
  • defaultRequiredFactorsForUser is optional, and by default it will be an empty array (or null, which means the same thing). It represents the input to the getMFARequirementsForAuth function.

Tenant info fetching

  • When we get info for a tenant, we need to also communicate the firstfactors array. We have to distinguish if that tenant does not have the firstFactors configured vs it has it configured, but just a sub set of the enabled methods. This can be easily done by returning an optional firstFactors in the tenant info.

If frontend uses dynamic login method: true

  • The frontend will call getLoginMethods API. This API will get the info from the tenant.
    • If the tenant has a configured firstFactors array (even if it's empty), it will return that
    • If there is nothing configured on the core, it will check if the user has defined the firstFactors array in the typeInput of the backend SDK, and if that's there, we will return the intersection of that and the enabled login methods for the tenant.
    • If the backend sdk's firstFactor is also not defined, then we return the auth recipes that are enabled for this tenant.

If frontend does not use dynamic login methods:

  • The user has to configure the firstFactors array on the frontend and backend.
  • If they do not configure it (on frontend or backend), you just treat it as if all auth recipes that are initialised are first factors).

How will the backend sign in / up APis know about the first factor?

  • The sign in / up APis from the core will return if that has the first factor config enabled for it or not, for that tenant. it is important to distinguish not configured vs it not being a first factor.
  • For example, the email password sign up API from the core can return:
{
  "status": "OK",
  "user": {...},
  "firstFactorConfig"?: boolean
  • If firstFactorConfig is missing, it means that there is no firstfactor configuration for that tenant, otherwise we use the boolean.
  • If it's not configured on the core, we use the firstFactor from the backend's typeInput, else, we just assume it as true.

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Oct 6, 2023

About passwordless factor ID

Instead of having factor id for passwordless where we have "otp-mobile" etc.. We do NOT have factor ID of passwordless. The config for passwordless.init on the frontend and backend need to be a super set of all the factor IDs that getMFARequirementsForAuth can return.

@porcellus
Copy link
Contributor

porcellus commented Oct 6, 2023

TODO

Planned return values for the default implementations of getMFARequirementsForAuth, getMFARequirementsForFactorSetup

Initialized recipes First factors Linked accounts (login methods) Has TOTP device getMFARequirementsForAuth(oneOf) getMFARequirementsForFactorSetup(oneOf)
TP, EP, TOTP undefined TP No [totp] []
TP, EP, TOTP undefined TP Yes [totp] [totp]
TP, EP, TOTP undefined TP, EP No [totp] []
TP, EP, TOTP undefined TP, EP Yes [totp] [totp]
TP, EP, TOTP TP TP, EP No [totp, emailpassword] [emailpassword, totp]
TP, EP, TOTP TP TP, EP Yes [totp, emailpassword, totp] [emailpassword, totp]

@porcellus
Copy link
Contributor

Factor ids associated with each recipe:

  • EmailPassword: emailpassword
  • ThirdParty: thirdparty
  • Passwordless: otp-phone, otp-email, link-phone, link-email
  • TOTP: totp
  • Other non-auth recipes do not have any factorIds associated with them.
  • Combination recipes: the union of the "sub-recipes"

@porcellus
Copy link
Contributor

porcellus commented Nov 21, 2023

Meeting notes:

  • we'll change the behaviour of sign in/up APIs to NOT overwrite the session if one already exists, but still return success (status: "OK" )
  • apps will be able to revert to the old behaviour through a flag added to the session recipe (overwriteSessionDuringSignIn)
  • we will only allow factor setup when signing up (i.e.: do not link pre-existing accounts through the MFA recipe)
  • Just to clarify, if someone calls the createNewSession manually whilst they already have a session, that should override the current one.

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Nov 23, 2023

EDIT: 23rd November, 2023

For the default implementation of isAllowedToSetupFactor:

  • if the next array is empty, then we should allow
  • if not and any of the items in the array are set up, then you disallow.

@porcellus
Copy link
Contributor

Implementing recovery codes (will be added as part of an example app):

  1. Add API endpoints/UI to create/manage recovery codes (e.g.: save these into user metadata)
  2. Add an endpoint that'll verify&save a recovery code into the session payload
  3. Override isAllowedToSetupFactor to check if the access token payload has a valid recovery code.
  4. Override factor setup to also remove the recovery code from the DB/user metadata
  5. Implement a UI flow to reset:
    1. Allow inputting the recovery code
    2. After the recovery code is verified redirect to the factor chooser (or directly to a factor setup page)
    3. Each factor can be forced into "setup mode" by adding ?setup=true to the URL or by overriding
    4. getFactorsSetUpByUser to return empty if the user has a recovery code in the session.
  6. Override UI components to add links pointing to the entry point of the above reset flow

@porcellus
Copy link
Contributor

porcellus commented Dec 7, 2023

All TODOS have been moved from here to the main list

@sattvikc
Copy link
Collaborator

sattvikc commented Dec 8, 2023

BE TODOs:

  • Change type of userContext to Record<string, any>
  • MFA tests

@sattvikc
Copy link
Collaborator

sattvikc commented Dec 15, 2023

Flow for factor login: https://supertokens.slack.com/archives/C051BN9QJUX/p1702645397935389

  • no session -> normal operation
  • with session:
    • mfa disabled ->
      • if session overwrite is not allowed -> we don’t do auto account linking and no manual account linking (even if user has switched on automatic account linking)
        • the 2nd account already exists -> no op (do not change the db)
        • the 2nd account does not exist -> isSignUpAllowed -> create a recipe user
      • if session overwrite is allowed -> we ignore the input session and just do normal operation as if there was no input session.
    • mfa enabled -> we don’t do auto account linking (even if user has switched on automatic account linking)
      • the 2nd account already exists:
        • if user is already linked to first account -> modify session’s completed and next array
        • if user is not already linked to first account -> Contact support case (cause we can’t do account linking here cause the other account may have some info already in it, and we do not call shouldDoAutomaticAccountLinking function)
      • the 2nd account does not exist -> creating and linking (if linking is allowed, if not, we aren’t creating either + isAllowedToSetupFactor + (2nd factor is verification || login method with same email and its verified))
        • If linking is not allowed, we return a support status code
        • The code path should never use the session overwrite boolean in this case!

Logic for the above discussion:

  • mfa disabled
    • no session (normal operation)
      • sign in
        • recipe signIn
        • check isSignInAllowed
        • auto account linking
        • create session
        • return
      • sign up
        • check isSignUpAllowed
        • recipe signUp (with auto account linking)
        • create session
        • return
    • with session
      • sign in
        • recipe signIn
        • if overwriteSessionDuringSignInUp === true
          • check isSignInAllowed
          • auto account linking
          • create session
        • return
      • sign up
        • check isSignUpAllowed
        • if overwriteSessionDuringSignInUp === true
          • recipe signUp (with auto account linking)
          • create session
        • else
          • recipe signUp (without auto account linking)
        • return
  • mfa enabled
    • no session (normal operation + check for valid first factor + mark factor as complete)
      • sign in
        • recipe signIn
        • check isSignInAllowed
        • check if valid first factor
        • auto account linking
        • create session
        • mark factor as complete in session
        • return
      • sign up
        • check isSignUpAllowed
        • check if valid first factor
        • recipe signUp (with auto account linking)
        • create session
        • mark factor as complete in session
        • return
    • with session
      • sign in
        • recipe signIn
        • check isSignInAllowed
        • check if factor user is linked to session user (support code if failed)
        • mark factor as complete in session
        • return
      • sign up
        • check for matching verified email in session user (support code if failed)
        • check if allowed to setup (returns claim error if failed)
        • check if factor user can be linked to session user (if failed, support code / unauthorized)
        • recipe signUp (with auto account linking)
        • link factor user to session user (if failed, recurse or unauthorized)
        • create session
        • mark factor as complete in session
        • return

@porcellus
Copy link
Contributor

Notes from 24.01.02 discussion:
Let’s look at this use-case:

  • the user is required to do email verification before setting up otp-phone (let's say to lower sms sending costs)
  • They can do this by:
    • Overriding isAllowedToSetupFactor… on the BE
    • Adding the MFA validator after the EV validator on the FE (I need to double check, but this can happen based on recipe init order as well)

In this case:

  • If we simply filter the next array and only add whatever is available at that point in time:
    • the next array will be empty after an EP sign up
    • this would make requests using that session pass the default MFA claim validator which is definitely not the right thing to do
  • If we filter the next array and add a special (access-denied) factor:
    • The requests now fail the default MFA claim validator, which is the right thing
    • What updates the next array in the claim? EV endpoints should not care about the MFA claim so they can’t. We’d end up relying on people calling the MFA info endpoint once again.
    • Also we should avoid adding special cases like this if possible

What I propose instead:

  • We add a flag (v: boolean) to the MFA claim. This will explicitly flag if the session passes the default requirements or not, making it very easy to explain.
  • we can add a filtered next options array to the MFA info API response
  • we could drop the next array as we only really use it as a flag and as a way to provide hints about where to redirect/what to show on the chooser screen, but that can be achieved in other ways
    • Doing it without increasing the redirection and call-count will require a tiny bit of caching on the FE and/or making the assumption that if you navigated to the factor screen you are allowed to either complete or set it up and just rely on the initial API calls to tell us otherwise.

The way we can communicate implementation for custom UIs:

  1. Check if the flag is true
  2. If false, call MFA info to see what to do next:
    1. If the available next factors array is empty show an access denied screen
    2. if the available next factors array has a single item go there
    3. if the available next factors array has multiple items show a chooser

If someone wants to get smart and use the next array in the claim (if we choose to leave it there) they can do so, but that would not be the standard recommendation. Our implementation could still use it.

Changes:

  • Remove the next array from the claim and add the flag
  • Update the related validator to use the flag instead of the next array
  • Add a filtered (in next array && (in isAllowedToSetup || in isAlreadySetup) next array into the response of MFA info endpoint
  • Cache the MFA info response in the FE (we want to avoid adding the email/phone info into the query params and people were confused about using the hash before)
    • One possible solution to explore: cache the info in session storage + add cache key as a query param.
    • check how many requests we make when navigating to/refreshing a protected page, with and without a factor to redirect to
  • Update the claim failure redirection impl to use the MFA info API instead of the next array.
  • Update the docs to reflect the above recommendations

@sattvikc
Copy link
Collaborator

sattvikc commented Jan 3, 2024

Jan 3, 2024

Invalid first factor error is not a support code but instead a 401 error. Support codes are only meant to be used when the implementations are all correct but the user is unable to complete something because of an application state. Invalid first factor is either user trying to call unintended API or a dev mistake or a config error.

@sattvikc sattvikc mentioned this issue Jan 29, 2024
13 tasks
This was referenced Mar 13, 2024
@rishabhpoddar
Copy link
Member Author

This has been released in core version >= 9.0, node SDK version >= 17.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants