From 576fc0b9475048881986ec52eccef8e589e8c0e6 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 23 May 2016 12:05:09 +1000 Subject: [PATCH] refactor ZanataRestSecurityInterceptor --- .../rest/ZanataRestSecurityInterceptor.java | 75 +++++++++++++++---- .../zanata/security/SecurityFunctions.java | 25 ++++--- 2 files changed, 76 insertions(+), 24 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/rest/ZanataRestSecurityInterceptor.java b/zanata-war/src/main/java/org/zanata/rest/ZanataRestSecurityInterceptor.java index 25ea0438cd..ecef1f2d61 100644 --- a/zanata-war/src/main/java/org/zanata/rest/ZanataRestSecurityInterceptor.java +++ b/zanata-war/src/main/java/org/zanata/rest/ZanataRestSecurityInterceptor.java @@ -23,12 +23,14 @@ import org.apache.oltu.oauth2.rs.response.OAuthRSResponse; import org.jboss.resteasy.annotations.interception.SecurityPrecedence; import org.zanata.model.HAccount; +import org.zanata.rest.oauth.OAuthUtil; import org.zanata.security.SecurityFunctions; import org.zanata.security.ZanataIdentity; import org.zanata.security.annotations.Authenticated; import org.zanata.security.oauth.SecurityTokens; import org.zanata.util.HttpUtil; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.googlecode.totallylazy.Either; import lombok.extern.slf4j.Slf4j; @@ -73,34 +75,37 @@ public void filter(ContainerRequestContext context) return; } - String username = HttpUtil.getUsername(context.getHeaders()); - String apiKey = HttpUtil.getApiKey(context.getHeaders()); + Tokens tokens = new Tokens(context, request); - if (StringUtils.isNotEmpty(username) && StringUtils.isNotEmpty(apiKey)) { + if (!tokens.canAuthenticate() && + SecurityFunctions.doesRestPathAllowAnonymousAccess( + context.getMethod(), context.getUriInfo().getPath())) { + // if we don't have any information to authenticate but the + // requesting API allows anonymous access, we let it go + return; + } + + if (tokens.canAuthenticateUsingApiKey()) { // if apiKey presents, we use apiKey for security check - zanataIdentity.getCredentials().setUsername(username); - zanataIdentity.setApiKey(apiKey); + zanataIdentity.getCredentials().setUsername(tokens.username.get()); + zanataIdentity.setApiKey(tokens.apiKey.get()); + zanataIdentity.tryLogin(); } else { Either usernameOrResponse = getUsernameOrResponse(); if (usernameOrResponse.isRight()) { - if (SecurityFunctions - .canAccessRestPath(zanataIdentity, context.getMethod(), - context.getUriInfo().getPath())) { - return; - } context.abortWith(usernameOrResponse.right()); return; } - username = usernameOrResponse.left(); + String username = usernameOrResponse.left(); zanataIdentity.getCredentials().setUsername(username); zanataIdentity.setOAuthRequest(true); + zanataIdentity.tryLogin(); } - zanataIdentity.tryLogin(); if (!SecurityFunctions.canAccessRestPath(zanataIdentity, context.getMethod(), context.getUriInfo().getPath())) { - log.info(InvalidApiKeyUtil.getMessage(username, apiKey)); + log.info("can not authenticate REST request: {}", tokens); context.abortWith(Response.status(Status.UNAUTHORIZED) .entity("Invalid token") .build()); @@ -171,4 +176,48 @@ private Response buildUnauthorizedResponse(String message) { private Response buildServerErrorResponse(String message) { return Response.serverError().entity(message).build(); } + + private static class Tokens { + + private final Optional username; + private final Optional apiKey; + private final Optional accessToken; + private final Optional authorizationCode; + + Tokens(ContainerRequestContext context, HttpServletRequest request) { + String username = HttpUtil.getUsername(context.getHeaders()); + String apiKey = HttpUtil.getApiKey(context.getHeaders()); + this.username = optionalNotEmptyString(username); + this.apiKey = optionalNotEmptyString(apiKey); + accessToken = OAuthUtil.getAccessToken(request); + String authorizationCode = request.getParameter(OAuth.OAUTH_CODE); + this.authorizationCode = optionalNotEmptyString(authorizationCode); + } + + private static Optional optionalNotEmptyString(String value) { + return StringUtils.isNotEmpty(value) ? Optional.of(value) : Optional.empty(); + } + + boolean canAuthenticateUsingApiKey() { + return username.isPresent() && apiKey.isPresent(); + } + + boolean canAuthenticateUsingOAuth() { + return authorizationCode.isPresent() || accessToken.isPresent(); + } + + boolean canAuthenticate() { + return canAuthenticateUsingApiKey() || authorizationCode.isPresent() || accessToken.isPresent(); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("username", username) + .add("apiKey", apiKey) + .add("accessToken", accessToken) + .add("authorizationCode", authorizationCode) + .toString(); + } + } } diff --git a/zanata-war/src/main/java/org/zanata/security/SecurityFunctions.java b/zanata-war/src/main/java/org/zanata/security/SecurityFunctions.java index 97fa171df3..bdf26885a5 100644 --- a/zanata-war/src/main/java/org/zanata/security/SecurityFunctions.java +++ b/zanata-war/src/main/java/org/zanata/security/SecurityFunctions.java @@ -42,9 +42,7 @@ import org.zanata.util.ServiceLocator; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import javax.inject.Inject; -import javax.ws.rs.HttpMethod; import java.util.Optional; @@ -568,19 +566,23 @@ public boolean canDownloadTMX() { public static boolean canAccessRestPath( @Nonnull ZanataIdentity identity, String httpMethod, String restServicePath) { - // This is to allow data injection for function-test/rest-test - if (isTestServicePath(restServicePath)) { - log.debug("Allow rest access for Zanata test"); - return true; - } - if (identity.isLoggedIn()) { + if (doesRestPathAllowAnonymousAccess(httpMethod, restServicePath)) { return true; } - return isRestPathAllowedAnonymousAccess(httpMethod); + return identity.isLoggedIn(); } - private static boolean isRestPathAllowedAnonymousAccess(String httpMethod) { - return HttpUtil.isReadMethod(httpMethod); + /** + * Check if the REST api allow anonymous access + * @param httpMethod {@link javax.ws.rs.HttpMethod} + * @param servicePath service path of rest request. + * See annotation @Path in REST service class. + * @return + */ + public static boolean doesRestPathAllowAnonymousAccess(String httpMethod, + String servicePath) { + return HttpUtil.isReadMethod(httpMethod) || + isTestServicePath(servicePath); } /** @@ -590,6 +592,7 @@ private static boolean isRestPathAllowedAnonymousAccess(String httpMethod) { * See annotation @Path in REST service class. */ private static boolean isTestServicePath(String servicePath) { + // This is to allow data injection for function-test/rest-test return servicePath != null && ( // when being called in RestLimitingFilter