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

After redirecting from consent -- runtime error: invalid memory address or nil pointer dereference #988

Closed
mindcrash opened this issue Aug 15, 2018 · 4 comments · Fixed by #989

Comments

@mindcrash
Copy link

mindcrash commented Aug 15, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Hydra crashes after accepting consent and redirecting

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

  1. Setup Hydra as documented
  2. Setup Login / Consent endpoints, with body of Consent endpoint looking like
    await hydra.getConsentRequest(challenge)
        .then(function (response) {
          // for now local users always will give implicit consent for accessing apps
            return hydra.acceptConsentRequest(challenge, {
                grant_scope: response.requested_scope,
            }).then(function (response) {
                 // crash happens after this redirect
                res.statusCode = 302
                res.setHeader("Location", response.redirect_to)
                res.end()
            });
        })

What is the expected behavior?

Hydra doesn't crash after redirecting

Which version of the software is affected?

At least 1.0.0-beta8

Log output

time="2018-08-15T13:12:10Z" level=info msg="started handling request" method=GET remote="172.17.0.1:57622" request="/oauth2/auth?client_id=test&consent_verifier=7b7ae6f82c66468d896c14814e0a4ca9&max_age=0&nonce=rfpxwypbbzrnzzghpvpngesb&prompt=&redirect_uri=http%3A%2F%2F127.0.0.1%3A3000%2Fcallback&response_type=code&scope=openid&state=itjdxedmnehhwaqotojpdrot"
2018/08/15 13:12:10 http2: panic serving 172.17.0.1:57622: runtime error: invalid memory address or nil pointer dereference
goroutine 94 [running]:
net/http.(*http2serverConn).runHandler.func1(0xc42000c350, 0xc4201bdfaf, 0xc4201b0000)
        /usr/local/go/src/net/http/h2_bundle.go:5753 +0x190
panic(0xada4c0, 0xfaa9f0)
        /usr/local/go/src/runtime/panic.go:502 +0x229
github.com/ory/hydra/oauth2.(*Handler).AuthHandler(0xc420096340, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0x0, 0x0, 0x0)
        /go/src/github.com/ory/hydra/oauth2/handler.go:634 +0x66b
github.com/ory/hydra/oauth2.(*Handler).AuthHandler-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0x0, 0x0, 0x0)
        /go/src/github.com/ory/hydra/oauth2/handler.go:161 +0x66
github.com/ory/hydra/vendor/github.com/julienschmidt/httprouter.(*Router).ServeHTTP(0xc420271c20, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/julienschmidt/httprouter/router.go:299 +0x6c1
github.com/ory/hydra/vendor/github.com/urfave/negroni.Wrap.func1(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc4202636e0)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:46 +0x4d
github.com/ory/hydra/vendor/github.com/urfave/negroni.HandlerFunc.ServeHTTP(0xc4202b40a0, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc4202636e0)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:29 +0x4e
github.com/ory/hydra/vendor/github.com/urfave/negroni.middleware.ServeHTTP(0xc4fa40, 0xc4202b40a0, 0xc4202b4180, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0xa5
github.com/ory/hydra/vendor/github.com/urfave/negroni.(middleware).ServeHTTP-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0x60
net/http.HandlerFunc.ServeHTTP(0xc4202636c0, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /usr/local/go/src/net/http/server.go:1947 +0x44
github.com/ory/hydra/cmd/server.(*Handler).rejectInsecureRequests(0xc420265dc0, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc4202636c0)
        /go/src/github.com/ory/hydra/cmd/server/handler.go:260 +0x5f
github.com/ory/hydra/cmd/server.(*Handler).(github.com/ory/hydra/cmd/server.rejectInsecureRequests)-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc4202636c0)
        /go/src/github.com/ory/hydra/cmd/server/handler.go:58 +0x52
github.com/ory/hydra/vendor/github.com/urfave/negroni.HandlerFunc.ServeHTTP(0xc420073d80, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc4202636c0)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:29 +0x4e
github.com/ory/hydra/vendor/github.com/urfave/negroni.middleware.ServeHTTP(0xc4fa40, 0xc420073d80, 0xc4202b4160, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0xa5
github.com/ory/hydra/vendor/github.com/urfave/negroni.(middleware).ServeHTTP-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0x60
github.com/ory/hydra/vendor/github.com/ory/metrics-middleware.(*MetricsManager).ServeHTTP(0xc4200ac0f0, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc420263680)
        /go/src/github.com/ory/hydra/vendor/github.com/ory/metrics-middleware/middleware.go:160 +0x1e6
github.com/ory/hydra/vendor/github.com/urfave/negroni.middleware.ServeHTTP(0xc4e800, 0xc4200ac0f0, 0xc4202b4120, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0xa5
github.com/ory/hydra/vendor/github.com/urfave/negroni.(middleware).ServeHTTP-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0x60
github.com/ory/hydra/metrics/prometheus.(*MetricsManager).ServeHTTP(0xc420080198, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc420263660)
        /go/src/github.com/ory/hydra/metrics/prometheus/middleware.go:26 +0x44
github.com/ory/hydra/vendor/github.com/urfave/negroni.middleware.ServeHTTP(0xc4e480, 0xc420080198, 0xc4202b4100, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0xa5
github.com/ory/hydra/vendor/github.com/urfave/negroni.(middleware).ServeHTTP-fm(0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0x60
github.com/ory/hydra/vendor/github.com/meatballhat/negroni-logrus.(*Middleware).ServeHTTP(0xc42008e540, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000, 0xc420263620)
        /go/src/github.com/ory/hydra/vendor/github.com/meatballhat/negroni-logrus/middleware.go:136 +0x2e6
github.com/ory/hydra/vendor/github.com/urfave/negroni.middleware.ServeHTTP(0xc4e740, 0xc42008e540, 0xc4202b40e0, 0x7f8534ccc4f0, 0xc42000c358, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:38 +0xa5
github.com/ory/hydra/vendor/github.com/urfave/negroni.(*Negroni).ServeHTTP(0xc42001c120, 0xc58500, 0xc42000c350, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/urfave/negroni/negroni.go:96 +0xf2
github.com/ory/hydra/vendor/github.com/rs/cors.(*Cors).Handler.func1(0xc58500, 0xc42000c350, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/rs/cors/cors.go:200 +0xfe
net/http.HandlerFunc.ServeHTTP(0xc4202b4200, 0xc58500, 0xc42000c350, 0xc4202df000)
        /usr/local/go/src/net/http/server.go:1947 +0x44
github.com/ory/hydra/vendor/github.com/gorilla/context.ClearHandler.func1(0xc58500, 0xc42000c350, 0xc4202df000)
        /go/src/github.com/ory/hydra/vendor/github.com/gorilla/context/context.go:141 +0x8b
net/http.HandlerFunc.ServeHTTP(0xc4202b4220, 0xc58500, 0xc42000c350, 0xc4202df000)
        /usr/local/go/src/net/http/server.go:1947 +0x44
net/http.serverHandler.ServeHTTP(0xc42026a000, 0xc58500, 0xc42000c350, 0xc4202df000)
        /usr/local/go/src/net/http/server.go:2694 +0xbc
net/http.initNPNRequest.ServeHTTP(0xc4200dc000, 0xc42026a000, 0xc58500, 0xc42000c350, 0xc4202df000)
        /usr/local/go/src/net/http/server.go:3260 +0x9a
net/http.(Handler).ServeHTTP-fm(0xc58500, 0xc42000c350, 0xc4202df000)
        /usr/local/go/src/net/http/h2_bundle.go:5475 +0x4d
net/http.(*http2serverConn).runHandler(0xc4201b0000, 0xc42000c350, 0xc4202df000, 0xc420263600)
        /usr/local/go/src/net/http/h2_bundle.go:5760 +0x89
created by net/http.(*http2serverConn).processHeaders
        /usr/local/go/src/net/http/h2_bundle.go:5494 +0x46b
@mindcrash
Copy link
Author

After re-checking my consent code I noticed this crash was triggered because I forgot to add the following two fields to the consent payload: remember and remember_for

Maybe some payload validation (and logging) could come in handy here since it is quite easy to overlook things like this in custom endpoints...

@aeneasr
Copy link
Member

aeneasr commented Aug 16, 2018

Huh, that's interesting, it seems like the nil pointer is coming from the session object. I'll investigate.

@aeneasr
Copy link
Member

aeneasr commented Aug 16, 2018

Would it be possible for you to provide your login+consent app as a fully functioning app? I'd like to use it to reproduce the issue.

@aeneasr
Copy link
Member

aeneasr commented Aug 16, 2018

Oh I think I know what the issue is. Because you omitted session (which is fine!) its value is nil but we're accessing a member of said value with session.Session.IDToken which then causes the panic. This is a bug in Hydra and will be fixed. Thank you for reporting!

aeneasr pushed a commit that referenced this issue Aug 16, 2018
This resolves a panic when the session is not available. Closes #988

Signed-off-by: arekkas <aeneas@ory.am>
aeneasr pushed a commit that referenced this issue Aug 16, 2018
This resolves a panic when the session is not available. Closes #988

Signed-off-by: arekkas <aeneas@ory.am>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants