Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hydra + Kratos login flow #3108

Closed
3 of 6 tasks
friedemannsommer opened this issue Feb 15, 2023 · 6 comments · Fixed by #3152
Closed
3 of 6 tasks

Hydra + Kratos login flow #3108

friedemannsommer opened this issue Feb 15, 2023 · 6 comments · Fixed by #3152
Assignees
Labels
bug Something is not working.

Comments

@friedemannsommer
Copy link

friedemannsommer commented Feb 15, 2023

Preflight checklist

Describe the bug

I'm trying to use the Hydra integration via the login_challenge argument for Kratos,
but the resulting responses are not what I would expect based on the API documentation.

  • I would expect Kratos to request the OAuth2 login request from Hydra (which it does) and then add this
    to the oauth2_login_request field of the login flow, but this field is only present if the flow is (re-)requested via getLoginFlow.
  • After a successful login, Kratos should redirect to the redirect_to URL which will be returned from Hydra via the acceptOAuth2LoginRequest request.

Reproducing the bug

oauth2_login_request field missing in createBrowserLoginFlow response

  1. flow = createBrowserLoginFlow({ loginChallenge: 'login_challenge' })
  2. flow will contain the oauth2_login_challenge field but oauth2_login_request isn't set
  3. flow = getLoginFlow({ id: flow.id })
  4. now the flow contains the oauth2_login_challenge and oauth2_login_request fields

redirect_browser_to after login with login_challenge

  1. flow = createBrowserLoginFlow({ loginChallenge: 'login_challenge' })
  2. flow = updateLoginFlow({ flow: flow.id, updateLoginFlowBody: { ... }})
  3. flow will now be the SuccessfulNativeLogin response instead of browser_location_change_required

Relevant log output

started handling request http_request=map[headers:map[accept:application/json, text/plain, */* connection:close cookie:[csrf_token=base64]] host:127.0.0.1 method:GET path:/self-service/login/browser query:login_challenge=123456789 remote:127.0.0.1]
[DEBUG] GET https://hydra-admin/admin/oauth2/auth/requests/login?login_challenge=123456789 audience=application service_name=Ory Kratos service_version=v0.11.1
completed handling request http_request=started handling request http_request=map[headers:map[accept:application/json, text/plain, */* connection:close cookie:[csrf_token=base64]] host:127.0.0.1 method:GET path:/self-service/login/browser query:login_challenge=123456789 remote:127.0.0.1] http_response=map[headers:map[cache-control:private, no-cache, no-store, must-revalidate content-type:application/json; charset=utf-8 vary:Cookie] size:1880 status:200 text_status:OK took:40.0ms]

Relevant configuration

selfservice:
  methods:
    password:
      enabled: true

oauth2_provider:
  url: https://hydra-admin

log:
  level: debug

Version

v0.11.1

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes

Additional Context

The createBrowserLoginFlow (selfservice/flow/login/handler.go) function requests the Hydra login request and checks it, but seemingly doesn't add it the flow. But the getLoginFlow (selfservice/flow/login/handler.go) function does add the Hydra login request as field.

if ar.OAuth2LoginChallenge.Valid {
hlr, err := h.d.Hydra().GetLoginRequest(r.Context(), ar.OAuth2LoginChallenge)
if err != nil {
// We don't redirect back to the third party on errors because Hydra doesn't
// give us the 3rd party return_uri when it redirects to the login UI.
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
}
ar.HydraLoginRequest = hlr
}

@friedemannsommer friedemannsommer added the bug Something is not working. label Feb 15, 2023
@fabhuebner
Copy link

+1

@vinckr
Copy link
Member

vinckr commented Feb 21, 2023

Hello @friedemannsommer & @fabhuebner

See this PR: ory/examples#67

This is a community effort to create a self-hosted Ory Kratos + Hydra integration example.
Any contribution / review / feedback etc is much appreciated 🙌

@friedemannsommer
Copy link
Author

friedemannsommer commented Feb 21, 2023

@vinckr If I understand the example correctly, it's using the Kratos init flow redirect and then loads the given flow which prevents part of the issue I described.

if (!isQuerySet(flow)) {
  logger.debug("No flow ID found in URL query initializing login flow", {
    query: req.query,
  })
  res.redirect(303, initFlowUrl)
  return
}

Example source references:

Sadly I'm unable to implement it like this and must rely on the JSON response of these endpoints.

Which leads me to think, it is expected that the oauth2_login_request is missing in the JSON response of createBrowserLoginFlow (if a valid login_challenge was given)?

Based on the current implementation of the example I don't understand how the redirect after a successful login should work. Does Kratos redirect to Hydra if the form data is passed directly to /self-service/login?flow=<UUIDv4> (explicitly not using updateLoginFlow method)? I'll try to check this and report back.

@vinckr
Copy link
Member

vinckr commented Feb 23, 2023

Hey @friedemannsommer

thanks for the detailed response.
The example is currently not in a working state, so that is why some things might be weird.
I also have to get into the gritty details some more...
Maybe we can include your use case as well in it (or it requires a different example setup? 🤔 )

@friedemannsommer
Copy link
Author

friedemannsommer commented Feb 28, 2023

I've created a (very basic) example project to demonstrate the described issues.
github.com/friedemannsommer/hydra-kratos-nextjs-integration

So far I have not managed to test if Kratos correctly redirects to Hydra after a successful login, if the login flow doesn't use the JSON API.

aeneasr pushed a commit that referenced this issue Mar 8, 2023
The oauth2_login_request field was missing when initially creating the login flow.

Closes #3108
@friedemannsommer
Copy link
Author

@vinckr @CaptainStandby Thank you for addressing this issue, should i create a separate issue (or rather discussion) for the redirect after login topic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants