From 07cb83c672326848162998a9cfbc8ca34af42bf0 Mon Sep 17 00:00:00 2001 From: hackerman <3372410+aeneasr@users.noreply.github.com> Date: Wed, 29 Jan 2025 15:46:43 +0100 Subject: [PATCH] fix: set correct request url in acc linking and oidc flows (#4282) --- internal/client-go/go.sum | 2 ++ selfservice/flow/login/handler.go | 6 ------ selfservice/strategy/oidc/strategy_login.go | 6 ++---- selfservice/strategy/oidc/strategy_registration.go | 10 ++++++---- selfservice/strategy/oidc/strategy_test.go | 1 + 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/internal/client-go/go.sum b/internal/client-go/go.sum index c966c8ddfd0d..734252e68153 100644 --- a/internal/client-go/go.sum +++ b/internal/client-go/go.sum @@ -4,6 +4,8 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e h1:bRhVy7zSSasaqNksaRZiA5EEI+Ei4I1nO5Jh72wfHlg= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= +golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go index f98ba66cd3fb..3ded1c05f332 100644 --- a/selfservice/flow/login/handler.go +++ b/selfservice/flow/login/handler.go @@ -110,12 +110,6 @@ func WithOrganizationID(organizationID uuid.NullUUID) FlowOption { } } -func WithRequestedAAL(aal identity.AuthenticatorAssuranceLevel) FlowOption { - return func(f *Flow) { - f.RequestedAAL = aal - } -} - func WithInternalContext(internalContext []byte) FlowOption { return func(f *Flow) { f.InternalContext = internalContext diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index 392009ec2241..ffe04c7c3e75 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -144,13 +144,11 @@ func (s *Strategy) processLogin(ctx context.Context, w http.ResponseWriter, r *h registrationFlow.OrganizationID = loginFlow.OrganizationID registrationFlow.IDToken = loginFlow.IDToken registrationFlow.RawIDTokenNonce = loginFlow.RawIDTokenNonce - registrationFlow.RequestURL, err = x.TakeOverReturnToParameter(loginFlow.RequestURL, registrationFlow.RequestURL) registrationFlow.TransientPayload = loginFlow.TransientPayload registrationFlow.Active = s.ID() - if err != nil { - return nil, s.handleError(ctx, w, r, loginFlow, provider.Config().ID, nil, err) - } + // We are converting the flow here, but want to retain the original request URL. + registrationFlow.RequestURL = loginFlow.RequestURL if _, err := s.processRegistration(ctx, w, r, registrationFlow, token, claims, provider, container); err != nil { return registrationFlow, err diff --git a/selfservice/strategy/oidc/strategy_registration.go b/selfservice/strategy/oidc/strategy_registration.go index 5ed061119e7b..ccb6287cdfb2 100644 --- a/selfservice/strategy/oidc/strategy_registration.go +++ b/selfservice/strategy/oidc/strategy_registration.go @@ -272,10 +272,12 @@ func (s *Strategy) registrationToLogin(ctx context.Context, w http.ResponseWrite return nil, err } - lf.RequestURL, err = x.TakeOverReturnToParameter(rf.RequestURL, lf.RequestURL) - if err != nil { - return nil, err - } + // In this scenario, we are performing account linking. The request URL is set to the "original" registration URL + // the user used to try and sign up (which triggered the account linking flow). + // + // In this scenario we want to keep the original request url instead of the current request url, as the current + // request url is an "in-between" state where we are half-way through performing account linking. + lf.RequestURL = rf.RequestURL lf.TransientPayload = rf.TransientPayload lf.Active = s.ID() lf.OrganizationID = rf.OrganizationID diff --git a/selfservice/strategy/oidc/strategy_test.go b/selfservice/strategy/oidc/strategy_test.go index dde3e1bda607..927d0c27457b 100644 --- a/selfservice/strategy/oidc/strategy_test.go +++ b/selfservice/strategy/oidc/strategy_test.go @@ -1578,6 +1578,7 @@ func TestStrategy(t *testing.T) { assert.True(t, gjson.GetBytes(body, "ui.nodes.#(attributes.name==password)").Exists(), "%s", body) assert.Equal(t, "new-login-if-email-exist-with-password-strategy@ory.sh", gjson.GetBytes(body, "ui.messages.#(id==1010016).context.duplicateIdentifier").String()) assert.Equal(t, gjson.GetBytes(body, "organization_id").String(), orgID.String()) + assert.NotContains(t, gjson.GetBytes(body, "request_url").String(), "code=") linkingLoginFlow.ID = gjson.GetBytes(body, "id").String() linkingLoginFlow.UIAction = gjson.GetBytes(body, "ui.action").String() linkingLoginFlow.CSRFToken = gjson.GetBytes(body, `ui.nodes.#(attributes.name=="csrf_token").attributes.value`).String()