-
Notifications
You must be signed in to change notification settings - Fork 396
Enforce literal charactors in callback uri validation with a configuration #2900
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
Enforce literal charactors in callback uri validation with a configuration #2900
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2900 +/- ##
============================================
+ Coverage 56.08% 57.81% +1.72%
+ Complexity 9354 9088 -266
============================================
Files 669 669
Lines 53762 50708 -3054
Branches 11860 11226 -634
============================================
- Hits 30155 29317 -838
+ Misses 19378 17305 -2073
+ Partials 4229 4086 -143
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:
|
d6cfc7b
to
fb54307
Compare
...on/identity/oauth2/authz/validators/AbstractResponseTypeRequestValidatorCallbackURITest.java
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/oauth2/authz/validators/AbstractResponseTypeRequestValidator.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.
Pull Request Overview
This pull request enhances OAuth2 callback URI validation by introducing configurable literal character enforcement in regular expressions. The main improvement addresses security concerns where regex special characters (., +, ?) could cause unintended matches in callback URI patterns.
- Adds configuration-driven literal character enforcement for regex validation
- Implements diagnostic logging to identify validation failures when literal enforcement would apply
- Updates test coverage to validate the new enforcement behavior across various callback URI scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
AbstractResponseTypeRequestValidator.java | Core implementation of configurable literal character enforcement and diagnostic logging |
AbstractResponseTypeRequestValidatorCallbackURITest.java | Comprehensive test suite for the new callback URI validation logic |
testng.xml | Test configuration update to include the new test class |
OAuth2ServiceTest.java | Simplified existing test data provider by removing redundant test cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Escape (.), (+), (?) only when followed by a letter/digit (so .com, .org, etc. get escaped), | ||
but don't touch .* or .+ or .{n} . |
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.
The word 'charactors' in the PR title should be 'characters'. Also, the comment should end with proper punctuation and formatting.
Escape (.), (+), (?) only when followed by a letter/digit (so .com, .org, etc. get escaped), | |
but don't touch .* or .+ or .{n} . | |
Escape (.), (+), (?) only when followed by a letter/digit (so .com, .org, etc. get escaped), | |
but don't touch .* or .+ or .{n}. |
Copilot uses AI. Check for mistakes.
...a/org/wso2/carbon/identity/oauth2/authz/validators/AbstractResponseTypeRequestValidator.java
Show resolved
Hide resolved
b4d63eb
to
62a05d6
Compare
Proposed changes in this pull request
This pull request updates the callback URI validation logic in the OAuth2 authorization request validator. The main change is an improvement to how regular expressions are handled to ensure more accurate matching of callback URIs.
Callback URI validation improvements:
.
,?
,+
in the regular expression only when they are followed by a letter or digit (e.g., .com, .org), which prevents unintended matches and avoids interfering with wildcard patterns like .* or .+ or .{n}. when the particular configuration is enabledChanges derived from
Depends on