Skip to content

Commit

Permalink
fix: don't assume the login challenge to be a UUID
Browse files Browse the repository at this point in the history
For compatibility with ory/hydra#3515, which
now encodes the whole flow in the login challenge, we cannot further
assume that the challenge is a UUID.
  • Loading branch information
hperl committed Jun 16, 2023
1 parent d9efc0a commit a7eb0f9
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 40 deletions.
7 changes: 6 additions & 1 deletion hydra/hydra.go
Expand Up @@ -50,7 +50,12 @@ func GetLoginChallengeID(conf *config.Config, r *http.Request) (sqlxx.NullString
return "", errors.WithStack(herodot.ErrInternalServerError.WithReason("refusing to parse login_challenge query parameter because " + config.ViperKeyOAuth2ProviderURL + " is invalid or unset"))
}

return sqlxx.NullString(r.URL.Query().Get("login_challenge")), nil
loginChallenge := r.URL.Query().Get("login_challenge")
if loginChallenge == "" {
return "", errors.WithStack(herodot.ErrBadRequest.WithReason("the login_challenge parameter is present but empty"))
}

return sqlxx.NullString(loginChallenge), nil
}

func (h *DefaultHydra) getAdminURL(ctx context.Context) (string, error) {
Expand Down
9 changes: 9 additions & 0 deletions hydra/hydra_test.go
Expand Up @@ -64,6 +64,15 @@ func TestGetLoginChallengeID(t *testing.T) {
want: "",
assertErr: assert.NoError,
},
{
name: "empty login challenge; hydra is configured",
args: args{
conf: configWithHydra,
r: requestFromChallenge(""),
},
want: "",
assertErr: assert.Error,
},
{
name: "login_challenge is present; Hydra is not configured",
args: args{
Expand Down
@@ -0,0 +1,17 @@
{
"id": "cccccccc-dda4-4700-9e42-35731f2af91e",
"oauth2_login_challenge": "challenge data",
"type": "api",
"expires_at": "2013-10-07T08:23:19Z",
"issued_at": "2013-10-07T08:23:19Z",
"request_url": "http://kratos:4433/self-service/browser/flows/login",
"ui": {
"action": "",
"method": "",
"nodes": null
},
"created_at": "2013-10-07T08:23:19Z",
"updated_at": "2013-10-07T08:23:19Z",
"refresh": false,
"requested_aal": "aal1"
}
@@ -0,0 +1,8 @@
INSERT INTO selfservice_login_flows (id, nid, request_url, issued_at, expires_at, active_method, csrf_token, created_at,
updated_at, forced, type, ui, internal_context, oauth2_login_challenge_data)
VALUES ('cccccccc-dda4-4700-9e42-35731f2af91e',
'884f556e-eb3a-4b9f-bee3-11345642c6c0',
'http://kratos:4433/self-service/browser/flows/login',
'2013-10-07 08:23:19', '2013-10-07 08:23:19', '',
'fpeVSZ9ZH7YvUkhXsOVEIssxbfauh5lcoQSYxTcN0XkMneg1L42h+HtvisjlNjBF4ElcD2jApCHoJYq2u9sVWg==',
'2013-10-07 08:23:19', '2013-10-07 08:23:19', false, 'api', '{}', '{"foo":"bar"}', 'challenge data');
4 changes: 2 additions & 2 deletions selfservice/flow/login/hook.go
Expand Up @@ -233,8 +233,8 @@ func (e *HookExecutor) PostLoginHook(

// If Kratos is used as a Hydra login provider, we need to redirect back to Hydra by returning a 422 status
// with the post login challenge URL as the body.
if a.OAuth2LoginChallenge.Valid {
postChallengeURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), a.OAuth2LoginChallenge.UUID, i.ID.String(), s.AMR)
if a.OAuth2LoginChallenge != "" {
postChallengeURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
if err != nil {
return err
}
Expand Down
37 changes: 0 additions & 37 deletions test/e2e/cypress/integration/profiles/oidc-provider/error.spec.ts

This file was deleted.

0 comments on commit a7eb0f9

Please sign in to comment.