diff --git a/selfservice/strategy/code/.schema/login.schema.json b/selfservice/strategy/code/.schema/login.schema.json index 9a8a0aa5cf25..22286b039149 100644 --- a/selfservice/strategy/code/.schema/login.schema.json +++ b/selfservice/strategy/code/.schema/login.schema.json @@ -4,10 +4,7 @@ "type": "object", "properties": { "method": { - "type": "string", - "enum": [ - "code" - ] + "type": "string" }, "code": { "type": "string" diff --git a/selfservice/strategy/code/.schema/recovery.schema.json b/selfservice/strategy/code/.schema/recovery.schema.json index 011503132baa..eb04bd359002 100644 --- a/selfservice/strategy/code/.schema/recovery.schema.json +++ b/selfservice/strategy/code/.schema/recovery.schema.json @@ -4,11 +4,7 @@ "type": "object", "properties": { "method": { - "type": "string", - "enum": [ - "code", - "link" - ] + "type": "string" }, "code": { "type": "string" diff --git a/selfservice/strategy/code/.schema/registration.schema.json b/selfservice/strategy/code/.schema/registration.schema.json index 90f245c107c5..0cd5282eb9ee 100644 --- a/selfservice/strategy/code/.schema/registration.schema.json +++ b/selfservice/strategy/code/.schema/registration.schema.json @@ -4,10 +4,7 @@ "type": "object", "properties": { "method": { - "type": "string", - "enum": [ - "code" - ] + "type": "string" }, "csrf_token": { "type": "string" diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 652de2104248..cb734b1bf4a1 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -165,11 +165,11 @@ func (s *Strategy) methodEnabledAndAllowedFromRequest(r *http.Request, f *login. decoderx.HTTPDecoderAllowedMethods("POST", "PUT", "PATCH", "GET"), decoderx.HTTPDecoderSetValidatePayloads(false), decoderx.HTTPDecoderJSONFollowsFormFormat()); err != nil { - return nil, errors.WithStack(err) + return &method, errors.WithStack(err) } if err := flow.MethodEnabledAndAllowed(r.Context(), f.GetFlowName(), s.ID().String(), method.Method, s.deps); err != nil { - return nil, err + return &method, err } return &method, nil @@ -192,9 +192,14 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, } if p, err := s.methodEnabledAndAllowedFromRequest(r, f); errors.Is(err, flow.ErrStrategyNotResponsible) { - if !(s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled && (p == nil || len(p.Address) > 0)) { + if !s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled { + return nil, err + } + + if p == nil || len(p.Address) == 0 { return nil, err } + // In this special case we only expect `address` to be set. } else if err != nil { return nil, err @@ -261,6 +266,10 @@ func (s *Strategy) findIdentifierInVerifiableAddress(i *identity.Identity, ident func (s *Strategy) findIdentityForIdentifier(ctx context.Context, identifier string, requestedAAL identity.AuthenticatorAssuranceLevel, session *session.Session) (_ *identity.Identity, _ []Address, err error) { ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.findIdentityForIdentifier") + span.SetAttributes( + attribute.String("flow.requested_aal", string(requestedAAL)), + ) + defer otelx.End(span, &err) if len(identifier) == 0 { @@ -279,7 +288,7 @@ func (s *Strategy) findIdentityForIdentifier(ctx context.Context, identifier str // we need to gracefully handle this flow. // // TODO this section should be removed at some point when we are sure that all identities have a code credential. - if errors.Is(err, schema.NewNoCodeAuthnCredentials()) { + if codeCred := new(schema.ValidationError); errors.As(err, &codeCred) && codeCred.ValidationError.Message == "account does not exist or has not setup up sign in with code" { fallbackAllowed := s.deps.Config().SelfServiceCodeMethodMissingCredentialFallbackEnabled(ctx) span.SetAttributes( attribute.Bool(config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, fallbackAllowed), @@ -290,7 +299,7 @@ func (s *Strategy) findIdentityForIdentifier(ctx context.Context, identifier str return nil, nil, errors.WithStack(schema.NewNoCodeAuthnCredentials()) } - _, err := s.findIdentifierInVerifiableAddress(session.Identity, identifier) + address, err := s.findIdentifierInVerifiableAddress(session.Identity, identifier) if err != nil { return nil, nil, err } @@ -307,6 +316,7 @@ func (s *Strategy) findIdentityForIdentifier(ctx context.Context, identifier str // // So we accept that the identity in this case will simply not have code credentials, and we will rely on the // fallback mechanism to authenticate the user. + return session.Identity, []Address{*address}, nil } else if err != nil { return nil, nil, err } diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go index 9c98a929dcc9..d93972162852 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -751,7 +751,7 @@ func TestLoginCodeStrategy(t *testing.T) { }) t.Run("case=should be able to get AAL2 session", func(t *testing.T) { - run := func(t *testing.T, withoutCodeCredential bool, overrideCodeCredential *identity.Credentials) (*state, *http.Client) { + run := func(t *testing.T, withoutCodeCredential bool, overrideCodeCredential *identity.Credentials, overrideAllCredentials map[identity.CredentialsType]identity.Credentials) (*state, *http.Client) { user := createIdentity(ctx, t, reg, withoutCodeCredential) if overrideCodeCredential != nil { toUpdate := user.Credentials[identity.CredentialsTypeCodeAuth] @@ -763,6 +763,10 @@ func TestLoginCodeStrategy(t *testing.T) { } user.Credentials[identity.CredentialsTypeCodeAuth] = toUpdate } + if overrideAllCredentials != nil { + user.Credentials = overrideAllCredentials + } + require.NoError(t, reg.PrivilegedIdentityPool().UpdateIdentity(ctx, user)) var cl *http.Client var f *oryClient.LoginFlow @@ -805,14 +809,14 @@ func TestLoginCodeStrategy(t *testing.T) { testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") // has code identifier conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, false) // fallback enabled - _, cl := run(t, true, nil) + _, cl := run(t, true, nil, nil) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") }) t.Run("case=disabling mfa does not lock out the users", func(t *testing.T) { testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") // has code identifier - s, cl := run(t, true, nil) + s, cl := run(t, true, nil, nil) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") email := gjson.GetBytes(s.identity.Traits, "email").String() @@ -865,36 +869,48 @@ func TestLoginCodeStrategy(t *testing.T) { conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, false) }) - _, cl := run(t, false, nil) + _, cl := run(t, false, nil, nil) + testhelpers.EnsureAAL(t, cl, public, "aal2", "code") + }) + + t.Run("case=missing code credential with fallback works even when identity schema has no code identifier set", func(t *testing.T) { + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/no-code.schema.json") // missing the code identifier + conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, true) // fallback enabled + t.Cleanup(func() { + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") + conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, false) + }) + + _, cl := run(t, false, nil, nil) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") }) t.Run("case=missing code credential with fallback works even when identity schema has no code identifier set", func(t *testing.T) { - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/default.schema.json") // missing the code identifier + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/no-code-id.schema.json") // missing the code identifier conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, true) // fallback enabled t.Cleanup(func() { testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, false) }) - _, cl := run(t, false, nil) + _, cl := run(t, true, &identity.Credentials{}, map[identity.CredentialsType]identity.Credentials{}) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") }) t.Run("case=legacy code credential with fallback works when identity schema has the code identifier not set", func(t *testing.T) { - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/default.schema.json") // has code identifier + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/no-code.schema.json") // has code identifier conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, true) // fallback enabled t.Cleanup(func() { testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") // has code identifier conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, false) }) - _, cl := run(t, false, &identity.Credentials{Config: []byte(`{"via":""}`)}) + _, cl := run(t, false, &identity.Credentials{Config: []byte(`{"via":""}`)}, nil) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") }) t.Run("case=legacy code credential with fallback works when identity schema has the code identifier not set", func(t *testing.T) { - testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/default.schema.json") // has code identifier + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/no-code.schema.json") // has code identifier conf.MustSet(ctx, config.ViperKeyCodeConfigMissingCredentialFallbackEnabled, true) // fallback enabled t.Cleanup(func() { testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") // has code identifier @@ -906,12 +922,11 @@ func TestLoginCodeStrategy(t *testing.T) { `{"address_type": "email", "used_at": {"Time": "0001-01-01T00:00:00Z", "Valid": false}}`, `{"address_type": "", "used_at": {"Time": "0001-01-01T00:00:00Z", "Valid": false}}`, `{"address_type": ""}`, - `{"address_type": "sms"}`, `{"address_type": "phone"}`, `{}`, } { t.Run(fmt.Sprintf("config=%d", k), func(t *testing.T) { - _, cl := run(t, false, &identity.Credentials{Config: []byte(credentialsConfig)}) + _, cl := run(t, false, &identity.Credentials{Config: []byte(credentialsConfig)}, nil) testhelpers.EnsureAAL(t, cl, public, "aal2", "code") }) } @@ -1097,7 +1112,6 @@ func TestLoginCodeStrategy(t *testing.T) { } require.Equal(t, "No value found for trait email_1 in the current identity.", gjson.GetBytes(body, "reason").String(), "%s", body) }) - }) }) } diff --git a/selfservice/strategy/code/stub/no-code-id.schema.json b/selfservice/strategy/code/stub/no-code-id.schema.json new file mode 100644 index 000000000000..3d2dee0d0bd0 --- /dev/null +++ b/selfservice/strategy/code/stub/no-code-id.schema.json @@ -0,0 +1,26 @@ +{ + "$id": "https://example.com/person.schema.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Person", + "type": "object", + "properties": { + "traits": { + "type": "object", + "properties": { + "email": { + "type": "string", + "ory.sh/kratos": { + "credentials": { + }, + "verification": { + "via": "email" + }, + "recovery": { + "via": "email" + } + } + } + } + } + } +} diff --git a/selfservice/strategy/idfirst/.schema/login.schema.json b/selfservice/strategy/idfirst/.schema/login.schema.json index 02ebd4ca9d9f..9e8be40eff58 100644 --- a/selfservice/strategy/idfirst/.schema/login.schema.json +++ b/selfservice/strategy/idfirst/.schema/login.schema.json @@ -11,10 +11,7 @@ "minLength": 1 }, "method": { - "type": "string", - "enum": [ - "identifier_first" - ] + "type": "string" }, "transient_payload": { "type": "object", diff --git a/session/manager_http.go b/session/manager_http.go index c87dc20862e7..cf68e7289737 100644 --- a/session/manager_http.go +++ b/session/manager_http.go @@ -242,7 +242,7 @@ func (s *ManagerHTTP) FetchFromRequest(ctx context.Context, r *http.Request) (_ return nil, errors.WithStack(NewErrNoCredentialsForSession()) } - se, err := s.r.SessionPersister().GetSessionByToken(ctx, token, ExpandEverything, identity.ExpandDefault) + se, err := s.r.SessionPersister().GetSessionByToken(ctx, token, ExpandEverything, identity.ExpandEverything) if err != nil { if errors.Is(err, herodot.ErrNotFound) || errors.Is(err, sqlcon.ErrNoRows) { return nil, errors.WithStack(NewErrNoActiveSessionFound()) diff --git a/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts b/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts index 40e6c7e7984a..047496a5ef37 100644 --- a/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts @@ -34,6 +34,7 @@ context("2FA lookup secrets", () => { beforeEach(() => { cy.visit(base) cy.clearAllCookies() + cy.useConfig((builder) => builder.disableCodeMfa()) email = gen.email() password = gen.password() cy.registerApi({ diff --git a/test/e2e/cypress/integration/profiles/mfa/settings.spec.ts b/test/e2e/cypress/integration/profiles/mfa/settings.spec.ts index 2b2e312a136a..329cb0ffc04b 100644 --- a/test/e2e/cypress/integration/profiles/mfa/settings.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/settings.spec.ts @@ -39,6 +39,7 @@ context("2FA UI settings tests", () => { beforeEach(() => { cy.clearAllCookies() + cy.useConfig((builder) => builder.disableCodeMfa()) cy.login({ email, password, cookieUrl: base }) cy.visit(settings) }) diff --git a/test/e2e/cypress/integration/profiles/mfa/totp.spec.ts b/test/e2e/cypress/integration/profiles/mfa/totp.spec.ts index a193bcb6ceed..d3523dc241b1 100644 --- a/test/e2e/cypress/integration/profiles/mfa/totp.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/totp.spec.ts @@ -34,7 +34,7 @@ context("2FA TOTP", () => { beforeEach(() => { cy.useConfig((builder) => - builder.longPrivilegedSessionTime().useLaxAal(), + builder.longPrivilegedSessionTime().useLaxAal().disableCodeMfa(), ) email = gen.email() password = gen.password() diff --git a/test/e2e/cypress/integration/profiles/mfa/webauthn.spec.ts b/test/e2e/cypress/integration/profiles/mfa/webauthn.spec.ts index 07be1001d323..9e6b260a7922 100644 --- a/test/e2e/cypress/integration/profiles/mfa/webauthn.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/webauthn.spec.ts @@ -37,6 +37,7 @@ context("2FA WebAuthn", () => { beforeEach(() => { cy.clearAllCookies() + cy.useConfig((builder) => builder.disableCodeMfa()) email = gen.email() password = gen.password() cy.registerApi({ diff --git a/test/e2e/cypress/integration/profiles/mobile/settings/success.spec.ts b/test/e2e/cypress/integration/profiles/mobile/settings/success.spec.ts index 89d295bfba7f..fe65ac033e74 100644 --- a/test/e2e/cypress/integration/profiles/mobile/settings/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/mobile/settings/success.spec.ts @@ -25,6 +25,7 @@ context("Mobile Profile", () => { beforeEach(() => { cy.loginMobile({ email, password }) + cy.location("pathname").should("not.contain", "/Login") cy.visit(MOBILE_URL + "/Settings") }) @@ -67,6 +68,7 @@ context("Mobile Profile", () => { fields: { "traits.website": website }, }) cy.loginMobile({ email, password }) + cy.location("pathname").should("not.contain", "/Login") cy.visit(MOBILE_URL + "/Settings") }) diff --git a/test/e2e/cypress/support/configHelpers.ts b/test/e2e/cypress/support/configHelpers.ts index bce083d63d77..566bb475fa58 100644 --- a/test/e2e/cypress/support/configHelpers.ts +++ b/test/e2e/cypress/support/configHelpers.ts @@ -25,6 +25,11 @@ export class ConfigBuilder { return this } + public disableCodeMfa() { + this.config.selfservice.methods.code.mfa_enabled = false + return this + } + public enableRecovery() { if (!this.config.selfservice.flows.recovery) { this.config.selfservice.flows.recovery = {} diff --git a/test/e2e/profiles/mfa/.kratos.yml b/test/e2e/profiles/mfa/.kratos.yml index 91f0bfb71042..081b3bb541b7 100644 --- a/test/e2e/profiles/mfa/.kratos.yml +++ b/test/e2e/profiles/mfa/.kratos.yml @@ -27,6 +27,8 @@ selfservice: ui_url: http://localhost:4455/recovery methods: + code: + mfa_enabled: true totp: enabled: true config: