ory / hydra

The most scalable and customizable OpenID Certified™ OpenID Connect and OAuth Provider on the market. Become an OpenID Connect and OAuth2 Provider over night. Broad support for related RFCs. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.63k stars 1.5k forks source link

RP-Initiated Logout doesn't work #1546

Closed leorb closed 5 years ago

leorb commented 5 years ago

Describe the bug

When the browser is directed to the logout endpoint, Hydra responds with "invalid_request" error.

Reproducing the bug

Steps to reproduce the behavior:

  1. Set OAUTH2_ISSUER_URL issuer parameter value for Hydra to a URL that does not contain a trailing slash ie. http://localhost:4444
  2. Start Hydra
  3. Mint an ID token
  4. Go to the logout endpoint, specifying the ID token in the id_token_hint parameter as recommended by the spec
  5. See the error.

Server logs

Stack trace: 
github.com/ory/hydra/consent.(*DefaultStrategy).issueLogoutVerifier
    /go/src/github.com/ory/hydra/consent/strategy_default.go:776
github.com/ory/hydra/consent.(*DefaultStrategy).HandleOpenIDConnectLogout
    /go/src/github.com/ory/hydra/consent/strategy_default.go:941
github.com/ory/hydra/oauth2.(*Handler).LogoutHandler
    /go/src/github.com/ory/hydra/oauth2/handler.go:121
github.com/julienschmidt/httprouter.(*Router).ServeHTTP
    /go/pkg/mod/github.com/julienschmidt/httprouter@v1.2.0/router.go:334

Expected behavior Hydra should log out the End-User and redirect the browser to the post_logout_redirect_uri.

Environment

Additional context It seems that the problem is something along these lines:

  1. Hydra appends a slash to the ISSUER_URL (discussed in #1041 and #1482)
  2. The issuer + slash are subsequently reflected in the iss field of the token
  3. logout verification fails because the issuer in the id_token_hint is not the same as the issuer specified in the config.

A possible workaround might be to specify the trailing slash in the config file.

aeneasr commented 5 years ago

Nice find! I think this can easily be fixed by TrimRight(..., "/") + "/" here

aeneasr commented 5 years ago

Are you up for a PR? :)

leorb commented 5 years ago

Thank you!