Skip to content

Refactor FormAuthenticator to remove code duplication #942

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.

Sign up for GitHub

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

Merged
dsoumis merged 1 commit into from
Jan 19, 2026
Merged

Refactor FormAuthenticator to remove code duplication #942

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions java/org/apache/catalina/authenticator/FormAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,7 @@ protected boolean isContinuationRequired(Request request) {
// a resource is protected for some HTTP methods but not protected for
// GET which is used after authentication when redirecting to the
// protected resource.
// TODO: This is similar to the FormAuthenticator.matchRequest() logic
// Is there a way to remove the duplication?
Session session = request.getSessionInternal(false);
if (session != null) {
SavedRequest savedRequest = (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
return savedRequest != null && decodedRequestURI.equals(savedRequest.getDecodedRequestURI());
}

return false;
return matchRequest(request, false);
}


Expand Down Expand Up @@ -499,15 +491,21 @@ protected void forwardToErrorPage(Request request, HttpServletResponse response,
}


protected boolean matchRequest(Request request) {
return matchRequest(request, true);
}

/**
* Does this request match the saved one (so that it must be the redirect we signaled after successful
* authentication?)
*
* @param request The request to be verified
* @param strict <code>true</code> to check for a valid Principal and valid Session ID, <code>false</code> to only
* check for a valid saved request and matching URI
*
* @return <code>true</code> if the requests matched the saved one
*/
protected boolean matchRequest(Request request) {
protected boolean matchRequest(Request request, boolean strict) {
// Has a session been created?
Session session = request.getSessionInternal(false);
if (session == null) {
Expand All @@ -520,17 +518,19 @@ protected boolean matchRequest(Request request) {
return false;
}

// Is there a saved principal?
if (cache && session.getPrincipal() == null || !cache && request.getPrincipal() == null) {
return false;
}

// Does session id match?
if (getChangeSessionIdOnAuthentication()) {
String expectedSessionId = (String) session.getNote(Constants.SESSION_ID_NOTE);
if (expectedSessionId == null || !expectedSessionId.equals(request.getRequestedSessionId())) {
if (strict) {
// Is there a saved principal?
if (cache && session.getPrincipal() == null || !cache && request.getPrincipal() == null) {
return false;
}

// Does session id match?
if (getChangeSessionIdOnAuthentication()) {
String expectedSessionId = (String) session.getNote(Constants.SESSION_ID_NOTE);
if (expectedSessionId == null || !expectedSessionId.equals(request.getRequestedSessionId())) {
return false;
}
}
}

// Does the request URI match?
Expand Down