Enhance outbound provisioning with resident SP fallback and sub-organization support#7765
Merged
PasinduYeshan merged 6 commits intowso2:masterfrom Mar 3, 2026
Merged
Conversation
9 tasks
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a logging dependency and OSGi import; introduced RuntimeProvisioningConfig.jit flag and propagated JIT state through OutboundProvisioningManager → ProvisioningThread; added ProvisioningUtil.maskIfRequired; adjusted LOCAL_SP fallback and tenant-domain lookups; added org-scoped API resources/scopes; updated tests and a singleton lock fix. Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
...sioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java
Show resolved
Hide resolved
...sioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java
Show resolved
Hide resolved
...ity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java
Show resolved
Hide resolved
...ntity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.java
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/provisioning/listener/DefaultInboundUserProvisioningListener.java
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/provisioning/listener/DefaultInboundUserProvisioningListener.java
Show resolved
Hide resolved
.../src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java
Show resolved
Hide resolved
.../src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
9 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate copyright year to include 2026.
The file is being modified in 2026 but the header still reads
Copyright (c) 2014. As per coding guidelines, the copyright year must be the current year or a range ending in the current year.📝 Proposed fix
- * Copyright (c) 2014, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2014-2026, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.As per coding guidelines: "Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java` at line 2, The file header in OutboundProvisioningManager.java still shows "Copyright (c) 2014" and must be updated to include 2026; edit the top-of-file license comment in the OutboundProvisioningManager class file to use a current-year form (e.g., "2014-2026" or just "2026") so the copyright range or end-year reflects 2026.components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate copyright year to match the current year range.
This file is changed in the PR but the copyright header still reads
Copyright (c) 2014, WSO2 Inc.. The sibling fileProvisioningThread.javain this same PR already carries the correct form. As per coding guidelines, the copyright year must be the current year or a range ending in the current year.🔧 Proposed fix
-* Copyright (c) 2014, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. +* Copyright (c) 2014-2026, WSO2 LLC. (http://www.wso2.com).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.java` at line 2, Update the copyright header in ProvisioningUtil.java to use the current year or a year range ending in the current year (matching the sibling ProvisioningThread.java); locate the file by the class name ProvisioningUtil and replace the existing "Copyright (c) 2014, WSO2 Inc." line with the correct form (e.g., "Copyright (c) 2014-2026, WSO2 Inc." or just "Copyright (c) 2026, WSO2 Inc.") so the header across the provisioning classes is consistent.
🧹 Nitpick comments (2)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java (1)
303-309: Guard against null/blank tenant domain fromthreadLocalServiceProvider.getTenantDomain().If
getTenantDomain()returns null or empty,getServiceProviderNameByClientIdwill fail and the exception is silently caught, causing provisioning to be skipped without diagnostic context. UseStringUtils.isNotBlank()with a fallback totenantDomainName(already resolved fromCarbonContextat line 292) to prevent silent failures:Suggested defensive guard
+String spTenantDomain = StringUtils.isNotBlank(threadLocalServiceProvider.getTenantDomain()) + ? threadLocalServiceProvider.getTenantDomain() + : tenantDomainName; serviceProvider = ApplicationManagementService.getInstance() .getServiceProviderNameByClientId( threadLocalServiceProvider.getServiceProviderName(), IdentityApplicationConstants.OAuth2.NAME, - threadLocalServiceProvider.getTenantDomain()); + spTenantDomain);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java` around lines 303 - 309, Guard against a null/blank tenant domain returned by threadLocalServiceProvider.getTenantDomain() before calling getServiceProviderNameByClientId: use StringUtils.isNotBlank(threadLocalServiceProvider.getTenantDomain()) and if false fall back to the already-resolved tenantDomainName (from CarbonContext) so getServiceProviderNameByClientId(...) is always invoked with a non-empty tenant domain; update the call site around getServiceProviderNameByClientId(...) and adjust the surrounding error handling/logging to avoid silent provisioning skips when tenant domain is missing.components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java (1)
396-407: Add a WARN log whenLOCAL_SPcannot be resolved.When
localSPisnullthe fallback silently proceeds with an empty connector map, meaning outbound provisioning is quietly skipped with no trace in the logs. Since the fallback is only entered when provisioning is expected to proceed (the config explicitly opts out of app-level provisioning), failing to resolveLOCAL_SPis a deviation from expected behaviour that warrants a warning.🔍 Proposed improvement
if (localSP != null) { serviceProvider = localSP; connectors = getOutboundProvisioningConnectors(localSP, spTenantDomainName); inboundClaimDialect = IdentityProvisioningConstants.WSO2_CARBON_DIALECT; // LOCAL_SP uses WSO2_CARBON_DIALECT; spClaimMappings is not needed. spClaimMappings = null; + } else { + log.warn("LOCAL_SP not found for tenant: " + spTenantDomainName + + ". Skipping outbound provisioning fallback."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java` around lines 396 - 407, When the fallback branch that looks up LOCAL_SP via ApplicationManagementService.getInstance().getServiceProvider(...) yields localSP==null there is no log, so outbound provisioning silently stops; add a warning log (using the existing logger in this class) inside the if-block where localSP is null to warn that resolving LOCAL_SP failed and outbound provisioning will be skipped — reference the symbols isApplicationBasedOutboundProvisioningEnabled(), LOCAL_SP, localSP, connectors, getOutboundProvisioningConnectors(...) and serviceProvider so the log clearly states that LOCAL_SP could not be resolved for the tenant (include spTenantDomainName) and that connectors remain empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Line 2: The file header in OutboundProvisioningManager.java still shows
"Copyright (c) 2014" and must be updated to include 2026; edit the top-of-file
license comment in the OutboundProvisioningManager class file to use a
current-year form (e.g., "2014-2026" or just "2026") so the copyright range or
end-year reflects 2026.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.java`:
- Line 2: Update the copyright header in ProvisioningUtil.java to use the
current year or a year range ending in the current year (matching the sibling
ProvisioningThread.java); locate the file by the class name ProvisioningUtil and
replace the existing "Copyright (c) 2014, WSO2 Inc." line with the correct form
(e.g., "Copyright (c) 2014-2026, WSO2 Inc." or just "Copyright (c) 2026, WSO2
Inc.") so the header across the provisioning classes is consistent.
---
Nitpick comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java`:
- Around line 303-309: Guard against a null/blank tenant domain returned by
threadLocalServiceProvider.getTenantDomain() before calling
getServiceProviderNameByClientId: use
StringUtils.isNotBlank(threadLocalServiceProvider.getTenantDomain()) and if
false fall back to the already-resolved tenantDomainName (from CarbonContext) so
getServiceProviderNameByClientId(...) is always invoked with a non-empty tenant
domain; update the call site around getServiceProviderNameByClientId(...) and
adjust the surrounding error handling/logging to avoid silent provisioning skips
when tenant domain is missing.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Around line 396-407: When the fallback branch that looks up LOCAL_SP via
ApplicationManagementService.getInstance().getServiceProvider(...) yields
localSP==null there is no log, so outbound provisioning silently stops; add a
warning log (using the existing logger in this class) inside the if-block where
localSP is null to warn that resolving LOCAL_SP failed and outbound provisioning
will be skipped — reference the symbols
isApplicationBasedOutboundProvisioningEnabled(), LOCAL_SP, localSP, connectors,
getOutboundProvisioningConnectors(...) and serviceProvider so the log clearly
states that LOCAL_SP could not be resolved for the tenant (include
spTenantDomainName) and that connectors remain empty.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/provisioning/org.wso2.carbon.identity.provisioning/pom.xmlcomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/DefaultInboundUserProvisioningListener.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves outbound provisioning reliability and log safety by adding a resident-SP (LOCAL_SP) connector fallback, enhancing error log context with optional masking, and fixing tenant-domain usage when resolving OAuth-based service provider identifiers.
Changes:
- Add LOCAL_SP fallback connector resolution when application-based outbound provisioning is disabled and an app has no outbound connectors.
- Introduce log masking support via
LoggerUtilsand improve outbound provisioning error messages with more context. - Adjust OAuth-based SP name resolution to use the correct tenant domain in provisioning listeners.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/ProvisioningErrorListener.java | Updates copyright header and fixes tenant domain passed for OAuth client-id → SP-name resolution. |
| components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/listener/DefaultInboundUserProvisioningListener.java | Fixes tenant domain passed for OAuth client-id → SP-name resolution. |
| components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningUtil.java | Adds a helper to mask log values using central log-mgt utilities. |
| components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java | Enhances failure log message with masked entity name and more context. |
| components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java | Refines resident-SP selection logic and adds LOCAL_SP fallback behavior when connectors are missing. |
| components/provisioning/org.wso2.carbon.identity.provisioning/pom.xml | Adds central log-mgt dependency and OSGi import for masking utilities. |
Comment on lines
1
to
5
| /* | ||
| * Copyright (c) 2022, WSO2 Inc. (http://www.wso2.org) | ||
| * Copyright (c) 2022-2026, WSO2 Inc. (http://www.wso2.org) | ||
| * | ||
| * WSO2 Inc. licenses this file to you under the Apache License, | ||
| * Version 2.0 (the "License"); you may not use this file except |
There was a problem hiding this comment.
License header does not match the required WSO2 Apache-2.0 header format (company name/URL/text) expected for Java files. Please replace this header with the standard 2026 WSO2 LLC Apache 2.0 header used in this repo to ensure license compliance consistency.
Comment on lines
+394
to
+398
| // When application-based outbound provisioning is disabled and the application has no | ||
| // outbound provisioning connectors configured, fall back to LOCAL_SP (resident app) connectors. | ||
| if (!isApplicationBasedOutboundProvisioningEnabled() && MapUtils.isEmpty(connectors) | ||
| && !LOCAL_SP.equals(serviceProviderIdentifier)) { | ||
| ServiceProvider localSP = ApplicationManagementService.getInstance() |
There was a problem hiding this comment.
When falling back to LOCAL_SP you replace serviceProvider (and related dialect/claim-mapping decisions), but provisioningEntityTenantDomainName was computed earlier based on the original service provider’s isSaasApp() value. This can cause the provisioning thread to pick the wrong tenant flow after the fallback. Consider recomputing provisioningEntityTenantDomainName (and any other SP-derived variables) after selecting the final serviceProvider to use.
Comment on lines
+394
to
+397
| // When application-based outbound provisioning is disabled and the application has no | ||
| // outbound provisioning connectors configured, fall back to LOCAL_SP (resident app) connectors. | ||
| if (!isApplicationBasedOutboundProvisioningEnabled() && MapUtils.isEmpty(connectors) | ||
| && !LOCAL_SP.equals(serviceProviderIdentifier)) { |
There was a problem hiding this comment.
The new LOCAL_SP fallback path (when application-based outbound provisioning is disabled and an app has no connectors) introduces important behavior changes, but there is no corresponding test coverage in the provisioning module tests. Please add a unit/integration test that exercises this fallback (connectors empty for app, LOCAL_SP connectors present) and asserts the effective connectors/dialect/tenant flow used.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #7765 +/- ##
============================================
- Coverage 52.31% 51.26% -1.06%
+ Complexity 19862 19847 -15
============================================
Files 2130 2161 +31
Lines 124116 126789 +2673
Branches 24549 26280 +1731
============================================
+ Hits 64935 64999 +64
- Misses 51181 53541 +2360
- Partials 8000 8249 +249
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml (1)
3-3:⚠️ Potential issue | 🟡 MinorStale copyright year —
system-api-resource.xmlstill reads2023-2024while the siblingapi-resource-collection.xmlwas bumped to2026in this PR.📝 Proposed fix
- ~ Copyright (c) 2023-2024, WSO2 LLC. (http://www.wso2.com). + ~ Copyright (c) 2023-2026, WSO2 LLC. (http://www.wso2.com).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml` at line 3, Update the copyright year displayed in the header of the system-api-resource.xml file from "2023-2024" to "2026" to match the sibling api-resource-collection.xml; locate the copyright comment line near the top of the XML (the line containing "Copyright (c) 2023-2024, WSO2 LLC.") and replace the year range so it reads "2026".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml`:
- Line 543: The new (no-version) org_connections collection is missing the Scope
entry added to the legacy v0 collection; open the org_connections collection
definition (the Read section of the no-version collection) and add a Scope
element with name="internal_org_group_mgt_view" (matching the legacy entry) so
org-scoped Connections consumers get group-read access; ensure you add it to the
Read permissions block for the org_connections collection and do not create a
duplicate scope entry.
---
Outside diff comments:
In
`@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml`:
- Line 3: Update the copyright year displayed in the header of the
system-api-resource.xml file from "2023-2024" to "2026" to match the sibling
api-resource-collection.xml; locate the copyright comment line near the top of
the XML (the line containing "Copyright (c) 2023-2024, WSO2 LLC.") and replace
the year range so it reads "2026".
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/provisioning/org.wso2.carbon.identity.provisioning/pom.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml.j2features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2
...g.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml
Outdated
Show resolved
Hide resolved
PasinduYeshan
force-pushed
the
feat/suborg-outbound
branch
from
c11da38 to
eced60b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/cache/ServiceProviderProvisioningConnectorCache.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate license header year to current range.
Line 2 still uses
2014; please update to a range ending in2026(for example,2014-2026) to meet repo policy.As per coding guidelines,
**/*.java: Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/cache/ServiceProviderProvisioningConnectorCache.java` at line 2, Update the license header year range at the top of the file to include the current year (change the single year "2014" to a range ending in 2026, e.g., "2014-2026") in the file containing the ServiceProviderProvisioningConnectorCache class so the Java source header matches repository policy; locate the existing header comment in ServiceProviderProvisioningConnectorCache.java and replace the year text accordingly.components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java (1)
1-2:⚠️ Potential issue | 🟡 MinorUpdate the copyright year to include the current year.
The copyright header shows
2014, but per coding guidelines it should be the current year or a range ending in the current year (e.g.,2014-2026).Proposed fix
/* - * Copyright (c) 2014, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2014-2026, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. *As per coding guidelines: "Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java` around lines 1 - 2, The file header copyright is outdated (shows 2014); update the license header at the top of OutboundProvisioningManager.java so the copyright year is current or a range ending in the current year (e.g., change "2014" to "2014-2026" or "2026"); locate the existing block comment at the top of the file (the /* ... */ header) and replace the year accordingly while preserving the rest of the license text.
🧹 Nitpick comments (2)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java (1)
378-384: Consider logging when LOCAL_SP lookup returns null.If
ApplicationManagementService.getInstance().getServiceProvider(LOCAL_SP, spTenantDomainName)returns null, the code silently continues without any provisioning connectors. A debug log for this edge case would aid troubleshooting.Proposed enhancement
if (localSP != null) { serviceProvider = localSP; connectors = getOutboundProvisioningConnectors(localSP, spTenantDomainName); inboundClaimDialect = IdentityProvisioningConstants.WSO2_CARBON_DIALECT; // LOCAL_SP uses WSO2_CARBON_DIALECT; spClaimMappings is not needed. spClaimMappings = null; + } else { + if (log.isDebugEnabled()) { + log.debug("LOCAL_SP not found for tenant: " + spTenantDomainName + ". Skipping fallback."); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java` around lines 378 - 384, Add a debug log for the case when ApplicationManagementService.getInstance().getServiceProvider(LOCAL_SP, spTenantDomainName) returns null so the silent fall-through is visible; specifically, after the retrieval that assigns localSP (and before or instead of silently continuing), log at debug level that localSP is null for the given spTenantDomainName and that no local serviceProvider/connectors will be used, referencing localSP, serviceProvider, getOutboundProvisioningConnectors, spClaimMappings and IdentityProvisioningConstants.WSO2_CARBON_DIALECT so maintainers can locate the logic.components/provisioning/org.wso2.carbon.identity.provisioning/src/test/java/org/wso2/carbon/identity/provisioning/ProvisioningThreadTest.java (1)
168-168: Normalize inline comment casing at Line 168.Please capitalize the first word in this comment to match repository comment conventions.
As per coding guidelines, "Comments should start with a space and first letter capitalized."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/test/java/org/wso2/carbon/identity/provisioning/ProvisioningThreadTest.java` at line 168, Update the inline comment in ProvisioningThreadTest to follow comment conventions by capitalizing the first word and keeping the leading space after the slashes: change the comment "// jitProvisioningEnabledForIdP=false must not block regular provisioning." to start with a capitalized first word (e.g., "// JitProvisioningEnabledForIdP=false must not block regular provisioning.") so it matches repository comment casing rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java`:
- Around line 66-74: Add a Javadoc for the new public ProvisioningThread
constructor that documents the purpose of the constructor and each parameter
(including the new boolean jitProvisioningEnabledForIdP), explains any
side-effects (e.g., it delegates to the other ProvisioningThread(...)
constructor and sets jitProvisioningEnabledForIdP), and notes thread-safety or
nullability expectations for parameters such as provisioningEntity, connector,
dao, and tenant domain names; place the Javadoc immediately above the public
ProvisioningThread(ProvisioningEntity,..., boolean jitProvisioningEnabledForIdP)
constructor and follow project Javadoc style for param and return descriptions.
- Around line 42-43: The field jitProvisioningEnabledForIdP is never initialized
in the legacy constructors of ProvisioningThread, so the new guard (checking
jitProvisioningEnabledForIdP) will always short-circuit JIT requests; update the
existing constructors in class ProvisioningThread (the ones that currently set
tenantDomain, subject, remoteIdpName, claimMapping, etc.) to explicitly
initialize jitProvisioningEnabledForIdP to the prior default (true) or take it
as a parameter and assign it, ensuring the new guard behaves the same for
objects created via those constructors; locate references to
jitProvisioningEnabledForIdP and the constructors in ProvisioningThread and add
the assignment there.
- Around line 106-108: The code mutates the shared cached connector instance by
setting connector.jitProvisioningEnabled in ProvisioningThread, causing data
races; remove the direct assignment and instead thread-scope the JIT flag (e.g.,
add a boolean parameter or a per-request context) and pass it into the
connector's provisioning flow. Concretely, stop writing
connector.jitProvisioningEnabled in ProvisioningThread, extend the Connector API
(e.g., Connector.provision(..., boolean jitProvisioningEnabled) or use a
RequestContext/ThreadLocal read-only accessor) and update ProvisioningThread to
pass jitProvisioningEnabledForIdP through to the connector.provision call so no
shared cached field is mutated. Ensure all connector implementations and callers
are updated to accept the new parameter or to read the request-scoped context.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/RuntimeProvisioningConfig.java`:
- Around line 47-50: Update the Javadoc comment for the getter that returns
whether JIT provisioning is enabled by adding a trailing period to the sentence
"Get JIT provisioning enabled or not." (the Javadoc block immediately above the
method that returns JIT provisioning enabled) so the description ends with a
period to satisfy comment punctuation rules.
---
Outside diff comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/cache/ServiceProviderProvisioningConnectorCache.java`:
- Line 2: Update the license header year range at the top of the file to include
the current year (change the single year "2014" to a range ending in 2026, e.g.,
"2014-2026") in the file containing the
ServiceProviderProvisioningConnectorCache class so the Java source header
matches repository policy; locate the existing header comment in
ServiceProviderProvisioningConnectorCache.java and replace the year text
accordingly.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Around line 1-2: The file header copyright is outdated (shows 2014); update
the license header at the top of OutboundProvisioningManager.java so the
copyright year is current or a range ending in the current year (e.g., change
"2014" to "2014-2026" or "2026"); locate the existing block comment at the top
of the file (the /* ... */ header) and replace the year accordingly while
preserving the rest of the license text.
---
Nitpick comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Around line 378-384: Add a debug log for the case when
ApplicationManagementService.getInstance().getServiceProvider(LOCAL_SP,
spTenantDomainName) returns null so the silent fall-through is visible;
specifically, after the retrieval that assigns localSP (and before or instead of
silently continuing), log at debug level that localSP is null for the given
spTenantDomainName and that no local serviceProvider/connectors will be used,
referencing localSP, serviceProvider, getOutboundProvisioningConnectors,
spClaimMappings and IdentityProvisioningConstants.WSO2_CARBON_DIALECT so
maintainers can locate the logic.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/test/java/org/wso2/carbon/identity/provisioning/ProvisioningThreadTest.java`:
- Line 168: Update the inline comment in ProvisioningThreadTest to follow
comment conventions by capitalizing the first word and keeping the leading space
after the slashes: change the comment "// jitProvisioningEnabledForIdP=false
must not block regular provisioning." to start with a capitalized first word
(e.g., "// JitProvisioningEnabledForIdP=false must not block regular
provisioning.") so it matches repository comment casing rules.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/RuntimeProvisioningConfig.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/cache/ServiceProviderProvisioningConnectorCache.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/test/java/org/wso2/carbon/identity/provisioning/ProvisioningThreadTest.java
...ity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java
Show resolved
Hide resolved
...ity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java
Show resolved
Hide resolved
...ity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java
Outdated
Show resolved
Hide resolved
...visioning/src/main/java/org/wso2/carbon/identity/provisioning/RuntimeProvisioningConfig.java
Show resolved
Hide resolved
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java (1)
1-17:⚠️ Potential issue | 🟡 MinorUpdate file license header to current-year format.
This header still uses the old single-year form and should be updated to match the repository’s required Apache header year format (e.g., range ending in 2026).
As per coding guidelines, "Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026). Use the Apache License 2.0 header format specified."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java` around lines 1 - 17, Update the file header license in OutboundProvisioningManager.java to the repository's current Apache 2.0 header format by replacing the old single-year copyright line with the correct year or range ending in the current year (e.g., "2014-2026"), keeping the rest of the standard Apache License 2.0 text intact; ensure the header appears at the very top of the file above the OutboundProvisioningManager class declaration and matches the project's canonical header format used across other Java files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Around line 207-217: The JIT flag is read from the potentially stale fIdP
before re-resolving the IdP in getOutboundProvisioningConnector, which can
propagate outdated state into RuntimeProvisioningConfig; fix by computing
enableJitProvisioning after obtaining the resolved IdP (i.e., move or
re-evaluate fIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled() once
getOutboundProvisioningConnector(...) returns the up-to-date connector/IdP) and
then set proConfig.setJitProvisioningEnabled(...) using that fresh value so
RuntimeProvisioningConfig receives the correct per-IdP JIT state (refer to fIdP,
getOutboundProvisioningConnector, RuntimeProvisioningConfig, and proConfig).
---
Outside diff comments:
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`:
- Around line 1-17: Update the file header license in
OutboundProvisioningManager.java to the repository's current Apache 2.0 header
format by replacing the old single-year copyright line with the correct year or
range ending in the current year (e.g., "2014-2026"), keeping the rest of the
standard Apache License 2.0 text intact; ensure the header appears at the very
top of the file above the OutboundProvisioningManager class declaration and
matches the project's canonical header format used across other Java files.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.javacomponents/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/RuntimeProvisioningConfig.java
🚧 Files skipped from review as they are similar to previous changes (1)
- components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/ProvisioningThread.java
Comment on lines
+207
to
+217
| boolean enableJitProvisioning = fIdP.getJustInTimeProvisioningConfig() != null | ||
| && fIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled(); | ||
|
|
||
| if (fIdP.getJustInTimeProvisioningConfig() != null | ||
| && fIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled()) { | ||
| enableJitProvisioning = true; | ||
| } | ||
|
|
||
| connector = getOutboundProvisioningConnector(fIdP, | ||
| registeredConnectorFactories, tenantDomain, | ||
| enableJitProvisioning); | ||
| connector = getOutboundProvisioningConnector(fIdP, registeredConnectorFactories, tenantDomain); | ||
| // add to the provisioning connectors list. there will be one item for each | ||
| // provisioning identity provider found in the out-bound provisioning | ||
| // configuration of the local service provider. | ||
| if (connector != null) { | ||
| RuntimeProvisioningConfig proConfig = new RuntimeProvisioningConfig(); | ||
| proConfig | ||
| .setProvisioningConnectorEntry(new SimpleEntry<>( | ||
| connectorType, connector)); | ||
| proConfig.setJitProvisioningEnabled(enableJitProvisioning); | ||
| proConfig.setProvisioningConnectorEntry(new SimpleEntry<>(connectorType, connector)); |
Contributor
There was a problem hiding this comment.
JIT enablement is derived from a potentially stale IdP object.
enableJitProvisioning is computed from fIdP before the enabled IdP is re-resolved in getOutboundProvisioningConnector(...). This can propagate outdated JIT state into RuntimeProvisioningConfig and cause incorrect per-IdP behavior.
Suggested direction
- boolean enableJitProvisioning = fIdP.getJustInTimeProvisioningConfig() != null
- && fIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled();
- connector = getOutboundProvisioningConnector(fIdP, registeredConnectorFactories, tenantDomain);
+ connector = getOutboundProvisioningConnector(fIdP, registeredConnectorFactories, tenantDomain);
+ IdentityProvider effectiveIdP = IdentityProviderManager.getInstance()
+ .getEnabledIdPByName(fIdP.getIdentityProviderName(), tenantDomain);
+ boolean enableJitProvisioning = effectiveIdP != null
+ && effectiveIdP.getJustInTimeProvisioningConfig() != null
+ && effectiveIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@components/provisioning/org.wso2.carbon.identity.provisioning/src/main/java/org/wso2/carbon/identity/provisioning/OutboundProvisioningManager.java`
around lines 207 - 217, The JIT flag is read from the potentially stale fIdP
before re-resolving the IdP in getOutboundProvisioningConnector, which can
propagate outdated state into RuntimeProvisioningConfig; fix by computing
enableJitProvisioning after obtaining the resolved IdP (i.e., move or
re-evaluate fIdP.getJustInTimeProvisioningConfig().isProvisioningEnabled() once
getOutboundProvisioningConnector(...) returns the up-to-date connector/IdP) and
then set proConfig.setJitProvisioningEnabled(...) using that fresh value so
RuntimeProvisioningConfig receives the correct per-IdP JIT state (refer to fIdP,
getOutboundProvisioningConnector, RuntimeProvisioningConfig, and proConfig).
Contributor
Author
There was a problem hiding this comment.
This is intentional. At this point, the fIdP is created using the application's configured outbound JIT provisioning settings. getOutboundProvisioningConnector fetches an IdP-level JIT enablement check, which is not relevant in the context of outbound provisioning.
Contributor
There was a problem hiding this comment.
@PasinduYeshan, thank you for the clarification! That makes perfect sense—the application's outbound provisioning configuration should take precedence for JIT settings in the outbound flow, rather than the IdP's general JIT configuration. I appreciate you explaining the architectural intent here.
Contributor
Author
sadilchamishka
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and fixes to the outbound provisioning logic, focusing on enhanced error logging, improved connector fallback behavior, and better handling of service provider identification. The changes also add support for log masking to protect sensitive information and update dependencies to enable these features.
Outbound Provisioning Logic and Error Handling:
LOCAL_SP). This ensures outbound provisioning can still proceed in such scenarios.ProvisioningThreadnow include masked entity names and additional context (IDP, connector, entity type, and operation), improving traceability while protecting sensitive data. [1] [2]Log Masking and Dependency Updates:
org.wso2.carbon.identity.central.log.mgtand updated the import package list to support log masking utilities. [1] [2] [3]ProvisioningUtil.maskIfRequiredto mask sensitive values in logs when log masking is enabled.Service Provider Identification Fixes:
Other Minor Changes:
These changes collectively improve the reliability, security, and maintainability of the outbound provisioning component.
Related Issues