diff --git a/go.mod b/go.mod index 24bc9a2cff10..f3a839bacaff 100644 --- a/go.mod +++ b/go.mod @@ -110,6 +110,7 @@ require ( github.com/cortesi/moddwatch v0.1.0 // indirect github.com/cortesi/termlog v0.0.0-20210222042314-a1eec763abec // indirect github.com/rjeczalik/notify v0.9.3 // indirect + github.com/wI2L/jsondiff v0.6.0 // indirect golang.org/x/term v0.22.0 // indirect gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect mvdan.cc/sh/v3 v3.6.0 // indirect diff --git a/go.sum b/go.sum index 7e9d0ef27bb4..44ff24e8a182 100644 --- a/go.sum +++ b/go.sum @@ -872,6 +872,8 @@ github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9r github.com/toqueteos/webbrowser v1.2.0/go.mod h1:XWoZq4cyp9WeUeak7w7LXRUQf1F1ATJMir8RTqb4ayM= github.com/urfave/negroni v1.0.0 h1:kIimOitoypq34K7TG7DUaJ9kq/N4Ofuwi1sjz0KipXc= github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4= +github.com/wI2L/jsondiff v0.6.0 h1:zrsH3FbfVa3JO9llxrcDy/XLkYPLgoMX6Mz3T2PP2AI= +github.com/wI2L/jsondiff v0.6.0/go.mod h1:D6aQ5gKgPF9g17j+E9N7aasmU1O+XvfmWm1y8UMmNpw= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= diff --git a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_correct_value.json b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_correct_value.json index 4279880cdf5d..3673c6943f03 100644 --- a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_correct_value.json +++ b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_correct_value.json @@ -9,8 +9,8 @@ "config": { "addresses": [ { - "address": "hi@example.org", - "channel": "email" + "channel": "email", + "address": "hi@example.org" } ] }, diff --git a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_one_identifier.json b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_one_identifier.json index 4279880cdf5d..3673c6943f03 100644 --- a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_one_identifier.json +++ b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_one_identifier.json @@ -9,8 +9,8 @@ "config": { "addresses": [ { - "address": "hi@example.org", - "channel": "email" + "channel": "email", + "address": "hi@example.org" } ] }, diff --git a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_two_identifiers.json b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_two_identifiers.json index f0447e80d2d8..9163f3c3886b 100644 --- a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_two_identifiers.json +++ b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_email_empty_space_value-with_two_identifiers.json @@ -10,12 +10,12 @@ "config": { "addresses": [ { - "address": "foo@example.org", - "channel": "email" + "channel": "email", + "address": "foo@example.org" }, { - "address": "bar@example.org", - "channel": "email" + "channel": "email", + "address": "bar@example.org" } ] }, diff --git a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_empty_value.json b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_empty_value.json index 4279880cdf5d..3673c6943f03 100644 --- a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_empty_value.json +++ b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_empty_value.json @@ -9,8 +9,8 @@ "config": { "addresses": [ { - "address": "hi@example.org", - "channel": "email" + "channel": "email", + "address": "hi@example.org" } ] }, diff --git a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_unknown_value.json b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_unknown_value.json index 4279880cdf5d..3673c6943f03 100644 --- a/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_unknown_value.json +++ b/identity/.snapshots/TestUpgradeCredentials-type=code-from=v0_with_unknown_value.json @@ -9,8 +9,8 @@ "config": { "addresses": [ { - "address": "hi@example.org", - "channel": "email" + "channel": "email", + "address": "hi@example.org" } ] }, diff --git a/identity/credentials.go b/identity/credentials.go index 3164fa3dafa6..9fc2d93851bb 100644 --- a/identity/credentials.go +++ b/identity/credentials.go @@ -10,6 +10,7 @@ import ( "time" "github.com/gofrs/uuid" + "github.com/wI2L/jsondiff" "github.com/ory/kratos/ui/node" "github.com/ory/x/sqlxx" @@ -248,7 +249,13 @@ func CredentialsEqual(a, b map[CredentialsType]Credentials) bool { return false } - if string(expect.Config) != string(actual.Config) { + // Try to normalize configs (remove spaces etc). + patch, err := jsondiff.CompareJSON(actual.Config, expect.Config) + if err != nil { + return false + } + + if len(patch) > 0 { return false } diff --git a/identity/credentials_migrate.go b/identity/credentials_migrate.go index 8df394a6f6de..dea1a8a44b8d 100644 --- a/identity/credentials_migrate.go +++ b/identity/credentials_migrate.go @@ -100,15 +100,21 @@ func UpgradeCodeCredentials(c *Credentials) (err error) { continue } - c.Config, err = sjson.SetBytes(c.Config, "addresses.-1", map[string]any{ - "address": id, - "channel": channel, + c.Config, err = sjson.SetBytes(c.Config, "addresses.-1", &CredentialsCodeAddress{ + Address: id, + Channel: channel, }) if err != nil { return errors.WithStack(err) } } + // This is needed because sjson adds spaces which can trip string comparisons. + c.Config, err = json.Marshal(json.RawMessage(c.Config)) + if err != nil { + return errors.WithStack(err) + } + c.Version = 1 } return nil diff --git a/identity/extension_credentials.go b/identity/extension_credentials.go index fb614b4558f0..faae191b89ca 100644 --- a/identity/extension_credentials.go +++ b/identity/extension_credentials.go @@ -97,7 +97,7 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch conf.Addresses = append(conf.Addresses, CredentialsCodeAddress{ Channel: via, - Address: fmt.Sprintf("%s", value), + Address: value, }) conf.Addresses = lo.UniqBy(conf.Addresses, func(item CredentialsCodeAddress) string { diff --git a/identity/manager.go b/identity/manager.go index dba4d31fbf10..2429df260fbe 100644 --- a/identity/manager.go +++ b/identity/manager.go @@ -350,6 +350,7 @@ func (m *Manager) requiresPrivilegedAccess(ctx context.Context, original, update if !CredentialsEqual(updated.Credentials, original.Credentials) { // reset the identity *updated = *original + return errors.WithStack(ErrProtectedFieldModified) } diff --git a/internal/testhelpers/session.go b/internal/testhelpers/session.go index 69a79d9bf009..c614f1afe36d 100644 --- a/internal/testhelpers/session.go +++ b/internal/testhelpers/session.go @@ -161,7 +161,7 @@ func NewHTTPClientWithArbitrarySessionTokenAndTraits(t *testing.T, ctx context.C func NewHTTPClientWithArbitrarySessionCookie(t *testing.T, ctx context.Context, reg *driver.RegistryDefault) *http.Client { req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) - req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) + req = req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) id := x.NewUUID() s, err := NewActiveSession(req, reg, &identity.Identity{ID: id, State: identity.StateActive, Traits: []byte("{}"), Credentials: map[identity.CredentialsType]identity.Credentials{ @@ -178,7 +178,7 @@ func NewHTTPClientWithArbitrarySessionCookie(t *testing.T, ctx context.Context, func NewNoRedirectHTTPClientWithArbitrarySessionCookie(t *testing.T, ctx context.Context, reg *driver.RegistryDefault) *http.Client { req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) - req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) + req = req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) id := x.NewUUID() s, err := NewActiveSession(req, reg, &identity.Identity{ID: id, State: identity.StateActive, @@ -196,7 +196,7 @@ func NewNoRedirectHTTPClientWithArbitrarySessionCookie(t *testing.T, ctx context func NewHTTPClientWithIdentitySessionCookie(t *testing.T, ctx context.Context, reg *driver.RegistryDefault, id *identity.Identity) *http.Client { req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) - req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) + req = req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) s, err := NewActiveSession(req, reg, id, time.Now(), @@ -223,7 +223,7 @@ func NewHTTPClientWithIdentitySessionCookieLocalhost(t *testing.T, ctx context.C func NewHTTPClientWithIdentitySessionToken(t *testing.T, ctx context.Context, reg *driver.RegistryDefault, id *identity.Identity) *http.Client { req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) - req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) + req = req.WithContext(confighelpers.WithConfigValue(ctx, "session.lifespan", time.Hour)) s, err := NewActiveSession(req, reg, id, time.Now(), diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 5859449e2074..652de2104248 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -147,19 +147,6 @@ func (s *Strategy) findIdentityByIdentifier(ctx context.Context, identifier stri return id, cred, false, nil } -func (s *Strategy) decode(r *http.Request) (*updateLoginFlowWithCodeMethod, error) { - var p updateLoginFlowWithCodeMethod - if err := s.dx.Decode(r, &p, - decoderx.HTTPDecoderSetValidatePayloads(true), - decoderx.HTTPKeepRequestBody(true), - decoderx.MustHTTPRawJSONSchemaCompiler(loginMethodSchema), - decoderx.HTTPDecoderAllowedMethods("POST"), - decoderx.HTTPDecoderJSONFollowsFormFormat()); err != nil { - return nil, err - } - return &p, nil -} - type decodedMethod struct { Method string `json:"method" form:"method"` Address string `json:"address" form:"address"` @@ -205,7 +192,7 @@ 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 && s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled && (p == nil || len(p.Address) > 0)) { + if !(s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled && (p == nil || len(p.Address) > 0)) { return nil, err } // In this special case we only expect `address` to be set. @@ -303,13 +290,11 @@ func (s *Strategy) findIdentityForIdentifier(ctx context.Context, identifier str return nil, nil, errors.WithStack(schema.NewNoCodeAuthnCredentials()) } - address, err := s.findIdentifierInVerifiableAddress(session.Identity, identifier) + _, err := s.findIdentifierInVerifiableAddress(session.Identity, identifier) if err != nil { return nil, nil, err } - addresses = []Address{*address} - // We only end up here if the identity's identity schema does not have the `code` identifier extension defined. // We know that this is the case for a couple of projects who use 2FA with the code credential. // diff --git a/test/e2e/cypress/integration/profiles/code/login/error.spec.ts b/test/e2e/cypress/integration/profiles/code/login/error.spec.ts index 3310a966627f..244d40ab8132 100644 --- a/test/e2e/cypress/integration/profiles/code/login/error.spec.ts +++ b/test/e2e/cypress/integration/profiles/code/login/error.spec.ts @@ -160,6 +160,12 @@ context("Login error messages with code method", () => { "contain", "Property identifier is missing", ) + } else if (app === "react") { + // The backspace trick is not working in React. + cy.get('[data-testid="ui/message/4010008"]').should( + "contain", + "code is invalid", + ) } else { cy.get('[data-testid="ui/message/4000002"]').should( "contain",