fix(security): complete the default-deny whitelist for anonymous endpoints - #347
Merged
hirokiterashima merged 1 commit intoSep 9, 2026
Conversation
…oints The Spring Boot 3 upgrade (WISE-Community#311) added anyRequest().authenticated() but the permitAll whitelist was incomplete. This re-audits the security configuration against every unguarded frontend route and fills the gaps. Static asset paths served by WebConfig resource handlers (portal scripts, themes, translations, the VLE engine, project icons) were not excluded from the security filter chain, so the login page could not load its own JS, CSS or translations for anonymous visitors. Password recovery for teachers was actively blocked: /api/teacher/forgot/** matched the /api/teacher/** role rule (requires TEACHER) before reaching the terminal rule, making recovery impossible without the role the user is trying to recover. Student forgot paths fell through to authenticated. Both are now permitted before the role rules. Remaining anonymous endpoints are now explicitly permitted: the contact form, news, announcements (called on every page load by the root component), project info (used by the contact and library pages), Google registration checks, project preview, survey entry, and framework error dispatch paths. Remove @EnableWebSecurity(debug = true) which logs request headers and session cookies on every request. Gate DebugController behind @Profile(dev) and remove its permitAll rule so it is not registered or reachable in production. Remove the duplicate /api/user/info permitAll.
hirokiterashima
self-requested a review
September 9, 2026 16:13
hirokiterashima
approved these changes
Sep 9, 2026
hirokiterashima
left a comment
Member
There was a problem hiding this comment.
LGTM. Thanks for catching and fixing the endpoints that were omitted!
I agree that the existing WebSecurityConfigAuthorizationTest should be extended to cover the newly permitted endpoints to prevent regression in the future.
I also noticed some legacy endpoints that are no longer in use, so I'll be removing them in the coming months.
Member
|
🎉 This PR is included in version 1.25.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Following @hirokiterashima's response on #326 noting that the Spring Boot 3 upgrade (#311) should cover the default-deny changes, I re-audited the security configuration against the frontend routing to verify the coverage. The audit cross-referenced every unguarded frontend route with the backend
authorizeHttpRequestsrules and found several gaps where anonymous endpoints are now blocked by theanyRequest().authenticated()terminal rule (or, in the case of teacher password recovery, by the/api/teacher/**role rule that precedes it).This PR completes the whitelist so that the default-deny rule does not break anonymous flows.
Changes
1. Permit static asset paths served by WebConfig resource handlers
The resource handler paths (
/pages/resources/**,/portal/javascript/**,/portal/themes/**,/portal/translate/**,/vle/**,/projectIcons/**) are not excluded from the security filter chain. Without an explicitpermitAll(), they fall through toanyRequest().authenticated()and return 302 redirects to/loginfor anonymous visitors -- which means the login page itself cannot load its scripts, styles, or translations./studentuploads/**is intentionally left out so student files continue to require authentication./curriculum/**was already permitted.2. Permit password recovery endpoints before the role rules
/api/teacher/forgot/**is matched by the existing/api/teacher/**rule, which requires the TEACHER role. A teacher who forgot their password cannot recover it because recovery requires the role they are trying to log back in to use. This PR places the forgot-passwordpermitAll()rules for both student and teacher before the/api/teacher/**role rule so that first-match semantics resolve correctly.3. Permit remaining anonymous endpoints
The following endpoints are called by unguarded frontend routes but were not in the whitelist:
POST /api/contact/contact(public contact form)GET /api/news/**/newsand home pageGET /api/announcementGET /api/project/info/*/contact?projectId=and library detailGET /api/google-user/check-user-exists/join/*(Google registration)GET /api/google-user/check-user-matches/join/*(Google registration)GET /previewproject.htmlGET /run-survey/**GET /error,/errors/**,/favicon.ico4. Remove security debug mode
@EnableWebSecurity(debug = true)logs the full filter chain invocation for every request, including headers and session cookies. Changed to@EnableWebSecurity.DebugControllerwas publicly accessible (/api/debug/**permitAll) with no@Profilegate, exposingWebAuthenticationDetails(which includes the session ID) to any authenticated caller. Added@Profile(dev)and removed the permitAll rule.5. Cleanup
/api/user/infopermitAll (it appeared in both arequestMatchers(String...)group and a separateAntPathRequestMatcherrule).What is NOT changed
anyRequest().authenticated()terminal rule itself (already correct)./api/j_acegi_security_checkbyWISEAuthenticationProcessingFilter,/api/logoutbyLogoutFilter) do not need permitAll rules and are not added.Notes for future consideration
Two of the newly permitted endpoints have pre-existing design concerns unrelated to the default-deny change (they were already publicly reachable before #311):
/api/project/info/*:ProjectAPIController.getProjectInfo()returns the owner's and shared owners'username,firstName, andlastNamewith no@Securedannotation. Now that this is an explicitpermitAll, it may be worth adding a restricted-field projection or requiring authentication for the owner details./run-survey/**:SurveyAPIControllercreates a realUserrecord on every GET request with no rate limiting or CAPTCHA. The only guard is a 1000-workgroup cap per run. This is by design for the survey feature but could be abused for mass account creation.Both are out of scope for this PR since they are existing behavior, not regressions.
Testing
./mvnw package).WebSecurityConfigAuthorizationTestshould be extended to cover the newly permitted endpoints. I can add those tests in a follow-up if preferred.