Skip to content

Commit

Permalink
chore(server): clear context after req done (#2470)
Browse files Browse the repository at this point in the history
Co-authored-by: vaughn.zhang <[email protected]>
Co-authored-by: imbajin <[email protected]>
  • Loading branch information
3 people authored Mar 19, 2024
1 parent 7b33574 commit 277f76e
Show file tree
Hide file tree
Showing 13 changed files with 105 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,6 +57,9 @@ public class AccessLogFilter implements ContainerResponseFilter {
@Context
private jakarta.inject.Provider<HugeConfig> configProvider;

@Context
private jakarta.inject.Provider<GraphManager> managerProvider;

public static boolean needRecordLog(ContainerRequestContext context) {
// TODO: add test for 'path' result ('/gremlin' or 'gremlin')
String path = context.getUriInfo().getPath();
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -71,15 +71,15 @@ public class AuthenticationFilter implements ContainerRequestFilter {

private static final Logger LOG = Log.logger(AuthenticationFilter.class);

private static final List<String> WHITE_API_LIST = ImmutableList.of(
"graphs/*/auth/login",
private static final AntPathMatcher MATCHER = new AntPathMatcher();
private static final Set<String> 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<String> 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";

Expand All @@ -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);
Expand All @@ -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;
}

Expand All @@ -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();

Expand All @@ -134,38 +135,32 @@ 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));
}
}

Map<String, String> credentials = new HashMap<>();
// 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);
Expand All @@ -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);
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -103,10 +107,7 @@ default User authenticate(final Map<String, String> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> ARTHAS_DISABLED_COMMANDS =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -263,6 +265,10 @@ public HugeAuthenticator.User authenticate(Map<String, String> credentials)
return this.authenticator().authenticate(credentials);
}

public void unauthorize(SecurityContext context) {
this.authenticator().unauthorize(context);
}

public AuthManager authManager() {
return this.authenticator().authManager();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Loading

0 comments on commit 277f76e

Please sign in to comment.