Skip to content
This repository has been archived by the owner on Jul 15, 2021. It is now read-only.

Parse token error ignored #73

Open
stevesloka opened this issue Oct 23, 2018 · 3 comments · Fixed by jcrood/gangway#9
Open

Parse token error ignored #73

stevesloka opened this issue Oct 23, 2018 · 3 comments · Fixed by jcrood/gangway#9

Comments

@stevesloka
Copy link
Contributor

Currently, the parse token method (https://github.com/heptiolabs/gangway/blob/master/cmd/gangway/handlers.go#L190) which takes an id_token and processes swallows the returned error.

Need to look into how to properly handle this error and not ignore. For exampe, if you pass a token foo it won't return an error.

u5surf added a commit to u5surf/gangway that referenced this issue Oct 24, 2018
u5surf added a commit to u5surf/gangway that referenced this issue Oct 24, 2018
@u5surf
Copy link

u5surf commented Oct 24, 2018

@stevesloka
i fixed cmd/gangway/handlers.go and TestParseToken in cmd/gangway/handlers_test.go
but make check failed to following message that has handled errors correctly

$ make check
go test -v ./...
=== RUN   TestConfigNotFound
--- PASS: TestConfigNotFound (0.00s)
=== RUN   TestEnvionmentOverrides
--- PASS: TestEnvionmentOverrides (0.00s)
=== RUN   TestGetRootPathPrefix
=== RUN   TestGetRootPathPrefix/specified_default
=== RUN   TestGetRootPathPrefix/not_specified
=== RUN   TestGetRootPathPrefix/specified
--- PASS: TestGetRootPathPrefix (0.00s)
    --- PASS: TestGetRootPathPrefix/specified_default (0.00s)
    --- PASS: TestGetRootPathPrefix/not_specified (0.00s)
    --- PASS: TestGetRootPathPrefix/specified (0.00s)
=== RUN   TestHomeHandler
--- PASS: TestHomeHandler (0.00s)
=== RUN   TestCallbackHandler
--- PASS: TestCallbackHandler (0.01s)
=== RUN   TestUnauthedCommandlineHandlerRedirect
time="2018-10-24T22:54:53+09:00" level=error msg="Failed to open CA file. open : no such file or directory"
--- PASS: TestUnauthedCommandlineHandlerRedirect (0.03s)
=== RUN   TestParseToken
--- FAIL: TestParseToken (0.00s)
	handlers_test.go:93: Fatal Error parsing token. Unexpected signing method
=== RUN   TestGenerateSessionKeys
--- PASS: TestGenerateSessionKeys (0.01s)
=== RUN   TestInitSessionStore
--- PASS: TestInitSessionStore (0.01s)
=== RUN   TestCleanupSession
--- PASS: TestCleanupSession (0.02s)
FAIL
FAIL	github.com/heptiolabs/gangway/cmd/gangway	0.107s
make: *** [test] Error 1

i focused the result of decoding Header of idToken in TestParseToken
alg is RS256, however the code expects to parse HMAC256(HS256) encoded JWT

code:
https://github.com/heptiolabs/gangway/blob/481502a833e571e064533ab2dada21ae590f4cca/cmd/gangway/handlers.go#L191

idToken in testcode:
https://github.com/heptiolabs/gangway/blob/481502a833e571e064533ab2dada21ae590f4cca/cmd/gangway/handlers_test.go#L89

decode base64:
$ echo eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9 | base64 -D
{"alg":"RS256","typ":"JWT"}

I wonder which "alg" I should choose.

@stevesloka
Copy link
Contributor Author

@u5surf yeah that's what I ran into when I found this bug. I'm not 100% sure how to make the library parse correctly. If you look at this second example, I think we can remove the check for the signature and the token will show up as 'invalid'.

See (https://godoc.org/github.com/dgrijalva/jwt-go#Parse) under the example Example (ErrorChecking).

u5surf added a commit to u5surf/gangway that referenced this issue Oct 29, 2018
@u5surf
Copy link

u5surf commented Oct 29, 2018

@stevesloka I chached checking rules following ErrorChecking of https://godoc.org/github.com/dgrijalva/jwt-go#Parse

jcrood pushed a commit to jcrood/gangway that referenced this issue Jun 7, 2022
Also correct the JWT signatures in the now failing handler tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants