diff --git a/helm/hugegraph/README.md b/helm/hugegraph/README.md index fb8a01b93d..c8c48bd6c2 100644 --- a/helm/hugegraph/README.md +++ b/helm/hugegraph/README.md @@ -813,8 +813,9 @@ before anything reaches the cluster: applied with the backslashes and answer 401 to the Secret value (measured on `:latest`, 2026-10-03). This guard is temporary: it goes once the Server decodes Basic auth as UTF-8 and splits on the first colon only, and - the entrypoint writes spaces so they read back unchanged (the TODOs at the - decoding site and at `encode_prop_value` are in apache/hugegraph#3260). + the entrypoint writes spaces so they read back unchanged (Server side: + apache/hugegraph#3284, once the image this chart deploys includes that + fix; the TODO at `encode_prop_value` is in apache/hugegraph#3260). - A non-ClusterIP `pd.service.type` requires `pd.service.allowInsecureExposure=true`, a non-ClusterIP `server.service.type` requires `server.service.allowInsecureExposure=true` diff --git a/helm/hugegraph/templates/server-deployment.yaml b/helm/hugegraph/templates/server-deployment.yaml index bfbdf16e6a..48d24ebaf9 100644 --- a/helm/hugegraph/templates/server-deployment.yaml +++ b/helm/hugegraph/templates/server-deployment.yaml @@ -146,8 +146,8 @@ spec: # 401 to every request made with it. Temporary: drop this check, # the colon case above and the matching schema pattern once the # Server decodes Basic auth as UTF-8 and splits the credential on - # its first colon only (TODO in AuthenticationFilter, - # apache/hugegraph#3260). + # its first colon only (apache/hugegraph#3284) in the image this + # chart deploys. if (( $(printf '%s' "${PASSWORD}" | LC_ALL=C tr -d '\040-\176' | wc -c) > 0 )); then echo "auth Secret key 'password' must be printable ASCII (bytes" \ "0x20-0x7E: no tabs, newlines or non-ASCII characters): the" \ diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java index 939ccfe22b..216fff97e9 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java @@ -67,7 +67,7 @@ public class CypherAPI extends API { private static final ObjectReader REQUEST_READER = new ObjectMapper() .readerFor(Object.class) .with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS); - private final Base64.Decoder decoder = Base64.getUrlDecoder(); + private final Base64.Decoder decoder = Base64.getDecoder(); private final String basic = "Basic "; private final String bearer = "Bearer "; @@ -221,7 +221,7 @@ private Pair toUserPass(String auth) { String encoded = auth.substring(basic.length()); byte[] userPass = this.decoder.decode(encoded); String authorization = new String(userPass, UTF8); - split = authorization.split(":"); + split = authorization.split(":", 2); } catch (Exception e) { LOG.error("Failed convert auth to credential.", e); return null; 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 13bb05d8d9..ed37c6592c 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 @@ -20,6 +20,7 @@ import static org.apache.hugegraph.config.ServerOptions.WHITE_IP_STATUS; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.security.Principal; import java.util.HashMap; import java.util.List; @@ -39,7 +40,6 @@ import org.apache.hugegraph.util.Log; import org.apache.tinkerpop.gremlin.server.auth.AuthenticationException; import org.glassfish.grizzly.http.server.Request; -import org.glassfish.grizzly.utils.Charsets; import org.gridkit.jvmtool.cmd.AntPathMatcher; import org.slf4j.Logger; @@ -184,19 +184,16 @@ protected User authenticate(ContainerRequestContext context) { if (auth.startsWith(BASIC_AUTH_PREFIX)) { auth = auth.substring(BASIC_AUTH_PREFIX.length()); - // TODO: decode the Basic credential as UTF-8 and split it on the first colon only. - // Decoding as ASCII and splitting on every colon makes a non-ASCII password answer - // 401 and a password containing ':' answer 400, although both were accepted at - // account creation. The Helm chart (helm/hugegraph) refuses such admin passwords in - // its schema and Server wrapper; drop that guard once this is fixed. - auth = new String(DatatypeConverter.parseBase64Binary(auth), Charsets.ASCII_CHARSET); - String[] values = auth.split(":"); - if (values.length != 2) { + // RFC 7617 section 2.1 allows only UTF-8 as the declared charset, and it bars a + // colon from the user-id alone, so split on the first one and keep the rest + auth = new String(DatatypeConverter.parseBase64Binary(auth), StandardCharsets.UTF_8); + int colon = auth.indexOf(':'); + if (colon < 0) { throw new BadRequestException("Invalid syntax for username and password"); } - final String username = values[0]; - final String password = values[1]; + final String username = auth.substring(0, colon); + final String password = auth.substring(colon + 1); if (StringUtils.isEmpty(username) || StringUtils.isEmpty(password)) { throw new BadRequestException("Invalid syntax for username and password"); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java index ba5884739a..28d253a5cf 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java @@ -19,9 +19,13 @@ import static org.apache.hugegraph.testutil.Assert.assertContains; +import java.nio.charset.StandardCharsets; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; +import java.util.Deque; import java.util.List; import java.util.Map; @@ -30,10 +34,14 @@ import org.junit.Before; import org.junit.Test; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import jakarta.ws.rs.client.Entity; +import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; public class CypherApiTest extends BaseApiTest { @@ -392,6 +400,58 @@ public void testAuthenticationAndGraphRouting() { "name", "alice", "dave", "marko", "peter"); } + @Test + public void testBasicAuthWithColonPassword() { + // The group, target and access setup below uses the standalone auth API; in PD mode + // groups are graphspace-scoped. The header parsing under test is backend-independent. + assumeStandaloneMode(); + // CypherAPI parses the Basic header itself, apart from AuthenticationFilter + String user = "cypher_colon_user"; + // With this user name these encode to standard Base64 containing '+' and '/' + Assert.assertTrue(basicCredential(user, "a:b:c~").contains("+")); + Assert.assertTrue(basicCredential(user, "a:b:c?").contains("/")); + Deque created = new ArrayDeque<>(); + RestClient noAuthClient = new RestClient(baseUrl(), false); + try { + String groupId = this.createAuth(created, "auth/groups", + "{\"group_name\":\"cypher_colon_group\"}"); + String targetId = this.createAuth(created, "graphspaces/DEFAULT/auth/targets", + "{\"target_name\":\"cypher_colon_target\"," + + "\"target_graph\":\"hugegraph\"," + + "\"target_url\":\"127.0.0.1:8080\"," + + "\"target_resources\":[{\"type\":\"ALL\"}]}"); + for (String permission : ImmutableList.of("READ", "EXECUTE")) { + this.createAuth(created, "graphspaces/DEFAULT/auth/accesses", String.format( + "{\"group\":\"%s\",\"target\":\"%s\",\"access_permission\":\"%s\"}", + groupId, targetId, permission)); + } + + for (String password : ImmutableList.of("a:b:c", "a:b:c~", "a:b:c?")) { + Deque member = new ArrayDeque<>(); + try { + String userId = this.createAuth(member, "graphspaces/DEFAULT/auth/users", + String.format("{\"user_name\":\"%s\"," + + "\"user_password\":\"%s\"}", + user, password)); + this.createAuth(member, "graphspaces/DEFAULT/auth/belongs", String.format( + "{\"user\":\"%s\",\"group\":\"%s\"}", userId, groupId)); + + Response r = basicAuthCypher(noAuthClient, user, password); + assertColumn(assertCypherSuccessData(assertResponseStatus(200, r)), + "name", "marko", "peter"); + + r = basicAuthCypher(noAuthClient, user, "wrong" + password); + assertResponseStatus(401, r); + } finally { + this.deleteAuth(member); + } + } + } finally { + noAuthClient.close(); + this.deleteAuth(created); + } + } + @Test public void testSpecifiedGraphRouting() { assumeStandaloneMode(); @@ -411,6 +471,40 @@ public void testSpecifiedGraphRouting() { } } + private String createAuth(Deque created, String path, String body) { + String content = assertResponseStatus(201, client().post(path, body)); + String id = assertJsonContains(content, "id"); + created.push(new String[]{path, id}); + return id; + } + + private void deleteAuth(Deque created) { + // Attempt every deletion first, then fail on any record that was left behind + List failures = new ArrayList<>(); + while (!created.isEmpty()) { + String[] entry = created.pop(); + Response r = client().delete(entry[0], entry[1]); + if (r.getStatus() != 204) { + failures.add(String.format("%s/%s: %s %s", entry[0], entry[1], + r.getStatus(), r.readEntity(String.class))); + } + } + Assert.assertTrue("Failed to delete auth records: " + failures, failures.isEmpty()); + } + + private static String basicCredential(String user, String password) { + byte[] credential = (user + ":" + password).getBytes(StandardCharsets.UTF_8); + return Base64.getEncoder().encodeToString(credential); + } + + private static Response basicAuthCypher(RestClient client, String user, String password) { + // Built by hand: Jersey's HttpAuthenticationFeature encodes the credential as ISO-8859-1 + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.add(HttpHeaders.AUTHORIZATION, "Basic " + basicCredential(user, password)); + return client.target().path(PATH).queryParam("cypher", QUERY) + .request().headers(headers).get(); + } + private List query(String cypher) { return assertCypherSuccessData(assertResponseStatus(200, client().post(PATH, cypher))); } 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 0b6df93438..de01f7538e 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 @@ -17,7 +17,9 @@ package org.apache.hugegraph.api; +import java.nio.charset.StandardCharsets; import java.nio.file.Paths; +import java.util.Base64; import java.util.Map; import org.apache.hugegraph.testutil.Assert; @@ -136,6 +138,63 @@ public void testVerify() { assertResponseStatus(401, r); } + @Test + public void testBasicAuthWithNonAsciiOrColonPassword() { + // RFC 7617: the credential is UTF-8 and only the first colon separates the fields + String[][] users = {{"utf8user", "ädminpass1"}, + {"coloned", "new:pass1234"}}; + RestClient noAuthClient = new RestClient(baseUrl(), false); + try { + for (String[] user : users) { + Response r = this.createUser(user[0], user[1]); + String result = assertResponseStatus(201, r); + Map created = JsonUtil.fromJson( + result, new TypeReference>() {}); + String id = (String) created.get("id"); + try { + r = basicAuthGet(noAuthClient, user[0] + ":" + user[1]); + assertResponseStatus(200, r); + + r = basicAuthGet(noAuthClient, user[0] + ":wrong" + user[1]); + assertResponseStatus(401, r); + } finally { + assertResponseStatus(204, this.deleteUser(id)); + } + } + } finally { + noAuthClient.close(); + } + } + + @Test + public void testBasicAuthWithMalformedCredential() { + RestClient noAuthClient = new RestClient(baseUrl(), false); + try { + assertResponseStatus(200, basicAuthGet(noAuthClient, "test:test")); + + // No colon, an empty user-id or an empty password + for (String credential : new String[]{"admin", "admin:", ":pa"}) { + assertResponseStatus(400, basicAuthGet(noAuthClient, credential)); + } + + // Only the first colon separates the fields, so a trailing colon belongs to + // the password + assertResponseStatus(401, basicAuthGet(noAuthClient, "test:test:")); + assertResponseStatus(401, basicAuthGet(noAuthClient, "test::")); + } finally { + noAuthClient.close(); + } + } + + private static Response basicAuthGet(RestClient client, String userPass) { + // Built by hand: Jersey's HttpAuthenticationFeature encodes the credential as ISO-8859-1 + byte[] credential = userPass.getBytes(StandardCharsets.UTF_8); + MultivaluedMap headers = new MultivaluedHashMap<>(); + headers.add(HttpHeaders.AUTHORIZATION, + "Basic " + Base64.getEncoder().encodeToString(credential)); + return client.get("graphspaces/DEFAULT/graphs", headers); + } + private Response createUser(String name, String password) { String user = "{\"user_name\":\"%s\",\"user_password\":\"%s" + "\",\"user_email\":\"user1@baidu.com\"," +