Skip to content

Commit

Permalink
fix: authentication Filter should not process OPTIONS request (#5525)
Browse files Browse the repository at this point in the history
method #5524
  • Loading branch information
yurem authored Jul 12, 2023
1 parent e099649 commit aed5e4f
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,9 @@ public class AppConfiguration implements Configuration {
@DocProperty(description = "Defines if Response body will be logged. Default value is false", defaultValue = "false")
private Boolean httpLoggingResponseBodyContent = false;

@DocProperty(description = "Force Authentication Filtker to process OPTIONS request", defaultValue = "true")
private Boolean skipAuthenticationFilterOptionsMethod = true;

public Map<String, String> getDateFormatterPatterns() {
return dateFormatterPatterns;
}
Expand Down Expand Up @@ -3219,4 +3222,13 @@ public Boolean getHttpLoggingResponseBodyContent() {
public void setHttpLoggingResponseBodyContent(Boolean httpLoggingResponseBodyContent) {
this.httpLoggingResponseBodyContent = httpLoggingResponseBodyContent;
}

public Boolean isSkipAuthenticationFilterOptionsMethod() {
return skipAuthenticationFilterOptionsMethod;
}

public void setSkipAuthenticationFilterOptionsMethod(Boolean skipAuthenticationFilterOptionsMethod) {
this.skipAuthenticationFilterOptionsMethod = skipAuthenticationFilterOptionsMethod;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
final String requestUrl = httpRequest.getRequestURL().toString();
log.trace("Get request to: '{}'", requestUrl);

final String method = httpRequest.getMethod();
if (appConfiguration.isSkipAuthenticationFilterOptionsMethod() && "OPTIONS".equals(method)) {
log.trace("Ignoring '{}' request to to: '{}'", method, requestUrl);
filterChain.doFilter(httpRequest, httpResponse);
return;
}

boolean tokenEndpoint = requestUrl.endsWith("/token");
boolean tokenRevocationEndpoint = requestUrl.endsWith("/revoke");
boolean backchannelAuthenticationEnpoint = requestUrl.endsWith("/bc-authorize");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,11 @@ public void doFilter(final ServletRequest servletRequest,
// Determines the CORS request type.
AbstractCorsFilter.CORSRequestType requestType = checkRequestType(request);

dumpRequestDetails("before doFilter", request, requestType);

// Adds CORS specific attributes to request.
if (decorateRequest) {
AbstractCorsFilter.decorateCORSProperties(request, requestType);
decorateCORSProperties(request, requestType);
}
switch (requestType) {
case SIMPLE:
Expand All @@ -125,6 +127,8 @@ public void doFilter(final ServletRequest servletRequest,
this.handleInvalidCORS(request, response, filterChain);
break;
}

dumpRequestDetails("after doFilter", request, requestType);
}

@Override
Expand Down Expand Up @@ -154,18 +158,21 @@ protected void handleSimpleCORS(final HttpServletRequest request,
AbstractCorsFilter.CORSRequestType.SIMPLE,
AbstractCorsFilter.CORSRequestType.ACTUAL));
}
dumpRequestDetails("before handleSimpleCORS", request, requestType);

final String origin = request
.getHeader(AbstractCorsFilter.REQUEST_HEADER_ORIGIN);
final String method = request.getMethod();

// Section 6.1.2
if (!isOriginAllowed(request, origin)) {
log.trace("handleSimpleCORS: handleInvalidCORS");
handleInvalidCORS(request, response, filterChain);
return;
}

if (!allowedHttpMethods.contains(method)) {
log.trace("handleSimpleCORS: handleInvalidCORS");
handleInvalidCORS(request, response, filterChain);
return;
}
Expand Down Expand Up @@ -209,6 +216,8 @@ protected void handleSimpleCORS(final HttpServletRequest request,
exposedHeadersString);
}

dumpRequestDetails("after handleSimpleCORS", request, requestType);

// Forward the request down the filter chain.
filterChain.doFilter(request, response);
}
Expand All @@ -234,6 +243,8 @@ protected void handlePreflightCORS(final HttpServletRequest request,
CORSRequestType.PRE_FLIGHT.name().toLowerCase()));
}

dumpRequestDetails("before handlePreflightCORS", request, requestType);

final String origin = request
.getHeader(AbstractCorsFilter.REQUEST_HEADER_ORIGIN);

Expand Down Expand Up @@ -322,6 +333,8 @@ protected void handlePreflightCORS(final HttpServletRequest request,
join(allowedHttpHeaders, ","));
}

dumpRequestDetails("after handlePreflightCORS", request, requestType);

// Do not forward the request down the filter chain.
}

Expand Down Expand Up @@ -363,7 +376,7 @@ private void handleInvalidCORS(final HttpServletRequest request,
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
response.resetBuffer();

if (log.isDebugEnabled()) {
if (log.isDebugEnabled() || log.isTraceEnabled()) {
// Debug so no need for i18n
StringBuilder message =
new StringBuilder("Invalid CORS request; Origin=");
Expand Down Expand Up @@ -402,7 +415,7 @@ public void destroy() {
* @param request The {@link HttpServletRequest} object.
* @param corsRequestType The {@link CORSRequestType} object.
*/
protected static void decorateCORSProperties(
protected void decorateCORSProperties(
final HttpServletRequest request,
final CORSRequestType corsRequestType) {
if (request == null) {
Expand All @@ -415,6 +428,8 @@ protected static void decorateCORSProperties(
SM.getString("corsFilter.nullRequestType"));
}

dumpRequestDetails("before decorateCORSProperties", request, corsRequestType);

switch (corsRequestType) {
case SIMPLE:
request.setAttribute(
Expand Down Expand Up @@ -462,8 +477,9 @@ protected static void decorateCORSProperties(
// Don't set any attributes
break;
}
}

dumpRequestDetails("after decorateCORSProperties", request, corsRequestType);
}

/**
* Joins elements of {@link Set} into a string, where each element is
Expand Down Expand Up @@ -739,6 +755,9 @@ public boolean isAnyOriginAllowed(ServletRequest servletRequest) {
}

protected void setContextClientAllowedOrigins(ServletRequest servletRequest, Collection<String> clientAllowedOrigins) {
if (log.isTraceEnabled()) {
log.trace("setContextClientAllowedOrigins: {}", clientAllowedOrigins);
}
servletRequest.setAttribute(PARAM_CLIENT_ALLOWED_ORIGINS, clientAllowedOrigins);
}

Expand All @@ -759,6 +778,33 @@ protected boolean hasContextClientAllowedOrigins(ServletRequest servletRequest)
return false;
}

private void dumpRequestDetails(final String prefix, final HttpServletRequest request, final CORSRequestType corsRequestType) {
if (!log.isTraceEnabled()) {
return;
}
StringBuilder allAttributes = new StringBuilder("[");
for (Iterator<String> it = request.getAttributeNames().asIterator(); it.hasNext();) {
if (allAttributes.length() > 1) {
allAttributes.append(",");
}
String attributeName = (String) it.next();
allAttributes.append(attributeName).append(" = ").append(request.getAttribute(attributeName));
}
allAttributes.append("]");

StringBuilder allHeaders = new StringBuilder("[");
for (Iterator<String> it = request.getHeaderNames().asIterator(); it.hasNext();) {
if (allHeaders.length() > 1) {
allHeaders.append(",");
}
String HeaderName = (String) it.next();
allHeaders.append(HeaderName).append(" = ").append(request.getHeader(HeaderName));
}
allHeaders.append("]");

log.trace("{}: request method {} to URI {}, corsType {}, attributes {}, headers {}", prefix, request.getMethod(), request.getRequestURI(), corsRequestType, allAttributes, allHeaders);
}

/**
* Returns a {@link Set} of headers that should be exposed by browser.
*/
Expand Down

0 comments on commit aed5e4f

Please sign in to comment.