Skip to content

Commit

Permalink
Infer the JWS signing algorithm name by looking at the provided key
Browse files Browse the repository at this point in the history
This handles cases where the JWS message or the key do not have a proper `alg`
header. The `alg` header is optional[0], so some identity providers may not
supply it (such as Microsoft Identity[1])

[0]: https://datatracker.ietf.org/doc/html/rfc7517#section-4.4
[1]: https://login.microsoftonline.com/common/discovery/v2.0/keys

Signed-off-by: Erik Haugrud <[email protected]>
  • Loading branch information
erik-h committed Apr 13, 2024
1 parent 8e2e849 commit b264d29
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
2 changes: 1 addition & 1 deletion internal/authz/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ func (o *oidcHandler) isValidIDToken(ctx context.Context, log telemetry.Logger,
return false, codes.Internal
}

if _, err := jws.Verify([]byte(idTokenString), jws.WithKeySet(jwtSet)); err != nil {
if _, err := jws.Verify([]byte(idTokenString), jws.WithKeySet(jwtSet, jws.WithInferAlgorithmFromKey(true))); err != nil {
log.Error("error verifying id token with fetched jwks", err)
return false, codes.Internal
}
Expand Down
29 changes: 26 additions & 3 deletions internal/authz/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ func TestOIDCProcess(t *testing.T) {

unknownJWKPriv, _ := newKeyPair(t)
jwkPriv, jwkPub := newKeyPair(t)
bytes, err := json.Marshal(newKeySet(t, jwkPub))
noAlgJwkPriv, noAlgJwkPub := newKeyPair(t)
noAlgJwkPriv.Set(jwk.KeyIDKey, noAlgKeyId)

Check failure on line 198 in internal/authz/oidc_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `noAlgJwkPriv.Set` is not checked (errcheck)
noAlgJwkPub.Set(jwk.KeyIDKey, noAlgKeyId)

Check failure on line 199 in internal/authz/oidc_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `noAlgJwkPub.Set` is not checked (errcheck)
noAlgJwkPub.Remove(jwk.AlgorithmKey)

Check failure on line 200 in internal/authz/oidc_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `noAlgJwkPub.Remove` is not checked (errcheck)

bytes, err := json.Marshal(newKeySet(t, jwkPub, noAlgJwkPub))
require.NoError(t, err)
basicOIDCConfig.JwksConfig = &oidcv1.OIDCConfig_Jwks{
Jwks: string(bytes),
Expand Down Expand Up @@ -360,6 +365,23 @@ func TestOIDCProcess(t *testing.T) {
requireStoredTokens(t, store, newSessionID, false)
},
},
{
name: "successfully retrieve new tokens when 'alg' is not specified in JWK",
req: callbackRequest,
storedAuthState: validAuthState,
mockTokensResponse: &idpTokensResponse{
IDToken: newJWT(t, noAlgJwkPriv, jwt.NewBuilder().Audience([]string{"test-client-id"}).Claim("nonce", newNonce)),
AccessToken: "access-token",
TokenType: "Bearer",
},
responseVerify: func(t *testing.T, resp *envoy.CheckResponse) {
require.Equal(t, int32(codes.Unauthenticated), resp.GetStatus().GetCode())
requireStandardResponseHeaders(t, resp)
requireRedirectResponse(t, resp.GetDeniedResponse(), requestedAppURL, nil)
requireStoredTokens(t, store, sessionID, true)
requireStoredTokens(t, store, newSessionID, false)
},
},
{
name: "request is invalid, query parameters are missing",
req: modifyCallbackRequestPath("/callback?"),
Expand Down Expand Up @@ -1405,8 +1427,9 @@ func modifyCallbackRequestPath(path string) *envoy.CheckRequest {
}

const (
keyID = "test"
keyAlg = jwa.RS256
keyID = "test"
keyAlg = jwa.RS256
noAlgKeyId = "noAlgTest"

Check failure on line 1432 in internal/authz/oidc_test.go

View workflow job for this annotation

GitHub Actions / lint

var-naming: const noAlgKeyId should be noAlgKeyID (revive)
)

func newKeySet(t *testing.T, keys ...jwk.Key) jwk.Set {
Expand Down

0 comments on commit b264d29

Please sign in to comment.