Skip to content

Conversation

@SujanSanjula96
Copy link
Contributor

Proposed changes in this pull request

$Subject

Copilot AI review requested due to automatic review settings October 20, 2025 06:13
Copy link
Contributor

Copilot AI left a 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 improves tenant perspective organization context rewrite configurations by making them more flexible and configurable. The changes introduce conditional configuration options and dynamic template rendering for organization context rewrites.

  • Adds a new configuration flag to control authorization path enablement
  • Introduces dynamic template loops for configurable context rewrite paths
  • Conditionally includes authorization-related API paths and servlet contexts

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
org.wso2.carbon.identity.core.server.feature.default.json Adds new configuration flag for enabling authorization paths in tenant perspective
identity.xml.j2 Implements dynamic template rendering and conditional inclusion of authorization paths and servlet contexts

Comment on lines +4033 to +4038
{% for org_context_in_tenant_perspective in org_context_in_tenant_perspective.rewrite %}
{% for base_path in org_context_in_tenant_perspective.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in org_context_in_tenant_perspective.sub_paths %}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop variable name conflicts with the parent object name 'org_context_in_tenant_perspective'. This will shadow the parent variable and make nested properties inaccessible within the loop.

Suggested change
{% for org_context_in_tenant_perspective in org_context_in_tenant_perspective.rewrite %}
{% for base_path in org_context_in_tenant_perspective.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in org_context_in_tenant_perspective.sub_paths %}
{% for rewrite_context in org_context_in_tenant_perspective.rewrite %}
{% for base_path in rewrite_context.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in rewrite_context.sub_paths %}

Copilot uses AI. Check for mistakes.
Comment on lines +4077 to +4083
<Servlet>
{% for servlet in org_context_in_tenant_perspective.rewrite.servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
{% for servlet in org_context_in_tenant_perspective.rewrite.custom_servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These loops assume that 'org_context_in_tenant_perspective.rewrite.servlets' and 'org_context_in_tenant_perspective.rewrite.custom_servlets' are iterable collections, but there's no validation to ensure these properties exist or are properly structured.

Suggested change
<Servlet>
{% for servlet in org_context_in_tenant_perspective.rewrite.servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
{% for servlet in org_context_in_tenant_perspective.rewrite.custom_servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
<Servlet>
{% if org_context_in_tenant_perspective.rewrite.servlets is defined and org_context_in_tenant_perspective.rewrite.servlets %}
{% for servlet in org_context_in_tenant_perspective.rewrite.servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
{% endif %}
{% if org_context_in_tenant_perspective.rewrite.custom_servlets is defined and org_context_in_tenant_perspective.rewrite.custom_servlets %}
{% for servlet in org_context_in_tenant_perspective.rewrite.custom_servlets %}
<Context>{{servlet}}</Context>
{% endfor %}
{% endif %}

Copilot uses AI. Check for mistakes.
Comment on lines +4033 to +4038
{% for org_context_in_tenant_perspective in org_context_in_tenant_perspective.rewrite %}
{% for base_path in org_context_in_tenant_perspective.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in org_context_in_tenant_perspective.sub_paths %}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to variable shadowing in the outer loop (line 4033), this will try to access 'base_path' property on the loop item instead of the original configuration object, which will likely cause template rendering errors.

Suggested change
{% for org_context_in_tenant_perspective in org_context_in_tenant_perspective.rewrite %}
{% for base_path in org_context_in_tenant_perspective.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in org_context_in_tenant_perspective.sub_paths %}
{% for rewrite_item in org_context_in_tenant_perspective.rewrite %}
{% for base_path in rewrite_item.base_path %}
<Context>
<BasePath>{{base_path}}</BasePath>
<SubPaths>
{% for sub_path in rewrite_item.sub_paths %}

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

@SujanSanjula96 SujanSanjula96 marked this pull request as draft October 20, 2025 06:24
@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.87%. Comparing base (b6259f1) to head (6b39d10).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #7566      +/-   ##
============================================
+ Coverage     50.85%   50.87%   +0.02%     
+ Complexity    19001    18999       -2     
============================================
  Files          2099     2099              
  Lines        121819   121947     +128     
  Branches      25296    25349      +53     
============================================
+ Hits          61952    62046      +94     
- Misses        51867    51892      +25     
- Partials       8000     8009       +9     
Flag Coverage Δ
unit 35.23% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant