-
Notifications
You must be signed in to change notification settings - Fork 173
Add OAuth authorization server integration tests #3531
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
base: main
Are you sure you want to change the base?
Conversation
Add integration test for the full OAuth 2.0 Authorization Code flow with PKCE using mockoidc as the upstream IDP. TestIntegration_FullPKCEFlow validates the complete OAuth flow including authorization through the upstream IDP, client state preservation, token exchange with PKCE verification, and refresh token issuance via the offline_access scope. The test also verifies RFC 8707 resource parameter handling for audience binding and comprehensive JWT claims (iss, sub, aud, iat, exp, scp).
2fc4b0a to
fd666fb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3531 +/- ##
==========================================
- Coverage 65.35% 65.31% -0.04%
==========================================
Files 403 403
Lines 39210 39210
==========================================
- Hits 25625 25610 -15
- Misses 11599 11614 +15
Partials 1986 1986 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pkg/authserver/server_impl.go
Outdated
| // - Strategy: how to generate and validate tokens | ||
| // - Factories: which OAuth grant types to enable (each adds handlers for specific flows) | ||
| return compose.Compose( | ||
| provider := compose.Compose( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can just return the composer.Compose() without adding a var here
pkg/authserver/integration_test.go
Outdated
| require.True(t, ok, "scp claim should be an array") | ||
| scopeStrings := make([]string, len(scope)) | ||
| for i, s := range scope { | ||
| scopeStrings[i] = s.(string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should validate the type here
| } | ||
|
|
||
| tokenResp := makeTokenRequest(t, serverURL, params) | ||
| defer tokenResp.Body.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are closing the body twice... here and on parsetokenresponse
yrobla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved with some comments
Remove defer resp.Body.Close() from parseTokenResponse since the caller (exchangeCodeForTokens) is responsible for closing the body. Following Go's convention that the caller of the function returning an *http.Response owns the resource and handles cleanup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Check that each element in the scp claim array is a string, consistent with how the outer array type assertion is validated. Provides a clear test failure message instead of a panic if the token structure is unexpected. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove intermediate variable assignment and return the result directly, following idiomatic Go style. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add integration test for the full OAuth 2.0 Authorization Code flow with PKCE using mockoidc as the upstream IDP.
TestIntegration_FullPKCEFlow validates the complete OAuth flow including authorization through the upstream IDP, client state preservation, token exchange with PKCE verification, and refresh token issuance via the offline_access scope. The test also verifies RFC 8707 resource parameter handling for audience binding and comprehensive JWT claims (iss, sub, aud, iat, exp, scp).