Add file-based import and export endpoints for OIDC scopes#1053
Open
RovinKYK wants to merge 3 commits intowso2:masterfrom
Open
Add file-based import and export endpoints for OIDC scopes#1053RovinKYK wants to merge 3 commits intowso2:masterfrom
RovinKYK wants to merge 3 commits intowso2:masterfrom
Conversation
Contributor
WalkthroughAdds file-based import/export/update for OIDC scopes: new API endpoints, service methods, model for file-backed scope configuration, and three new error codes with descriptions; updates OpenAPI spec and REST implementation to handle multipart file uploads and file downloads. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as OidcApiServiceImpl
participant Service as OidcScopeManagementService
participant Store as ScopeStore
Client->>API: POST /oidc/scopes/import (multipart file)
API->>Service: importScopeFromFile(fileStream, attachment)
Service->>Service: parse file -> ScopeConfiguration -> Scope
Service->>Store: persist new Scope
Store-->>Service: created scope name
Service-->>API: created scope name
API-->>Client: 201 Created (Location header)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
...so2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java
Show resolved
Hide resolved
...so2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java
Show resolved
Hide resolved
...va/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/impl/OidcApiServiceImpl.java
Show resolved
Hide resolved
...va/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/impl/OidcApiServiceImpl.java
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/model/ScopeConfiguration.java
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/model/ScopeConfiguration.java
Show resolved
Hide resolved
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
RovinKYK
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.common/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/common/OidcScopeConstants.java`:
- Line 31: INVALID_REQUEST and ERROR_CODE_INVALID_INPUT use confusingly similar
numeric codes ("OAUTH-60001" vs "60001"); update ERROR_CODE_INVALID_INPUT
(constant name ERROR_CODE_INVALID_INPUT) to use a clearly distinct code (e.g.,
change its value to "OAUTH-60002" or "60002" with an explicit different prefix)
or reuse INVALID_REQUEST's code where appropriate so codes are not
ambiguous—modify the constant value in OidcScopeConstants accordingly and ensure
any usages reflect the new distinct code.
In
`@components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.v1/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java`:
- Around line 230-236: The code constructs a ScopeUpdateRequest from the
uploaded scope but always calls updateScope(scopeName, updateRequest) without
checking whether the uploaded scope's name (e.g., scope.getName()) matches the
path variable scopeName; add a guard in OidcScopeManagementService to compare
scopeName with scope.getName() and throw a BadRequest/Conflict (HTTP 400/409) if
they differ, otherwise proceed to call updateScope(scopeName, updateRequest);
ensure the validation occurs before creating/using ScopeUpdateRequest so
mismatched uploads are rejected with a clear error.
- Around line 262-275: In getScopeFromFile, validate that fileInputStream and
fileDetail (and fileDetail.getDataHandler()) are not null before using them; if
any are null, throw an IdentityOAuthClientException indicating a bad client
request (e.g., "Missing or invalid upload file") so it returns a client error
instead of NPE/500; after validation proceed to build the FileContent and call
generateModelFromFile as before, and keep the existing try/catch/finally for IO
handling.
- Around line 166-168: Replace the UnsupportedOperationException in
OidcScopeManagementService with API error handling that returns HTTP 400 for
missing/invalid fileType: detect the blank fileType (the current
StringUtils.isBlank(fileType) check) and throw/return an API error representing
Bad Request (HTTP 400) with a clear message like "No valid media type found" and
appropriate error code instead of letting it surface as a 500; use the project’s
API error helper/exception builder (the APIError/BadRequest-style helper used
elsewhere in the service layer) so the controller returns a 400 response.
In
`@components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.v1/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/impl/OidcApiServiceImpl.java`:
- Around line 69-82: The filename from FileContent (fileContent.getFileName())
is used directly in the Content-Disposition header in exportScopeToFile, which
allows CR/LF or quote injection; sanitize/normalize the filename before using
it: remove or replace CR (\r), LF (\n) and both single and double quotes, and
fall back to a safe default (e.g., "scope-export.txt") if the result is empty;
then use the sanitized value when building the Content-Disposition header so
exportScopeToFile never inserts raw filename characters into the response
header.
🧹 Nitpick comments (6)
components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.common/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/common/OidcScopeConstants.java (1)
36-44: Naming convention inconsistency with existing entries.The new enum constants use an
ERROR_CODE_prefix (e.g.,ERROR_CODE_ERROR_EXPORTING_SCOPE), while existing entries likeINVALID_REQUEST,ERROR_CONFLICT_REQUEST, andSCOPE_NOT_FOUNDdo not follow this pattern. Additionally,ERROR_CODE_ERROR_EXPORTING_SCOPEcontains "ERROR" twice, which is redundant.For consistency, consider aligning with the existing naming style:
♻️ Suggested naming alignment
- ERROR_CODE_ERROR_EXPORTING_SCOPE("65001", + ERROR_EXPORTING_SCOPE("65001", "Unable to export the OIDC scope configurations."), - ERROR_CODE_ERROR_IMPORTING_SCOPE("65002", + ERROR_IMPORTING_SCOPE("65002", "Unable to import the OIDC scope configurations."), - ERROR_CODE_ERROR_UPDATING_SCOPE("65003", + ERROR_UPDATING_SCOPE("65003", "Unable to update the OIDC scope configurations."), // Client Errors - 600xx - ERROR_CODE_INVALID_INPUT("60001", "Invalid Input"); + INVALID_INPUT("60001", "Invalid Input");components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.v1/src/main/resources/oidc-scope-management.yaml (4)
93-131: Consider adding 415 Unsupported Media Type response.The endpoint accepts XML, YAML, or JSON files but doesn't specify a 415 response for unsupported file formats. This would improve API clarity when clients upload files with unsupported formats.
📝 Suggested addition
$ref: '#/components/responses/Conflict' + 415: + description: Unsupported file format. + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' $ref: '#/components/responses/ServerError'
260-291: Response content types don't cover all Accept header variants.The Accept header enum includes
text/yaml,text/xml, andtext/json, but the responsecontentsection only definesapplication/json,application/xml, andapplication/yaml. Clients sendingtext/*variants won't see matching response content definitions.Either align the response content types with all accepted variants, or simplify the Accept enum to only include the
application/*variants that are actually returned.📝 Option 1: Simplify Accept header enum
schema: type: string enum: - application/json - application/xml - application/yaml - - application/x-yaml - - text/yaml - - text/xml - - text/json default: application/yaml
302-348: Clarify behavior when path{id}differs from scope name in file.The endpoint updates scope
{id}from an imported file. If the file contains a different scope name than the path parameter, the expected behavior is ambiguous. Consider documenting whether:
- The path
{id}takes precedence (file's name field is ignored)- A mismatch results in a 400 error
- The scope is renamed
This clarification in the description would prevent unexpected behavior for API consumers.
388-396: Pre-existing issue: Header name should beLocation, notSet-Cookie.The
Createdresponse component usesSet-Cookieas the header name, but the description indicates it should be the location of the newly created scope. The new import endpoint correctly usesLocation(line 118). Consider fixing this for consistency.📝 Suggested fix
Created: description: Created headers: - Set-Cookie: + Location: description: Location of the newly created scope schema: type: string format: uri example: https://apis.is.com/t/wso2.com/api/server/v1/oidc/scopes/Scope1components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.v1/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/model/ScopeConfiguration.java (1)
104-109: Defensively copyclaimsin setter/getter to prevent external mutation.Right now external callers can mutate internal state or pass null. Consider normalizing to an internal copy and returning copies.
Suggested diff
public List<String> getClaims() { - return claims; + return claims == null ? new ArrayList<>() : new ArrayList<>(claims); } public void setClaims(List<String> claims) { - this.claims = claims; + this.claims = claims == null ? new ArrayList<>() : new ArrayList<>(claims); }
...ava/org/wso2/carbon/identity/api/server/oidc/scope/management/common/OidcScopeConstants.java
Show resolved
Hide resolved
...so2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java
Show resolved
Hide resolved
...so2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java
Show resolved
Hide resolved
...so2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java
Show resolved
Hide resolved
...va/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/impl/OidcApiServiceImpl.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds file-based import and export capabilities for OIDC scopes, aligned with existing application, IDP, and userstore import/export patterns and the shared file serialization utilities.
Changes:
- Extends the OIDC scope OpenAPI definition with new
/oidc/scopes/import,/oidc/scopes/{id}/export, and/oidc/scopes/{id}/importoperations and aFileUploadschema for multipart file handling. - Implements core service logic to serialize/deserialize OIDC scopes to/from XML, JSON, and YAML using the common
FileSerializationUtil, plus a dedicatedScopeConfigurationmodel for file representation. - Wires the new operations through the generated JAX-RS API (
OidcApi,OidcApiService) and the concrete implementation (OidcApiServiceImpl), and adds new error codes for import/export/update failures.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.../oidc-scope-management.yaml |
Adds import/export endpoints and the FileUpload schema for OIDC scopes, defining the HTTP contracts for file-based operations. |
.../model/ScopeConfiguration.java |
Introduces a JAXB-annotated DTO used as the on-disk representation of scope configurations for serialization/deserialization. |
.../impl/OidcApiServiceImpl.java |
Implements the new API methods to delegate to the core service and construct appropriate Response objects (headers, content type, body) for import/export. |
.../core/OidcScopeManagementService.java |
Implements the core logic to export scopes to FileContent, import scopes from uploaded files, update existing scopes from files, and handle related errors via the shared file-serialization utilities. |
.../gen/.../model/Scope.java |
Cleans up XML binding imports in the generated Scope model, keeping it as a pure JSON-validation DTO used throughout the scope APIs. |
.../gen/.../model/FileUpload.java |
Adds a generated FileUpload model with validation for the uploaded file, matching the new OpenAPI FileUpload schema. |
.../gen/.../OidcApiService.java |
Extends the generated service interface with methods for exporting scopes to files and importing/updating scopes from uploaded files. |
.../gen/.../OidcApi.java |
Adds the new JAX-RS endpoints (paths, verbs, Swagger metadata, multipart parameters) for file-based scope import/export/update, delegating to OidcApiService. |
.../common/OidcScopeConstants.java |
Introduces new error codes/messages for failures during scope export, import, and update, used by the core service for structured API errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...n.identity.api.server.oidc.scope.management.v1/src/main/resources/oidc-scope-management.yaml
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.common/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/common/OidcScopeConstants.java`:
- Around line 48-61: Make the enum's description field final to ensure
immutability: change the ErrorMessage enum's "description" field to final and
update the two constructors so the two-argument constructor delegates to the
three-argument one (or explicitly sets this.description = null) to guarantee a
defined value; modify ErrorMessage(String code, String message) to call
ErrorMessage(code, message, null) (or set description = null) and leave
ErrorMessage(String code, String message, String description) assigning the
final field, and ensure any getDescription() continues to read the final
description field.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.oidc.scope.management/org.wso2.carbon.identity.api.server.oidc.scope.management.v1/src/main/java/org/wso2/carbon/identity/api/server/oidc/scope/management/v1/core/OidcScopeManagementService.java (1)
372-380: Minor formatting inconsistency.The closing brace at line 380 has 3 spaces of indentation instead of 4, which is inconsistent with the rest of the file.
Suggested fix
private APIError handleException(Response.Status status, OidcScopeConstants.ErrorMessage errorMessage) { ErrorResponse errorResponse = new ErrorResponse.Builder() .withCode(errorMessage.getCode()) .withMessage(errorMessage.getMessage()) .withDescription(errorMessage.getDescription()) .build(LOG, errorMessage.getMessage()); return new APIError(status, errorResponse); - } + }
...ava/org/wso2/carbon/identity/api/server/oidc/scope/management/common/OidcScopeConstants.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR introduces file-based configuration management capabilities for OIDC scopes. This enhancement enables administrators to export, import, and update OIDC scope configurations using standard configuration file formats (XML, YAML, JSON), facilitating easier scope management, backup, and migration workflows.
Related to https://github.com/wso2-enterprise/iam-product-management/issues/662
Goals
Approach
1. New API Endpoints:
2. Implementation Details:
User stories
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit