reduce some code duplication for AppEngineAuthentication

Signed-off-by: Lachlan Roberts <[email protected]>
GoogleCloudPlatform · Jul 11, 2024 · 24f949a · 24f949a
1 parent f5f9c8f
commit 24f949a
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 119 deletions.
diff --git a/...dk_jetty12/src/main/java/com/google/apphosting/runtime/jetty/AppEngineAuthentication.java b/...dk_jetty12/src/main/java/com/google/apphosting/runtime/jetty/AppEngineAuthentication.java
@@ -106,7 +106,7 @@ public static void configureSecurityHandler(ConstraintSecurityHandler handler) {
   private static class AppEngineAuthenticator extends LoginAuthenticator {
 
     /**
-     * Checks if the request could to to the login page.
+     * Checks if the request could to the login page.
      *
      * @param uri The uri requested.
      * @return True if the uri starts with "/_ah/", false otherwise.
@@ -125,7 +125,7 @@ public String getAuthMethod() {
      * j.c.g.apphosting.utils.jetty.AppEngineAuthentication.AppEngineAuthenticator.authenticate().
      *
      * <p>If authentication is required but the request comes from an untrusted ip, 307s the request
-     * back to the trusted appserver. Otherwise it will auth the request and return a login url if
+     * back to the trusted appserver. Otherwise, it will auth the request and return a login url if
      * needed.
      *
      * <p>From org.eclipse.jetty.server.Authentication:
@@ -138,7 +138,7 @@ public String getAuthMethod() {
      *     for both successful and unsuccessful authentications), then the result will implement
      *     {@link Authentication.ResponseSent}. If Authentication is not mandatory, then a {@link
      *     Authentication.Deferred} may be returned.
-     * @throws ServerAuthException
+     * @throws ServerAuthException in an error occurs during authentication.
      */
     @Override
     public Authentication validateRequest(
@@ -300,13 +300,6 @@ public void setIdentityService(IdentityService identityService) {
       this.identityService = identityService;
     }
 
-    /**
-     * Validate a user identity. Validate that a UserIdentity previously created by a call to {@link
-     * #login(String, Object, ServletRequest)} is still valid.
-     *
-     * @param user The user to validate
-     * @return true if authentication has not been revoked for the user.
-     */
     @Override
     public boolean validate(UserIdentity user) {
       logger.atInfo().log("validate(%s) throwing UnsupportedOperationException.", user);
@@ -331,7 +324,7 @@ public User getUser() {
 
     @Override
     public String getName() {
-      if ((user.getFederatedIdentity() != null) && (user.getFederatedIdentity().length() > 0)) {
+      if ((user.getFederatedIdentity() != null) && (!user.getFederatedIdentity().isEmpty())) {
         return user.getFederatedIdentity();
       }
       return user.getEmail();
@@ -402,8 +395,8 @@ public boolean isUserInRole(String role) {
           return userService.isUserAdmin();
         } else {
           // TODO: I'm not sure this will happen in
-          // practice. If it does, we may need to pass an
-          // application's admin list down somehow.
+          //   practice. If it does, we may need to pass an
+          //   application's admin list down somehow.
           logger.atSevere().log("Cannot tell if non-logged-in user %s is an admin.", user);
           return false;
         }

diff --git a/...etty12/src/main/java/com/google/apphosting/runtime/jetty/EE10AppEngineAuthentication.java b/...etty12/src/main/java/com/google/apphosting/runtime/jetty/EE10AppEngineAuthentication.java
@@ -20,6 +20,8 @@
 import com.google.appengine.api.users.UserService;
 import com.google.appengine.api.users.UserServiceFactory;
 import com.google.apphosting.api.ApiProxy;
+import com.google.apphosting.runtime.jetty.AppEngineAuthentication.AppEnginePrincipal;
+import com.google.apphosting.runtime.jetty.AppEngineAuthentication.AppEngineUserIdentity;
 import com.google.common.flogger.GoogleLogger;
 import jakarta.servlet.http.HttpServletResponse;
 import java.security.Principal;
@@ -231,7 +233,7 @@ public UserIdentity login(
      *
      * @return A AppEngineUserIdentity if a user is logged in, or null otherwise.
      */
-    private AppEngineUserIdentity loadUser() {
+    private AppEngineUserIdentity  loadUser() {
       UserService userService = UserServiceFactory.getUserService();
       User engineUser = userService.getCurrentUser();
       if (engineUser == null) {
@@ -264,109 +266,4 @@ public boolean validate(UserIdentity user) {
       throw new UnsupportedOperationException();
     }
   }
-
-  /**
-   * {@code AppEnginePrincipal} is an implementation of {@link Principal} that represents a
-   * logged-in Google App Engine user.
-   */
-  public static class AppEnginePrincipal implements Principal {
-    private final User user;
-
-    public AppEnginePrincipal(User user) {
-      this.user = user;
-    }
-
-    public User getUser() {
-      return user;
-    }
-
-    @Override
-    public String getName() {
-      if ((user.getFederatedIdentity() != null) && (!user.getFederatedIdentity().isEmpty())) {
-        return user.getFederatedIdentity();
-      }
-      return user.getEmail();
-    }
-
-    @Override
-    public boolean equals(Object other) {
-      if (other instanceof AppEnginePrincipal) {
-        return user.equals(((AppEnginePrincipal) other).user);
-      } else {
-        return false;
-      }
-    }
-
-    @Override
-    public String toString() {
-      return user.toString();
-    }
-
-    @Override
-    public int hashCode() {
-      return user.hashCode();
-    }
-  }
-
-  /**
-   * {@code AppEngineUserIdentity} is an implementation of {@link UserIdentity} that represents a
-   * logged-in Google App Engine user.
-   */
-  public static class AppEngineUserIdentity implements UserIdentity {
-
-    private final AppEnginePrincipal userPrincipal;
-
-    public AppEngineUserIdentity(AppEnginePrincipal userPrincipal) {
-      this.userPrincipal = userPrincipal;
-    }
-
-    /*
-     * Only used by jaas and jaspi.
-     */
-    @Override
-    public Subject getSubject() {
-      logger.atInfo().log("getSubject() throwing UnsupportedOperationException.");
-      throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Principal getUserPrincipal() {
-      return userPrincipal;
-    }
-
-    @Override
-    public boolean isUserInRole(String role) {
-      UserService userService = UserServiceFactory.getUserService();
-      logger.atFine().log("Checking if principal %s is in role %s", userPrincipal, role);
-      if (userPrincipal == null) {
-        logger.atInfo().log("isUserInRole() called with null principal.");
-        return false;
-      }
-
-      if (USER_ROLE.equals(role)) {
-        return true;
-      }
-
-      if (ADMIN_ROLE.equals(role)) {
-        User user = userPrincipal.getUser();
-        if (user.equals(userService.getCurrentUser())) {
-          return userService.isUserAdmin();
-        } else {
-          // TODO: I'm not sure this will happen in
-          // practice. If it does, we may need to pass an
-          // application's admin list down somehow.
-          logger.atSevere().log("Cannot tell if non-logged-in user %s is an admin.", user);
-          return false;
-        }
-      } else {
-        logger.atWarning().log("Unknown role: %s.", role);
-        return false;
-      }
-    }
-
-    @Override
-    public String toString() {
-      return AppEngineUserIdentity.class.getSimpleName() + "('" + userPrincipal + "')";
-    }
-  }
 }