Skip to content

Conversation

@Malith-19
Copy link
Contributor

@Malith-19 Malith-19 commented Dec 5, 2025

Purpose

  • $subject

Related issues

Before merge

Summary by CodeRabbit

  • New Features

    • Added an application-level option to control extending renewed OAuth2 refresh token expiry; API, DTO and schema updated so clients can read/set this via application payloads. Server will populate the setting from defaults when not explicitly provided.
  • Chores

    • Bumped OAuth2-related module versions and updated license headers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Walkthrough

Adds a boolean OpenAPI field extendRenewedRefreshTokenExpiryTime to RefreshTokenConfiguration, propagates it through API↔DTO converters, updates the DTO setter signature to accept a boolean, uses OAuthServerConfiguration to supply a default when absent, and bumps an inbound OAuth2 dependency version.

Changes

Cohort / File(s) Summary
API model → DTO converter
components/org.wso2.carbon.identity.api.server.application.management/.../core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java
Reads extendRenewedRefreshTokenExpiryTime from the API model; if null, obtains default from OAuthServerConfiguration.isExtendRenewedTokenExpiryTimeEnabled(), then sets it on OAuthConsumerAppDTO via the boolean setter. Added import for OAuthServerConfiguration.
DTO → API model builder
components/org.wso2.carbon.identity.api.server.application.management/.../core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java
buildRefreshTokenConfiguration now includes .extendRenewedRefreshTokenExpiryTime(oAuthConsumerAppDTO.getExtendRenewedRefreshTokenExpiryTime()) to propagate the boolean value into the API model.
DTO / Model changes
components/org.wso2.carbon.identity.api.server.application.management/.../model/OAuthConsumerAppDTO.java, components/.../model/RefreshTokenConfiguration.java
OAuthConsumerAppDTO setter signature changed to setExtendRenewedRefreshTokenExpiryTime(boolean). RefreshTokenConfiguration builder gained extendRenewedRefreshTokenExpiryTime(Boolean) fluent accessor.
OpenAPI schema
components/org.wso2.carbon.identity.api.server.application.management/.../src/main/resources/applications.yaml
Added boolean property extendRenewedRefreshTokenExpiryTime to the RefreshTokenConfiguration schema (description and example included).
Build configuration
pom.xml
Bumped identity.inbound.oauth2 dependency version in project properties from 7.0.3507.0.409.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check the defaulting logic that uses OAuthServerConfiguration.isExtendRenewedTokenExpiryTimeEnabled() when the API model value is null.
  • Verify the DTO setter signature change (String → boolean) is applied consistently and callers updated.
  • Confirm OpenAPI schema metadata matches runtime expectations and examples.
  • Ensure the dependency bump does not introduce incompatible API changes in other modules.

Poem

🐰 I nibble lines of YAML bright,
A boolean hop into the night.
DTO ears and builder paws,
Defaults fetched from server laws.
Hop, commit — a carrot byte.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is incomplete. It lacks most required sections including Goals, Approach, User stories, Developer Checklist, Release note, Documentation, Training, Certification, Marketing, Automation tests, Security checks, Samples, Migrations, Test environment, and Learning. Complete the pull request description by filling in all required template sections, especially Goals, Approach, Release note, Documentation, Automation tests, and Security checks.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a new application property (extendRenewedRefreshTokenExpiryTime) to the refresh token configuration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85baa34 and 44c4239.

📒 Files selected for processing (1)
  • 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/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • 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/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines 143 to 144
consumerAppDTO.setRenewRefreshTokenEnabled(renewRefreshToken);
String extendRefreshTokenExpiryTime = refreshToken.getExtendRenewedRefreshTokenExpiryTime() != null ?

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 1

Suggested change
consumerAppDTO.setRenewRefreshTokenEnabled(renewRefreshToken);
String extendRefreshTokenExpiryTime = refreshToken.getExtendRenewedRefreshTokenExpiryTime() != null ?
consumerAppDTO.setRenewRefreshTokenEnabled(renewRefreshToken);
log.debug("Updated refresh token renewal configuration: " + renewRefreshToken);
String extendRefreshTokenExpiryTime = refreshToken.getExtendRenewedRefreshTokenExpiryTime() != null ?

Copy link

@wso2-engineering wso2-engineering bot left a 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

⚠️ Warning: AI-Generated Review Comments

  • 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.
Comment Accepted (Y/N) Reason
#### Log Improvement Suggestion No: 1
#### Log Improvement Suggestion No: 2
#### Log Improvement Suggestion No: 3

String renewRefreshToken = refreshToken.getRenewRefreshToken() != null ?
String.valueOf(refreshToken.getRenewRefreshToken()) : null;
consumerAppDTO.setRenewRefreshTokenEnabled(renewRefreshToken);
String extendRefreshTokenExpiryTime = refreshToken.getExtendRenewedRefreshTokenExpiryTime() != null ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a boolean instead of a String?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Followed the same pattern used for the renewRefreshToken property.

@Malith-19 Malith-19 merged commit 893f57d into wso2:master Dec 18, 2025
5 checks passed
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.

3 participants