Remove xsrfScope and xsrfProtection authentication attributes

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159121132
This commit is contained in:
mountford 2017-06-15 10:31:47 -07:00 committed by Ben McIlwain
parent 580c41f2d6
commit 7d2f53a6fe
19 changed files with 103 additions and 185 deletions

View file

@ -46,13 +46,6 @@ public @interface Action {
*/
boolean automaticallyPrintOk() default false;
// TODO(b/26304887): Flip default to true.
/** Enables XSRF protection on all HTTP methods except GET and HEAD. */
boolean xsrfProtection() default false;
/** Arbitrary value included in the XSRF token hash. */
String xsrfScope() default "app";
/**
* Require user be logged-in or 302 redirect to the Google auth login page.
*

View file

@ -15,10 +15,8 @@
package google.registry.request;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.net.HttpHeaders.LOCATION;
import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
import static google.registry.security.XsrfTokenManager.X_CSRF_TOKEN;
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED;
import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
@ -28,7 +26,6 @@ import com.google.appengine.api.users.UserService;
import com.google.common.base.Optional;
import google.registry.request.auth.AuthResult;
import google.registry.request.auth.RequestAuthenticator;
import google.registry.security.XsrfTokenManager;
import google.registry.util.FormattingLogger;
import google.registry.util.TypeUtils.TypeInstantiator;
import java.io.IOException;
@ -61,9 +58,6 @@ import javax.servlet.http.HttpServletResponse;
*
* <h3>Security Features</h3>
*
* <p>XSRF protection is built into this class. It can be enabled or disabled on individual actions
* using {@link Action#xsrfProtection() xsrfProtection} setting.
*
* <p>This class also enforces the {@link Action#requireLogin() requireLogin} setting.
*
* @param <C> request component type
@ -76,7 +70,6 @@ public class RequestHandler<C> {
private final Provider<? extends RequestComponentBuilder<C>> requestComponentBuilderProvider;
private final UserService userService;
private final RequestAuthenticator requestAuthenticator;
private final XsrfTokenManager xsrfTokenManager;
/**
* Constructor for subclasses to create a new request handler for a specific request component.
@ -90,15 +83,12 @@ public class RequestHandler<C> {
* request-derived modules provided by this class)
* @param userService an instance of the App Engine UserService API
* @param requestAuthenticator an instance of the {@link RequestAuthenticator} class
* @param xsrfTokenManager an instance of the {@link XsrfTokenManager} class
*/
protected RequestHandler(
Provider<? extends RequestComponentBuilder<C>> requestComponentBuilderProvider,
UserService userService,
RequestAuthenticator requestAuthenticator,
XsrfTokenManager xsrfTokenManager) {
this(null, requestComponentBuilderProvider, userService, requestAuthenticator,
xsrfTokenManager);
RequestAuthenticator requestAuthenticator) {
this(null, requestComponentBuilderProvider, userService, requestAuthenticator);
}
/** Creates a new RequestHandler with an explicit component class for test purposes. */
@ -106,22 +96,19 @@ public class RequestHandler<C> {
Class<C> component,
Provider<? extends RequestComponentBuilder<C>> requestComponentBuilderProvider,
UserService userService,
RequestAuthenticator requestAuthenticator,
XsrfTokenManager xsrfTokenManager) {
RequestAuthenticator requestAuthenticator) {
return new RequestHandler<>(
checkNotNull(component),
requestComponentBuilderProvider,
userService,
requestAuthenticator,
xsrfTokenManager);
requestAuthenticator);
}
private RequestHandler(
@Nullable Class<C> component,
Provider<? extends RequestComponentBuilder<C>> requestComponentBuilderProvider,
UserService userService,
RequestAuthenticator requestAuthenticator,
XsrfTokenManager xsrfTokenManager) {
RequestAuthenticator requestAuthenticator) {
// If the component class isn't explicitly provided, infer it from the class's own typing.
// This is safe only for use by subclasses of RequestHandler where the generic parameter is
// preserved at runtime, so only expose that option via the protected constructor.
@ -130,7 +117,6 @@ public class RequestHandler<C> {
this.requestComponentBuilderProvider = checkNotNull(requestComponentBuilderProvider);
this.userService = checkNotNull(userService);
this.requestAuthenticator = checkNotNull(requestAuthenticator);
this.xsrfTokenManager = checkNotNull(xsrfTokenManager);
}
/** Runs the appropriate action for a servlet request. */
@ -163,11 +149,6 @@ public class RequestHandler<C> {
rsp.setHeader(LOCATION, userService.createLoginURL(req.getRequestURI()));
return;
}
if (route.get().shouldXsrfProtect(method)
&& !xsrfTokenManager.validateToken(nullToEmpty(req.getHeader(X_CSRF_TOKEN)))) {
rsp.sendError(SC_FORBIDDEN, "Invalid " + X_CSRF_TOKEN);
return;
}
Optional<AuthResult> authResult =
requestAuthenticator.authorize(route.get().action().auth(), req);
if (!authResult.isPresent()) {

View file

@ -42,9 +42,4 @@ abstract class Route {
}
return false;
}
boolean shouldXsrfProtect(Action.Method requestMethod) {
return action().xsrfProtection()
&& (requestMethod != Action.Method.GET) && (requestMethod != Action.Method.HEAD);
}
}

View file

@ -38,8 +38,6 @@ import java.util.Map;
* <li>the simple name of the action class
* <li>the allowable HTTP methods
* <li>whether to automatically print "ok" in the response
* <li>whether XSRF protection is enabled
* <li>the XSRF scope
* <li>whether login is required
* <li>the allowable authentication methods
* <li>the minimum authentication level
@ -53,12 +51,11 @@ public class RouterDisplayHelper {
private static final String PATH = "path";
private static final String CLASS = "class";
private static final String METHODS = "methods";
private static final String XSRF_SCOPE = "xsrfScope";
private static final String AUTH_METHODS = "authMethods";
private static final String MINIMUM_LEVEL = "minLevel";
private static final String FORMAT =
"%%-%ds %%-%ds %%-%ds %%-2s %%-4s %%-%ds %%-5s %%-%ds %%-%ds %%s";
"%%-%ds %%-%ds %%-%ds %%-2s %%-5s %%-%ds %%-%ds %%s";
/** Returns a string representation of the routing map in the specified component. */
public static String extractHumanReadableRoutesFromComponent(Class<?> componentClass) {
@ -71,7 +68,6 @@ public class RouterDisplayHelper {
columnWidths.get(PATH),
columnWidths.get(CLASS),
columnWidths.get(METHODS),
columnWidths.get(XSRF_SCOPE),
columnWidths.get(AUTH_METHODS),
columnWidths.get(MINIMUM_LEVEL));
}
@ -83,8 +79,6 @@ public class RouterDisplayHelper {
"CLASS",
"METHODS",
"OK",
"XSRF",
"SCOPE",
"LOGIN",
"AUTH_METHODS",
"MIN",
@ -98,8 +92,6 @@ public class RouterDisplayHelper {
route.actionClass().getSimpleName(),
Joiner.on(",").join(route.action().method()),
route.action().automaticallyPrintOk() ? "y" : "n",
route.action().xsrfProtection() ? "y" : "n",
route.action().xsrfScope(),
route.action().requireLogin() ? "y" : "n",
Joiner.on(",").join(route.action().auth().methods()),
route.action().auth().minimumLevel(),
@ -112,7 +104,6 @@ public class RouterDisplayHelper {
int pathWidth = 4;
int classWidth = 5;
int methodsWidth = 7;
int xsrfScopeWidth = 5;
int authMethodsWidth = 12;
int minLevelWidth = 3;
for (Route route : routes) {
@ -131,10 +122,6 @@ public class RouterDisplayHelper {
if (len > methodsWidth) {
methodsWidth = len;
}
len = route.action().xsrfScope().length();
if (len > xsrfScopeWidth) {
xsrfScopeWidth = len;
}
len = Joiner.on(",").join(route.action().auth().methods()).length();
if (len > authMethodsWidth) {
authMethodsWidth = len;
@ -150,7 +137,6 @@ public class RouterDisplayHelper {
.put(PATH, pathWidth)
.put(CLASS, classWidth)
.put(METHODS, methodsWidth)
.put(XSRF_SCOPE, xsrfScopeWidth)
.put(AUTH_METHODS, authMethodsWidth)
.put(MINIMUM_LEVEL, minLevelWidth)
.build());