diff --git a/e2e/tests/login/device_token/disabled_if_no_2fa/test.yaml b/e2e/tests/login/device_token/disabled_if_no_2fa/test.yaml new file mode 100644 index 0000000000..91676d2925 --- /dev/null +++ b/e2e/tests/login/device_token/disabled_if_no_2fa/test.yaml @@ -0,0 +1,226 @@ +name: Login - Device Token - Disabled if no secondary authenticator +authgear.yaml: + override: | + authentication: + identities: + - login_id + primary_authenticators: + - password + secondary_authenticators: + - password + secondary_authentication_mode: required + identity: + login_id: + keys: + - type: username +before: + - type: user_import + user_import: users.json +steps: + - action: "create" + input: | + { + "type": "login", + "name": "default" + } + output: + result: | + { + "action": { + "type": "identify", + "data": { + "type": "identification_data", + "options": "[[array]]" + } + } + } + + - action: input + input: | + { + "identification": "username", + "login_id": "e2e_login_device_token" + } + output: + result: | + { + "action": { + "type": "authenticate", + "data": { + "device_token_enabled": false, + "options": [ + { + "authentication": "primary_password" + } + ], + "type": "authentication_data" + } + } + } + + - action: input + input: | + { + "authentication": "primary_password", + "password": "password" + } + output: + result: | + { + "action": { + "type": "authenticate", + "data": { + "device_token_enabled": true, + "options": [ + { + "authentication": "secondary_password" + } + ], + "type": "authentication_data" + } + } + } + - action: input + input: | + { + "authentication": "secondary_password", + "password": "password", + "request_device_token": true + } + output: + result: | + { + "action": { + "type": "finished" + } + } + + # Login again. Expect 2fa is skipped by using device token + - action: "create" + input: | + { + "type": "login", + "name": "default" + } + output: + result: | + { + "action": { + "type": "identify", + "data": { + "type": "identification_data", + "options": "[[array]]" + } + } + } + - action: input + input: | + { + "identification": "username", + "login_id": "e2e_login_device_token" + } + output: + result: | + { + "action": { + "type": "authenticate", + "data": { + "device_token_enabled": false, + "options": [ + { + "authentication": "primary_password" + } + ], + "type": "authentication_data" + } + } + } + - action: input + input: | + { + "authentication": "primary_password", + "password": "password" + } + output: + result: | + { + "action": { + "type": "finished" + } + } + + # Remove the 2fa. + - action: query + query: | + WITH authenticator_ids AS ( + SELECT id FROM _auth_authenticator + WHERE app_id = '{{ .AppID }}' + AND kind = 'secondary' + ), del_1 AS ( + DELETE FROM _auth_authenticator_password + WHERE id IN (SELECT id FROM authenticator_ids) + ) + DELETE FROM _auth_authenticator + WHERE id IN (SELECT id FROM authenticator_ids); + # Login again, should be blocked because 2fa is required. + - action: "create" + input: | + { + "type": "login", + "name": "default" + } + output: + result: | + { + "action": { + "type": "identify", + "data": { + "type": "identification_data", + "options": "[[array]]" + } + } + } + + - action: input + input: | + { + "identification": "username", + "login_id": "e2e_login_device_token" + } + output: + result: | + { + "action": { + "type": "authenticate", + "data": { + "device_token_enabled": false, + "options": [ + { + "authentication": "primary_password" + } + ], + "type": "authentication_data" + } + } + } + + - action: input + input: | + { + "authentication": "primary_password", + "password": "password" + } + output: + error: | + { + "name": "Invalid", + "reason": "InvariantViolated", + "message": "no authenticator", + "code": 400, + "info": { + "FlowType": "login", + "cause": { + "kind": "NoAuthenticator" + } + } + } diff --git a/e2e/tests/login/device_token/disabled_if_no_2fa/users.json b/e2e/tests/login/device_token/disabled_if_no_2fa/users.json new file mode 100644 index 0000000000..aedb403702 --- /dev/null +++ b/e2e/tests/login/device_token/disabled_if_no_2fa/users.json @@ -0,0 +1,19 @@ +{ + "identifier": "preferred_username", + "records": [ + { + "preferred_username": "e2e_login_device_token", + "name": "Login", + "password": { + "type": "bcrypt", + "password_hash": "$2y$10$/wLavnCmYGP/zzpw/mR1iOK5y5hGyrEFJtmaIbvFf9VA6l2O4NMKO" + }, + "mfa": { + "password": { + "type": "bcrypt", + "password_hash": "$2y$10$/wLavnCmYGP/zzpw/mR1iOK5y5hGyrEFJtmaIbvFf9VA6l2O4NMKO" + } + } + } + ] +} diff --git a/pkg/lib/authenticationflow/declarative/intent_login_flow_step_authenticate.go b/pkg/lib/authenticationflow/declarative/intent_login_flow_step_authenticate.go index d0c91cc3f8..18505d79e5 100644 --- a/pkg/lib/authenticationflow/declarative/intent_login_flow_step_authenticate.go +++ b/pkg/lib/authenticationflow/declarative/intent_login_flow_step_authenticate.go @@ -40,11 +40,12 @@ func init() { // NodeDoConsumeRecoveryCode (MilestoneDidAuthenticate) type IntentLoginFlowStepAuthenticate struct { - FlowReference authflow.FlowReference `json:"flow_reference,omitempty"` - JSONPointer jsonpointer.T `json:"json_pointer,omitempty"` - StepName string `json:"step_name,omitempty"` - UserID string `json:"user_id,omitempty"` - Options []AuthenticateOption `json:"options"` + FlowReference authflow.FlowReference `json:"flow_reference,omitempty"` + JSONPointer jsonpointer.T `json:"json_pointer,omitempty"` + StepName string `json:"step_name,omitempty"` + UserID string `json:"user_id,omitempty"` + Options []AuthenticateOption `json:"options"` + DeviceTokenEnabled bool `json:"device_token_enabled"` } var _ authflow.TargetStep = &IntentLoginFlowStepAuthenticate{} @@ -91,12 +92,13 @@ func NewIntentLoginFlowStepAuthenticate(ctx context.Context, deps *authflow.Depe } step := i.step(current) - options, err := getAuthenticationOptionsForLogin(ctx, deps, flows, i.UserID, step) + options, deviceTokenEnabled, err := getAuthenticationOptionsForLogin(ctx, deps, flows, i.UserID, step) if err != nil { return nil, err } i.Options = options + i.DeviceTokenEnabled = deviceTokenEnabled return i, nil } @@ -115,9 +117,6 @@ func (i *IntentLoginFlowStepAuthenticate) CanReactTo(ctx context.Context, deps * } step := i.step(current) - deviceTokenIndex := i.deviceTokenIndex(step) - deviceTokenEnabled := deviceTokenIndex >= 0 - _, _, deviceTokenInspected := authflow.FindMilestoneInCurrentFlow[MilestoneDeviceTokenInspected](flows) authenticationMethodSelected := false @@ -139,7 +138,7 @@ func (i *IntentLoginFlowStepAuthenticate) CanReactTo(ctx context.Context, deps * createAuthenticatorMilestones := authflow.FindAllMilestones[MilestoneFlowCreateAuthenticator](flows.Root) switch { - case deviceTokenEnabled && !deviceTokenInspected: + case i.DeviceTokenEnabled && !deviceTokenInspected: // Inspect the device token return nil, nil case !authenticationMethodSelected: @@ -161,7 +160,7 @@ func (i *IntentLoginFlowStepAuthenticate) CanReactTo(ctx context.Context, deps * FlowRootObject: flowRootObject, JSONPointer: i.JSONPointer, Options: i.Options, - DeviceTokenEnabled: deviceTokenEnabled, + DeviceTokenEnabled: i.DeviceTokenEnabled, ShouldBypassBotProtection: shouldBypassBotProtection, BotProtectionCfg: deps.Config.BotProtection, }, nil @@ -170,7 +169,7 @@ func (i *IntentLoginFlowStepAuthenticate) CanReactTo(ctx context.Context, deps * // We expect the selected authentication method to be authenticated before this intent becomes input reactor again. panic(fmt.Errorf("unauthenticated")) - case deviceTokenEnabled && !deviceTokenCreatedIfRequested: + case i.DeviceTokenEnabled && !deviceTokenCreatedIfRequested: // We look at the current input to see if device token is request. // So we do not need to take another input. return nil, nil @@ -189,9 +188,6 @@ func (i *IntentLoginFlowStepAuthenticate) ReactTo(ctx context.Context, deps *aut } step := i.step(current) - deviceTokenIndex := i.deviceTokenIndex(step) - deviceTokenEnabled := deviceTokenIndex >= 0 - _, _, deviceTokenInspected := authflow.FindMilestoneInCurrentFlow[MilestoneDeviceTokenInspected](flows) authenticationMethodSelected := false @@ -211,7 +207,7 @@ func (i *IntentLoginFlowStepAuthenticate) ReactTo(ctx context.Context, deps *aut _, _, nestedStepsHandled := authflow.FindMilestoneInCurrentFlow[MilestoneNestedSteps](flows) switch { - case deviceTokenEnabled && !deviceTokenInspected: + case i.DeviceTokenEnabled && !deviceTokenInspected: return authflow.NewSubFlow(&IntentInspectDeviceToken{ UserID: i.UserID, }), nil @@ -296,9 +292,9 @@ func (i *IntentLoginFlowStepAuthenticate) ReactTo(ctx context.Context, deps *aut return nil, authflow.ErrIncompatibleInput case !authenticated: panic(fmt.Errorf("unauthenticated")) - case deviceTokenEnabled && !deviceTokenCreatedIfRequested: + case i.DeviceTokenEnabled && !deviceTokenCreatedIfRequested: return authflow.NewSubFlow(&IntentCreateDeviceTokenIfRequested{ - JSONPointer: authflow.JSONPointerForOneOf(i.JSONPointer, deviceTokenIndex), + JSONPointer: authflow.JSONPointerForOneOf(i.JSONPointer, i.deviceTokenIndex(step)), UserID: i.UserID, }), nil case !nestedStepsHandled: @@ -313,14 +309,6 @@ func (i *IntentLoginFlowStepAuthenticate) ReactTo(ctx context.Context, deps *aut } func (i *IntentLoginFlowStepAuthenticate) OutputData(ctx context.Context, deps *authflow.Dependencies, flows authflow.Flows) (authflow.Data, error) { - current, err := i.currentFlowObject(deps) - if err != nil { - return nil, err - } - step := i.step(current) - - deviceTokenIndex := i.deviceTokenIndex(step) - deviceTokenEnabled := deviceTokenIndex >= 0 options := []AuthenticateOptionForOutput{} for _, o := range i.Options { @@ -329,7 +317,7 @@ func (i *IntentLoginFlowStepAuthenticate) OutputData(ctx context.Context, deps * return NewStepAuthenticateData(StepAuthenticateData{ Options: options, - DeviceTokenEnabled: deviceTokenEnabled, + DeviceTokenEnabled: i.DeviceTokenEnabled, }), nil } diff --git a/pkg/lib/authenticationflow/declarative/utils_common.go b/pkg/lib/authenticationflow/declarative/utils_common.go index a546d6a97d..b548177ec2 100644 --- a/pkg/lib/authenticationflow/declarative/utils_common.go +++ b/pkg/lib/authenticationflow/declarative/utils_common.go @@ -200,23 +200,27 @@ func findFlowRootObjectInFlow(deps *authflow.Dependencies, flows authflow.Flows) } // nolint: gocognit -func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Dependencies, flows authflow.Flows, userID string, step *config.AuthenticationFlowLoginFlowStep) ([]AuthenticateOption, error) { - options := []AuthenticateOption{} +func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Dependencies, flows authflow.Flows, userID string, step *config.AuthenticationFlowLoginFlowStep) ( + options []AuthenticateOption, + deviceTokenEnabled bool, + err error, +) { + options = []AuthenticateOption{} identities, err := deps.Identities.ListByUser(userID) if err != nil { - return nil, err + return nil, false, err } authenticators, err := deps.Authenticators.List(userID) if err != nil { - return nil, err + return nil, false, err } secondaryAuthenticators := authenticator.ApplyFilters(authenticators, authenticator.KeepKind(model.AuthenticatorKindSecondary)) userRecoveryCodes, err := deps.MFA.ListRecoveryCodes(userID) if err != nil { - return nil, err + return nil, false, err } passkeyAuthenticators := authenticator.ApplyFilters( authenticators, @@ -358,8 +362,9 @@ func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Depend for _, branch := range step.OneOf { switch branch.Authentication { case config.AuthenticationFlowAuthenticationDeviceToken: - // Device token is handled transparently. - break + if len(secondaryAuthenticators) > 0 { + deviceTokenEnabled = true + } case config.AuthenticationFlowAuthenticationRecoveryCode: options = useAuthenticationOptionAddRecoveryCodes(options, userHasRecoveryCode, branch.BotProtection) case config.AuthenticationFlowAuthenticationPrimaryPassword: @@ -367,7 +372,7 @@ func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Depend case config.AuthenticationFlowAuthenticationPrimaryPasskey: options, err = useAuthenticationOptionAddPasskey(options, deps, userHasPasskey, userID, branch.BotProtection) if err != nil { - return nil, err + return nil, false, err } case config.AuthenticationFlowAuthenticationSecondaryPassword: options = useAuthenticationOptionAddSecondaryPassword(options, userHasSecondaryPassword, branch.BotProtection) @@ -379,7 +384,7 @@ func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Depend if targetStepName := branch.TargetStep; targetStepName != "" { info, err := findIdentity(targetStepName) if err != nil { - return nil, err + return nil, false, err } options = useAuthenticationOptionAddPrimaryOOBOTPOfIdentity(options, deps, branch.Authentication, info, branch.BotProtection) @@ -393,7 +398,7 @@ func getAuthenticationOptionsForLogin(ctx context.Context, deps *authflow.Depend } } - return options, nil + return options, deviceTokenEnabled, nil } // nolint:gocognit diff --git a/pkg/lib/facade/coordinator.go b/pkg/lib/facade/coordinator.go index 7a166b1c95..68f1b1342e 100644 --- a/pkg/lib/facade/coordinator.go +++ b/pkg/lib/facade/coordinator.go @@ -889,6 +889,10 @@ func (c *Coordinator) removeOrphans(userID string) error { if err != nil { return err } + err = c.MFA.InvalidateAllDeviceTokens(userID) + if err != nil { + return err + } } return nil