bitflicker64 commented on code in PR #3189: URL: https://github.com/apache/hugegraph/pull/3189#discussion_r3939639187
########## docker/conf/hubble/hstore.properties.example: ########## @@ -20,6 +20,11 @@ pd.enabled=true server.direct_url=http://server:8080 pd.peers=pd:8686 pd.server=pd:8620 +# PD REST credential. The password must equal PD's auth.secret-key, which has +# no default: set it to the same value as HG_PD_AUTH_SECRET_KEY in .env. While +# it is empty, Hubble's PD-backed views get HTTP 401 from PD. +operations.pd.username=hubble +operations.pd.password= Review Comment: Fixed in e964b68, following your layout. The tracked files are now `conf/hubble/hstore.properties.example` and `hstore-ha.properties.example`; both Compose files mount `<name>.local.properties`; `docker/conf/hubble/*.local.properties` is in `.gitignore` next to `docker/.env`. `set-hubble-pd-password.sh <name> [secret]` generates the local file from the example (no sed, backslashes doubled, mode 644 for the read-only mount, refuses empty or line-break secrets). The `.env` recipe and the deployment guide run it before `up`, and `test-compose.sh` generates both before every render and smoke, then restores a developer's own local files. Render passes with the CI secret and with `a&b#c\d`. ########## hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh: ########## @@ -101,10 +119,12 @@ if env | grep '^hugegraph\.' > /dev/null; then check_any_pd_stores() { for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do - if curl ${PD_AUTH_ARGS} -f -s \ + if printf 'user = \"%s:%s\"\n' \ + \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ + curl -K - -f -s \ Review Comment: Fixed in e964b68, taking the shape you sketched. curl now runs alone with `-w '\n%{http_code}'`, the code is split off the body, and a 401 logs `PD at <peer> refused the credential (401): PD_AUTH_PASSWORD must match PD's auth.secret-key` and aborts via a distinct return code, so the `until` loop exits instead of retrying; the outer handler now separates a real timeout (124) from an abort. The mock curl honours `-w`, and a new `auth-401` case asserts non-zero exit, the message, exactly one call, and no timeout wording; 7/7 pass. ########## hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java: ########## @@ -85,6 +98,17 @@ public class PDConfig { private ConfigService configService; private IdService idService; + @Override + public void afterPropertiesSet() { + if (PUBLISHED_SECRET_KEY.equals(this.secretKey)) { Review Comment: Fixed in e964b68, taking the log option rather than the hard gate. `PDConfig.afterPropertiesSet` now emits the same "auth.secret-key is not configured" ERROR when the key is empty, so it is in the boot log before any request. Verified on the packaged dist with the shipped config: the line appears at `[main]` during startup (it lands in `hugegraph-pd-stdout.log`, where the boot phase logs before the file appender takes over). Kept it a log rather than a throw so a tarball upgrade still starts for gRPC clients while the operator adds the key. ########## hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java: ########## @@ -59,6 +59,10 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons authority = authority.replace("Basic ", ""); return authenticate(authority, token, tokenCall, DEFAULT_HANDLE); } catch (Exception e) { + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + // RFC 7235 requires a challenge on a 401; without it clients that + // authenticate reactively never retry with credentials + response.setHeader("WWW-Authenticate", "Basic realm=\"hugegraph-pd\""); response.setContentType("application/json"); response.getWriter().println(new API().toJSON(e)); Review Comment: Fixed in e964b68. The 401 body is now the constant `{"status":-1,"error":"Unauthorized"}` for every refusal, and the reason (missing header, unknown service name, wrong password) goes to `log.debug` with the method and path. Verified on the dist: wrong password, unknown name and no header all return the identical body with the challenge header. Debug rather than warn since the path is reachable unauthenticated and I did not want a log-flood lever; the one-shot boot and first-use ERRORs still cover the misconfiguration case. -- 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]
