1276 | add configurable userinfo response_type (JSON/JWS/JWE)#1362
1276 | add configurable userinfo response_type (JSON/JWS/JWE)#1362nandhu-kumar wants to merge 1 commit intoasgardeo:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a UserInfo response type enum and field, propagated Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1362 +/- ##
=======================================
Coverage 89.67% 89.68%
=======================================
Files 649 649
Lines 42534 42546 +12
Branches 2454 2454
=======================================
+ Hits 38144 38156 +12
Misses 2375 2375
Partials 2015 2015
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3c8147f to
5ad6f0f
Compare
|
We should add the |
|
Shall we also squash the commits? |
5ad6f0f to
2ff5c77
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/application/store.go (1)
276-286:⚠️ Potential issue | 🟠 MajorPopulate UserInfo.ResponseType in GetOAuthApplication.
Right now the storedresponse_typeis dropped when rebuildingUserInfoConfig, so the OAuth-app fetch endpoint returns it empty even if configured.🔧 Suggested fix
- userInfoConfig = &model.UserInfoConfig{ - UserAttributes: userAttributes, - } + userInfoConfig = &model.UserInfoConfig{ + ResponseType: oAuthConfig.UserInfo.ResponseType, + UserAttributes: userAttributes, + }
🤖 Fix all issues with AI agents
In `@tests/integration/application/model.go`:
- Around line 82-84: The equality logic for the UserInfo struct omits the
ResponseType field, so update the UserInfo equality comparisons (e.g.,
UserInfo.Equal or compareUserInfo helpers used in tests) to include ResponseType
alongside existing checks like UserAttributes, ensuring both equality
methods/compare helpers compare the ResponseType string; make the same change in
the other test equality helper referenced around the second location (lines
266-274) so response_type regressions are detected.
2ff5c77 to
0b9648c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/internal/application/store.go`:
- Around line 70-71: GetOAuthApplication currently constructs the UserInfoConfig
without setting the ResponseType, which causes configured JWS/JWE values to be
lost; update the UserInfo block in GetOAuthApplication to populate the
ResponseType field (using the model.UserInfoResponseType type) from the stored
application record (same source used for UserAttributes) so the returned
UserInfoConfig includes ResponseType along with UserAttributes.
🧹 Nitpick comments (2)
backend/internal/application/model/constants.go (1)
31-43: Type the UserInfoResponseType constants to the enum type for stronger typing.This avoids accidental mixing with other string‑typed response types and improves compile‑time checks.
♻️ Suggested change
const ( // UserInfoResponseTypeJSON represents the JSON userinfo response type. - UserInfoResponseTypeJSON = "JSON" + UserInfoResponseTypeJSON UserInfoResponseType = "JSON" // UserInfoResponseTypeJWS represents the JWS userinfo response type. - UserInfoResponseTypeJWS = "JWS" + UserInfoResponseTypeJWS UserInfoResponseType = "JWS" // UserInfoResponseTypeJWE represents the JWE userinfo response type. - UserInfoResponseTypeJWE = "JWE" + UserInfoResponseTypeJWE UserInfoResponseType = "JWE" )backend/internal/application/model/oauth_app.go (1)
50-51: Add a jsonschema description foruser_info.response_typeto keep schema docs consistent.♻️ Suggested change
- ResponseType UserInfoResponseType `json:"response_type,omitempty" yaml:"response_type,omitempty"` + ResponseType UserInfoResponseType `json:"response_type,omitempty" yaml:"response_type,omitempty" jsonschema:"UserInfo response type. Allowed values: JSON, JWS, JWE."`
0b9648c to
73e518e
Compare
- Added response_type to userInfoConfig - Added validation logic (JSON/JWS/JWE) - Added persistense support in store layer - Added integration test coverage - Fixed lint issues - Persist userinfo response_type in store layer and refactor to typed constant Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
73e518e to
c35ae32
Compare
Purpose
Currently, OAuth UserInfo response is always returned as plain JSON. However, as per the specification, the server should support returning signed (JWS) or encrypted (JWE) UserInfo responses.
This PR introduces support in the Thunder Application management endpoint to configure the
userinfo.response_typeflag per application.Allowed values:
The configured value is persisted in the database (OAuth config JSON) and returned via the Application APIs.
No Breaking Changes
This change is backward compatible.
If
response_typeis not provided or is invalid, it defaults toJSON.Approach
response_typefield insideUserInfoConfig.user_info.response_type.JSONwhen the flag is not provided or invalid.response_typeinsideoauth_config_json.Note:
This PR only introduces configuration and persistence support. Actual signing/encryption logic in the UserInfo endpoint will be handled separately (#1220 ).
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
Tests