-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
add missing url patterns for AdminServiceAuthenticationFilter #5291
Conversation
WalkthroughThe changes in this pull request introduce multiple updates and features to the Apollo system, primarily in version 2.4.0. Enhancements include new URL patterns for the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
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
🧹 Outside diff range and nitpick comments (1)
CHANGES.md (1)
27-27
: Consider enhancing the changelog entry with security impact.While the entry correctly documents the change, consider expanding it to highlight the security implications:
-* [Fix: add missing url patterns for AdminServiceAuthenticationFilter](https://github.com/apolloconfig/apollo/pull/5291) +* [Security: add missing url patterns for AdminServiceAuthenticationFilter to prevent unauthorized access to sensitive endpoints](https://github.com/apolloconfig/apollo/pull/5291)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
CHANGES.md
(1 hunks)apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceAutoConfiguration.java
(1 hunks)
🔇 Additional comments (1)
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceAutoConfiguration.java (1)
39-39
: LGTM! Security enhancement properly implemented.
The addition of URL patterns /apollo/audit/*
, /items-search/*
, and /server/*
effectively addresses the security vulnerability, particularly for the /server/config/find-all-config
endpoint. The implementation follows the existing pattern structure and provides comprehensive coverage.
Let's verify the impact of these new patterns:
Also applies to: 44-44, 47-47
✅ Verification successful
Security enhancement verification completed - Implementation is correct and safe
The added URL patterns are properly secured:
/apollo/audit/*
: All audit endpoints have@PreAuthorize
annotations requiring query permissions/items-search/*
: Part of the protected item management API, consistent with other item endpoints/server/*
: Covers sensitive server config endpoints that are only accessed via AdminServiceAPI
The patterns are used by legitimate internal APIs and don't affect any public endpoints. The Portal's AdminServiceAPI is the only consumer of these endpoints, making proper authenticated calls.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any public endpoints or documentation that might be affected
# Search for endpoint definitions under the new patterns
echo "Searching for affected endpoints..."
rg -l "(/apollo/audit/|/items-search/|/server/)" --type java
# Search for any documentation mentioning these endpoints
echo "Checking documentation for public API references..."
rg -l "(/apollo/audit/|/items-search/|/server/)" --type md
Length of output: 1283
Script:
#!/bin/bash
# Let's examine the endpoint implementations to understand their access patterns
# Check ServerConfigController implementations
echo "=== Server Config Controller implementations ==="
ast-grep --pattern 'class ServerConfigController {
$$$
}'
# Check ItemController implementation
echo "=== Item Controller implementation ==="
ast-grep --pattern 'class ItemController {
$$$
}'
# Check audit controller implementation
echo "=== Audit Controller implementation ==="
ast-grep --pattern 'class ApolloAuditController {
$$$
}'
# Look for any @PreAuthorize or security annotations on these endpoints
echo "=== Security annotations on endpoints ==="
rg "@(PreAuthorize|Secured|RolesAllowed)" apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/
rg "@(PreAuthorize|Secured|RolesAllowed)" apollo-audit/apollo-audit-impl/src/main/java/
Length of output: 1777
Script:
#!/bin/bash
# Let's check the actual endpoint mappings and their implementations
echo "=== Server Config Controller endpoints ==="
rg "@(GetMapping|PostMapping|RequestMapping|PutMapping|DeleteMapping)" -B2 -A2 apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ServerConfigController.java
echo "=== Item Controller endpoints ==="
rg "@(GetMapping|PostMapping|RequestMapping|PutMapping|DeleteMapping)" -B2 -A2 apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java
echo "=== Audit Controller endpoints ==="
rg "@(GetMapping|PostMapping|RequestMapping|PutMapping|DeleteMapping)" -B2 -A2 apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/controller/ApolloAuditController.java
# Check if these endpoints are used in portal API calls
echo "=== Portal API usage ==="
rg "(/apollo/audit/|/items-search/|/server/)" apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/
Length of output: 5716
What's the purpose of this PR
add missing url patterns for AdminServiceAuthenticationFilter
Which issue(s) this PR fixes:
Fixes #5290
Brief changelog
/apollo/audit/*
,/items-search/*
and/server/*
url patterns forAdminServiceAuthenticationFilter
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
New Features
ConsumerToken
.ConfigService
.AdminServiceAuthenticationFilter
.Bug Fixes
AdminServiceAuthenticationFilter
.