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.

Sign up for GitHub

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

AppEngineAuthenticator does need to extend LoginAuthenticator for EE10 #247

Merged
merged 2 commits into from
Jul 12, 2024
Merged

AppEngineAuthenticator does need to extend LoginAuthenticator for EE10 #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5f9c8f
AppEngineAuthenticator does need to extend LoginAuthenticator for EE10
lachlan-roberts Jul 11, 2024
24f949a
reduce some code duplication for AppEngineAuthentication
lachlan-roberts Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions ...dk_jetty12/src/main/java/com/google/apphosting/runtime/jetty/AppEngineAuthentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
126 changes: 15 additions & 111 deletions ...etty12/src/main/java/com/google/apphosting/runtime/jetty/EE10AppEngineAuthentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,8 +97,8 @@ protected Constraint getConstraint(String pathInContext, Request request) {
}
};

LoginService loginService = new AppEngineLoginService();
LoginAuthenticator authenticator = new AppEngineAuthenticator();
AppEngineLoginService loginService = new AppEngineLoginService();
AppEngineAuthenticator authenticator = new AppEngineAuthenticator();
DefaultIdentityService identityService = new DefaultIdentityService();

// Set allowed roles.
Expand All @@ -112,7 +114,9 @@ protected Constraint getConstraint(String pathInContext, Request request) {
* {@code AppEngineAuthenticator} is a custom {@link Authenticator} that knows how to redirect the
* current request to a login URL in order to authenticate the user.
*/
private static class AppEngineAuthenticator extends LoginAuthenticator {
private static class AppEngineAuthenticator implements Authenticator {

private LoginService _loginService;

/**
* Checks if the request could go to the login page.
Expand All @@ -124,6 +128,11 @@ private static boolean isLoginOrErrorPage(String uri) {
return uri.startsWith(AUTH_URL_PREFIX);
}

@Override
public void setConfiguration(Configuration configuration) {
_loginService = configuration.getLoginService();
}

@Override
public String getAuthenticationType() {
return AUTH_METHOD;
Expand All @@ -146,7 +155,7 @@ public Constraint.Authorization getConstraintAuthentication(
return Constraint.Authorization.ALLOWED;
}

return super.getConstraintAuthentication(pathInContext, existing, getSession);
return Authenticator.super.getConstraintAuthentication(pathInContext, existing, getSession);
}

/**
Expand All @@ -172,7 +181,7 @@ public AuthenticationState validateRequest(Request req, Response res, Callback c
UserIdentity user = _loginService.login(null, null, null, null);
logger.atFine().log("authenticate() returning new principal for %s", user);
if (user != null) {
return new UserAuthenticationSucceeded(getAuthenticationType(), user);
return new LoginAuthenticator.UserAuthenticationSucceeded(getAuthenticationType(), user);
}
}

Expand Down Expand Up @@ -224,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) {
Expand Down Expand Up @@ -257,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 + "')";
}
}
}
Loading