arshilkxwork commented on code in PR #3290:
URL: https://github.com/apache/hugegraph/pull/3290#discussion_r4251183024
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/CypherApiTest.java:
##########
@@ -411,6 +471,33 @@ public void testSpecifiedGraphRouting() {
}
}
+ private String createAuth(Deque<String[]> 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<String[]> created) {
+ while (!created.isEmpty()) {
+ String[] entry = created.pop();
+ client().delete(entry[0], entry[1]);
Review Comment:
Thank you. In c9339f0, `deleteAuth` attempts every deletion and then fails
the test if any of them did not return 204, listing the affected records. A
cleanup failure is therefore no longer silent, and the remaining records are
still removed. `testBasicAuthWithNonAsciiOrColonPassword` in `LoginApiTest` now
asserts 204 on its user deletion for the same reason.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java:
##########
@@ -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) {
Review Comment:
Thank you. c9339f0 adds `LoginApiTest#testBasicAuthWithMalformedCredential`,
which sends hand-built headers. After a control request with `test:test` that
returns 200, it expects 400 for `admin`, `admin:` and `:pa`, and 401 for
`test:test:` and `test::`, using the user the class creates in `setup()`. The
two trailing-colon cases correspond to the compatibility notes in the
description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
