Skip to content

Commit 91a4e5b

Browse files
author
John Rood
committed
Propagate JWT parse error, fixes vmware-archive#73.
Also correct the JWT signatures in the now failing handler tests.
1 parent ccacd51 commit 91a4e5b

File tree

4 files changed

+31
-10
lines changed

4 files changed

+31
-10
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ Fixes https://github.com/vmware-archive/gangway/issues/71
4747
* BREAKING - corrected ENV variable name of `customHTMLTemplatesDir` to `CUSTOM_HTML_TEMPLATES_DIR`,
4848
this was (incorrectly) `CUSTOM_HTTP_TEMPLATES_DIR`
4949
* Config option `showClaims` to show/hide received claims
50-
50+
* Propagate JWT parse error (https://github.com/vmware-archive/gangway/issues/73)
5151

5252
## v3.3.0 (2021-07-15)
5353

‎cmd/gangway/handlers_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func TestCommandLineHandler(t *testing.T) {
146146
"default": {
147147
params: map[string]string{
148148
"state": "Uv38ByGCZU8WP18PmmIdcpVmx00QA3xNe7sEB9Hixkk=",
149-
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.zNG4Dnxr76J0p4phfsAUYWunioct0krkMiunMynlQsU",
149+
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.QxBLa0qPxN2GgrUKc22BTBTks_TUs5xxDDtCKbRtmBg",
150150
"refresh_token": "bar",
151151
"code": "0cj0VQzNl36e4P2L&state=jdep4ov52FeUuzWLDDtSXaF4b5%2F%2FCUJ52xlE69ehnQ8%3D",
152152
},
@@ -158,7 +158,7 @@ func TestCommandLineHandler(t *testing.T) {
158158
"incorrect username claim": {
159159
params: map[string]string{
160160
"state": "Uv38ByGCZU8WP18PmmIdcpVmx00QA3xNe7sEB9Hixkk=",
161-
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.zNG4Dnxr76J0p4phfsAUYWunioct0krkMiunMynlQsU",
161+
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.QxBLa0qPxN2GgrUKc22BTBTks_TUs5xxDDtCKbRtmBg",
162162
"refresh_token": "bar",
163163
"code": "0cj0VQzNl36e4P2L&state=jdep4ov52FeUuzWLDDtSXaF4b5%2F%2FCUJ52xlE69ehnQ8%3D",
164164
},
@@ -169,7 +169,7 @@ func TestCommandLineHandler(t *testing.T) {
169169
"no email claim": {
170170
params: map[string]string{
171171
"state": "Uv38ByGCZU8WP18PmmIdcpVmx00QA3xNe7sEB9Hixkk=",
172-
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.zNG4Dnxr76J0p4phfsAUYWunioct0krkMiunMynlQsU",
172+
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.QxBLa0qPxN2GgrUKc22BTBTks_TUs5xxDDtCKbRtmBg",
173173
"refresh_token": "bar",
174174
"code": "0cj0VQzNl36e4P2L&state=jdep4ov52FeUuzWLDDtSXaF4b5%2F%2FCUJ52xlE69ehnQ8%3D",
175175
},
@@ -277,7 +277,7 @@ func TestKubeconfigHandler(t *testing.T) {
277277
ClientSecret: "someClientSecret",
278278
},
279279
params: map[string]string{
280-
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.zNG4Dnxr76J0p4phfsAUYWunioct0krkMiunMynlQsU",
280+
"id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.a5ug38N-hzHYsrFMx3puWfCSD_44lFUUugTsr8J9vH0",
281281
"refresh_token": "bar",
282282
},
283283
expectedStatusCode: http.StatusOK,
@@ -286,7 +286,7 @@ func TestKubeconfigHandler(t *testing.T) {
286286
expectedAuthInfoAuthProviderConfig: map[string]string{
287287
"client-id": "someClientID",
288288
"client-secret": "someClientSecret",
289-
"id-token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.zNG4Dnxr76J0p4phfsAUYWunioct0krkMiunMynlQsU",
289+
"id-token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJHYW5nd2F5VGVzdCIsImlhdCI6MTU0MDA0NjM0NywiZXhwIjoxODg3MjAxNTQ3LCJhdWQiOiJnYW5nd2F5LmhlcHRpby5jb20iLCJzdWIiOiJnYW5nd2F5QGhlcHRpby5jb20iLCJHaXZlbk5hbWUiOiJHYW5nIiwiU3VybmFtZSI6IldheSIsIkVtYWlsIjoiZ2FuZ3dheUBoZXB0aW8uY29tIiwiR3JvdXBzIjoiZGV2LGFkbWluIn0.a5ug38N-hzHYsrFMx3puWfCSD_44lFUUugTsr8J9vH0",
290290
"refresh-token": "bar",
291291
"idp-issuer-url": "GangwayTest",
292292
"idp-certificate-authority-data": "",

‎internal/oidc/token.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ type Token struct {
3333

3434
// ParseToken returns a jwt token from an idToken, returns error if it cannot parse
3535
func ParseToken(idToken, clientSecret string) (*jwt.Token, error) {
36-
token, _ := jwt.Parse(idToken, func(token *jwt.Token) (interface{}, error) {
36+
token, err := jwt.Parse(idToken, func(token *jwt.Token) (interface{}, error) {
3737
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
3838
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
3939
}
4040
return []byte(clientSecret), nil
4141
})
4242

43-
return token, nil
43+
return token, err
4444
}
4545

4646
// Exchange takes an oauth2 auth token and exchanges for an id_token

‎internal/oidc/token_test.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func TestParseToken(t *testing.T) {
5555
"rsa": {
5656
idToken: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.EkN-DOsnsuRjRO6BxXemmJDm3HbxrbRzXglbN2S4sOkopdU4IsDxTI8jO19W_A4K8ZPJijNLis4EZsHeY559a4DFOd50_OqgHGuERTqYZyuhtF39yxJPAjUESwxk2J5k_4zM3O-vtd1Ghyo4IbqKKSy6J9mTniYJPenn5-HIirE",
5757
clientSecret: "",
58-
expectError: false,
58+
expectError: true,
5959
want: &jwt.Token{
6060
Raw: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.EkN-DOsnsuRjRO6BxXemmJDm3HbxrbRzXglbN2S4sOkopdU4IsDxTI8jO19W_A4K8ZPJijNLis4EZsHeY559a4DFOd50_OqgHGuERTqYZyuhtF39yxJPAjUESwxk2J5k_4zM3O-vtd1Ghyo4IbqKKSy6J9mTniYJPenn5-HIirE",
6161
Method: jwt.SigningMethodRS256,
@@ -72,6 +72,25 @@ func TestParseToken(t *testing.T) {
7272
Valid: false,
7373
},
7474
},
75+
"invalid": {
76+
idToken: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0In0.HE7fK0xOQwFEr4WDgRWj4teRPZ6i3GLwD5YCm6Pwu_c",
77+
clientSecret: "",
78+
expectError: true,
79+
want: nil,
80+
},
81+
"alg-none": {
82+
// uses {"alg":"none","typ":"JWT"}
83+
idToken: "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0K.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0aiIsImlhdCI6MTQyMjc3OTYzOH0K.",
84+
clientSecret: "",
85+
expectError: true,
86+
want: nil,
87+
},
88+
"abc": {
89+
idToken: "I know my ABCs",
90+
clientSecret: "",
91+
expectError: true,
92+
want: nil,
93+
},
7594
}
7695

7796
for name, tc := range tests {
@@ -82,8 +101,10 @@ func TestParseToken(t *testing.T) {
82101
// If we expect an error, check that it's thrown
83102
if tc.expectError {
84103
if err == nil {
85-
t.Fatalf("Error was returned but not expected: %v", err)
104+
t.Fatalf("No error was returned but one expected")
86105
}
106+
} else if err != nil {
107+
t.Fatalf("Error was returned but not expected: %v", err)
87108
} else {
88109
// We don't expect an error, check the result
89110
if got.Valid != tc.want.Valid {

0 commit comments

Comments
 (0)