From 277f76ef036a7c1f44f10f97a39a6cf8575a44da Mon Sep 17 00:00:00 2001 From: SunnyBoy-WYH <48077841+SunnyBoy-WYH@users.noreply.github.com> Date: Tue, 19 Mar 2024 17:43:33 +0800 Subject: [PATCH] chore(server): clear context after req done (#2470) Co-authored-by: vaughn.zhang Co-authored-by: imbajin --- .../apache/hugegraph/api/auth/LoginAPI.java | 6 +- .../hugegraph/api/filter/AccessLogFilter.java | 12 ++++ .../api/filter/AuthenticationFilter.java | 59 +++++++++---------- .../hugegraph/auth/ConfigAuthenticator.java | 6 ++ .../hugegraph/auth/HugeAuthenticator.java | 9 +-- .../hugegraph/auth/HugeGraphAuthProxy.java | 6 +- .../hugegraph/auth/StandardAuthenticator.java | 8 +++ .../hugegraph/config/ServerOptions.java | 2 +- .../apache/hugegraph/core/GraphManager.java | 6 ++ .../static/conf/rest-server.properties | 2 +- .../apache/hugegraph/api/ArthasApiTest.java | 38 ++++++++---- .../org/apache/hugegraph/api/BaseApiTest.java | 7 +-- .../apache/hugegraph/api/LoginApiTest.java | 12 ++-- 13 files changed, 105 insertions(+), 68 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/LoginAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/LoginAPI.java index ba2ab3f4fe..74af573e39 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/LoginAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/LoginAPI.java @@ -70,11 +70,9 @@ public String login(@Context GraphManager manager, @PathParam("graph") String gr checkCreatingBody(jsonLogin); try { - String token = manager.authManager() - .loginUser(jsonLogin.name, jsonLogin.password); + String token = manager.authManager().loginUser(jsonLogin.name, jsonLogin.password); HugeGraph g = graph(manager, graph); - return manager.serializer(g) - .writeMap(ImmutableMap.of("token", token)); + return manager.serializer(g).writeMap(ImmutableMap.of("token", token)); } catch (AuthenticationException e) { throw new NotAuthorizedException(e.getMessage(), e); } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java index 7a4a9b97d7..d429db4d9b 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AccessLogFilter.java @@ -26,8 +26,10 @@ import java.io.IOException; import java.net.URI; +import org.apache.hugegraph.auth.HugeAuthenticator; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.ServerOptions; +import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.metrics.MetricsUtil; import org.apache.hugegraph.util.Log; import org.slf4j.Logger; @@ -55,6 +57,9 @@ public class AccessLogFilter implements ContainerResponseFilter { @Context private jakarta.inject.Provider configProvider; + @Context + private jakarta.inject.Provider managerProvider; + public static boolean needRecordLog(ContainerRequestContext context) { // TODO: add test for 'path' result ('/gremlin' or 'gremlin') String path = context.getUriInfo().getPath(); @@ -114,6 +119,13 @@ public void filter(ContainerRequestContext requestContext, executeTime, null, method, path, uri.getQuery()); } } + + // Unset the context in "HugeAuthenticator", need distinguish Graph/Auth server lifecycle + GraphManager manager = managerProvider.get(); + // TODO: transfer Authorizer if we need after. + if (manager.requireAuthentication()) { + manager.unauthorize(requestContext.getSecurityContext()); + } } private boolean statusOk(int status) { diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java index 8505d435dd..b15adf3b90 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java @@ -45,7 +45,7 @@ import org.slf4j.Logger; import com.alipay.remoting.util.StringUtils; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import jakarta.annotation.Priority; import jakarta.ws.rs.BadRequestException; @@ -71,15 +71,15 @@ public class AuthenticationFilter implements ContainerRequestFilter { private static final Logger LOG = Log.logger(AuthenticationFilter.class); - private static final List WHITE_API_LIST = ImmutableList.of( - "graphs/*/auth/login", + private static final AntPathMatcher MATCHER = new AntPathMatcher(); + private static final Set FIXED_WHITE_API_SET = ImmutableSet.of( "versions", "openapi.json" ); - private static final AntPathMatcher MATCHER = new AntPathMatcher(); - - private static String whiteIpStatus; + /** Remove auth/login API from whitelist */ + private static final Set FLEXIBLE_WHITE_API_SET = ImmutableSet.of(); + private static Boolean enabledWhiteIpCheck; private static final String STRING_WHITE_IP_LIST = "whiteiplist"; private static final String STRING_ENABLE = "enable"; @@ -94,7 +94,7 @@ public class AuthenticationFilter implements ContainerRequestFilter { @Override public void filter(ContainerRequestContext context) throws IOException { - if (AuthenticationFilter.isWhiteAPI(context)) { + if (isWhiteAPI(context)) { return; } User user = this.authenticate(context); @@ -107,7 +107,7 @@ protected User authenticate(ContainerRequestContext context) { E.checkState(manager != null, "Context GraphManager is absent"); if (!manager.requireAuthentication()) { - // Return anonymous user with admin role if disable authentication + // Return anonymous user with an admin role if disable authentication return User.ANONYMOUS; } @@ -121,11 +121,12 @@ protected User authenticate(ContainerRequestContext context) { } // Check whiteIp - if (whiteIpStatus == null) { - whiteIpStatus = this.configProvider.get().get(WHITE_IP_STATUS); + if (enabledWhiteIpCheck == null) { + String whiteIpStatus = this.configProvider.get().get(WHITE_IP_STATUS); + enabledWhiteIpCheck = Objects.equals(whiteIpStatus, STRING_ENABLE); } - if (Objects.equals(whiteIpStatus, STRING_ENABLE) && request != null) { + if (enabledWhiteIpCheck && request != null) { peer = request.getRemoteAddr() + ":" + request.getRemotePort(); path = request.getRequestURI(); @@ -134,9 +135,8 @@ protected User authenticate(ContainerRequestContext context) { boolean whiteIpEnabled = manager.authManager().getWhiteIpStatus(); if (!path.contains(STRING_WHITE_IP_LIST) && whiteIpEnabled && !whiteIpList.contains(remoteIp)) { - throw new ForbiddenException( - String.format("Remote ip '%s' is not permitted", - remoteIp)); + throw new ForbiddenException(String.format("Remote ip '%s' is not permitted", + remoteIp)); } } @@ -144,28 +144,23 @@ protected User authenticate(ContainerRequestContext context) { // Extract authentication credentials String auth = context.getHeaderString(HttpHeaders.AUTHORIZATION); if (auth == null) { - throw new NotAuthorizedException( - "Authentication credentials are required", - "Missing authentication credentials"); + throw new NotAuthorizedException("Authentication credentials are required", + "Missing authentication credentials"); } if (auth.startsWith(BASIC_AUTH_PREFIX)) { auth = auth.substring(BASIC_AUTH_PREFIX.length()); - auth = new String(DatatypeConverter.parseBase64Binary(auth), - Charsets.ASCII_CHARSET); + auth = new String(DatatypeConverter.parseBase64Binary(auth), Charsets.ASCII_CHARSET); String[] values = auth.split(":"); if (values.length != 2) { - throw new BadRequestException( - "Invalid syntax for username and password"); + throw new BadRequestException("Invalid syntax for username and password"); } final String username = values[0]; final String password = values[1]; - if (StringUtils.isEmpty(username) || - StringUtils.isEmpty(password)) { - throw new BadRequestException( - "Invalid syntax for username and password"); + if (StringUtils.isEmpty(username) || StringUtils.isEmpty(password)) { + throw new BadRequestException("Invalid syntax for username and password"); } credentials.put(HugeAuthenticator.KEY_USERNAME, username); @@ -174,8 +169,7 @@ protected User authenticate(ContainerRequestContext context) { String token = auth.substring(BEARER_TOKEN_PREFIX.length()); credentials.put(HugeAuthenticator.KEY_TOKEN, token); } else { - throw new BadRequestException( - "Only HTTP Basic or Bearer authentication is supported"); + throw new BadRequestException("Only HTTP Basic or Bearer authentication is supported"); } credentials.put(HugeAuthenticator.KEY_ADDRESS, peer); @@ -185,8 +179,7 @@ protected User authenticate(ContainerRequestContext context) { try { return manager.authenticate(credentials); } catch (AuthenticationException e) { - throw new NotAuthorizedException("Authentication failed", - e.getMessage()); + throw new NotAuthorizedException("Authentication failed", e.getMessage()); } } @@ -250,7 +243,7 @@ private boolean matchPermission(String required) { requiredPerm = RequiredPerm.fromPermission(required); /* - * Replace owner value(it may be a variable) if the permission + * Replace owner value (it may be a variable) if the permission * format like: "$owner=$graph $action=vertex_write" */ String owner = requiredPerm.owner(); @@ -316,7 +309,11 @@ public boolean equals(Object obj) { public static boolean isWhiteAPI(ContainerRequestContext context) { String path = context.getUriInfo().getPath(); - for (String whiteApi : WHITE_API_LIST) { + if (FIXED_WHITE_API_SET.contains(path)) { + return true; + } + + for (String whiteApi : FLEXIBLE_WHITE_API_SET) { if (MATCHER.match(whiteApi, path)) { return true; } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/ConfigAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/ConfigAuthenticator.java index 21cf39682d..eaad573d5e 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/ConfigAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/ConfigAuthenticator.java @@ -30,6 +30,8 @@ import org.apache.hugegraph.util.E; import org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens; +import jakarta.ws.rs.core.SecurityContext; + public class ConfigAuthenticator implements HugeAuthenticator { public static final String KEY_USERNAME = CredentialGraphTokens.PROPERTY_USERNAME; @@ -80,6 +82,10 @@ public UserWithRole authenticate(final String username, return new UserWithRole(IdGenerator.of(username), username, role); } + @Override + public void unauthorize(SecurityContext context) { + } + @Override public AuthManager authManager() { throw new NotImplementedException("AuthManager is unsupported by ConfigAuthenticator"); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java index 1e67680953..02911c8d98 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java @@ -39,6 +39,8 @@ import org.apache.tinkerpop.gremlin.server.auth.Authenticator; import org.apache.tinkerpop.shaded.jackson.annotation.JsonProperty; +import jakarta.ws.rs.core.SecurityContext; + public interface HugeAuthenticator extends Authenticator { String KEY_USERNAME = CredentialGraphTokens.PROPERTY_USERNAME; @@ -64,6 +66,8 @@ public interface HugeAuthenticator extends Authenticator { UserWithRole authenticate(String username, String password, String token); + void unauthorize(SecurityContext context); + AuthManager authManager(); HugeGraph graph(); @@ -103,10 +107,7 @@ default User authenticate(final Map credentials) } HugeGraphAuthProxy.logUser(user, credentials.get(KEY_PATH)); - /* - * Set authentication context - * TODO: unset context after finishing a request - */ + // TODO: Ensure context lifecycle in GraphServer & AuthServer(#AccessLogFilter) HugeGraphAuthProxy.setContext(new Context(user)); return user; diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index 796c38c320..e611d166f4 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -1752,9 +1752,9 @@ public void apply(Traversal.Admin traversal) { } /* - * Verify gremlin-execute permission for user gremlin(in gremlin- - * server-exec worker) and gremlin job(in task worker). - * But don't check permission in rest worker, because the following + * Verify gremlin-execute permission for user gremlin (in gremlin-server-exec worker) + * and gremlin job(in task worker). + * But don't check permission in rest worker because the following * places need to call traversal(): * 1.vertices/edges rest api * 2.oltp rest api (like crosspointpath/neighborrank) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java index 3e276046f9..ad100875b0 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java @@ -27,6 +27,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.api.filter.AuthenticationFilter; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.ServerOptions; @@ -39,6 +40,8 @@ import org.apache.tinkerpop.gremlin.server.auth.AuthenticationException; import org.apache.tinkerpop.gremlin.structure.util.GraphFactory; +import jakarta.ws.rs.core.SecurityContext; + public class StandardAuthenticator implements HugeAuthenticator { private static final String INITING_STORE = "initing_store"; @@ -192,6 +195,11 @@ public UserWithRole authenticate(String username, String password, userWithRole.username(), role); } + @Override + public void unauthorize(SecurityContext context) { + HugeGraphAuthProxy.resetContext(); + } + @Override public AuthManager authManager() { return this.graph().authManager(); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index b46735c8f0..63da169c99 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -294,7 +294,7 @@ public static synchronized ServerOptions instance() { "arthas.ip", "The IP provided by Arthas, it can be accessible from the outside.", disallowEmpty(), - "0.0.0.0" + "127.0.0.1" ); public static final ConfigOption ARTHAS_DISABLED_COMMANDS = diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index cdd318428b..37939c2019 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -76,6 +76,8 @@ import com.alipay.sofa.rpc.config.ServerConfig; +import jakarta.ws.rs.core.SecurityContext; + public final class GraphManager { private static final Logger LOG = Log.logger(GraphManager.class); @@ -263,6 +265,10 @@ public HugeAuthenticator.User authenticate(Map credentials) return this.authenticator().authenticate(credentials); } + public void unauthorize(SecurityContext context) { + this.authenticator().unauthorize(context); + } + public AuthManager authManager() { return this.authenticator().authManager(); } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index 23f78c5824..b67905fc19 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -12,7 +12,7 @@ batch.max_write_threads=0 # configuration of arthas arthas.telnet_port=8562 arthas.http_port=8561 -arthas.ip=0.0.0.0 +arthas.ip=127.0.0.1 arthas.disabled_commands=jad # authentication configs diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ArthasApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ArthasApiTest.java index d79fa772b5..2f92324808 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ArthasApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ArthasApiTest.java @@ -38,21 +38,35 @@ public void testArthasStart() { @Test public void testArthasApi() { - String body = "{\n" + - " \"action\": \"exec\",\n" + - " \"command\": \"version\"\n" + - "}"; + // command exec + String execBody = "{\n" + + " \"action\": \"exec\",\n" + + " \"command\": \"version\"\n" + + "}"; RestClient arthasApiClient = new RestClient(ARTHAS_API_BASE_URL, false); - // If the request header contains basic auth, - // and if we are not set auth when arthas attach hg, arthas will auth it and return 401. - // ref:https://arthas.aliyun.com/en/doc/auth.html#configure-username-and-password - Response r = arthasApiClient.post(ARTHAS_API_PATH, body); - String result = assertResponseStatus(200, r); + Response execResponse = arthasApiClient.post(ARTHAS_API_PATH, execBody); + String result = assertResponseStatus(200, execResponse); assertJsonContains(result, "state"); assertJsonContains(result, "body"); - RestClient arthasApiClientWithAuth = new RestClient(ARTHAS_API_BASE_URL); - r = arthasApiClientWithAuth.post(ARTHAS_API_PATH, body); - assertResponseStatus(401, r); + // command session + String sessionBody = "{\n" + + " \"action\":\"init_session\"\n" + + "}"; + Response sessionResponse = arthasApiClient.post(ARTHAS_API_PATH, sessionBody); + String sessionResult = assertResponseStatus(200, sessionResponse); + assertJsonContains(sessionResult, "sessionId"); + assertJsonContains(sessionResult, "consumerId"); + assertJsonContains(sessionResult, "state"); + + // join session: using invalid sessionId + String joinSessionBody = "{\n" + + " \"action\":\"join_session\",\n" + + " \"sessionId\" : \"xxx\"\n" + + "}"; + Response joinSessionResponse = arthasApiClient.post(ARTHAS_API_PATH, joinSessionBody); + String joinSessionResult = assertResponseStatus(200, joinSessionResponse); + assertJsonContains(joinSessionResult, "message"); + assertJsonContains(joinSessionResult, "state"); } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java index 43b8cdbd1a..4b6c0ed7f4 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java @@ -101,8 +101,8 @@ public static RestClient newClient() { public static class RestClient { - private Client client; - private WebTarget target; + private final Client client; + private final WebTarget target; public RestClient(String url) { this(url, true); @@ -113,8 +113,7 @@ public RestClient(String url, Boolean enableAuth) { this.client.register(EncodingFilter.class); this.client.register(GZipEncoder.class); if (enableAuth) { - this.client.register(HttpAuthenticationFeature.basic(USERNAME, - PASSWORD)); + this.client.register(HttpAuthenticationFeature.basic(USERNAME, PASSWORD)); } this.target = this.client.target(url); } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/LoginApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/LoginApiTest.java index b323efa361..e7e3455a45 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/LoginApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/LoginApiTest.java @@ -116,8 +116,7 @@ public void testVerify() { assertJsonContains(result, "user_name"); Map user = JsonUtil.fromJson(result, - new TypeReference>() { - }); + new TypeReference>() {}); Assert.assertEquals(this.userId4Test, user.get("user_id")); Assert.assertEquals("test", user.get("user_name")); @@ -140,8 +139,7 @@ public void testVerify() { private Response createUser(String name, String password) { String user = "{\"user_name\":\"%s\",\"user_password\":\"%s" + "\",\"user_email\":\"user1@baidu.com\"," + - "\"user_phone\":\"123456789\",\"user_avatar\":\"image1" + - ".jpg\"}"; + "\"user_phone\":\"123456789\",\"user_avatar\":\"image1.jpg\"}"; return this.client().post(USER_PATH, String.format(user, name, password)); } @@ -151,16 +149,14 @@ private Response deleteUser(String id) { private Response login(String name, String password) { String login = Paths.get(PATH, "login").toString(); - String loginUser = "{\"user_name\":\"%s\"," + - "\"user_password\":\"%s\"}"; + String loginUser = "{\"user_name\":\"%s\",\"user_password\":\"%s\"}"; return client().post(login, String.format(loginUser, name, password)); } private String tokenFromResponse(String content) { Map data = JsonUtil.fromJson(content, - new TypeReference>() { - }); + new TypeReference>() {}); return (String) data.get("token"); } }