zhang-arvin commented on code in PR #12779:
URL: https://github.com/apache/gravitino/pull/12779#discussion_r3951631811
##########
server-common/src/main/java/org/apache/gravitino/server/web/HealthCheckPathMatcher.java:
##########
@@ -56,6 +55,8 @@ public boolean isHealthCheckPath(String path) {
|| path.startsWith("/health/")
|| path.equals("/health.html")
|| path.equals("/api/health")
- || path.startsWith("/api/health/");
+ || path.startsWith("/api/health/")
+ || path.equals("/")
+ || path.startsWith("/ui/");
Review Comment:
Good question. The UI paths are deliberately listed in
`HealthCheckPathMatcher` as exemptions, not as filter targets: now that the
`AuthenticationFilter` covers `/*` (outside the old `/api/*` scope), if the UI
path were also subject to authentication, unauthenticated users would be
blocked before they could ever load the login page (`/` / `/ui/*`), producing a
chicken-and-egg situation — nobody could authenticate because the login UI
itself would require authentication. So the matcher keeps `/`, `/ui/*` (and the
health aliases) as bypass paths so the login page stays anonymously reachable,
while every other non-`/api/*` path now goes through the auth filter.
I've also pushed a small follow-up commit extracting the repeated `"/*"`
into a named constant (`ROOT_PATH`) as LiJie20190102 suggested, and will add
tests for it.
##########
server/src/main/java/org/apache/gravitino/server/GravitinoServer.java:
##########
@@ -196,12 +196,12 @@ protected void configure() {
server.addServlet(new HealthAliasServlet(), "/health/*");
server.addServlet(new HealthAliasServlet(), "/health.html");
- server.addFilter(new RequestContextFilter(), API_ANY_PATH);
+ server.addFilter(new RequestContextFilter(), "/*");
Review Comment:
Thanks for the review! Extracted `"/*"` into a named constant `ROOT_PATH`
alongside `API_ANY_PATH` and replaced the 5 occurrences in the `configure`
method. Pushed to the branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
