diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index 85070509402c..0e0820fb4696 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -5,6 +5,7 @@ package code import ( "context" + "encoding/json" "net/http" "sort" "strings" @@ -138,11 +139,49 @@ func (s *Strategy) CountActiveFirstFactorCredentials(ctx context.Context, cc map func (s *Strategy) CountActiveMultiFactorCredentials(ctx context.Context, cc map[identity.CredentialsType]identity.Credentials) (int, error) { codeConfig := s.deps.Config().SelfServiceCodeStrategy(ctx) - if codeConfig.MFAEnabled { - return 1, nil + if !codeConfig.MFAEnabled { + return 0, nil } - return 0, nil + // Get code credentials if they exist + creds, ok := cc[identity.CredentialsTypeCodeAuth] + if !ok { + return 0, nil + } + + // Check if the credentials config is valid JSON + if !gjson.Valid(string(creds.Config)) { + return 0, nil + } + + // Check for v0 format with address_type field + if gjson.GetBytes(creds.Config, "address_type").Exists() { + addressType := gjson.GetBytes(creds.Config, "address_type").String() + if addressType != "" { + return 1, nil + } + return 0, nil + } + + var conf identity.CredentialsCode + if err := json.Unmarshal(creds.Config, &conf); err != nil { + return 0, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to unmarshal credentials config: %s", err)) + } + + // If no addresses configured, return 0 + if len(conf.Addresses) == 0 { + return 0, nil + } + + // Count valid addresses configured for MFA + validAddresses := 0 + for _, addr := range conf.Addresses { + if addr.Address != "" { + validAddresses++ + } + } + + return validAddresses, nil } func NewStrategy(deps any) *Strategy { diff --git a/selfservice/strategy/code/strategy_test.go b/selfservice/strategy/code/strategy_test.go index 5edbef5ec718..b58c08001cdc 100644 --- a/selfservice/strategy/code/strategy_test.go +++ b/selfservice/strategy/code/strategy_test.go @@ -166,13 +166,13 @@ func TestCountActiveCredentials(t *testing.T) { }}, mfaEnabled: true, enabled: false, - expected: 1, + expected: 0, }, { in: map[identity.CredentialsType]identity.Credentials{}, mfaEnabled: true, enabled: true, - expected: 1, + expected: 0, }, { in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { @@ -181,8 +181,53 @@ func TestCountActiveCredentials(t *testing.T) { }}, mfaEnabled: true, enabled: true, + expected: 0, + }, + { + in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { + Type: strategy.ID(), + Config: []byte(`{"address_type":"email","used_at":{"Time":"0001-01-01T00:00:00Z","Valid":false}}`), + }}, + mfaEnabled: true, + enabled: true, expected: 1, }, + { + in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { + Type: strategy.ID(), + Config: []byte(`{"addresses":[{"channel":"email","address":"test@ory.sh"}]}`), + }}, + mfaEnabled: true, + enabled: true, + expected: 1, + }, + { + in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { + Type: strategy.ID(), + Config: []byte(`{"addresses":[]}`), + }}, + mfaEnabled: true, + enabled: true, + expected: 0, + }, + { + in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { + Type: strategy.ID(), + Config: []byte(`{"addresses":[{"channel":"sms","address":"+1234567890"}]}`), + }}, + mfaEnabled: true, + enabled: true, + expected: 1, + }, + { + in: map[identity.CredentialsType]identity.Credentials{strategy.ID(): { + Type: strategy.ID(), + Config: []byte(`{"addresses":[{"channel":"sms","address":"+1234567890"},{"channel":"email","address":"test@ory.sh"}]}`), + }}, + mfaEnabled: true, + enabled: true, + expected: 2, + }, } { t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { ctx := confighelpers.WithConfigValue(ctx, "selfservice.methods.code.mfa_enabled", tc.mfaEnabled) diff --git a/session/manager_http_test.go b/session/manager_http_test.go index 0bbfdd625461..507a1ab7caa6 100644 --- a/session/manager_http_test.go +++ b/session/manager_http_test.go @@ -515,11 +515,18 @@ func TestDoesSessionSatisfy(t *testing.T) { Identifiers: []string{testhelpers.RandomEmail()}, Config: []byte(`{"address_type":"email","used_at":{"Time":"0001-01-01T00:00:00Z","Valid":false}}`), } - //codeEmpty := identity.Credentials{ - // Type: identity.CredentialsTypeCodeAuth, - // Identifiers: []string{testhelpers.RandomEmail()}, - // Config: []byte(`{}`), - //} + + codeV2 := identity.Credentials{ + Type: identity.CredentialsTypeCodeAuth, + Identifiers: []string{testhelpers.RandomEmail()}, + Config: []byte(`{"addresses":[{"channel":"email","address":"test@ory.sh"}]}`), + } + + codeEmpty := identity.Credentials{ + Type: identity.CredentialsTypeCodeAuth, + Identifiers: []string{}, + Config: []byte(`{}`), + } oidc := identity.Credentials{ Type: identity.CredentialsTypeOIDC, @@ -630,6 +637,38 @@ func TestDoesSessionSatisfy(t *testing.T) { withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeCodeAuth]}, // No error }, + { + desc: "with highest_available a otp codeV2 user is aal1", + matcher: config.HighestAvailableAAL, + creds: []identity.Credentials{codeV2}, + withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeCodeAuth]}, + // No error + }, + { + desc: "with highest_available a empty mfa code user is aal1", + matcher: config.HighestAvailableAAL, + creds: []identity.Credentials{codeEmpty}, + withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeCodeAuth]}, + withContext: func(t *testing.T, ctx context.Context) context.Context { + return confighelpers.WithConfigValues(ctx, map[string]any{ + "selfservice.methods.code.mfa_enabled": true, + }) + }, + // No error + }, + { + desc: "with highest_available a password user with empty mfa code is aal1", + matcher: config.HighestAvailableAAL, + creds: []identity.Credentials{password, codeEmpty}, + withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypePassword]}, + withContext: func(t *testing.T, ctx context.Context) context.Context { + return confighelpers.WithConfigValues(ctx, map[string]any{ + "selfservice.methods.code.passwordless_enabled": false, + "selfservice.methods.code.mfa_enabled": true, + }) + }, + // No error + }, { desc: "with highest_available a oidc user is aal1", matcher: config.HighestAvailableAAL, @@ -690,7 +729,20 @@ func TestDoesSessionSatisfy(t *testing.T) { { desc: "with highest_available a recovery link user requires aal2 if they have 2fa code configured", matcher: config.HighestAvailableAAL, - creds: []identity.Credentials{}, + creds: []identity.Credentials{code}, + withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeRecoveryLink]}, + withContext: func(t *testing.T, ctx context.Context) context.Context { + return confighelpers.WithConfigValues(ctx, map[string]any{ + "selfservice.methods.code.passwordless_enabled": false, + "selfservice.methods.code.mfa_enabled": true, + }) + }, + errIs: new(session.ErrAALNotSatisfied), + }, + { + desc: "with highest_available a recovery link user requires aal2 if they have 2fa code v2 configured", + matcher: config.HighestAvailableAAL, + creds: []identity.Credentials{codeV2}, withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeRecoveryLink]}, withContext: func(t *testing.T, ctx context.Context) context.Context { return confighelpers.WithConfigValues(ctx, map[string]any{ @@ -773,6 +825,19 @@ func TestDoesSessionSatisfy(t *testing.T) { }) }, }, + { + desc: "has=aal1, requested=highest, available=aal2, credential=password+codeV2-mfa", + matcher: config.HighestAvailableAAL, + creds: []identity.Credentials{password, codeV2}, + withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypePassword]}, + errAs: new(session.ErrAALNotSatisfied), + withContext: func(t *testing.T, ctx context.Context) context.Context { + return confighelpers.WithConfigValues(ctx, map[string]any{ + "selfservice.methods.code.passwordless_enabled": false, + "selfservice.methods.code.mfa_enabled": true, + }) + }, + }, { desc: "has=aal1, requested=highest, available=aal2, credential=password+lookup_secrets", matcher: config.HighestAvailableAAL, diff --git a/test/e2e/cypress/integration/profiles/mfa/code_optional.spec.ts b/test/e2e/cypress/integration/profiles/mfa/code_optional.spec.ts new file mode 100644 index 000000000000..74f603cce34f --- /dev/null +++ b/test/e2e/cypress/integration/profiles/mfa/code_optional.spec.ts @@ -0,0 +1,217 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +import { gen } from "../../../helpers" +import { routes as express } from "../../../helpers/express" + +context("2FA code with optional field", () => { + ;[ + { + login: express.login, + settings: express.settings, + base: express.base, + app: "express" as "express", + profile: "mfa-optional", + }, + ].forEach(({ settings, login, profile, app, base }) => { + describe(`for app ${app}`, () => { + before(() => { + cy.useConfigProfile(profile) + cy.proxy(app) + }) + + describe("when using highest_available aal with empty optionalMfaEmail field", () => { + let email: string + let password: string + + beforeEach(() => { + email = gen.email() + password = gen.password() + + // Configure system to use highest available AAL + cy.useConfig((builder) => + builder.longPrivilegedSessionTime().useHighestAvailable(), + ) + + // Register a user without setting up code MFA (empty optionalMfaEmail field) + cy.register({ + email, + password, + fields: { "traits.optionalMfaEmail": "" }, + }) + cy.deleteMail() + }) + + it("should not show 2FA page during login when optionalMfaEmail field is empty", () => { + // Log out first + cy.clearAllCookies() + + // Login with the user + cy.visit(login) + cy.get('input[name="identifier"]').type(email) + cy.get('input[name="password"]').type(password) + cy.get('button[name="method"][value="password"]').click() + + // Should be logged in directly without 2FA page + cy.getSession({ + expectAal: "aal1", + expectMethods: ["password"], + }) + + // Verify we're not asked for 2FA when visiting settings + cy.visit(settings) + cy.location("pathname").should("contain", "/settings") + cy.get('input[name="traits.email"]').should("contain.value", email) + }) + }) + + describe("when using highest_available aal with configured optionalMfaEmail field", () => { + let email: string + let mfaEmail: string + let password: string + + beforeEach(() => { + email = gen.email() + mfaEmail = gen.email() + password = gen.password() + + // Configure system to use highest available AAL + cy.useConfig((builder) => + builder.longPrivilegedSessionTime().useHighestAvailable(), + ) + + // Register a user with optionalMfaEmail field set + cy.register({ + email, + password, + fields: { "traits.optionalMfaEmail": mfaEmail }, + }) + cy.deleteMail() + }) + + it("should show 2FA page during login when optionalMfaEmail field is configured", () => { + // Log out first + cy.clearAllCookies() + + // Login with the user + cy.visit(login) + cy.get('input[name="identifier"]').type(email) + cy.get('input[name="password"]').type(password) + cy.get('button[name="method"][value="password"]').click() + + // Should be asked for 2FA + cy.location("pathname").should("contain", "/login") + cy.get("[name='address']").should("be.visible").click() + + // Should see the code input field + cy.get("input[name='code']").should("be.visible") + + // Get the code from email and enter it + cy.getLoginCodeFromEmail(mfaEmail).then((code) => { + cy.get("input[name='code']").type(code) + cy.contains("Continue").click() + }) + + // Should be logged in with AAL2 + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "code"], + }) + }) + + it("should require 2FA for settings flow when optionalMfaEmail field is configured", () => { + // Log out first + cy.clearAllCookies() + + // Try to access settings directly + cy.visit(settings) + + // Should be redirected to login + cy.location("pathname").should("contain", "/login") + + // Login with password + cy.get('input[name="identifier"]').type(email) + cy.get('input[name="password"]').type(password) + cy.get('button[name="method"][value="password"]').click() + + // Should be asked for 2FA + cy.location("pathname").should("contain", "/login") + cy.get("[name='address']").should("be.visible").click() + + // Should see the code input field + cy.get("input[name='code']").should("be.visible") + + // Get the code from email and enter it + cy.getLoginCodeFromEmail(mfaEmail).then((code) => { + cy.get("input[name='code']").type(code) + cy.contains("Continue").click() + }) + + // Go the the settings page again + cy.visit(settings) + + // Should now be at settings with AAL2 + cy.location("pathname").should("contain", "/settings") + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "code"], + }) + }) + }) + + describe("when user with configured optionalMfaEmail logs in with aal1 session", () => { + let email: string + let mfaEmail: string + let password: string + + beforeEach(() => { + email = gen.email() + mfaEmail = gen.email() + password = gen.password() + + // Configure system to allow aal1 login but still require highest_available for settings + cy.useConfig((builder) => + builder + .longPrivilegedSessionTime() + .useLaxSessionAal() + .useHighestSettingsFlowAal(), + ) + + // Register a user with optionalMfaEmail field set + cy.register({ + email, + password, + fields: { "traits.optionalMfaEmail": mfaEmail }, + }) + cy.deleteMail() + }) + + it("should not allow access to settings page with aal1 session", () => { + // Log out first + cy.clearAllCookies() + + // Login with just password (aal1) + cy.visit(login) + cy.get('input[name="identifier"]').type(email) + cy.get('input[name="password"]').type(password) + cy.get('button[name="method"][value="password"]').click() + + // Verify we have an aal1 session + cy.getSession({ + expectAal: "aal1", + expectMethods: ["password"], + }) + + // Try to access settings page + cy.visit(settings) + + // Should be redirected to login for 2FA + cy.location("pathname").should("contain", "/login") + + // Verify we're being asked for 2FA + cy.get("[name='address']").should("be.visible") + }) + }) + }) + }) +}) diff --git a/test/e2e/cypress/support/configHelpers.ts b/test/e2e/cypress/support/configHelpers.ts index 566bb475fa58..11edb0380292 100644 --- a/test/e2e/cypress/support/configHelpers.ts +++ b/test/e2e/cypress/support/configHelpers.ts @@ -132,18 +132,34 @@ export class ConfigBuilder { return this } - public useLaxAal() { - this.config.selfservice.flows.settings.required_aal = "aal1" + public useLaxSessionAal() { this.config.session.whoami.required_aal = "aal1" return this } - public useHighestAvailable() { - this.config.selfservice.flows.settings.required_aal = "highest_available" + public useLaxSettingsFlowAal() { + this.config.selfservice.flows.settings.required_aal = "aal1" + return this + } + + public useLaxAal() { + return this.useLaxSessionAal().useLaxSettingsFlowAal() + } + + public useHighestSessionAal() { this.config.session.whoami.required_aal = "highest_available" return this } + public useHighestSettingsFlowAal() { + this.config.selfservice.flows.settings.required_aal = "highest_available" + return this + } + + public useHighestAvailable() { + return this.useHighestSessionAal().useHighestSettingsFlowAal() + } + public enableCode() { this.config.selfservice.methods.code.enabled = true return this diff --git a/test/e2e/profiles/mfa-optional/.kratos.yml b/test/e2e/profiles/mfa-optional/.kratos.yml new file mode 100644 index 000000000000..061f9a5704fd --- /dev/null +++ b/test/e2e/profiles/mfa-optional/.kratos.yml @@ -0,0 +1,53 @@ +selfservice: + flows: + settings: + ui_url: http://localhost:4455/settings + privileged_session_max_age: 5m + required_aal: highest_available + + logout: + after: + default_browser_return_url: http://localhost:4455/login + + registration: + enable_legacy_one_step: true + ui_url: http://localhost:4455/registration + after: + password: + hooks: + - hook: session + + login: + ui_url: http://localhost:4455/login + error: + ui_url: http://localhost:4455/error + verification: + ui_url: http://localhost:4455/verify + recovery: + ui_url: http://localhost:4455/recovery + + methods: + code: + mfa_enabled: true + totp: + enabled: true + config: + issuer: issuer.ory.sh + lookup_secret: + enabled: true + webauthn: + enabled: true + config: + rp: + id: localhost + origin: http://localhost:4455 + display_name: Ory + +identity: + schemas: + - id: default + url: file://test/e2e/profiles/mfa-optional/identity.traits.schema.json + +session: + whoami: + required_aal: highest_available diff --git a/test/e2e/profiles/mfa-optional/identity.traits.schema.json b/test/e2e/profiles/mfa-optional/identity.traits.schema.json new file mode 100644 index 000000000000..4a8d8667c0ce --- /dev/null +++ b/test/e2e/profiles/mfa-optional/identity.traits.schema.json @@ -0,0 +1,45 @@ +{ + "$id": "https://schemas.ory.sh/presets/kratos/quickstart/email-password/identity.schema.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Person", + "type": "object", + "properties": { + "traits": { + "type": "object", + "properties": { + "email": { + "type": "string", + "format": "email", + "title": "Your E-Mail", + "minLength": 3, + "ory.sh/kratos": { + "credentials": { + "password": { + "identifier": true + }, + "webauthn": { + "identifier": true + } + } + } + }, + "optionalMfaEmail": { + "type": "string", + "format": "email", + "title": "Your E-Mail for MFA (Optional)", + "minLength": 3, + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "email" + } + } + } + } + }, + "required": ["email"], + "additionalProperties": false + } + } +} diff --git a/test/e2e/run.sh b/test/e2e/run.sh index a0a6d449fc6c..9fd44eef560f 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -253,7 +253,7 @@ run() { nc -zv localhost 4433 && (echo "Port 4433 unavailable, used by" ; lsof -i:4433 ; exit 1) ls -la . - for profile in code email mobile oidc recovery recovery-mfa verification mfa spa network passwordless passkey webhooks oidc-provider oidc-provider-mfa two-steps; do + for profile in code email mobile oidc recovery recovery-mfa verification mfa mfa-optional spa network passwordless passkey webhooks oidc-provider oidc-provider-mfa two-steps; do yq ea '. as $item ireduce ({}; . * $item )' test/e2e/profiles/kratos.base.yml "test/e2e/profiles/${profile}/.kratos.yml" > test/e2e/kratos.${profile}.yml cat "test/e2e/kratos.${profile}.yml" | envsubst | sponge "test/e2e/kratos.${profile}.yml" done