Adarsh-Me commented on code in PR #3192:
URL: https://github.com/apache/hugegraph/pull/3192#discussion_r3995734426
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh:
##########
@@ -41,16 +41,43 @@ if [ ! -d "$BAK_CONF" ]; then
cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak"
cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak"
cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak"
+fi
+
+# The appends below are guarded per file and match only an absent or still
+# commented-out definition, so they are no-ops on any config that already
+# carries authentication (e.g. a mounted one, or a re-run of this script).
+# The guards accept every spelling java.util.Properties reads as the key —
+# '=' or ':' or bare-whitespace separators, leading whitespace and
+# backslash-escaped dots — and the gremlin.graph flip tolerates CRLF
+# endings, which a mounted config saved on Windows carries. Appending
+# unconditionally used to create duplicate definitions that the
+# properties parser (first definition wins) and the yaml parser (last wins)
+# resolved in opposite directions, leaving Gremlin and REST on different
+# authenticators.
+AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}"
+if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:'
"${CONF}/${GREMLIN_SERVER_CONF}"; then
sed -i -e '$a\authentication: {' \
- -e '$a\ authenticator:
org.apache.hugegraph.auth.StandardAuthenticator,' \
+ -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \
-e '$a\ authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
-e '$a\ config: {tokens: conf/rest-server.properties}' \
-e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF}
+fi
- sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \
- -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+if ! grep -Eq
'^[[:blank:]]*auth[\\]?\.authenticator[[:blank:]]*([:=]|[[:blank:]])'
"${CONF}/${REST_SERVER_CONF}"; then
+ sed -i -e "\$a\\auth.authenticator=${AUTHENTICATOR_CLASS}"
${CONF}/${REST_SERVER_CONF}
+fi
+
+if ! grep -Eq
'^[[:blank:]]*auth[\\]?\.graph_store[[:blank:]]*([:=]|[[:blank:]])'
"${CONF}/${REST_SERVER_CONF}"; then
+ sed -i -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+fi
- sed -i
's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g'
${CONF}/graphs/${GRAPH_CONF}
+# GNU grep reads \r in a pattern as the letter r, so the carriage return a
+# CRLF line ends with is embedded as a byte: without it the anchored guard
+# misses a mounted CRLF config and the factory is never wrapped for auth
+# although both servers already believe authentication is on.
+CR=$'\r'
+if grep -Eq
"^gremlin\\.graph[[:blank:]]*=org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?\$"
"${CONF}/graphs/${GRAPH_CONF}"; then
Review Comment:
Fixed in 5f5051130d29089e34fec1d611d52efa5ba566b6. Guard and sed widened
together: leading blanks, escaped dot, colon/equals/bare-whitespace separators,
optional CR via the $CR byte variable. Verified against 8 spellings plus CRLF
(all rewrite), already-proxied and commented lines (both skipped), CR preserved
by the prefix-only sed. Suite passes.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -70,12 +74,108 @@ set_prop() {
get_prop_encoded() {
local key="$1" file="$2"
- local esc_key
- esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
- sed -nE \
-
"s~^[[:space:]]*${esc_key}([[:space:]]*[:=][[:space:]]*|[[:space:]]+)(.*)$~\\2~p"
\
- "${file}" | head -n 1
+ PROPS_MODE=get PROPS_KEY="${key}" PROPS_FILE="${file}" \
+ awk -f "${PROPS_AWK}" /dev/null
+}
+
+# First uncommented `authenticator:` inside the gremlin-server.yaml
+# authentication block, or on the `authentication:` line itself (a flow
+# mapping). snakeyaml resolves duplicate top-level keys to the last one,
+# but a mounted file carrying two authentication blocks is pathological;
+# report the first and let the mismatch WARN handle it. The scalar is
+# cleaned the way snakeyaml reads it — an inline comment (a '#' preceded
+# by whitespace), surrounding quotes and padding are stripped — because
+# java.util.Properties keeps all of those in the class name.
+get_yaml_authenticator() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 0
+ awk '
+ function scalar(s, out, i, n, c, q) {
+ out = ""
+ q = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (q != "") {
+ if (c == q) q = ""
+ else out = out c
+ continue
+ }
+ if (c == "\"" || c == "\047") { q = c; continue }
+ if (c == "#" &&
+ (out == "" || substr(out, length(out), 1) ~ /[ \t]/))
+ break
+ if (c == "," || c == "}" || c == "]") break
+ out = out c
+ }
+ sub(/^[ \t\r]+/, "", out)
+ sub(/[ \t\r]+$/, "", out)
+ return out
+ }
+ /^[ \t]*#/ { next }
+ /^[ \t]*authentication[ \t]*:/ {
+ inblk = 1
+ line = $0
+ sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line)
+ if (match(line, /authenticator[ \t]*:/)) {
+ print scalar(substr(line, RSTART + RLENGTH))
+ exit
+ }
+ next
+ }
+ inblk && /^[ \t]+authenticator[ \t]*:/ {
+ line = $0
+ sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line)
+ print scalar(line)
+ exit
+ }
+ ' "${yaml}"
+}
+
+# A mounted yaml can carry an authentication block whose authenticator
+# cannot be read (an empty or unparseable one). That is not the
+# both-empty case: exporting the default would override an explicit
+# choice that snakeyaml does resolve, so callers treat it as a mismatch.
+has_yaml_authentication_block() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 1
+ grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${yaml}"
+}
+
+# enable-auth.sh appends definitions to files it did not write. On a
+# mounted config those appended definitions are duplicates the two parsers
+# resolve in opposite directions — HugeConfig (commons-configuration) takes
+# the first, snakeyaml takes the last — so Gremlin and REST can land on
+# different authenticators with no error from either. Normalize both sides
+# to one definition of the same authenticator here; enable-auth.sh's
+# per-file guards then make its appends no-ops on anything already set.
+align_auth_config() {
+ local rest_auth yaml_auth
+
+ rest_auth=$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}")
+ yaml_auth=$(get_yaml_authenticator)
+ if [[ -z "${yaml_auth}" ]] && has_yaml_authentication_block; then
+ log "WARN: gremlin-server.yaml carries an authentication block" \
+ "without a readable authenticator; leaving both sides untouched"
+ return
+ fi
+ if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" !=
"${yaml_auth}" ]]; then
Review Comment:
Fixed in 5f5051130d29089e34fec1d611d52efa5ba566b6. align_auth_config now
reads the authenticator via a get-decoded mode (new get_prop wrapper) that
unescapes like java.util.Properties before comparing with the snakeyaml scalar,
so an escaped properties value and a plain yaml scalar no longer WARN; the
yaml-to-properties write on line 173 goes through the encoding setter set_prop.
Raw mode kept for the token-secret round trip. Covered by an
escaped-authenticator regression case; suite passes.
--
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]
