Change new authorization logic to log a warning rather than rejecting the request

This is the first step in rolling out the changes so that we can check via logging whether turning on the logic would reject anything it should not.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149050878
This commit is contained in:
mountford 2017-03-02 14:53:03 -08:00 committed by Ben McIlwain
parent b1b69656bd
commit ee2bd594c8
2 changed files with 11 additions and 2 deletions

View file

@ -171,8 +171,8 @@ public class RequestHandler<C> {
Optional<AuthResult> authResult = Optional<AuthResult> authResult =
requestAuthenticator.authorize(route.get().action().auth(), req); requestAuthenticator.authorize(route.get().action().auth(), req);
if (!authResult.isPresent()) { if (!authResult.isPresent()) {
rsp.sendError(SC_FORBIDDEN); logger.warning("Request would not have been authorized");
return; // TODO(b/28219927): Change this to call rsp.sendError(SC_FORBIDDEN) and return
} }
// Build a new request component using any modules we've constructed by this point. // Build a new request component using any modules we've constructed by this point.

View file

@ -48,6 +48,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
@ -426,6 +427,8 @@ public final class RequestHandlerTest {
verify(usersOnlyAction).run(); verify(usersOnlyAction).run();
} }
// TODO(b/28219927): turn this on once we actually do authorization
@Ignore
@Test @Test
public void testNoAuthNeeded_success() throws Exception { public void testNoAuthNeeded_success() throws Exception {
when(req.getMethod()).thenReturn("GET"); when(req.getMethod()).thenReturn("GET");
@ -436,6 +439,8 @@ public final class RequestHandlerTest {
assertThat(providedAuthResult.userAuthInfo()).isAbsent(); assertThat(providedAuthResult.userAuthInfo()).isAbsent();
} }
// TODO(b/28219927): turn this on once we actually do authorization
@Ignore
@Test @Test
public void testAuthNeeded_notLoggedIn() throws Exception { public void testAuthNeeded_notLoggedIn() throws Exception {
when(req.getMethod()).thenReturn("GET"); when(req.getMethod()).thenReturn("GET");
@ -445,6 +450,8 @@ public final class RequestHandlerTest {
assertThat(providedAuthResult).isNull(); assertThat(providedAuthResult).isNull();
} }
// TODO(b/28219927): turn this on once we actually do authorization
@Ignore
@Test @Test
public void testAuthNeeded_notAuthorized() throws Exception { public void testAuthNeeded_notAuthorized() throws Exception {
userService.setUser(testUser, false); userService.setUser(testUser, false);
@ -455,6 +462,8 @@ public final class RequestHandlerTest {
assertThat(providedAuthResult).isNull(); assertThat(providedAuthResult).isNull();
} }
// TODO(b/28219927): turn this on once we actually do authorization
@Ignore
@Test @Test
public void testAuthNeeded_success() throws Exception { public void testAuthNeeded_success() throws Exception {
userService.setUser(testUser, true); userService.setUser(testUser, true);