Skip to content
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.

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

Allow for OPTIONS requests to be passed through auth filters #1244

Merged
11 changes: 11 additions & 0 deletions src/server/src/main/docker/shiro.ini
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@
# by default it is not used. see cassandra-reaper.yaml

[main]
sessionManager = org.apache.shiro.web.session.mgt.DefaultWebSessionManager
sessionManager.sessionIdCookieEnabled = true
sessionManager.sessionIdCookie.secure = true
sessionManager.sessionIdCookie.sameSite = NONE
securityManager.sessionManager = $sessionManager

rememberMeManager = org.apache.shiro.web.mgt.CookieRememberMeManager
rememberMeManager.cookie.secure = true
rememberMeManager.cookie.sameSite = NONE
securityManager.rememberMeManager = $rememberMeManager

authc = org.apache.shiro.web.filter.authc.PassThruAuthenticationFilter
authc.loginUrl = /webui/login.html

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public void run(ReaperApplicationConfiguration config, Environment environment)
if (config.isEnableCrossOrigin() || System.getProperty("enableCrossOrigin") != null) {
FilterRegistration.Dynamic co = environment.servlets().addFilter("crossOriginRequests", CrossOriginFilter.class);
co.setInitParameter("allowedOrigins", "*");
co.setInitParameter("allowedHeaders", "X-Requested-With,Content-Type,Accept,Origin");
co.setInitParameter("allowedHeaders", "X-Requested-With,Content-Type,Accept,Origin,Authorization");
co.setInitParameter("allowedMethods", "OPTIONS,GET,PUT,POST,DELETE,HEAD,PATCH");
co.addMappingForUrlPatterns(EnumSet.allOf(DispatcherType.class), true, "/*");
}
Expand All @@ -205,7 +205,6 @@ public void run(ReaperApplicationConfiguration config, Environment environment)
environment.jersey().register(pingResource);

final ClusterResource addClusterResource = ClusterResource.create(context, cryptograph);

environment.jersey().register(addClusterResource);
final RepairRunResource addRepairRunResource = new RepairRunResource(context);
environment.jersey().register(addRepairRunResource);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
*
* Copyright 2022-2022 The Last Pickle Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.cassandrareaper.resources;

import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.HttpMethod;

import com.google.common.annotations.VisibleForTesting;

public final class RequestUtils {
public static final String ENABLE_CORS_ENV_VAR_NAME = "REAPER_ENABLE_CROSS_ORIGIN";

private RequestUtils() {}

public static boolean isOptionsRequest(ServletRequest request) {
if (request != null && request instanceof HttpServletRequest) {
if (((HttpServletRequest) request).getMethod().equalsIgnoreCase(HttpMethod.OPTIONS)) {
return true;
}
}
return false;
}

@VisibleForTesting
static boolean isCorsEnabled(String corsEnabledEnvVarValue) {
if (corsEnabledEnvVarValue != null) {
return Boolean.parseBoolean(corsEnabledEnvVarValue.trim().toLowerCase());
}
return false;
}

public static boolean isCorsEnabled() {
String corsEnabledEnvVarValue = System.getenv(ENABLE_CORS_ENV_VAR_NAME);
return isCorsEnabled(corsEnabledEnvVarValue);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing this and thinking about another requirement we have (which is to shut down the sessions despite being active after the configured timeout), I'm of the opinion that we shouldn't rely on env variable.
We can statically set a isCorsEnabled variable in RequestUtils which will be initialized in the ReaperApplication.java initialization phase, and feed on the corresponding config setting: config.isEnableCrossOrigin()

Suggested change
public final class RequestUtils {
public static final String ENABLE_CORS_ENV_VAR_NAME = "REAPER_ENABLE_CROSS_ORIGIN";
private RequestUtils() {}
public static boolean isOptionsRequest(ServletRequest request) {
if (request != null && request instanceof HttpServletRequest) {
if (((HttpServletRequest) request).getMethod().equalsIgnoreCase(HttpMethod.OPTIONS)) {
return true;
}
}
return false;
}
@VisibleForTesting
static boolean isCorsEnabled(String corsEnabledEnvVarValue) {
if (corsEnabledEnvVarValue != null) {
return Boolean.parseBoolean(corsEnabledEnvVarValue.trim().toLowerCase());
}
return false;
}
public static boolean isCorsEnabled() {
String corsEnabledEnvVarValue = System.getenv(ENABLE_CORS_ENV_VAR_NAME);
return isCorsEnabled(corsEnabledEnvVarValue);
}
}
public final class RequestUtils {
private static boolean isCorsEnabled = false;
private RequestUtils() {}
public static void setCorsEnabled(boolean enabled) {
isCorsEnabled = enabled;
}
public static boolean isCorsEnabled() {
return isCorsEnabled;
}
public static boolean isOptionsRequest(ServletRequest request) {
if (request != null && request instanceof HttpServletRequest) {
if (((HttpServletRequest) request).getMethod().equalsIgnoreCase(HttpMethod.OPTIONS)) {
return true;
}
}
return false;
}
}

Initialization can be done as follows in ReaperApplication.java:

 if (config.isAccessControlEnabled()) {
      RequestUtils.setCorsEnabled(config.isEnableCrossOrigin());
      SessionHandler sessionHandler = new SessionHandler();
      sessionHandler.setMaxInactiveInterval((int) config.getAccessControl().getSessionTimeout().getSeconds());
      environment.getApplicationContext().setSessionHandler(sessionHandler);
      environment.servlets().setSessionHandler(sessionHandler);
      environment.jersey().register(new ShiroExceptionMapper());
      environment.jersey().register(new LoginResource());
      environment.jersey().register(new ShiroJwtProvider(context));
    }

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so the latest commit switches to initializing with config property instead of directly using the env variable... duh, I should have seen that to start 😔 so thanks for that suggestion!

But, I wasn't sure why you suggested tangling up the enabling of CORS within the isAccessControlEnabled() block as you mention here?

I initialized things in place with the other CORS related configuration that already happened within ReaperApplication.java's startup code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, I wasn't sure why you suggested tangling up the enabling of CORS within the isAccessControlEnabled() block as you mention here?

I was under the impression that we needed it to allow OPTIONS calls when auth is enabled. Isn't it what you were trying to do here?
Either way, I'm fine with initializing it outside of that block 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, the two can be unrelated.

Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,39 @@

package io.cassandrareaper.resources.auth;

import io.cassandrareaper.resources.RequestUtils;

import java.io.IOException;

import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;

import com.google.common.annotations.VisibleForTesting;
import org.apache.shiro.subject.Subject;
import org.apache.shiro.web.filter.authz.HttpMethodPermissionFilter;
import org.apache.shiro.web.util.WebUtils;

public final class RestPermissionsFilter extends HttpMethodPermissionFilter {
private final boolean isCorsEnabled;

public RestPermissionsFilter() {
isCorsEnabled = RequestUtils.isCorsEnabled();
}

public RestPermissionsFilter() {}
@VisibleForTesting
boolean isCorsEnabled() {
return isCorsEnabled;
}

@Override
public boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue)
throws IOException {
if (isCorsEnabled() && RequestUtils.isOptionsRequest(request)) {
return true;
}
return super.isAccessAllowed(request, response, mappedValue);
}

@Override
protected Subject getSubject(ServletRequest request, ServletResponse response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,51 @@

package io.cassandrareaper.resources.auth;

import io.cassandrareaper.resources.RequestUtils;

import java.util.Optional;

import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;

import com.google.common.annotations.VisibleForTesting;
import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jws;
import io.jsonwebtoken.JwtException;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.lang.Strings;

import org.apache.shiro.subject.SimplePrincipalCollection;
import org.apache.shiro.subject.Subject;
import org.apache.shiro.web.filter.AccessControlFilter;
import org.apache.shiro.web.subject.WebSubject;
import org.apache.shiro.web.util.WebUtils;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class ShiroJwtVerifyingFilter extends AccessControlFilter {

private static final Logger LOG = LoggerFactory.getLogger(ShiroJwtVerifyingFilter.class);

public ShiroJwtVerifyingFilter() {}
private final boolean isCorsEnabled;

public ShiroJwtVerifyingFilter() {
isCorsEnabled = RequestUtils.isCorsEnabled();
}

@VisibleForTesting
boolean isCorsEnabled() {
return isCorsEnabled;
}

@Override
protected boolean isAccessAllowed(ServletRequest req, ServletResponse res, Object mappedValue) throws Exception {
if (isCorsEnabled() && RequestUtils.isOptionsRequest(req)) {
return true;
}

Subject nonJwt = getSubject(req, res);

return null != nonJwt.getPrincipal() && (nonJwt.isRemembered() || nonJwt.isAuthenticated())
Expand Down
11 changes: 11 additions & 0 deletions src/server/src/main/resources/shiro.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@
# limitations under the License.

[main]
sessionManager = org.apache.shiro.web.session.mgt.DefaultWebSessionManager
sessionManager.sessionIdCookieEnabled = true
sessionManager.sessionIdCookie.secure = true
sessionManager.sessionIdCookie.sameSite = NONE
securityManager.sessionManager = $sessionManager

rememberMeManager = org.apache.shiro.web.mgt.CookieRememberMeManager
rememberMeManager.cookie.secure = true
rememberMeManager.cookie.sameSite = NONE
securityManager.rememberMeManager = $rememberMeManager

authc = org.apache.shiro.web.filter.authc.PassThruAuthenticationFilter
authc.loginUrl = /webui/login.html

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
*
* Copyright 2019-2019 The Last Pickle Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.cassandrareaper.resources;

import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.HttpMethod;

import org.assertj.core.api.Assertions;
import org.junit.Test;

import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

public class RequestUtilsTest {
@Test
public void testIsCorsEnabledFromEnvironmentWithEmptyEnvironmentReturnsFalse() {
boolean allowAll = RequestUtils.isCorsEnabled();
Assertions.assertThat(allowAll).isFalse();
}

@Test
public void testIsCorsEnabledFromEnvironmentWithTrueEnvironmentReturnsTrue() {
boolean allowAll = RequestUtils.isCorsEnabled("true");
Assertions.assertThat(allowAll).isTrue();
}

@Test
public void testIsCorsEnabledFromEnvironmentWithFalseEnvironmentReturnsFalse() {
boolean allowAll = RequestUtils.isCorsEnabled("false");
Assertions.assertThat(allowAll).isFalse();
}

@Test
public void testIsCorsEnabledFromInvalidEnvironmentWithEnvironmentReturnsFalse() {
boolean allowAll = RequestUtils.isCorsEnabled("bad");
Assertions.assertThat(allowAll).isFalse();
}

@Test
public void testIsOptionsRequestInvalidInputReturnsFalse() {
boolean isOptionsRequest = RequestUtils.isOptionsRequest(null);
Assertions.assertThat(isOptionsRequest).isFalse();
}

@Test
public void testIsOptionsRequestOptionsServletInputReturnsTrue() {
HttpServletRequest mockServletRequest = spy(HttpServletRequest.class);
when(mockServletRequest.getMethod()).thenReturn(HttpMethod.OPTIONS);
boolean isOptionsRequest = RequestUtils.isOptionsRequest(mockServletRequest);
Assertions.assertThat(isOptionsRequest).isTrue();
}

@Test
public void testIsOptionsRequestGetServletInputReturnsTrue() {
HttpServletRequest mockServletRequest = spy(HttpServletRequest.class);
when(mockServletRequest.getMethod()).thenReturn(HttpMethod.GET);
boolean isOptionsRequest = RequestUtils.isOptionsRequest(mockServletRequest);
Assertions.assertThat(isOptionsRequest).isFalse();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
*
* Copyright 2022-2022 The Last Pickle Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.cassandrareaper.resources.auth;

import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.HttpMethod;

import org.assertj.core.api.Assertions;
import org.junit.Test;
import org.mockito.Mockito;

public class RestPermissionsFilterTest {

@Test
public void testOptionsRequestWithoutAuthorizationIsAllowed() throws Exception {
RestPermissionsFilter filter = Mockito.spy(RestPermissionsFilter.class);
HttpServletRequest mockHttpServletRequest = Mockito.spy(HttpServletRequest.class);
Mockito.when(mockHttpServletRequest.getMethod()).thenReturn(HttpMethod.OPTIONS);
Mockito.when(filter.isCorsEnabled()).thenReturn(true);

boolean allowed = filter.isAccessAllowed(
mockHttpServletRequest,
Mockito.mock(ServletResponse.class),
Mockito.mock(Object.class)
);
Assertions.assertThat(allowed).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.security.Principal;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.HttpMethod;

import org.apache.shiro.SecurityUtils;
import org.apache.shiro.mgt.DefaultSecurityManager;
Expand All @@ -31,7 +32,6 @@
import org.junit.Test;
import org.mockito.Mockito;


public final class ShiroJwtVerifyingFilterTest {

@Test
Expand Down Expand Up @@ -199,5 +199,19 @@ public void testAuthorizationValid() throws Exception {
}
}

@Test
public void testOptionsRequestWithoutAuthorizationIsAllowed() throws Exception {
ShiroJwtVerifyingFilter filter = Mockito.spy(ShiroJwtVerifyingFilter.class);
HttpServletRequest mockHttpServletRequest = Mockito.spy(HttpServletRequest.class);
Mockito.when(mockHttpServletRequest.getMethod()).thenReturn(HttpMethod.OPTIONS);
Mockito.when(filter.isCorsEnabled()).thenReturn(true);

boolean allowed = filter.isAccessAllowed(
mockHttpServletRequest,
Mockito.mock(ServletResponse.class),
Mockito.mock(Object.class)
);
Assertions.assertThat(allowed).isTrue();
}

}