Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into jonas-jonas/fixSess…
Browse files Browse the repository at this point in the history
…ionExpand
  • Loading branch information
jonas-jonas committed Apr 25, 2024
2 parents f971d23 + ec90929 commit 39919bf
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 22 deletions.
1 change: 1 addition & 0 deletions .trivyignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
CVE-2022-30065
CVE-2024-2961
CVE-2023-2650
2 changes: 1 addition & 1 deletion cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func main() {
}
}

if err := writeMessages(filepath.Join(os.Args[2], "concepts/ui-user-interface.mdx"), sortedMessages); err != nil {
if err := writeMessages(filepath.Join(os.Args[2], "concepts/ui-messages.md"), sortedMessages); err != nil {
_, _ = fmt.Fprintf(os.Stderr, "Unable to generate message table: %+v\n", err)
os.Exit(1)
}
Expand Down
15 changes: 11 additions & 4 deletions selfservice/flow/login/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,19 @@ func TestFlowLifecycle(t *testing.T) {
})

t.Run("case=returns session exchange code with any truthy value", func(t *testing.T) {
conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{"https://www.ory.sh", "https://example.com"})
parameters := []string{"true", "True", "1"}

for i := range parameters {
res, body := initFlow(t, url.Values{"return_session_token_exchange_code": {parameters[i]}}, true)
assert.Contains(t, res.Request.URL.String(), login.RouteInitAPIFlow)
assert.NotEmpty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
for _, param := range parameters {
t.Run("return_session_token_exchange_code="+param, func(t *testing.T) {
res, body := initFlow(t, url.Values{
"return_session_token_exchange_code": {param},
"return_to": {"https://example.com/redirect"},
}, true)
assert.Contains(t, res.Request.URL.String(), login.RouteInitAPIFlow)
assert.NotEmpty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
assert.Equal(t, "https://example.com/redirect", gjson.GetBytes(body, "return_to").String())
})
}
})

Expand Down
19 changes: 17 additions & 2 deletions selfservice/strategy/oidc/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func (s *Strategy) alreadyAuthenticated(w http.ResponseWriter, r *http.Request,
returnTo := s.d.Config().SelfServiceBrowserDefaultReturnTo(ctx)
if redirecter, ok := f.(flow.FlowWithRedirect); ok {
r, err := x.SecureRedirectTo(r, returnTo, redirecter.SecureRedirectToOpts(ctx, s.d)...)
if err != nil {
if err == nil {
returnTo = r
}
}
Expand Down Expand Up @@ -462,6 +462,9 @@ func (s *Strategy) HandleCallback(w http.ResponseWriter, r *http.Request, ps htt
case *login.Flow:
a.TransientPayload = cntnr.TransientPayload
if ff, err := s.processLogin(w, r, a, et, claims, provider, cntnr); err != nil {
if errors.Is(err, flow.ErrCompletedByStrategy) {
return
}
if ff != nil {
s.forwardError(w, r, ff, err)
return
Expand Down Expand Up @@ -631,7 +634,19 @@ func (s *Strategy) handleError(w http.ResponseWriter, r *http.Request, f flow.Fl
return err
}
// return a new login flow with the error message embedded in the login flow.
redirectURL := lf.AppendTo(s.d.Config().SelfServiceFlowLoginUI(r.Context()))
var redirectURL *url.URL
if lf.Type == flow.TypeAPI {
returnTo := s.d.Config().SelfServiceBrowserDefaultReturnTo(r.Context())
if redirecter, ok := f.(flow.FlowWithRedirect); ok {
secureReturnTo, err := x.SecureRedirectTo(r, returnTo, redirecter.SecureRedirectToOpts(r.Context(), s.d)...)
if err == nil {
returnTo = secureReturnTo
}
}
redirectURL = lf.AppendTo(returnTo)
} else {
redirectURL = lf.AppendTo(s.d.Config().SelfServiceFlowLoginUI(r.Context()))
}
if dc, err := flow.DuplicateCredentials(lf); err == nil && dc != nil {
redirectURL = urlx.CopyWithQuery(redirectURL, url.Values{"no_org_ui": {"true"}})

Expand Down
55 changes: 40 additions & 15 deletions selfservice/strategy/oidc/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func TestStrategy(t *testing.T) {
return res, body
}

makeAPICodeFlowRequest := func(t *testing.T, provider, action string) (returnToCode string) {
makeAPICodeFlowRequest := func(t *testing.T, provider, action string) (returnToURL *url.URL) {
res, err := testhelpers.NewDebugClient(t).Post(action, "application/json", strings.NewReader(fmt.Sprintf(`{
"method": "oidc",
"provider": %q
Expand All @@ -197,13 +197,10 @@ func TestStrategy(t *testing.T) {
res, err = testhelpers.NewClientWithCookieJar(t, nil, true).Get(changeLocation.RedirectBrowserTo)
require.NoError(t, err)

returnToURL := res.Request.URL
returnToURL = res.Request.URL
assert.True(t, strings.HasPrefix(returnToURL.String(), returnTS.URL+"/app_code"))

code := returnToURL.Query().Get("code")
assert.NotEmpty(t, code, "code query param was empty in the return_to URL")

return code
return returnToURL
}

exchangeCodeForToken := func(t *testing.T, codes sessiontokenexchange.Codes) (codeResponse session.CodeExchangeResponse, err error) {
Expand Down Expand Up @@ -553,12 +550,15 @@ func TestStrategy(t *testing.T) {
t.Run("suite=API with session token exchange code", func(t *testing.T) {
scope = []string{"openid"}

loginOrRegister := func(t *testing.T, id uuid.UUID, code string) {
loginOrRegister := func(t *testing.T, flowID uuid.UUID, code string) {
_, err := exchangeCodeForToken(t, sessiontokenexchange.Codes{InitCode: code})
require.Error(t, err)

action := assertFormValues(t, id, "valid")
returnToCode := makeAPICodeFlowRequest(t, "valid", action)
action := assertFormValues(t, flowID, "valid")
returnToURL := makeAPICodeFlowRequest(t, "valid", action)
returnToCode := returnToURL.Query().Get("code")
assert.NotEmpty(t, code, "code query param was empty in the return_to URL")

codeResponse, err := exchangeCodeForToken(t, sessiontokenexchange.Codes{
InitCode: code,
ReturnToCode: returnToCode,
Expand All @@ -568,11 +568,11 @@ func TestStrategy(t *testing.T) {
assert.NotEmpty(t, codeResponse.Token)
assert.Equal(t, subject, gjson.GetBytes(codeResponse.Session.Identity.Traits, "subject").String())
}
register := func(t *testing.T) {
performRegistration := func(t *testing.T) {
f := newAPIRegistrationFlow(t, returnTS.URL+"?return_session_token_exchange_code=true&return_to=/app_code", 1*time.Minute)
loginOrRegister(t, f.ID, f.SessionTokenExchangeCode)
}
login := func(t *testing.T) {
performLogin := func(t *testing.T) {
f := newAPILoginFlow(t, returnTS.URL+"?return_session_token_exchange_code=true&return_to=/app_code", 1*time.Minute)
loginOrRegister(t, f.ID, f.SessionTokenExchangeCode)
}
Expand All @@ -582,23 +582,48 @@ func TestStrategy(t *testing.T) {
first, then func(*testing.T)
}{{
name: "login-twice",
first: login, then: login,
first: performLogin, then: performLogin,
}, {
name: "login-then-register",
first: login, then: register,
first: performLogin, then: performRegistration,
}, {
name: "register-then-login",
first: register, then: login,
first: performRegistration, then: performLogin,
}, {
name: "register-twice",
first: register, then: register,
first: performRegistration, then: performRegistration,
}} {
t.Run("case="+tc.name, func(t *testing.T) {
subject = tc.name + "[email protected]"
tc.first(t)
tc.then(t)
})
}
t.Run("case=should use redirect_to URL on failure", func(t *testing.T) {
ctx := context.Background()
subject = "[email protected]"

i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
i.SetCredentials(identity.CredentialsTypePassword, identity.Credentials{
Identifiers: []string{subject},
})
i.Traits = identity.Traits(`{"subject":"` + subject + `"}`)
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(ctx, i))

f := newAPILoginFlow(t, returnTS.URL+"?return_session_token_exchange_code=true&return_to=/app_code", 1*time.Minute)

_, err := exchangeCodeForToken(t, sessiontokenexchange.Codes{InitCode: f.SessionTokenExchangeCode})
require.Error(t, err)

action := assertFormValues(t, f.ID, "valid")
returnToURL := makeAPICodeFlowRequest(t, "valid", action)
returnedFlow := returnToURL.Query().Get("flow")

require.NotEmpty(t, returnedFlow, "flow query param was empty in the return_to URL")
loginFlow, err := reg.LoginFlowPersister().GetLoginFlow(ctx, uuid.FromStringOrNil(returnedFlow))
require.NoError(t, err)
assert.Equal(t, text.ErrorValidationDuplicateCredentials, loginFlow.UI.Messages[0].ID)
})
})

t.Run("case=submit id_token during registration or login", func(t *testing.T) {
Expand Down

0 comments on commit 39919bf

Please sign in to comment.