imbajin commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3697167925
##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -83,6 +103,74 @@ public static void main(String[] args) throws Exception {
}
}
+ /**
+ * Skipping leaves the built-in admin to
GraphManager.initAdminUserIfNeeded()
+ * on the PD startup path, which writes it to PD metadata. Only an HStore
+ * auth graph reads that metadata back, so every other local built-in-auth
+ * configuration would start a server nobody can log in to. Remote auth and
+ * custom authenticators keep their identities elsewhere and are exempt.
+ */
+ private static void checkAdminBootstrapReachable(HugeConfig conf,
+ String restConf) {
+ if (!requiresLocalBuiltinAdmin(conf)) {
+ return;
+ }
+ if (!conf.get(ServerOptions.USE_PD)) {
Review Comment:
‼️ This validation still permits a fresh built-in-auth deployment to create
the public `admin/pa` credential. Once `usePD=true` and the auth graph is
HStore, startup reaches `GraphManager.loadMetaFromPD()`, which calls
`initAdminUserIfNeeded(conf.get(ServerOptions.ADMIN_PA))`; that option defaults
to `pa`, and this disabled path also discards Docker `PASSWORD`. The new
accepted-HStore test omits `auth.admin_pa`, so it currently pins the unsafe
case. Please fail closed unless an explicit non-empty admin password is
configured (or bootstrap a supplied secret securely), and add a regression
proving the default cannot create an administrator.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -74,9 +92,22 @@ if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
else
log "init hugegraph with auth mode"
./bin/enable-auth.sh
+ # init-store reads the password from stdin, and a disabled one returns
+ # before it gets there, so say plainly that PASSWORD is being dropped
+ case "${INIT_STORE_ENABLED}" in
+ n | f | no | off | false)
+ log "WARN: PASSWORD is ignored while init-store is disabled;" \
+ "the admin is created on the PD startup path from" \
+ "'auth.admin_pa', which defaults to the public value 'pa'"
;;
+ esac
echo "${PASSWORD}" | ./bin/init-store.sh
fi
- touch "${DOCKER_FOLDER}/${INIT_FLAG_FILE}"
+ # A disabled init-store initialized nothing, so recording it as done would
+ # stop a later re-enable from initializing.
+ case "${INIT_STORE_ENABLED}" in
+ n | f | no | off | false) ;;
+ *) touch "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ;;
Review Comment:
⚠️ This records initialization as complete when disablement comes from a
mounted config. With `HG_SERVER_INIT_STORE_ENABLED` unset, `INIT_STORE_ENABLED`
remains empty even if `rest-server.properties` contains
`init_store.enabled=false`; Java then returns successfully without
initializing, but this branch still touches `docker/init_complete`. Changing
the mounted property to `true` on a later restart will skip the real
initialization permanently. Please base marker creation on the effective parsed
property/result and add a mounted `false` → `true` lifecycle regression.
--
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]
