From 5fefa8906de9ec90fac1d96c289c807222d3601e Mon Sep 17 00:00:00 2001
From: mountford <mountford@google.com>
Date: Tue, 1 Aug 2017 12:21:13 -0700
Subject: [PATCH] Fix bug which caused exceptions when attempting to redirect
 to the console login page

When the registrar console code determines that a user has not logged in, it redirects to a login page. But when authenticating as an internal request (which should never happen), the redirection code encountered an exception, resulting in a 500 error.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=163867018
---
 .../ui/server/registrar/ConsoleUiAction.java  | 14 ++++++++++++-
 .../server/registrar/ConsoleUiActionTest.java | 20 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java
index 310b30f37..d97dc3863 100644
--- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java
+++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java
@@ -82,7 +82,19 @@ public final class ConsoleUiAction implements Runnable {
   public void run() {
     if (!authResult.userAuthInfo().isPresent()) {
       response.setStatus(SC_MOVED_TEMPORARILY);
-      response.setHeader(LOCATION, userService.createLoginURL(req.getRequestURI()));
+      String location;
+      try {
+        location = userService.createLoginURL(req.getRequestURI());
+      } catch (IllegalArgumentException e) {
+        // UserServiceImpl.createLoginURL() throws IllegalArgumentException if underlying API call
+        // returns an error code of NOT_ALLOWED. createLoginURL() assumes that the error is caused
+        // by an invalid URL. But in fact, the error can also occur if UserService doesn't have any
+        // user information, which happens when the request has been authenticated as internal. In
+        // this case, we want to avoid dying before we can send the redirect, so just redirect to
+        // the root path.
+        location = "/";
+      }
+      response.setHeader(LOCATION, location);
       return;
     }
     User user = authResult.userAuthInfo().get().user();
diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java
index 4e86b62bb..d12ed30f0 100644
--- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java
+++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java
@@ -14,7 +14,9 @@
 
 package google.registry.ui.server.registrar;
 
+import static com.google.common.net.HttpHeaders.LOCATION;
 import static com.google.common.truth.Truth.assertThat;
+import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -112,4 +114,22 @@ public class ConsoleUiActionTest {
     action.run();
     assertThat(response.getPayload()).contains("<h1>You need permission</h1>");
   }
+
+  @Test
+  public void testNoUser_redirect() throws Exception {
+    when(request.getRequestURI()).thenReturn("/test");
+    action.authResult = AuthResult.NOT_AUTHENTICATED;
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(SC_MOVED_TEMPORARILY);
+    assertThat(response.getHeaders().get(LOCATION)).isEqualTo("/_ah/login?continue=%2Ftest");
+  }
+
+  @Test
+  public void testNoUserInformationAtAll_redirectToRoot() throws Exception {
+    when(request.getRequestURI()).thenThrow(new IllegalArgumentException());
+    action.authResult = AuthResult.NOT_AUTHENTICATED;
+    action.run();
+    assertThat(response.getStatus()).isEqualTo(SC_MOVED_TEMPORARILY);
+    assertThat(response.getHeaders().get(LOCATION)).isEqualTo("/");
+  }
 }