refactor(SecurityFilter): Stop filters from being registered on all routes#361
Conversation
📝 WalkthroughWalkthroughConsolidated path-specific JWT filters into a generic QueryParameterJwtFilter, added AuthenticationCheckFilter to enforce authenticated SecurityContext, updated SecurityFilterChain beans (new wsSecurityChain and reordered Changes
Sequence Diagram(s)mermaid Client->>SecurityConfig: HTTP request (e.g., /api/... or /media/...) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java`:
- Around line 47-51: The try/catch in QueryParameterJwtFilter currently swallows
all Exceptions around jwtUtils.validateToken(token) and authenticateUser(token,
request), masking real server errors from authenticateUser (DB/mapper failures);
replace the broad "catch (Exception _)" with handling only
token-validation-specific exceptions (e.g. JWT parsing/validation exceptions) to
treat invalid tokens as unauthenticated, and for all other exceptions let them
propagate (or log and rethrow) so they surface as server errors; ensure you call
authenticateUser(token, request) unchanged and use the class logger (or rethrow
a ServletException/RuntimeException) for non-token errors instead of swallowing
them.
In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java`:
- Around line 249-260: The custom securityMatcher lambda using
request.getRequestURI() causes context-path-sensitive matching; update the
SecurityConfig so the matcher uses Spring's path matching or strips the context
path: either replace the lambda with Spring's string-based .securityMatcher(...)
patterns (like the other chains) or inside the lambda compute the
servlet-relative path by removing request.getContextPath() (e.g., substring
starting at request.getContextPath().length()) before comparing against
matchPaths and whitelistedPaths; adjust references to matchPaths and
whitelistedPaths comparisons accordingly so startsWith checks operate on the
stripped path.
- Around line 236-242: The jwtApiSecurityChain currently runs
authenticationCheckFilter which sends 401 for requests that pass through
jwtAuthenticationFilter without setting authentication; to fix, add the common
public routes from COMMON_PUBLIC_ENDPOINTS (e.g. "/api/docs",
"/api/openapi.json", "/api/v1/auth/**", "/api/v1/public-settings",
"/api/v1/healthcheck/**" or any other entries referenced by
COMMON_PUBLIC_ENDPOINTS) into the whitelistedPaths List so securityMatcher
permits them, or alternatively remove authenticationCheckFilter from
jwtApiSecurityChain if you intend those endpoints to remain accessible without
authentication; update the whitelistedPaths List in SecurityConfig (where
whitelistedPaths is defined) or adjust jwtApiSecurityChain accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 896f7c85-0101-439f-bb91-57325c6b1eda
📒 Files selected for processing (10)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.javabooklore-api/src/main/java/org/booklore/config/security/filter/AudiobookStreamingJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/CoverJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/CustomFontJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/EpubStreamingJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/JwtAuthenticationFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java
💤 Files with no reviewable changes (4)
- booklore-api/src/main/java/org/booklore/config/security/filter/AudiobookStreamingJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/CustomFontJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/CoverJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/EpubStreamingJwtFilter.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/config/security/filter/JwtAuthenticationFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
🧠 Learnings (3)
📚 Learning: 2026-03-25T21:02:57.527Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 195
File: booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java:15-19
Timestamp: 2026-03-25T21:02:57.527Z
Learning: In `booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java` and `booklore-api/src/main/java/org/booklore/config/logging/filter/RequestLoggingFilter.java`, logging all request headers and payloads at DEBUG level is intentional and accepted behavior in this project. The previous `LoggingFilter` also logged all headers. No header redaction or payload scrubbing is required — the DEBUG log level is considered sufficient access control. The new filter is not profile-gated (unlike the old `Profile("dev")` filter) but relies on `logger.isDebugEnabled()` for gating.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/JwtAuthenticationFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.java
📚 Learning: 2026-04-03T20:56:48.678Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 337
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java:38-40
Timestamp: 2026-04-03T20:56:48.678Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java` and `booklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.java`, the `forwardToKoboStore` field defaults to `false`. This means existing server installs that upgrade will have Kobo Store forwarding/combined init behavior silently disabled. This is **intentional by design** (per imnotjames, PR `#337`) and is expected to be communicated via release notes rather than through a migration/backfill.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.java
📚 Learning: 2026-03-25T19:09:09.638Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 189
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:113-116
Timestamp: 2026-03-25T19:09:09.638Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, when deciding whether to forward the `Authorization` header to the upstream Kobo server proxy on the `/v1/library/sync` endpoint, a simple null check (`request.getHeader(HttpHeaders.AUTHORIZATION) != null`) is the correct guard. The header is either absent or present — if present it may or may not be a valid token, but validity is determined by the upstream Kobo server, not by Grimmory. Blank/whitespace intermediate states do not occur in practice, and pre-validating the token value before forwarding is not the responsibility of this service.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.java
🪛 GitHub Check: CodeQL
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
[failure] 216-219: Disabled Spring CSRF protection
CSRF vulnerability due to protection being disabled.
🔀 Multi-repo context grimmory-tools/grimmory-docs
[::grimmory-tools/grimmory-docs::]
- docs/integration/kobo.md: lines ~77-86 — documents using the /api/kobo/ endpoint in Kobo integration examples (doc shows api_endpoint=http://.../api/kobo/your-token-here). This is a consumer-facing doc referencing the Kobo auth path the PR touches.
- docs/integration/opds.md: lines ~11, ~116-119, ~86 — references Komga API at /komga/api/ and OPDS endpoints under /api/v1/opds and other /api paths (OPDS auth/paths are listed in the docs). These overlap with the jwtApiSecurityChain matcher changes and OPDS whitelisting mentioned in the PR.
- docs/integration/komga-api.md: multiple lines — documents Komga compatibility and authentication via OPDS (/komga/** and /api/** references). Relevant to changes that gate /komga and /api routes.
- docs/authentication/overview.md and docs/authentication/oidc-settings.md: various lines — general authentication docs reference endpoints such as /api/v1/auth/refresh and server-side auth behavior; these endpoints are among those whitelisted/excluded in the PR.
Notes/implication: user-facing docs reference the exact API paths (/api/kobo, /api/v1/opds, /komga, /api/v1/auth/refresh) whose routing/filtering behavior was modified by the PR; docs may need review to ensure examples and guidance still match runtime behavior (especially around WebSocket /ws handling and new authentication-check/filter behavior).
🔇 Additional comments (1)
booklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.java (1)
8-17: The Spring Boot version in this project is 4.0.5, which fully supports@FilterRegistrationsince version 3.5.0. No compatibility issues exist with the@FilterRegistration(enabled = false)annotation used in this filter.
9943fc7 to
baafaf8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java (1)
49-57:⚠️ Potential issue | 🟠 MajorDo not swallow non-authentication failures in
doFilterInternal.At Line 55,
catch (Exception ex)also catches repository/transformer/runtime failures fromauthenticateUser(...)and then continues the chain, turning server faults into normal unauthenticated flow.Proposed tightening of exception handling
try { if (jwtUtils.validateToken(token)) { authenticateUser(token, request); } else { log.debug("Invalid token. Rejecting request."); } - } catch (Exception ex) { - log.error("Authentication error: {}", ex.getMessage(), ex); + } catch (UsernameNotFoundException ex) { + // Expected auth miss path: token user no longer exists + log.debug("Token user not found: {}", ex.getMessage()); }booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java (1)
260-266:⚠️ Potential issue | 🟠 MajorCustom matcher is context-path sensitive and can bypass this chain.
At Line 261, matching against
request.getRequestURI()includes the context path. Deployments with a non-root context path (e.g.,/booklore) can fail/api/**//komga/**matching and skip this chain.Proposed fix
.securityMatcher(request -> { - var pathContainer = PathContainer.parsePath(request.getRequestURI()); + String requestPath = request.getRequestURI().substring(request.getContextPath().length()); + if (requestPath.isEmpty()) { + requestPath = "/"; + } + var pathContainer = PathContainer.parsePath(requestPath); if (matchPatterns.parallelStream().noneMatch(p -> p.matches(pathContainer))) { return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java` around lines 260 - 266, The custom securityMatcher in SecurityConfig uses request.getRequestURI() which includes the servlet context path and can cause matchPatterns/whitelistedPatterns checks to fail in non-root deployments; change the matcher to strip the context path before parsing (e.g., compute String path = request.getRequestURI().substring(request.getContextPath().length()) and then use PathContainer.parsePath(path)) and use that normalized PathContainer for both matchPatterns.parallelStream().noneMatch(...) and whitelistedPatterns.parallelStream().anyMatch(...), keeping the logic in the securityMatcher method intact.
🧹 Nitpick comments (1)
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java (1)
53-53: Log text is misleading for current behavior.Line 53 says “Rejecting request”, but this filter now proceeds with
chain.doFilter(...). Update wording to avoid false operational signals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java` at line 53, The debug message in QueryParameterJwtFilter currently states "Invalid token. Rejecting request." which is misleading because the filter continues with chain.doFilter(...); update the log.debug call in QueryParameterJwtFilter to accurately reflect behavior (e.g., "Invalid token; continuing filter chain without authentication" or similar) so logs do not signal a rejected request while the request is passed along.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java`:
- Around line 280-282: The rule requestMatchers(req -> true) currently matches
every request and prevents subsequent rules from running; in SecurityConfig move
the dispatcherTypeMatchers(DispatcherType.ASYNC).permitAll() rule before any
broad matchers and remove or replace requestMatchers(req -> true) (do not use an
always-true matcher); ensure you keep anyRequest().authenticated() as the
catch-all after the ASYNC permit so async dispatches are allowed while all other
requests remain authenticated.
---
Duplicate comments:
In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java`:
- Around line 260-266: The custom securityMatcher in SecurityConfig uses
request.getRequestURI() which includes the servlet context path and can cause
matchPatterns/whitelistedPatterns checks to fail in non-root deployments; change
the matcher to strip the context path before parsing (e.g., compute String path
= request.getRequestURI().substring(request.getContextPath().length()) and then
use PathContainer.parsePath(path)) and use that normalized PathContainer for
both matchPatterns.parallelStream().noneMatch(...) and
whitelistedPatterns.parallelStream().anyMatch(...), keeping the logic in the
securityMatcher method intact.
---
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java`:
- Line 53: The debug message in QueryParameterJwtFilter currently states
"Invalid token. Rejecting request." which is misleading because the filter
continues with chain.doFilter(...); update the log.debug call in
QueryParameterJwtFilter to accurately reflect behavior (e.g., "Invalid token;
continuing filter chain without authentication" or similar) so logs do not
signal a rejected request while the request is passed along.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b16660d-6890-490f-8553-16b978fab9e7
📒 Files selected for processing (10)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.javabooklore-api/src/main/java/org/booklore/config/security/filter/AudiobookStreamingJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/CoverJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/CustomFontJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/EpubStreamingJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/JwtAuthenticationFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.javabooklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java
💤 Files with no reviewable changes (4)
- booklore-api/src/main/java/org/booklore/config/security/filter/CustomFontJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/CoverJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/EpubStreamingJwtFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/AudiobookStreamingJwtFilter.java
🚧 Files skipped from review as they are similar to previous changes (4)
- booklore-api/src/main/java/org/booklore/config/security/filter/JwtAuthenticationFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/AuthenticationCheckFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/KoboAuthFilter.java
- booklore-api/src/main/java/org/booklore/config/security/filter/KoreaderAuthFilter.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
🧠 Learnings (3)
📚 Learning: 2026-03-25T21:02:57.527Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 195
File: booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java:15-19
Timestamp: 2026-03-25T21:02:57.527Z
Learning: In `booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java` and `booklore-api/src/main/java/org/booklore/config/logging/filter/RequestLoggingFilter.java`, logging all request headers and payloads at DEBUG level is intentional and accepted behavior in this project. The previous `LoggingFilter` also logged all headers. No header redaction or payload scrubbing is required — the DEBUG log level is considered sufficient access control. The new filter is not profile-gated (unlike the old `Profile("dev")` filter) but relies on `logger.isDebugEnabled()` for gating.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
📚 Learning: 2026-03-27T15:06:25.913Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:151-153
Timestamp: 2026-03-27T15:06:25.913Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, the try/catch around `koboServerProxy.proxyCurrentRequest()` in `syncLibrary()` is intentionally scoped only to handle a missing/failed proxy response (null). Once a response is successfully obtained, its body and headers (including the upstream sync token decoded via `tokenGenerator.fromBase64()`) are processed without additional defensive wrapping. This is a deliberate design decision: other parts of the response are equally unguarded, and the goal is to handle proxy unavailability gracefully — not to catch every possible downstream processing error.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.java
📚 Learning: 2026-03-25T19:09:09.638Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 189
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:113-116
Timestamp: 2026-03-25T19:09:09.638Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, when deciding whether to forward the `Authorization` header to the upstream Kobo server proxy on the `/v1/library/sync` endpoint, a simple null check (`request.getHeader(HttpHeaders.AUTHORIZATION) != null`) is the correct guard. The header is either absent or present — if present it may or may not be a valid token, but validity is determined by the upstream Kobo server, not by Grimmory. Blank/whitespace intermediate states do not occur in practice, and pre-validating the token value before forwarding is not the responsibility of this service.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/filter/QueryParameterJwtFilter.javabooklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
[::grimmory-tools/grimmory-docs::]
- docs/integration/opds.md — multiple references to
/api/v1/opds,/komga/api, OPDS endpoints and examples (lines shown in search results: ~15, ~31–32, ~113–122, ~138–145, ~158–159). These document OPDS/Komga endpoints whose authentication/filtering behaviour was modified by the PR. - docs/integration/komga-api.md — documents Komga base
/komgaand/komga/api/v1/*endpoints and examples (search hits around lines ~9, ~38–39, ~121, ~236–257). Komga clients rely on these routes and may be affected by jwtApiSecurityChain matcher changes and ws/static resource ordering. - docs/integration/kobo.md — shows examples pointing Kobo to
/api/kobo/(search hits ~3, ~77, ~81, ~86). Kobo integration directly uses the /api/kobo path that PR changes touch (KoboAuthFilter registration/behavior). - docs/integration/opds.md & docs/integration/komga-api.md — mention OPDS endpoints
/api/v1/opdsand Komga file/stream endpoints (/komga/api/v1/books/{id}/file, thumbnails, per-page streaming) relevant to query-parameter token handling removed/centralized into QueryParameterJwtFilter. - docs/authentication/* (overview, oidc-settings) — reference endpoints such as
/api/v1/auth/refreshand other auth flows included in the PR’s whitelist/uri-matching changes (search hits present).
Summary: user-facing docs in this repo reference the exact API paths and client integrations (/komga, /api/v1/opds, /api/kobo, OPDS/Komga file streaming, and authentication endpoints) whose routing/filter and authentication behavior the PR refactors. These docs should be reviewed for consistency with the new filter registration, jwtApiSecurityChain matcher changes, and the introduction of wsSecurityChain and AuthenticationCheckFilter.
🔇 Additional comments (1)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java (1)
219-235: WebSocket HTTP chain split looks good.Keeping
/ws/**permit-all at HTTP handshake level is consistent when auth is enforced in the WebSocket interceptor layer.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java (1)
260-262:⚠️ Potential issue | 🟠 MajorContext-path-sensitive matcher can skip JWT chain for protected routes.
At Line 261, matching is done against
request.getRequestURI(), which includes the servlet context path. With a non-root context path (e.g./booklore), patterns like/api/**and/komga/**won’t match as intended, and secured requests may bypass this chain.Proposed fix
- var pathContainer = PathContainer.parsePath(request.getRequestURI()); + var requestUri = request.getRequestURI(); + var contextPath = request.getContextPath(); + var normalizedPath = (contextPath != null && !contextPath.isEmpty() && requestUri.startsWith(contextPath)) + ? requestUri.substring(contextPath.length()) + : requestUri; + var pathContainer = PathContainer.parsePath(normalizedPath);#!/bin/bash # Verify whether a servlet context path is configured and confirm matcher implementation. rg -n --iglob 'application*.yml' --iglob 'application*.properties' \ 'server\.servlet\.context-path|spring\.mvc\.servlet\.path' rg -n -C3 'securityMatcher\(request ->|getRequestURI\(\)|PathContainer\.parsePath' \ booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.javaAlso applies to: 265-266
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java` around lines 260 - 262, The matcher uses request.getRequestURI() which includes the servlet context path and can make matchPatterns (checked in the securityMatcher lambda using PathContainer.parsePath) miss protected routes; update the securityMatcher implementation to strip the servlet context path before parsing (e.g., compute String raw = request.getRequestURI(); String relative = raw.substring(request.getContextPath().length()); then PathContainer.parsePath(relative)) so matchPatterns.parallelStream().noneMatch(...) runs against the path relative to the servlet context and correctly matches routes like /api/** and /komga/**.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java`:
- Around line 260-262: The matcher uses request.getRequestURI() which includes
the servlet context path and can make matchPatterns (checked in the
securityMatcher lambda using PathContainer.parsePath) miss protected routes;
update the securityMatcher implementation to strip the servlet context path
before parsing (e.g., compute String raw = request.getRequestURI(); String
relative = raw.substring(request.getContextPath().length()); then
PathContainer.parsePath(relative)) so
matchPatterns.parallelStream().noneMatch(...) runs against the path relative to
the servlet context and correctly matches routes like /api/** and /komga/**.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c76290a-325b-489f-9ae7-6d91768d63f0
📒 Files selected for processing (1)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
🧠 Learnings (2)
📚 Learning: 2026-03-25T21:02:57.527Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 195
File: booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java:15-19
Timestamp: 2026-03-25T21:02:57.527Z
Learning: In `booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java` and `booklore-api/src/main/java/org/booklore/config/logging/filter/RequestLoggingFilter.java`, logging all request headers and payloads at DEBUG level is intentional and accepted behavior in this project. The previous `LoggingFilter` also logged all headers. No header redaction or payload scrubbing is required — the DEBUG log level is considered sufficient access control. The new filter is not profile-gated (unlike the old `Profile("dev")` filter) but relies on `logger.isDebugEnabled()` for gating.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
📚 Learning: 2026-03-25T19:09:09.638Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 189
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:113-116
Timestamp: 2026-03-25T19:09:09.638Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, when deciding whether to forward the `Authorization` header to the upstream Kobo server proxy on the `/v1/library/sync` endpoint, a simple null check (`request.getHeader(HttpHeaders.AUTHORIZATION) != null`) is the correct guard. The header is either absent or present — if present it may or may not be a valid token, but validity is determined by the upstream Kobo server, not by Grimmory. Blank/whitespace intermediate states do not occur in practice, and pre-validating the token value before forwarding is not the responsibility of this service.
Applied to files:
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java
🔀 Multi-repo context grimmory-tools/grimmory-docs
Linked repositories findings
grimmory-tools/grimmory-docs
-
docs/integration/opds.md — documents OPDS endpoints and usage of
/api/v1/opdsand references Komga endpoints such as/komga/api/v1/*(multiple GET endpoints including/komga/api/v1/books/{id}/file, thumbnails, per-page streaming). These are directly affected by jwtApiSecurityChain matcher and query-parameter token handling changes. [::grimmory-tools/grimmory-docs::docs/integration/opds.md] -
docs/integration/komga-api.md — documents Komga base
/komgaand/komga/api/v1/*endpoints (e.g.,/komga/api/v1/books/{bookId}/file,/komga/api/v1/books/{id}/pages/{num}) which rely on streaming/cover/file endpoints that previously used per-route JWT filters and are now routed through QueryParameterJwtFilter + JwtAuthenticationFilter. [::grimmory-tools/grimmory-docs::docs/integration/komga-api.md] -
docs/integration/kobo.md — shows examples and instructions pointing Kobo to
/api/kobo/{token}and describes Kobo sync behaviour. Changes to KoboAuthFilter/filter registration and removal of path gating may affect this integration; docs reference the exact/api/kobo/path and token usage. [::grimmory-tools/grimmory-docs::docs/integration/kobo.md] -
docs/tools/custom-fonts.md — documents custom-fonts UI and storage; PR extends custom-font authentication to accept Bearer headers in addition to URL parameters, so this doc area should be reviewed for any guidance on how clients should supply tokens for
/api/v1/custom-fonts/*. [::grimmory-tools/grimmory-docs::docs/tools/custom-fonts.md] -
docs/readers/epub-reader.md and docs/readers/audiobook-player.md — reader docs reference EPUB/audiobook streaming functionality; streaming endpoints
/api/v1/epub/...,/api/v1/audiobooks/...and related protections were changed (removal of specialized streaming filters and consolidation under QueryParameterJwtFilter/JwtAuthenticationFilter). Verify examples and client instructions remain accurate. [::grimmory-tools/grimmory-docs::docs/readers/epub-reader.md] [::grimmory-tools/grimmory-docs::docs/readers/audiobook-player.md] -
docs/tools/devices.md & docs/getting-started.md — include references to device integrations (KOReader, Kobo) and OPDS base paths; relevant due to ws/static ordering and jwtApiSecurityChain matcher adjustments that might affect client routing. [::grimmory-tools/grimmory-docs::docs/tools/devices.md] [::grimmory-tools/grimmory-docs::docs/getting-started.md]
Notes on findings quality: searches produced many matches in the docs repo that directly reference the exact API paths and integrations touched by the PR (Komga, OPDS, Kobo, custom-fonts, epub/audiobook streaming). These are consumer-facing docs that should be checked for consistency with the new security/filter behavior (token sources, public endpoint whitelist, and websocket /ws handling). No other code repos were available to search.
🔇 Additional comments (1)
booklore-api/src/main/java/org/booklore/config/security/SecurityConfig.java (1)
219-235: Nice separation for/ws/**traffic.Dedicated
wsSecurityChainat a higher priority keeps websocket routing isolated from JWT API matching and reduces accidental filter overlap.
|
This would also fix #405 |
|
This PR should not be merge until issue #378 is resolved. Reason: crash looping on route ‘/login’ |
…outes (grimmory-tools#361) * see pr message * fix: + use PathPattern for request matching + include common public route to whitelist + logging for failed queryjwt * forgot to remove debug code
…outes (grimmory-tools#361) * see pr message * fix: + use PathPattern for request matching + include common public route to whitelist + logging for failed queryjwt * forgot to remove debug code
…outes (#361) * see pr message * fix: + use PathPattern for request matching + include common public route to whitelist + logging for failed queryjwt * forgot to remove debug code
…outes (#361) * see pr message * fix: + use PathPattern for request matching + include common public route to whitelist + logging for failed queryjwt * forgot to remove debug code
…outes (grimmory-tools#361) * see pr message * fix: + use PathPattern for request matching + include common public route to whitelist + logging for failed queryjwt * forgot to remove debug code
Description
Formerly, all filters are registered across all routes regardless of whether they were added via
securityMatcher(...).addFilter()or not, which is unintuitive and requires repetitive checks. This PR aims to remove this behavior.It's done by explicitly creating a
@FilterRegistrationbean for each filter, allowing fine-grained control over init parameters. Then, disabling the automatic behavior by settingenabledinitial parameter tofalse.Changes
shouldNotFilter) on filtersAudiobookStreamingJwtFilter,CoverJwtFilter, andEpubStreamingJwtFilterwith the combination ofQueryParameterJwtFilterandJwtAuthenticationFIlter, which serves the same functionalitiesJwtAuthenticationFIlter,QueryParameterJwtFilter---AuthenticationCheckFilter)Other refactors:
KoreaderAuthFilterSummary by CodeRabbit
New Features
Security Updates
Behavior Changes