Improve OIDC error handling
This commit is contained in:
		
							parent
							
								
									ea5b8cc21f
								
							
						
					
					
						commit
						42f6cef7d6
					
				|  | @ -206,12 +206,15 @@ func (p *OIDCProvider) CreateSessionFromToken(ctx context.Context, token string) | ||||||
| func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, refresh bool) (*sessions.SessionState, error) { | func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, refresh bool) (*sessions.SessionState, error) { | ||||||
| 	idToken, err := p.verifyIDToken(ctx, token) | 	idToken, err := p.verifyIDToken(ctx, token) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, fmt.Errorf("could not verify id_token: %v", err) | 		switch err { | ||||||
| 	} | 		case ErrMissingIDToken: | ||||||
| 
 | 			// IDToken is mandatory in Redeem but optional in Refresh
 | ||||||
| 	// IDToken is mandatory in Redeem but optional in Refresh
 | 			if !refresh { | ||||||
| 	if idToken == nil && !refresh { | 				return nil, errors.New("token response did not contain an id_token") | ||||||
| 		return nil, errors.New("token response did not contain an id_token") | 			} | ||||||
|  | 		default: | ||||||
|  | 			return nil, fmt.Errorf("could not verify id_token: %v", err) | ||||||
|  | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	ss, err := p.buildSessionFromClaims(idToken) | 	ss, err := p.buildSessionFromClaims(idToken) | ||||||
|  |  | ||||||
|  | @ -129,9 +129,12 @@ type OIDCClaims struct { | ||||||
| func (p *ProviderData) verifyIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { | func (p *ProviderData) verifyIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { | ||||||
| 	rawIDToken := getIDToken(token) | 	rawIDToken := getIDToken(token) | ||||||
| 	if strings.TrimSpace(rawIDToken) != "" { | 	if strings.TrimSpace(rawIDToken) != "" { | ||||||
|  | 		if p.Verifier == nil { | ||||||
|  | 			return nil, ErrMissingOIDCVerifier | ||||||
|  | 		} | ||||||
| 		return p.Verifier.Verify(ctx, rawIDToken) | 		return p.Verifier.Verify(ctx, rawIDToken) | ||||||
| 	} | 	} | ||||||
| 	return nil, nil | 	return nil, ErrMissingIDToken | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // buildSessionFromClaims uses IDToken claims to populate a fresh SessionState
 | // buildSessionFromClaims uses IDToken claims to populate a fresh SessionState
 | ||||||
|  |  | ||||||
|  | @ -137,23 +137,33 @@ func TestProviderData_verifyIDToken(t *testing.T) { | ||||||
| 
 | 
 | ||||||
| 	testCases := map[string]struct { | 	testCases := map[string]struct { | ||||||
| 		IDToken       *idTokenClaims | 		IDToken       *idTokenClaims | ||||||
|  | 		Verifier      bool | ||||||
| 		ExpectIDToken bool | 		ExpectIDToken bool | ||||||
| 		ExpectedError error | 		ExpectedError error | ||||||
| 	}{ | 	}{ | ||||||
| 		"Valid ID Token": { | 		"Valid ID Token": { | ||||||
| 			IDToken:       &defaultIDToken, | 			IDToken:       &defaultIDToken, | ||||||
|  | 			Verifier:      true, | ||||||
| 			ExpectIDToken: true, | 			ExpectIDToken: true, | ||||||
| 			ExpectedError: nil, | 			ExpectedError: nil, | ||||||
| 		}, | 		}, | ||||||
| 		"Invalid ID Token": { | 		"Invalid ID Token": { | ||||||
| 			IDToken:       &failureIDToken, | 			IDToken:       &failureIDToken, | ||||||
|  | 			Verifier:      true, | ||||||
| 			ExpectIDToken: false, | 			ExpectIDToken: false, | ||||||
| 			ExpectedError: errors.New("failed to verify signature: the validation failed for subject [123456789]"), | 			ExpectedError: errors.New("failed to verify signature: the validation failed for subject [123456789]"), | ||||||
| 		}, | 		}, | ||||||
| 		"Missing ID Token": { | 		"Missing ID Token": { | ||||||
| 			IDToken:       nil, | 			IDToken:       nil, | ||||||
|  | 			Verifier:      true, | ||||||
| 			ExpectIDToken: false, | 			ExpectIDToken: false, | ||||||
| 			ExpectedError: nil, | 			ExpectedError: ErrMissingIDToken, | ||||||
|  | 		}, | ||||||
|  | 		"OIDC Verifier not Configured": { | ||||||
|  | 			IDToken:       &defaultIDToken, | ||||||
|  | 			Verifier:      false, | ||||||
|  | 			ExpectIDToken: false, | ||||||
|  | 			ExpectedError: ErrMissingOIDCVerifier, | ||||||
| 		}, | 		}, | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | @ -170,12 +180,13 @@ func TestProviderData_verifyIDToken(t *testing.T) { | ||||||
| 				}) | 				}) | ||||||
| 			} | 			} | ||||||
| 
 | 
 | ||||||
| 			provider := &ProviderData{ | 			provider := &ProviderData{} | ||||||
| 				Verifier: oidc.NewVerifier( | 			if tc.Verifier { | ||||||
|  | 				provider.Verifier = oidc.NewVerifier( | ||||||
| 					oidcIssuer, | 					oidcIssuer, | ||||||
| 					mockJWKS{}, | 					mockJWKS{}, | ||||||
| 					&oidc.Config{ClientID: oidcClientID}, | 					&oidc.Config{ClientID: oidcClientID}, | ||||||
| 				), | 				) | ||||||
| 			} | 			} | ||||||
| 			verified, err := provider.verifyIDToken(context.Background(), token) | 			verified, err := provider.verifyIDToken(context.Background(), token) | ||||||
| 			if err != nil { | 			if err != nil { | ||||||
|  |  | ||||||
|  | @ -22,6 +22,14 @@ var ( | ||||||
| 	// code
 | 	// code
 | ||||||
| 	ErrMissingCode = errors.New("missing code") | 	ErrMissingCode = errors.New("missing code") | ||||||
| 
 | 
 | ||||||
|  | 	// ErrMissingIDToken is returned when an oidc.Token does not contain the
 | ||||||
|  | 	// extra `id_token` field for an IDToken.
 | ||||||
|  | 	ErrMissingIDToken = errors.New("missing id_token") | ||||||
|  | 
 | ||||||
|  | 	// ErrMissingOIDCVerifier is returned when a provider didn't set `Verifier`
 | ||||||
|  | 	// but an attempt to call `Verifier.Verify` was about to be made.
 | ||||||
|  | 	ErrMissingOIDCVerifier = errors.New("oidc verifier is not configured") | ||||||
|  | 
 | ||||||
| 	_ Provider = (*ProviderData)(nil) | 	_ Provider = (*ProviderData)(nil) | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue