-
Notifications
You must be signed in to change notification settings - Fork 168
Introduce VC Management APIs #1035
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new VC Template Management API module (multi-module Maven layout): OpenAPI spec, REST implementation, core service, factory/holder, constants, and POM updates to expose public APIs and wire to an existing VCTemplateManager OSGi service; updates framework and OpenID4VC version properties. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as REST Client
participant API as VcTemplatesApiServiceImpl
participant Factory as ServerVCTemplateManagementServiceFactory
participant Service as ServerVCTemplateManagementService
participant Holder as VCTemplateManagementServiceHolder
participant Manager as VCTemplateManager (OSGi)
Client->>API: POST /vc-templates (VCTemplateCreationModel)
API->>Factory: getServerVCTemplateManagementService()
Factory->>Service: return singleton
Service->>Holder: getVCTemplateManager()
Holder->>Manager: retrieve via PrivilegedCarbonContext.getOSGiService()
Manager-->>Holder: VCTemplateManager instance
Holder-->>Service: VCTemplateManager
Service->>Manager: addTemplate(internalModel)
Manager-->>Service: created internal template
Service->>API: API model (created)
API->>Client: 201 Created + Location
Client->>API: GET /vc-templates?before=X&limit=10
API->>Service: listVCTemplates(before, null, filter, 10, attrs)
Service->>Manager: listTemplates(criteria)
Manager-->>Service: templates + totalCount
Service->>API: VCTemplateList (items + pagination links)
API-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
...entity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...rver/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java
Outdated
Show resolved
Hide resolved
...rver/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java
Outdated
Show resolved
Hide resolved
.../java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
.../java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
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.
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.
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.
Pull Request Overview
This PR introduces a new REST API for managing Verifiable Credential (VC) configurations in WSO2 Identity Server. The changes include a complete API implementation with OpenAPI specification, service layer, and dependency management.
- Adds VC Credential Configuration Management API with CRUD operations
- Updates the carbon identity framework version to 7.8.599-SNAPSHOT
- Implements service layer with proper error handling and model transformations
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Adds dependencies for VC config management components and updates framework version |
| components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml | Parent POM for the new VC config management module |
| VCCredentialConfigurations.yaml | OpenAPI specification defining the VC config management REST API |
| VcCredentialConfigurationsApiServiceImpl.java | REST API implementation delegating to service layer |
| ServerVCCredentialConfigManagementService.java | Core service layer handling business logic and model transformations |
| ServerVCCredentialConfigManagementServiceFactory.java | Factory for retrieving the service instance |
| VCCredentialConfigManagementServiceHolder.java | OSGi service holder for VCCredentialConfigManager |
| VCCredentialConfigManagementConstants.java | Constants for API path components |
| Generated model classes | Auto-generated DTOs from OpenAPI spec |
| Generated API interface files | Auto-generated API interfaces and service stubs |
| Component POM files | Maven build configuration for the new modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.java
Outdated
Show resolved
Hide resolved
...ement.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.java
Outdated
Show resolved
Hide resolved
...carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.java
Outdated
Show resolved
Hide resolved
...o2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.java
Outdated
Show resolved
Hide resolved
...g/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.java
Outdated
Show resolved
Hide resolved
...gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/CredentialMetadata.java
Outdated
Show resolved
Hide resolved
...1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/ClaimMapping.java
Outdated
Show resolved
Hide resolved
...ver.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml
Outdated
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/ClaimMapping.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/CredentialMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Metadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcCredentialConfigurationsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcCredentialConfigurationsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/VcCredentialConfigurationsApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcCredentialConfigurationsApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml(1 hunks)pom.xml(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcCredentialConfigurationsApiServiceImpl.java (4)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java (1)
VCCredentialConfigManagementConstants(24-32)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
ServerVCCredentialConfigManagementService(49-327)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (1)
ServerVCCredentialConfigManagementServiceFactory(28-52)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
VCCredentialConfigManagementServiceHolder(27-47)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
ServerVCCredentialConfigManagementService(49-327)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (8)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Metadata.java (1)
Metadata(33-98)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.java (1)
VCCredentialConfiguration(36-319)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.java (1)
VCCredentialConfigurationCreationModel(36-292)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.java (1)
VCCredentialConfigurationList(36-130)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.java (1)
VCCredentialConfigurationListItem(33-170)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.java (1)
VCCredentialConfigurationUpdateModel(32-79)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 268-273: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 216-219: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: CodeQL analysis (java)
- GitHub Check: Agent
- GitHub Check: build (17)
- GitHub Check: build (8)
- GitHub Check: build (11)
🔇 Additional comments (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java (1)
30-31: Constants read cleanBase path pieces match the new endpoints and there’s no surplus state. 👍
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml (1)
46-57: Dependency wiring looks goodProvided-scope framework deps match the runtime expectations, and the compiler plugin keeps Java 8 parity. 👍
...entity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java
Outdated
Show resolved
Hide resolved
...so2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
Show resolved
Hide resolved
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.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
33-60: Critical: OSGi service caching issue remains unresolvedThis caching pattern was previously flagged as critical. If the OSGi services are not available during class initialization,
nullis permanently cached, causing the factories' static blocks to fail withIllegalStateException. Fetch services on each call instead of caching them in static final fields.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (1)
315-315: Critical: Insecure token URL remains unresolvedThe
tokenUrlis still using HTTP instead of HTTPS, exposing OAuth tokens and credentials to interception. This security issue was previously flagged.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (1)
49-50: Consider removing unnecessaryvolatilemodifiersThe
volatilekeyword is unnecessary here since these fields are initialized once in the constructor and never modified afterward. While not harmful, it adds memory barrier overhead without providing benefits in this usage pattern.Apply this diff:
- private volatile ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; - private volatile ServerVCOfferManagementService serverVCOfferManagementService; + private ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; + private ServerVCOfferManagementService serverVCOfferManagementService;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/VcApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (7)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java
🧰 Additional context used
🧬 Code graph analysis (4)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (7)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Metadata.java (1)
Metadata(33-98)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.java (1)
VCCredentialConfiguration(36-295)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.java (1)
VCCredentialConfigurationCreationModel(36-269)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.java (1)
VCCredentialConfigurationList(36-130)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.java (1)
VCCredentialConfigurationListItem(33-170)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
VCCredentialConfigManagementServiceHolder(28-61)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (1)
ServerVCOfferManagementService(48-290)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java (1)
VCCredentialConfigManagementConstants(24-33)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
ServerVCCredentialConfigManagementService(49-334)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (1)
ServerVCOfferManagementService(48-290)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (1)
ServerVCCredentialConfigManagementServiceFactory(28-52)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java (1)
ServerVCOfferManagementServiceFactory(28-52)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.java (1)
VCOffer(35-153)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.java (1)
VCOfferCreationModel(35-128)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.java (1)
VCOfferList(36-130)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.java (1)
VCOfferListItem(33-120)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 356-361: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 308-311: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
.../carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java
Outdated
Show resolved
Hide resolved
.../java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml (1)
108-142: Clarify OpenAPI code generation workflow.The OpenAPI generator plugin is commented out with a note to uncomment when the API is updated. This manual approach can lead to inconsistencies between the spec and generated code if developers forget to regenerate.
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
274-283: Consider extracting shared error handling to a common utility.The
notFoundmethod andhandleVCConfigExceptionmethod (lines 285-315) contain error-handling logic that appears duplicated in ServerVCOfferManagementService. Both services use identical error code patterns ("VC-60001" for not found, "VC-65000" for unexpected errors) and response building logic.Consider extracting these methods to a shared utility class to reduce duplication and ensure consistent error responses across VC management services.
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (1)
49-50: Remove unnecessaryvolatilemodifiers.The service fields are marked
volatilebut are only assigned once in the constructor (not lazy-initialized) and the class doesn't implement any synchronization patterns that would benefit from volatile semantics.Apply this diff:
- private volatile ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; - private volatile ServerVCOfferManagementService serverVCOfferManagementService; + private final ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; + private final ServerVCOfferManagementService serverVCOfferManagementService;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/ClaimMapping.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/CredentialMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Metadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/VcApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (9)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java(2 hunks)components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java (1)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java (1)
APIResourceMgtEndpointConstants(30-159)
🔇 Additional comments (6)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java (1)
1885-1900: The logic correctly adds VC resource type authorization handling.The change properly includes VC (Verifiable Credential) resource type alongside BUSINESS and MCP resources, requiring the same authorization scope (
UPDATE_BUSINESS_AUTHORIZED_API_OPERATION). The null check onapiResourceis appropriately placed before accessing its type, ensuring safe handling of null values.components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java (1)
37-37: LGTM! Clean addition of VC resource type.The new
VC_RESOURCE_TYPEconstant and its inclusion inALLOWED_API_RESOURCE_TYPESfollow the established pattern used for MCP resources. The implementation is consistent and straightforward.Also applies to: 41-47
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java (2)
66-66: LGTM!The import is necessary for the updated system API protection logic.
615-624: Verify the security implications of exempting VC resources from system API protection.The logic now allows resources with types starting with
VC_RESOURCE_TYPEto bypass the system API protection check. Before merging, confirm that:
- VC resources are intentionally mutable and appropriate permission controls exist elsewhere
- This exemption aligns with your security model and governance policies
- The permission/authorization layer validates user access appropriately
Consider refactoring the multiple
startsWithchecks for better maintainability.The current chain of negated
startsWithchecks will become difficult to maintain as more resource types are added. Use a collection-based approach:+ private static final List<String> MODIFIABLE_RESOURCE_TYPE_PREFIXES = Arrays.asList( + APIResourceMgtEndpointConstants.BUSINESS_API_RESOURCE_TYPE, + MCP_SERVER_RESOURCE_TYPE, + VC_RESOURCE_TYPE + ); + private void handleSystemAPI(APIResource apiResource) { - if (apiResource.getType() != null && - !apiResource.getType().startsWith(APIResourceMgtEndpointConstants.BUSINESS_API_RESOURCE_TYPE) - && !apiResource.getType().startsWith(MCP_SERVER_RESOURCE_TYPE) - && !apiResource.getType().startsWith(VC_RESOURCE_TYPE)) { + if (apiResource.getType() != null && + MODIFIABLE_RESOURCE_TYPE_PREFIXES.stream() + .noneMatch(prefix -> apiResource.getType().startsWith(prefix))) { throw APIResourceMgtEndpointUtil.handleException(Response.Status.FORBIDDEN, ErrorMessage.ERROR_CODE_SYSTEM_API_RESOURCE_NOT_MODIFIABLE); } }components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
140-154: Past review comment appears incorrect.The past review comment states that variable
resultat line 140 is never read, but the code clearly uses it at lines 142-144, 152-154, and returns it. Similarly, theitemsvariable at line 147 is used at lines 152-153.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (1)
70-71: Defensive null handling looks good.The ternary operator correctly handles the case where
createdConfigurationmight be null when building the location URI, preventing potential NPEs.
...vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml
Show resolved
Hide resolved
.../carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 1
♻️ Duplicate comments (5)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (1)
313-316: Fix insecure OAuth2 token endpoint (duplicate concern).Line 315 uses
http://localhost:9443/oauth2/token, which leaks client credentials and authorization codes over plaintext. Change to HTTPS.authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml (1)
100-105: Update Java compiler target to Java 11 or higher (duplicate concern).Lines 102–103 set Java source/target to 1.8, which is end-of-life (March 2022) and unsupported by WSO2 Identity Server 6.x/7.x, which require Java 11 minimum. Update to at least Java 11.
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> <version>${maven.compiler.plugin.version}</version> <configuration> - <source>1.8</source> - <target>1.8</target> + <source>11</source> + <target>11</target> </configuration> </plugin>components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml (1)
38-42: Update Java compiler target to Java 11 or higher (duplicate concern).Lines 39–40 set Java source/target to 1.8, which is end-of-life and incompatible with WSO2 IS 6.x/7.x. Update to Java 11 or later to align with project standards.
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> <configuration> - <source>1.8</source> - <target>1.8</target> + <source>11</source> + <target>11</target> </configuration> </plugin>pom.xml (1)
790-799: Fix incorrect dependency coordinates for VC config modules (duplicate concern).Lines 792 and 797 reference non-existent artifacts
commonandv1. These do not match the actual module structure. Update to use the full artifact coordinates and addscope="provided"consistent with other server API dependencies.- <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> - <version>${project.version}</version> - </dependency> + <dependency> + <groupId>org.wso2.carbon.identity.server.api</groupId> + <artifactId>org.wso2.carbon.identity.api.server.vc.config.management.v1</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency>Remove the duplicate
.commonentry below (it is already declared at lines 875–880).components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (1)
232-292: Centralize duplicated error-handling helpers with existing config management service
notFound,handleVCConfigException,prefixCode, andincludeDataduplicate the same logic already present inServerVCCredentialConfigManagementService(as noted in the earlier review). Keeping two copies increases the risk of divergent behavior when error semantics change.Consider extracting these helpers into a shared component (e.g. a small
VCManagementErrorHandlerutility or base class constructed with aLoginstance) and delegating from both services, then removing the duplicated methods here and in the credential-config service.
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java (1)
615-623: VC resource types are now treated as modifiable (non‑system) – logic looks correctThe extended condition:
- Continues to forbid modifications when
typeis non‑null and does not start with any of: business API, MCP server, or VC resource prefixes.- Correctly allows VC resources (those whose type starts with
VC_RESOURCE_TYPE) to be patched/deleted along with business and MCP resources, while still blocking all other system types.Behavior is consistent with the method’s intent and with the existing pattern for business/MCP resources.
As an optional clean‑up, if you later find the set of “modifiable” types growing, consider centralizing that list (or a small helper) so
handleSystemAPIdoesn’t need to be updated each time.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java (1)
30-57: Make the factory class non-instantiable with a private constructor
ServerVCOfferManagementServiceFactoryis a pure static holder; it doesn’t need to be instantiated. To prevent accidental construction (and align with typical utility/factory patterns), consider adding a private constructor:public final class ServerVCOfferManagementServiceFactory { + private ServerVCOfferManagementServiceFactory() { + // Prevent instantiation. + } + private static final Log LOG = LogFactory.getLog(ServerVCOfferManagementServiceFactory.class); private static final ServerVCOfferManagementService SERVICE;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/ClaimMapping.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/CredentialMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Metadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/VcApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (15)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java(2 hunks)components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml(1 hunks)pom.xml(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java
🧰 Additional context used
🧬 Code graph analysis (3)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
VCCredentialConfigManagementServiceHolder(28-61)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (1)
ServerVCOfferManagementService(48-305)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
VCCredentialConfigManagementServiceHolder(28-61)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
ServerVCCredentialConfigManagementService(49-349)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.java (1)
VCOffer(35-153)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.java (1)
VCOfferCreationModel(35-128)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.java (1)
VCOfferList(36-130)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.java (1)
VCOfferListItem(33-120)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 356-361: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 308-311: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (4)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java (1)
61-67: ImportingVC_RESOURCE_TYPEis appropriate and scoped correctlyThe new static import cleanly exposes
VC_RESOURCE_TYPEfor use inhandleSystemAPIand keeps it consistent with the existing static import forMCP_SERVER_RESOURCE_TYPE. No issues here.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (1)
30-57: Singleton factory initialization pattern looks solidThe static initialization and singleton exposure are clear and consistent with the surrounding VC config management services: you fail fast with an
IllegalStateExceptionif theVCCredentialConfigManageris unavailable and log a debug message on successful initialization. No functional or lifecycle issues stand out here.pom.xml (1)
1077-1077: Module declaration is correct.Line 1077 properly adds the new VC config management component to the module list, following the existing naming convention and path structure.
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (1)
64-181: CRUD flows and model mappings look soundThe VC offer CRUD methods correctly:
- Scope operations by tenant via
ContextLoader.getTenantDomainFromContext().- Delegate to
VCOfferManagerfor persistence.- Map internal models to API models (
VCOffer,VCOfferList,VCOfferListItem) and back, handling null/empty lists safely.The control flow and exception wrapping around
VCConfigMgtExceptionare consistent and look functionally correct for the new API surface.Also applies to: 183-230, 294-304
...entity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 4
♻️ Duplicate comments (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (2)
315-315: Fix unresolved security issue: switch tokenUrl to HTTPS.This issue was flagged in a previous review but remains unresolved. The OAuth2 token endpoint uses cleartext HTTP, which exposes client secrets and authorization codes to interception. Update to HTTPS immediately.
authorizationCode: authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}
473-482: Resolve unresolved schema defect: define the requiredgrantsfield.This issue was flagged in a previous review but remains unresolved. The
VCOfferCreationModelschema listsgrantsas required (line 475) but fails to define it inproperties(lines 476–482). This breaks schema validation. Add the missinggrantsproperty with appropriate structure.VCOfferCreationModel: type: object required: [credentialConfigurationIds, grants] properties: displayName: type: string credentialConfigurationIds: type: array items: { type: string } + grants: + type: object + description: "Grant configuration for the credential offer." + properties: + grantType: + type: string + redirectUri: + type: string + scope: + type: array + items: + type: string + required: [grantType]
🧹 Nitpick comments (4)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (2)
356-362: AddmaxItemsconstraint to theclaimsarray.The
claimsarray lacks a maximum item constraint, allowing unbounded arrays. For predictable API behavior and memory efficiency, set a reasonable upper bound.claims: type: array items: type: string + maxItems: 100 example: ["givenname", "surname", "email"]
306-316: Ensure BasicAuth is only used over HTTPS.The
BasicAuthscheme transmits credentials in base64-encoded form (easily decoded) and must always use HTTPS. The current spec declares HTTPS servers but also includes an insecure HTTP token endpoint (line 315). After fixing line 315, document or enforce that BasicAuth is restricted to HTTPS channels only. Consider adding a note or explicit requirement in the security scheme description.securitySchemes: BasicAuth: type: http scheme: basic + description: "HTTP Basic Authentication. MUST only be used over HTTPS."components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (2)
49-50: Replacevolatilewithfinalfor immutable service references.The
volatilekeyword ensures visibility of mutable shared state across threads. Since these service references are assigned once in the constructor and never reassigned, they should be declaredfinalinstead.Apply this diff:
- private volatile ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; - private volatile ServerVCOfferManagementService serverVCOfferManagementService; + private final ServerVCCredentialConfigManagementService serverVCCredentialConfigManagementService; + private final ServerVCOfferManagementService serverVCOfferManagementService;
140-158: Simplify URI builders after adding null validation.After adding null validation in the create methods (per earlier comments), the null checks in these helpers become unnecessary. The resourceId/offerId parameters should never be null when called from create operations.
Apply this diff to simplify:
private URI buildConfigResourceLocation(String resourceId) { StringBuilder pathBuilder = new StringBuilder(Constants.V1_API_PATH_COMPONENT) .append(VCCredentialConfigManagementConstants.VC_CREDENTIAL_CONFIG_PATH_COMPONENT); - if (resourceId != null) { - pathBuilder.append(VCCredentialConfigManagementConstants.PATH_SEPARATOR).append(resourceId); - } + pathBuilder.append(VCCredentialConfigManagementConstants.PATH_SEPARATOR).append(resourceId); return ContextLoader.buildURIForHeader(pathBuilder.toString()); } private URI buildOfferResourceLocation(String offerId) { StringBuilder pathBuilder = new StringBuilder(Constants.V1_API_PATH_COMPONENT) .append(VCCredentialConfigManagementConstants.VC_OFFER_PATH_COMPONENT); - if (offerId != null) { - pathBuilder.append(VCCredentialConfigManagementConstants.PATH_SEPARATOR).append(offerId); - } + pathBuilder.append(VCCredentialConfigManagementConstants.PATH_SEPARATOR).append(offerId); return ContextLoader.buildURIForHeader(pathBuilder.toString()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.javais excluded by!**/gen/**
📒 Files selected for processing (7)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml(1 hunks)pom.xml(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml
- pom.xml
🧰 Additional context used
🧬 Code graph analysis (2)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (6)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java (1)
VCCredentialConfigManagementConstants(24-33)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.java (1)
VCOffer(35-153)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.java (1)
VCOfferCreationModel(35-128)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
ServerVCCredentialConfigManagementService(49-349)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java (1)
ServerVCCredentialConfigManagementServiceFactory(30-58)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCOfferManagementServiceFactory.java (1)
ServerVCOfferManagementServiceFactory(30-58)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOffer.java (1)
VCOffer(35-153)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferCreationModel.java (1)
VCOfferCreationModel(35-128)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferList.java (1)
VCOfferList(36-130)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCOfferListItem.java (1)
VCOfferListItem(33-118)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 356-361: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 308-311: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (4)
components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml (1)
1-37: Aggregator POM structure is well-formed and follows Maven best practices.The POM correctly defines an aggregator module with proper namespace configuration, Apache License header (2025 copyright), and valid parent reference to
identity-api-server:1.3.223-SNAPSHOT. Module ordering and naming align with project conventions.Verify that the referenced child modules (
org.wso2.carbon.identity.api.server.vc.config.management.v1andorg.wso2.carbon.identity.api.server.vc.config.management.common) exist and that this aggregator is registered as a module in the parent root POM to ensure the build includes all components.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml (3)
46-57: LGTM: Dependency scope and configuration.Both dependencies are correctly scoped as
provided, appropriate for a common/API module that exposes interfaces without bundling backend implementations. The dependencies align with the module's purpose:javax.ws.rs-apifor REST/JAX-RS APIs andorg.wso2.carbon.identity.vc.config.managementfor the identity framework service.
23-28: Verify parent POM declares dependency versions and registers this module.The module relies on a parent POM at
../pom.xmlfor version management ofjavax.ws.rs-apiandorg.wso2.carbon.identity.vc.config.managementdependencies. Confirm that the parent POM: (1) includes this module in its<modules>section, and (2) declares version properties or<dependencyManagement>entries for both dependencies to ensure consistent builds across the project.
39-40: Verify if Java 1.8 is the project-wide standard or if this module should target a newer version.The identity-api-server project supports building with JDK 8, 11, and 17. However, WSO2 Identity Server's current product recommendations for 2024–2025 deployments indicate JDK 17 or later should be used. Confirm whether targeting Java 1.8 aligns with the root POM configuration and other modules in this project, or if this module should be updated to align with modern standards.
.../carbon/identity/api/server/vc/config/management/v1/core/ServerVCOfferManagementService.java
Outdated
Show resolved
Hide resolved
.../java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
.../java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
...entity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
910-911: Align root pom Java compiler version with VC module and IS requirements.The root pom configures Java 1.8 (lines 910–911), which contradicts the Java 11+ requirement for WSO2 IS 6.x/7.x and conflicts with the VC module's updated compiler configuration. Update to Java 11 to maintain consistency across the build and ensure compatibility with the target platform.
<configuration> <encoding>UTF-8</encoding> - <source>1.8</source> - <target>1.8</target> + <source>11</source> + <target>11</target> <parameters>true</parameters> </configuration>
♻️ Duplicate comments (4)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
32-47: Critical: OSGi service permanently caches null if unavailable at class load.The static final
CONFIG_SERVICEis resolved duringServiceHolderclass initialization (whengetVCCredentialConfigManager()is first invoked). If theVCCredentialConfigManagerOSGi service or the Carbon context is not yet available at that moment,nullis cached permanently, breaking the API until restart.Apply this diff to fetch the service on each call and add defensive null-checking:
- private static class ServiceHolder { - - static final VCCredentialConfigManager CONFIG_SERVICE = (VCCredentialConfigManager) PrivilegedCarbonContext - .getThreadLocalCarbonContext().getOSGiService(VCCredentialConfigManager.class, null); - - } - /** * Get the {@link VCCredentialConfigManager} OSGi service. * * @return The VCCredentialConfigManager service instance. + * @throws IllegalStateException if the service is not available. */ public static VCCredentialConfigManager getVCCredentialConfigManager() { - return ServiceHolder.CONFIG_SERVICE; + VCCredentialConfigManager service = (VCCredentialConfigManager) PrivilegedCarbonContext + .getThreadLocalCarbonContext().getOSGiService(VCCredentialConfigManager.class, null); + if (service == null) { + throw new IllegalStateException("VCCredentialConfigManager service is not available"); + } + return service; }components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml (1)
102-103: Update Java compiler target to Java 11 or higher.Java 1.8 is end-of-life (March 2022) and incompatible with WSO2 Identity Server 6.x and 7.x, which require Java 11 (17 and 21 also supported in 7.x). Update both
<source>and<target>to at least 1.11 to align with current WSO2 platform requirements.- <configuration> - <source>1.8</source> - <target>1.8</target> - </configuration> + <configuration> + <source>11</source> + <target>11</target> + </configuration>components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (1)
281-281: Fix insecure token URL.
tokenUrlcurrently uses HTTP, which downgrades the OAuth code flow to cleartext and leaks client secrets and authorization codes. Switch to HTTPS to protect the token exchange.- tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token'pom.xml (1)
790-799: Remove non-existent artifact coordinates for VC config modules.Lines 790–799 declare dependencies on
org.wso2.carbon.identity.server.api:commonand:v1, which are not real artifacts. These prevent the VC config API JARs from being properly exported and will cause downstream resolution failures. Remove this block entirely; the correct artifactorg.wso2.carbon.identity.api.server.vc.config.management.commonis already declared at lines 875–880 with proper scope.- <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> - <version>${project.version}</version> - </dependency>
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
289-319: Consider mapping known NOT_FOUND client errors to 404 for consistency
handleVCConfigExceptioncurrently maps allVCConfigMgtClientExceptioninstances toBAD_REQUEST (400), whilenotFound()separately returnsNOT_FOUND (404)whenvcCredentialConfigManager.getreturnsnull. If the manager signals “not found” via a client exception/code instead ofnull, the same condition would be surfaced as 400, which is inconsistent.If the underlying manager uses a specific error code (e.g., for missing configuration), it would be worth special‑casing that here and returning 404 via
notFound()or an equivalent branch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/VcApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (13)
components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java(2 hunks)components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml(1 hunks)pom.xml(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/constants/APIResourceMgtEndpointConstants.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml
- components/org.wso2.carbon.identity.api.server.api.resource/org.wso2.carbon.identity.api.server.api.resource.v1/src/main/java/org/wso2/carbon/identity/api/server/api/resource/v1/core/ServerAPIResourceManagementService.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java
- components/org.wso2.carbon.identity.api.server.vc.config.management/pom.xml
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (4)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.java (1)
VCCredentialConfiguration(35-293)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.java (1)
VCCredentialConfigurationCreationModel(35-245)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.java (1)
VCCredentialConfigurationList(36-130)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 319-324: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 274-277: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (3)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.java (1)
1891-1893: Verify that theUPDATE_BUSINESS_AUTHORIZED_API_OPERATIONscope is appropriate for VC resources.The VC resource type is now grouped with BUSINESS and MCP types to use the
UPDATE_BUSINESS_AUTHORIZED_API_OPERATIONscope. The scope name specifically mentions "BUSINESS," creating potential semantic confusion when applied to VC resources.Confirm:
- Is this the intended authorization model for VC resources?
- Should the scope be renamed to something more generic to reflect its broader usage across multiple resource types?
- Consider adding a comment explaining why these three resource types share the same authorization requirements.
pom.xml (1)
1077-1077: VC config management module addition is correct.Module path components/org.wso2.carbon.identity.api.server.vc.config.management is properly added to the modules list and aligns with the dependency management and export updates introduced elsewhere in this PR.
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
64-99: Good use of tenant scoping and consistent logging in CRUD methodsResolving
tenantDomainviaContextLoader.getTenantDomainFromContext()and using it across add/delete/get keeps this service correctly tenant-aware, and the guarded debug logs provide useful traceability without noisy info-level logging.
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...so2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
Show resolved
Hide resolved
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.
Actionable comments posted: 6
♻️ Duplicate comments (7)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (2)
283-286: Use HTTPS for OAuthtokenUrl(cleartext creds / auth code risk).
tokenUrlishttp://...whileauthorizationUrlishttps://..., which is inconsistent and can downgrade the token exchange.authorizationCode: authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}
313-318: Bound theclaimsarrays withmaxItems(DoS / validation hardening).All three
claimsarrays are unbounded. Add a reasonablemaxItems(and optionallyminItems: 1,uniqueItems: true) consistently across schemas.claims: type: array items: type: string example: ["givenname", "surname", "email"] + maxItems: 100Also applies to: 341-346, 390-395
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java (1)
32-47: Don’t cache the OSGi service in a static-final field (can permanently cachenull).If the service isn’t available at class init,
CONFIG_SERVICEcan benullforever. Lookup per call (and fail fast with a clear exception).- private static class ServiceHolder { - - static final VCCredentialConfigManager CONFIG_SERVICE = (VCCredentialConfigManager) PrivilegedCarbonContext - .getThreadLocalCarbonContext().getOSGiService(VCCredentialConfigManager.class, null); - - } - /** * Get the {@link VCCredentialConfigManager} OSGi service. * * @return The VCCredentialConfigManager service instance. */ public static VCCredentialConfigManager getVCCredentialConfigManager() { - - return ServiceHolder.CONFIG_SERVICE; + VCCredentialConfigManager svc = (VCCredentialConfigManager) PrivilegedCarbonContext + .getThreadLocalCarbonContext() + .getOSGiService(VCCredentialConfigManager.class, null); + if (svc == null) { + throw new IllegalStateException("VCCredentialConfigManager service is not available"); + } + return svc; }components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
249-279: Fix copy/paste default error descriptions in offer methods.Both offer operations still say “Error while updating VC credential configuration”.
} catch (VCConfigMgtException e) { - throw handleVCConfigException(e, "Error while updating VC credential configuration", configId); + throw handleVCConfigException(e, "Error while generating VC credential offer", configId); } ... } catch (VCConfigMgtException e) { - throw handleVCConfigException(e, "Error while updating VC credential configuration", configId); + throw handleVCConfigException(e, "Error while revoking VC credential offer", configId); }components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml (1)
97-105: Update compiler target from Java 1.8 to repo/product baseline.Same issue as noted previously: hardcoding
1.8is likely incompatible if the platform baseline is Java 11+.<configuration> - <source>1.8</source> - <target>1.8</target> + <release>11</release> </configuration>#!/bin/bash # Confirm repo Java baseline and whether other modules are on 11+. rg -n --type xml "maven\.compiler\.(source|target)|<release>|java\.version" -Scomponents/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (1)
55-63: Don’t return201 CreatedwithLocation: .../nullwhen create fails.Validate the service response and ID before building
Response.created(...).VCCredentialConfiguration createdConfiguration = serverVCCredentialConfigManagementService .addVCCredentialConfiguration(vcCredentialConfigurationCreationModel); - URI location = buildConfigResourceLocation(createdConfiguration != null ? createdConfiguration.getId() : null); - return Response.created(location).entity(createdConfiguration).build(); + if (createdConfiguration == null || createdConfiguration.getId() == null) { + throw new IllegalStateException("Failed to create VC credential configuration."); + } + URI location = buildConfigResourceLocation(createdConfiguration.getId()); + return Response.created(location).entity(createdConfiguration).build(); }pom.xml (1)
790-799: Fix malformed artifact coordinates for VC config modules—issue persists from prior review.These entries reference non-existent artifacts
org.wso2.carbon.identity.server.api:commonand:v1, which lack proper artifact IDs. They will fail during dependency resolution and packaging.The correct artifact
org.wso2.carbon.identity.api.server.vc.config.management.commonis already declared properly at lines 875–880 with correct naming and scope. Either remove these duplicate entries or replace them with the actual artifact coordinates (ensure they include the full artifact name and mark asprovidedscope to match other server APIs).Apply this fix to remove the malformed entries:
- <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> - <version>${project.version}</version> - </dependency>
🧹 Nitpick comments (4)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml (1)
67-110: Add missing500response to POST for parity with other operations.Most operations include a
500response with theErrorschema, butPOST /vc/credential-configurationsdoes not—worth aligning for consistent client handling.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java (1)
24-35: Prefer primitives for simple constants (DEFAULT_LIMIT).
public static final Integer DEFAULT_LIMIT = 10;can beintto avoid boxing/nullability semantics.- public static final Integer DEFAULT_LIMIT = 10; + public static final int DEFAULT_LIMIT = 10;components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
414-423: Consider cappinglimitto a max.
validatedLimitenforces> 0but allows arbitrarily large values; adding an upper bound helps protect the backend.components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java (1)
43-53:volatilefield can befinal(no reassignment after ctor).
serverVCCredentialConfigManagementServiceis only assigned in the constructor; making itfinalsimplifies concurrency reasoning.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/PaginationLink.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VCCredentialConfigurationUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/VcApiService.javais excluded by!**/gen/**
📒 Files selected for processing (9)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/common/VCCredentialConfigManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml(1 hunks)pom.xml(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-10T02:37:06.312Z
Learnt from: mpmadhavig
Repo: wso2/identity-api-server PR: 1042
File: components/org.wso2.carbon.identity.api.server.idp/org.wso2.carbon.identity.api.server.idp.v1/src/main/java/org/wso2/carbon/identity/api/server/idp/v1/core/ServerIdpManagementService.java:515-534
Timestamp: 2025-12-10T02:37:06.312Z
Learning: In the file `components/org.wso2.carbon.identity.api.server.idp/org.wso2.carbon.identity.api.server.idp.v1/src/main/java/org/wso2/carbon/identity/api/server/idp/v1/core/ServerIdpManagementService.java`, the `createIDPImportRequest` method intentionally supports only JSON format for the new certificate export/import structure when the `SupportMultipleCertificateExport` feature flag is enabled. XML and YAML format support for the new certificate structure is deferred as future work.
Applied to files:
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/resources/VCCredentialConfigurations.yaml
[medium] 314-319: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 278-281: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (6)
components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java (1)
281-308:offerIdmapping is present—good.
toApiModel(...)correctly exposesofferIdto clients when set (needed for generate/revoke visibility).components/org.wso2.carbon.identity.api.server.vc.config.management/org.wso2.carbon.identity.api.server.vc.config.management.common/pom.xml (1)
33-44: > Likely an incorrect or invalid review comment.pom.xml (4)
632-637: Correct dependency declaration for OpenID4VC config management.The artifact naming, groupId, scope, and version reference are all correct. The dependency integration looks sound.
875-880: Correct VC config management common dependency declaration.The artifact naming, groupId, scope, and version are all correct. This entry follows the established pattern for server API common modules.
989-989: Version properties properly defined.The framework version bump and new OpenID4VC property are correctly declared. SNAPSHOT versions are appropriate for development branches.
Also applies to: 1028-1029
1080-1080: VC config management module correctly added.The new module is properly positioned and follows the established naming and directory structure conventions.
...vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...ntity/api/server/vc/config/management/v1/core/ServerVCCredentialConfigManagementService.java
Outdated
Show resolved
Hide resolved
...rver/vc/config/management/v1/factories/ServerVCCredentialConfigManagementServiceFactory.java
Outdated
Show resolved
Hide resolved
...so2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
Show resolved
Hide resolved
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.
Actionable comments posted: 6
♻️ Duplicate comments (4)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (2)
285-286: Fix insecure token URL.The
tokenUrluses HTTP which transmits OAuth tokens in cleartext. This was previously flagged.- tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token'
6-12: Prefer HTTPS for contact and license URLs.The
info.contact.urlandinfo.license.urluse HTTP. This was previously flagged.components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
196-205: Align paginationrelvalues with API contract.Code emits
rel=previous|next, but the OpenAPI example showsrel: before. This was previously flagged.pom.xml (1)
790-799: Fix dependency coordinates for VC template management modules.These dependency entries use bare
commonandv1artifactIds which are not valid artifacts. This was previously flagged for VC config modules and applies here too.<dependency> <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> + <artifactId>org.wso2.carbon.identity.api.server.vc.template.management.common</artifactId> <version>${project.version}</version> + <scope>provided</scope> </dependency> <dependency> <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> + <artifactId>org.wso2.carbon.identity.api.server.vc.template.management.v1</artifactId> <version>${project.version}</version> + <scope>provided</scope> </dependency>
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java (1)
24-35: Constants class is well-structured.The private constructor and
finalclass modifier correctly prevent instantiation and subclassing. Constants are appropriately named and scoped.Minor note: Consider using primitive
intinstead ofIntegerforDEFAULT_LIMITsince it's a constant that will never be null.- public static final Integer DEFAULT_LIMIT = 10; + public static final int DEFAULT_LIMIT = 10;components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java (1)
32-37: Consider defensive null handling for the OSGi service.The OSGi service retrieval doesn't verify availability. If
VCTemplateManageris not registered when this class loads,TEMPLATE_SERVICEwill benull, and downstream callers will receivenullfromgetVCTemplateManager(). While the factory layer checks for null and throwsIllegalStateException, this holder could be more defensive.Consider either:
- Validating the service at initialization and failing fast with a clear error, or
- Adding a null check in
getVCTemplateManager()with a descriptive exception, or- Documenting that callers must handle null returns
private static class ServiceHolder { static final VCTemplateManager TEMPLATE_SERVICE = (VCTemplateManager) PrivilegedCarbonContext .getThreadLocalCarbonContext().getOSGiService(VCTemplateManager.class, null); + + static { + if (TEMPLATE_SERVICE == null) { + throw new IllegalStateException("VCTemplateManager OSGi service is not available"); + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/PaginationLink.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/VcApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/pom.xml(1 hunks)pom.xml(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java (2)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java (1)
VCTemplateManagementServiceHolder(27-48)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
ServerVCTemplateManagementService(61-438)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
[medium] 318-323: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 278-281: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/pom.xml (1)
20-58: Maven module structure looks good.The module correctly defines parent coordinates, packaging, and dependencies with appropriate scopes. The structure follows the established patterns for server API modules in this codebase.
components/org.wso2.carbon.identity.api.server.vc.template.management/pom.xml (1)
20-37: Aggregator POM structure is correct.The module correctly establishes the multi-module structure for VC Template Management with appropriate parent coordinates and submodule declarations.
pom.xml (1)
632-637: New VC template management dependencies and module reference look correct.The additions for
org.wso2.carbon.identity.openid4vc.template.management,org.wso2.carbon.identity.api.server.vc.template.management.common, the version property, and the module reference are properly structured.Also applies to: 875-880, 1028-1030, 1080-1080
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java (2)
30-57: Factory implementation is correct.The factory correctly retrieves the
VCTemplateManagerOSGi service viaVCTemplateManagementServiceHolderand provides a public accessor for the singleton service instance. The null check with clear error message is good defensive programming.
35-47: This initialization pattern is consistent with the codebase standard.Eager initialization in a static block with null checks is the established pattern across recent service factory classes in this codebase (ServerWebhookManagementServiceFactory, ServerTenantManagementServiceFactory, SecretManagementServiceFactory, ServerScriptLibrariesServiceFactory, and others). The null check with a clear
IllegalStateExceptionmessage provides explicit failure diagnostics if the OSGi service is unavailable.This approach treats OSGi services as mandatory dependencies, which is appropriate for services fetched during factory initialization. Lazy initialization would deviate from the established pattern and is not necessary here.
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcApiServiceImpl.java (5)
66-84: LGTM!The
deleteVCTemplate,generateVCCredentialOffer, andgetVCTemplatemethods properly delegate to the service layer with appropriate HTTP status codes (204 No Content, 200 OK).
87-97: LGTM!The
listVCTemplatesandrevokeVCCredentialOffermethods correctly forward parameters to the service layer and return appropriate 200 OK responses.
100-105: LGTM!The
updateVCTemplatemethod correctly delegates to the service layer and returns the updated configuration with a 200 OK status.
107-115: Verify the need for null resourceId handling.The method conditionally appends
resourceIdonly if it's notnull(line 111). For aLocationheader returned after resource creation (POST), the ID should always be present. IfresourceIdcan legitimately benull, the resulting URI would be incomplete and potentially violate HTTP semantics.This relates to the null check in
addVCTemplate(line 61). If template creation always returns a valid ID, simplify this method:private URI buildTemplateLocation(String resourceId) { - StringBuilder pathBuilder = new StringBuilder(Constants.V1_API_PATH_COMPONENT) - .append(VCTemplateManagementConstants.VC_TEMPLATE_PATH_COMPONENT); - if (resourceId != null) { - pathBuilder.append(VCTemplateManagementConstants.PATH_SEPARATOR).append(resourceId); - } - return ContextLoader.buildURIForHeader(pathBuilder.toString()); + String path = Constants.V1_API_PATH_COMPONENT + + VCTemplateManagementConstants.VC_TEMPLATE_PATH_COMPONENT + + VCTemplateManagementConstants.PATH_SEPARATOR + + resourceId; + return ContextLoader.buildURIForHeader(path); }
57-63: Inconsistent null handling in template creation.The service method
addVCTemplatelacks the null validation thatgetVCTemplateperforms. WhengetVCTemplatereceives null from the template manager, it explicitly throwsnotFound(). TheaddVCTemplatemethod should follow the same pattern instead of allowing null to propagate throughtoApiModel(). The defensive null check on line 61 inVcApiServiceImplmasks this underlying inconsistency in error handling between similar operations.Update
ServerVCTemplateManagementService.addVCTemplateto validate the result fromvcTemplateManager.add()and throw an appropriate exception if null is returned, similar to the pattern used ingetVCTemplate.
...vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml
Show resolved
Hide resolved
...vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml
Outdated
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...ava/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcApiServiceImpl.java
Outdated
Show resolved
Hide resolved
...so2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
Show resolved
Hide resolved
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.
Actionable comments posted: 1
♻️ Duplicate comments (6)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml (3)
29-31: Fix inconsistent module name and description.The
<name>and<description>reference "VC Config Management" but the module is for "VC Template Management" based on the artifactId.- <name>WSO2 Identity Server - VC Config Management API</name> - <description>WSO2 IS - Verifiable Credential Configuration Management API</description> + <name>WSO2 Identity Server - VC Template Management API</name> + <description>WSO2 IS - Verifiable Credential Template Management API</description>
102-103: Update Java compiler target to Java 11 or higher.Java 1.8 is end-of-life and incompatible with WSO2 Identity Server 6.x and 7.x, which require Java 11 (17 and 21 also supported in 7.x).
<configuration> - <source>1.8</source> - <target>1.8</target> + <source>11</source> + <target>11</target> </configuration>
126-126: Mismatch betweendateLibrary=java17and Java 1.8 compiler target.The commented OpenAPI generator uses
<dateLibrary>java17</dateLibrary>but the module compiles as Java 1.8. If someone uncomments this configuration, the generated code will be incompatible. Either update the compiler target to Java 17 or change the dateLibrary to match the active compiler level.components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (3)
6-12: Prefer HTTPS for info URLs.The
contact.urlandlicense.urluse HTTP. Consider switching to HTTPS to avoid mixed-content warnings in rendered documentation.contact: name: WSO2 - url: 'http://wso2.com/products/identity-server/' + url: 'https://wso2.com/products/identity-server/' email: [email protected] license: name: Apache 2.0 - url: 'http://www.apache.org/licenses/LICENSE-2.0.html' + url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
283-286: Fix insecure token URL.The
tokenUrluses HTTP which transmits OAuth tokens in cleartext. This should use HTTPS.authorizationCode: authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}
317-321: AddmaxItemsconstraints to claims arrays.All
claimsarray properties lackmaxItemsconstraints, which could allow unbounded arrays leading to potential DoS or performance issues. Based on static analysis (CKV_OPENAPI_21), add reasonable limits.Apply to all three locations:
claims: type: array items: type: string example: ["given_name", "email"] + maxItems: 100Also applies to: 349-353, 398-402
🧹 Nitpick comments (4)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (2)
43-43: Remove unused LOG field.The
LOGfield is declared but never used in the class. Consider removing it to avoid dead code, or use it for debugging/error logging if needed.
57-64: Consider adding null check after service call for consistency.The
getVCTemplatemethod explicitly checks for null and throws a NOT_FOUND error (lines 127-129), butaddVCTemplateandupdateVCTemplatelack similar defensive checks after callingvcTemplateManager.add()andvcTemplateManager.update(). While the service layer uses exception-based error handling, adding a null check would provide defensive consistency across similar operations:public Response addVCTemplate(VCTemplateCreationModel vcTemplateCreationModel) { VCTemplate createdConfiguration = serverVCTemplateManagementService .addVCTemplate(vcTemplateCreationModel); + if (createdConfiguration == null) { + return Response.serverError().build(); + } URI location = buildTemplateLocation(createdConfiguration.getId()); return Response.created(location).entity(createdConfiguration).build(); }components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (2)
277-279: BasicAuth security scheme flagged for cleartext credentials.Static analysis (CKV_OPENAPI_3) flags BasicAuth as potentially allowing cleartext credentials. However, since the server URL (line 15) enforces HTTPS, credentials will be encrypted in transit. Consider adding a description clarifying that HTTPS is required.
securitySchemes: BasicAuth: type: http scheme: basic + description: "Basic authentication credentials must be transmitted over HTTPS."
357-370: Consider addingmaxItemsto VCTemplateList.links and VCTemplates arrays.For consistency with the claims arrays, the
linksandVCTemplatesarrays in VCTemplateList should also havemaxItemsconstraints to prevent unbounded responses.properties: totalResults: type: integer example: 1 links: type: array items: $ref: '#/components/schemas/PaginationLink' + maxItems: 10 VCTemplates: type: array items: $ref: '#/components/schemas/VCTemplateListItem' + maxItems: 1000
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/VcTemplatesApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (3)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java (1)
VCTemplateManagementConstants(24-35)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.java (1)
VCTemplate(35-268)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.java (1)
VCTemplateCreationModel(35-221)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
ServerVCTemplateManagementService(61-438)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java (1)
ServerVCTemplateManagementServiceFactory(30-58)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
[medium] 318-323: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 278-281: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (3)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml (1)
34-93: Dependencies look well-configured.The dependency declarations follow established patterns with appropriate exclusions for Jackson, Guava, and JSR311 to avoid version conflicts. The
providedscopes are correctly applied for runtime-provided libraries.components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (1)
66-106: LGTM on remaining endpoint implementations.The delete, get, list, update, generateOffer, and revokeOffer implementations correctly delegate to the service layer and return appropriate HTTP status codes (204 for delete, 200 for others).
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (1)
26-273: Well-structured API paths with consistent response handling.The endpoint definitions follow RESTful conventions with appropriate HTTP methods, status codes, and error responses. The operationIds align well with the Java implementation.
...so2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java
Show resolved
Hide resolved
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (2)
199-208: Consider aligning paginationrelvalues with API contract.The code emits
rel="previous"andrel="next", but a previous review noted the OpenAPI specification usesrel="before"andrel="after". Consider updating for consistency with the documented API contract.If the API contract indeed specifies
before/after, apply this diff:if (!isFirstPage) { String encodedString = Base64.getEncoder().encodeToString( templates.get(0).getCursorKey().toString().getBytes(StandardCharsets.UTF_8)); - result.addLinksItem(buildPaginationLink(url + "&before=" + encodedString, "previous")); + result.addLinksItem(buildPaginationLink(url + "&before=" + encodedString, "before")); } if (!isLastPage) { String encodedString = Base64.getEncoder().encodeToString(templates .get(templates.size() - 1).getCursorKey().toString().getBytes(StandardCharsets.UTF_8)); - result.addLinksItem(buildPaginationLink(url + "&after=" + encodedString, "next")); + result.addLinksItem(buildPaginationLink(url + "&after=" + encodedString, "after")); }
193-194: Direct list modification may fail at runtime.The code directly modifies the list returned by
searchResult.getTemplates(). If the underlying implementation returns an unmodifiable list, this will throwUnsupportedOperationExceptionwhen attempting to remove the extra item.Apply this diff to create a mutable copy:
List<org.wso2.carbon.identity.openid4vc.template.management.model.VCTemplate> templates = - searchResult.getTemplates(); + new ArrayList<>(searchResult.getTemplates());
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
427-435: Consider using the correct error code constant.The validation logic correctly rejects non-positive limits, but uses
ERROR_CODE_INVALID_CLAIMfor a query parameter error. Consider usingERROR_CODE_INVALID_QUERY_PARAM(already imported at line 60) for semantic clarity.Apply this diff:
limit = limit == null ? DEFAULT_LIMIT : limit; if (limit <= 0) { throw handleVCTemplateException( - VCTemplateMgtExceptionHandler.handleClientException(ERROR_CODE_INVALID_CLAIM, limit.toString())); + VCTemplateMgtExceptionHandler.handleClientException(ERROR_CODE_INVALID_QUERY_PARAM, limit.toString())); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (8)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/error/APIError.java (1)
APIError(25-63)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java (1)
VCTemplateManagementConstants(24-96)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/PaginationLink.java (1)
PaginationLink(33-118)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.java (1)
VCTemplate(35-268)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.java (1)
VCTemplateCreationModel(35-221)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateList.java (1)
VCTemplateList(37-161)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateListItem.java (1)
VCTemplateListItem(33-147)
🔇 Additional comments (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (10)
81-96: LGTM!The template creation flow is well-structured with proper tenant context handling, model conversion, and centralized error handling.
103-114: LGTM!Clean deletion logic with appropriate error handling.
122-138: LGTM!Retrieval logic correctly handles the not-found case and performs proper model conversion.
233-248: LGTM!Update logic is straightforward with proper model conversion and error handling.
250-264: LGTM!The offer generation flow correctly delegates to the template manager and handles errors appropriately.
266-280: LGTM!The offer revocation flow is properly implemented with appropriate error handling.
282-338: LGTM!Model conversion methods handle null values appropriately and map fields correctly for their respective use cases (creation vs. update).
340-412: LGTM!Error handling methods provide comprehensive coverage with appropriate status codes, error messages, and fallback logic for unexpected cases.
414-425: LGTM!List item conversion is clean and straightforward.
444-450: LGTM!Pagination link construction correctly builds the URI with proper path components.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pom.xml (1)
790-799: Critical: Fix the incorrect artifact coordinates (previously flagged).These dependency entries use invalid artifactIds (
commonandv1) that don't correspond to real artifacts. This issue was already identified in a previous review but remains unresolved. The correct artifact names should follow the full naming convention used throughout this POM, such as:
org.wso2.carbon.identity.api.server.vc.template.management.v1org.wso2.carbon.identity.api.server.vc.template.management.common(already declared at lines 875-880)These invalid entries will cause downstream build failures when Maven attempts to resolve dependencies.
🔎 Apply this diff to remove the invalid entries:
- <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> - <version>${project.version}</version> - </dependency>Note: The
.commonartifact is already correctly declared at lines 875-880. If a.v1artifact is needed, add it with the full name:<dependency> <groupId>org.wso2.carbon.identity.server.api</groupId> <artifactId>org.wso2.carbon.identity.api.server.vc.template.management.v1</artifactId> <version>${project.version}</version> <scope>provided</scope> </dependency>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pom.xml(6 hunks)
🔇 Additional comments (4)
pom.xml (4)
875-880: LGTM!The dependency management entry follows the correct naming convention and is consistent with other API server modules in this POM.
1080-1080: LGTM!The module reference is correctly added and follows the established pattern for other API server components.
989-989: Verify framework version compatibility.The
carbon.identity.framework.versionhas been updated to7.8.643. Ensure this version is compatible with the new VC template management features and doesn't introduce breaking changes to existing functionality. Review the framework's release notes and perform testing with the new VC features before merging.
632-637: Verify the external dependency version and artifact availability.The dependency declaration references
org.wso2.carbon.identity.openid4vc:org.wso2.carbon.identity.openid4vc.template.managementwith version resolved from thecarbon.identity.openid4vc.versionproperty. Confirm that this artifact exists in the configured Maven repositories and that version 1.0.0 is correct for the current integration scope.
...so2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
Outdated
Show resolved
Hide resolved
...so2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...ent.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/Error.java
Outdated
Show resolved
Hide resolved
|
PR builder started |
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.
Actionable comments posted: 3
♻️ Duplicate comments (8)
pom.xml (2)
1028-1029: Consider using SNAPSHOT version for WIP feature.The
carbon.identity.openid4vc.versionis set to1.0.0without a-SNAPSHOTsuffix. Since this PR is marked as "WIP" and introduces a feature under active development, using1.0.0-SNAPSHOTwould be more appropriate. Non-SNAPSHOT versions typically indicate released artifacts.🔎 Apply this diff if the dependency is still under development:
<!-- VC related versions --> - <carbon.identity.openid4vc.version>1.0.0</carbon.identity.openid4vc.version> + <carbon.identity.openid4vc.version>1.0.0-SNAPSHOT</carbon.identity.openid4vc.version>
790-799: Fix invalid dependency artifact coordinates.The
artifactIdvaluescommonandv1underorg.wso2.carbon.identity.server.apiare not valid Maven artifacts. These entries will cause downstream resolution failures during packaging. Based on the pattern in this PR, these should likely be removed or replaced with the actual artifactorg.wso2.carbon.identity.api.server.vc.template.management.v1.🔎 Consider removing these invalid entries or replacing with the correct artifact:
- <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> - <version>${project.version}</version> - </dependency>If a vc.template.management.v1 artifact is needed in dependencyManagement, add:
+ <dependency> + <groupId>org.wso2.carbon.identity.server.api</groupId> + <artifactId>org.wso2.carbon.identity.api.server.vc.template.management.v1</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency>components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (3)
6-12: Use HTTPS for contact and license URLs.The
contact.url(line 8) andlicense.url(line 12) currently usehttp://, which can cause mixed-content warnings and unnecessary redirects in rendered API documentation.🔎 Apply this diff to use HTTPS:
contact: name: WSO2 - url: 'http://wso2.com/products/identity-server/' + url: 'https://wso2.com/products/identity-server/' email: [email protected] license: name: Apache 2.0 - url: 'http://www.apache.org/licenses/LICENSE-2.0.html' + url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
283-286: Fix insecure OAuth2 token URL.The
tokenUrluseshttp://localhost:9443/..., which exposes OAuth client secrets and authorization codes over cleartext. This is a security vulnerability.🔎 Apply this diff to use HTTPS:
authorizationCode: authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}
317-321: AddmaxItemsconstraint to claims array in VCTemplate schema.The
claimsarray lacks amaxItemsconstraint, allowing unbounded arrays that could lead to DoS or performance degradation. This affectsVCTemplate.claims,VCTemplateCreationModel.claims(lines 349-353), andVCTemplateUpdateModel.claims(lines 398-402).🔎 Apply these diffs to add maxItems constraints:
For VCTemplate.claims (lines 317-321):
claims: type: array items: type: string example: ["given_name", "email"] + maxItems: 100For VCTemplateCreationModel.claims (lines 349-353):
claims: type: array items: type: string example: ["given_name", "email"] + maxItems: 100For VCTemplateUpdateModel.claims (lines 398-402):
claims: type: array items: type: string example: ["givenname", "surname", "email"] + maxItems: 100Based on static analysis hints, this aligns with OpenAPI security best practices (CKV_OPENAPI_21).
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (1)
108-116: Missing path separator between V1_API_PATH_COMPONENT and VC_TEMPLATE_PATH_COMPONENT.The path construction concatenates
/v1directly withvc-templateswithout an intermediate separator, producing/v1vc-templatesinstead of/v1/vc-templates. This will result in malformed Location headers for HTTP 201 responses.🔎 Apply this diff to fix the path construction:
private URI buildTemplateLocation(String resourceId) { StringBuilder pathBuilder = new StringBuilder(Constants.V1_API_PATH_COMPONENT) + .append(VCTemplateManagementConstants.PATH_SEPARATOR) .append(VCTemplateManagementConstants.VC_TEMPLATE_PATH_COMPONENT); if (resourceId != null) { pathBuilder.append(VCTemplateManagementConstants.PATH_SEPARATOR).append(resourceId); } return ContextLoader.buildURIForHeader(pathBuilder.toString()); }components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (2)
199-208: Align paginationrelvalues with API contract.Code emits
rel="previous"andrel="next", but based on past review feedback, the OpenAPI contract may use different values. Verify the API specification and ensure consistency.#!/bin/bash # Description: Find the OpenAPI specification to verify expected rel values # Search for OpenAPI/Swagger definitions related to VC templates fd -e yaml -e yml -e json | xargs rg -l "VCTemplate.*pagination|links.*rel" --type-add 'spec:*.{yaml,yml,json}' -t spec
173-198: Potential issue with direct list modification.The code directly modifies the
templateslist returned fromsearchResult.getTemplates(). If the underlying implementation returns an unmodifiable list, this will throwUnsupportedOperationException.🔎 Apply this diff to create a mutable copy:
List<org.wso2.carbon.identity.openid4vc.template.management.model.VCTemplate> templates = - searchResult.getTemplates(); + new ArrayList<>(searchResult.getTemplates());
🧹 Nitpick comments (4)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (3)
43-43: Remove unused LOG field.The logger is declared but never used throughout the implementation. Consider removing it to reduce unnecessary code.
🔎 Apply this diff to remove the unused field:
- private static final Log LOG = LogFactory.getLog(VcTemplatesApiServiceImpl.class); - private final ServerVCTemplateManagementService serverVCTemplateManagementService;
47-55: Clarify error message terminology.The error message refers to "server vc config management service" while the class and functionality relate to "template management." Consider updating the message for consistency.
🔎 Apply this diff to align the error message:
} catch (IllegalStateException e) { - throw new RuntimeException("Error occurred while initiating server vc config management service.", e); + throw new RuntimeException("Error occurred while initiating server vc template management service.", e); }
100-106: Rename parameter for consistency.The parameter
configIdshould be namedtemplateIdto maintain consistency with other methods in this class (deleteVCTemplate, getVCTemplate, etc.) and with the underlying service method signature.🔎 Apply this diff to standardize the parameter name:
- public Response updateVCTemplate(String configId, VCTemplateUpdateModel vcTemplateUpdateModel) { + public Response updateVCTemplate(String templateId, VCTemplateUpdateModel vcTemplateUpdateModel) { VCTemplate updatedConfiguration = serverVCTemplateManagementService - .updateVCTemplate(configId, vcTemplateUpdateModel); + .updateVCTemplate(templateId, vcTemplateUpdateModel); return Response.ok().entity(updatedConfiguration).build(); }components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
323-350: Consider extracting common mapping logic.Both
toInternalModeloverloads share identical logic for mappingdisplayName,format,claims, andexpiresIn. While the current implementation is functional, extracting common logic could improve maintainability.💡 Example refactoring approach:
Add a helper method to populate common fields:
private void populateCommonFields( org.wso2.carbon.identity.openid4vc.template.management.model.VCTemplate target, String displayName, String format, List<String> claims, Integer expiresIn) { target.setDisplayName(displayName); target.setFormat(format); if (claims != null) { target.setClaims(claims); } target.setExpiresIn(expiresIn); }Then simplify both methods to call the helper.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/PaginationLink.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/VcTemplatesApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (5)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml(1 hunks)pom.xml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (6)
components/org.wso2.carbon.identity.api.server.common/src/main/java/org/wso2/carbon/identity/api/server/common/ContextLoader.java (1)
ContextLoader(42-146)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java (1)
VCTemplateManagementConstants(24-96)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.java (1)
VCTemplate(35-268)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.java (1)
VCTemplateCreationModel(35-221)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (1)
ServerVCTemplateManagementService(65-463)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java (1)
ServerVCTemplateManagementServiceFactory(30-58)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
[medium] 318-323: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 278-281: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (8)
pom.xml (3)
632-637: LGTM: New OpenID4VC template management dependency.The dependency on
org.wso2.carbon.identity.openid4vc.template.managementis correctly configured withprovidedscope, consistent with other server API dependencies.
875-880: LGTM: New VC template management common module dependency.The
org.wso2.carbon.identity.api.server.vc.template.management.commondependency is correctly declared withprovidedscope, following the established pattern for common API modules.
1080-1080: LGTM: Module addition for VC template management.The new module
components/org.wso2.carbon.identity.api.server.vc.template.managementis properly added to the reactor build.components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (2)
1-17: LGTM! The copyright year is correct.The 2025 copyright year is appropriate for this new file created in 2025.
57-64: LGTM! Proper REST semantics implemented.The method correctly returns HTTP 201 with a Location header pointing to the newly created resource, following REST best practices.
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (3)
81-96: LGTM!The method correctly orchestrates template creation with proper tenant scoping, model conversion, and exception handling.
103-138: LGTM!Both
deleteVCTemplateandgetVCTemplatecorrectly implement their operations with proper tenant scoping, null handling, and exception management.
233-292: LGTM!All three methods (
updateVCTemplate,generateOffer,revokeOffer) correctly implement their operations with proper tenant scoping, model conversion, and exception handling. Javadoc is present for all public methods.
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
...on/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java
Show resolved
Hide resolved
|
PR builder started |
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.
Actionable comments posted: 2
♻️ Duplicate comments (13)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml (3)
6-12: Prefer HTTPS for info URLs.The
contact.urlandlicense.urlcurrently usehttp://. Switch to HTTPS to avoid mixed-content warnings and redirects in rendered documentation.
284-286: Fix insecure token URL.The
tokenUrluseshttp://localhost:9443/oauth2/token, which transmits OAuth tokens over cleartext HTTP. This creates a security vulnerability as client secrets and authorization codes can be intercepted.🔎 Apply this diff to use HTTPS:
authorizationUrl: 'https://localhost:9443/oauth2/authorize' - tokenUrl: 'http://localhost:9443/oauth2/token' + tokenUrl: 'https://localhost:9443/oauth2/token' scopes: {}
317-321: Add maxItems constraint to claims array.The
claimsarray inVCTemplatelacks amaxItemsconstraint, which could allow unbounded arrays leading to DoS or performance issues. This issue also applies toVCTemplateCreationModel.claims(lines 349-353) andVCTemplateUpdateModel.claims(lines 398-402).🔎 Apply this diff to add constraints:
For
VCTemplate.claims(lines 317-321):claims: type: array items: type: string example: ["given_name", "email"] + maxItems: 100Apply similar constraints to lines 349-353 and 398-402.
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java (7)
198-207: Align pagination rel values with API contract.The code emits
rel=previous|next, but consistency with the OpenAPI examples (which showrel: before) would improve API contract alignment.
69-72: Add null check for constructor parameter.The constructor accepts a
VCTemplateManagerparameter without validation. Ifnullis passed, all service methods will fail withNullPointerExceptionat runtime.🔎 Apply this diff to add validation:
public ServerVCTemplateManagementService(VCTemplateManager vcTemplateManager) { - + if (vcTemplateManager == null) { + throw new IllegalArgumentException("VCTemplateManager cannot be null"); + } this.vcTemplateManager = vcTemplateManager; }
192-196: Potential issue with direct list modification.The code directly modifies the
templateslist returned fromsearchResult.getTemplates(). If the underlying implementation returns an unmodifiable list, this will throwUnsupportedOperationException.🔎 Apply this diff to create a mutable copy:
- List<org.wso2.carbon.identity.openid4vc.template.management.model.VCTemplate> templates = - searchResult.getTemplates(); + List<org.wso2.carbon.identity.openid4vc.template.management.model.VCTemplate> templates = + new ArrayList<>(searchResult.getTemplates());
255-269: Add Javadoc and fix error message in generateOffer.The
generateOffermethod lacks Javadoc (as noted in past review), and the error message "Error while updating VC template" doesn't accurately describe the operation being performed.
277-291: Fix error message in revokeOffer.The error message "Error while updating VC template" doesn't accurately describe the revoke operation being performed.
362-390: Inconsistent error code prefix in fallback case.The fallback error code at line 383 uses
"VCM-65000"prefix, while other error codes use the"VC-"prefix. This inconsistency may confuse API consumers.🔎 Apply this diff to use consistent prefix:
} else { errorResponse = new ErrorResponse.Builder() - .withCode("VCM-65000") + .withCode("VC-65000") .withMessage("Unexpected error") .withDescription("An unexpected error occurred while processing the VC template request.") .build(LOG, "An unexpected error occurred while processing the VC template request.");
427-436: Use semantically correct error code for limit validation.The
validatedLimitmethod usesERROR_CODE_INVALID_CLAIMwhen the limit parameter is invalid. Sincelimitis a query parameter (not a claim), this is semantically incorrect.pom.xml (2)
1028-1029: Consider using SNAPSHOT version for active development.The version is set to
1.0.0without a-SNAPSHOTsuffix. Since this PR introduces a new feature under active development, consider using1.0.0-SNAPSHOTinstead to indicate the artifact is not yet released.
790-799: Fix incorrect dependency artifact coordinates.These dependencyManagement entries reference non-existent artifacts
commonandv1underorg.wso2.carbon.identity.server.api. This will cause resolution failures for any downstream consumers. The actual artifact should beorg.wso2.carbon.identity.api.server.vc.template.management.v1.🔎 Apply this diff to fix the coordinates:
<dependency> <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>common</artifactId> - <version>${project.version}</version> - </dependency> - <dependency> - <groupId>org.wso2.carbon.identity.server.api</groupId> - <artifactId>v1</artifactId> + <artifactId>org.wso2.carbon.identity.api.server.vc.template.management.v1</artifactId> <version>${project.version}</version> + <scope>provided</scope> </dependency>components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (1)
108-116: Missing path separator between V1_API_PATH_COMPONENT and VC_TEMPLATE_PATH_COMPONENT.This is the same issue identified in the previous review. The path construction concatenates
/v1directly withvc/templateswithout an intermediate separator, producing/v1vc/templatesinstead of/v1/vc/templates.🔎 Apply this diff to fix the path construction:
private URI buildTemplateLocation(String resourceId) { StringBuilder pathBuilder = new StringBuilder(Constants.V1_API_PATH_COMPONENT) + .append(VCTemplateManagementConstants.PATH_SEPARATOR) .append(VCTemplateManagementConstants.VC_TEMPLATE_PATH_COMPONENT); if (resourceId != null) { pathBuilder.append(VCTemplateManagementConstants.PATH_SEPARATOR).append(resourceId); } return ContextLoader.buildURIForHeader(pathBuilder.toString()); }
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java (1)
47-55: Consider adding null check for service initialization.If
ServerVCTemplateManagementServiceFactory.getServerVCTemplateManagementService()returnsnull, all subsequent API calls will fail withNullPointerException. Add validation after obtaining the service instance.🔎 Apply this diff to add validation:
public VcTemplatesApiServiceImpl() { try { this.serverVCTemplateManagementService = ServerVCTemplateManagementServiceFactory .getServerVCTemplateManagementService(); + if (this.serverVCTemplateManagementService == null) { + throw new IllegalStateException("ServerVCTemplateManagementService is not available"); + } } catch (IllegalStateException e) { throw new RuntimeException("Error occurred while initiating server vc config management service.", e); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/PaginationLink.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplate.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateCreationModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateList.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateListItem.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VCTemplateUpdateModel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/VcTemplatesApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/VcTemplatesApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/core/ServerVCTemplateManagementService.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/impl/VcTemplatesApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.vc.template.management/pom.xml(1 hunks)pom.xml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/org.wso2.carbon.identity.api.server.vc.template.management/pom.xml
- components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.common/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/common/VCTemplateManagementConstants.java
- components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/java/org/wso2/carbon/identity/api/server/vc/template/management/v1/factories/ServerVCTemplateManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/pom.xml
🧰 Additional context used
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.vc.template.management/org.wso2.carbon.identity.api.server.vc.template.management.v1/src/main/resources/VCTemplates.yaml
[medium] 318-323: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 278-281: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
...bon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java
Show resolved
Hide resolved
...bon/identity/api/server/vc/template/management/common/VCTemplateManagementServiceHolder.java
Show resolved
Hide resolved
|
PR builder completed |
Purpose
Introduce VC template management APIs
Related Issues
Goals
Approach
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
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.