fix(3149): pass the oauth flags as an array, and verify the source exists
The login source was never registered on any boot since it landed: forgejo-sso-source[422]: Command error: unexpected arguments: profile, email, groups' The flags were built as a shell STRING and word-split at the call site. Splitting happens after quote removal, so the quotes inside the value are just characters: --scopes 'openid profile email groups' reached forgejo as four words, three of them unexpected and one carrying an apostrophe. escapeShellArg interpolated into a string that is later word-split is a no-op that looks exactly like protection. An array carries the argument boundaries instead of re-deriving them from whitespace, and the shellcheck disable goes with it. Also assert the effect: the unit now fails if the source is absent from 'admin auth list' afterwards. The old failure exited non-zero and was still invisible to every check that read the rendered script rather than its result.
This commit is contained in:
parent
3c262b6c1b
commit
71b0f1c5b8
1 changed files with 26 additions and 7 deletions
|
|
@ -808,26 +808,45 @@ in
|
||||||
# principals here, and root can read the resting copy
|
# principals here, and root can read the resting copy
|
||||||
# anyway. Accepted deliberately (see docs/swarm/), not
|
# anyway. Accepted deliberately (see docs/swarm/), not
|
||||||
# overlooked; upstream gap filed.
|
# overlooked; upstream gap filed.
|
||||||
args="--provider openidConnect \
|
# An ARRAY, not a string. A string of arguments has to be
|
||||||
--key ${lib.escapeShellArg cfg.sso.clientId} \
|
# word-split at the call site to become argv, and splitting
|
||||||
--auto-discover-url ${lib.escapeShellArg autheliaDiscoveryUrl} \
|
# does not honour the quotes inside the value — the shell
|
||||||
--scopes ${lib.escapeShellArg "openid profile email groups"}"
|
# already finished quote removal by then, so
|
||||||
|
# `--scopes 'openid profile email groups'` arrives as four
|
||||||
|
# words with two stray apostrophes, and forgejo rejects the
|
||||||
|
# last three as unexpected arguments. An array carries the
|
||||||
|
# boundaries instead of re-deriving them from whitespace.
|
||||||
|
args=(
|
||||||
|
--provider openidConnect
|
||||||
|
--key ${lib.escapeShellArg cfg.sso.clientId}
|
||||||
|
--auto-discover-url ${lib.escapeShellArg autheliaDiscoveryUrl}
|
||||||
|
--scopes ${lib.escapeShellArg "openid profile email groups"}
|
||||||
|
)
|
||||||
|
|
||||||
# shellcheck disable=SC2086
|
|
||||||
if forgejo admin auth list | grep -q "[[:space:]]${ssoSourceName}[[:space:]]"; then
|
if forgejo admin auth list | grep -q "[[:space:]]${ssoSourceName}[[:space:]]"; then
|
||||||
id=$(forgejo admin auth list \
|
id=$(forgejo admin auth list \
|
||||||
| grep "[[:space:]]${ssoSourceName}[[:space:]]" \
|
| grep "[[:space:]]${ssoSourceName}[[:space:]]" \
|
||||||
| cut -f1)
|
| cut -f1)
|
||||||
forgejo admin auth update-oauth --id "$id" \
|
forgejo admin auth update-oauth --id "$id" \
|
||||||
--name ${lib.escapeShellArg ssoSourceName} \
|
--name ${lib.escapeShellArg ssoSourceName} \
|
||||||
--secret "$secret" $args
|
--secret "$secret" "''${args[@]}"
|
||||||
echo "updated OIDC login source ${ssoSourceName} (id $id)"
|
echo "updated OIDC login source ${ssoSourceName} (id $id)"
|
||||||
else
|
else
|
||||||
forgejo admin auth add-oauth \
|
forgejo admin auth add-oauth \
|
||||||
--name ${lib.escapeShellArg ssoSourceName} \
|
--name ${lib.escapeShellArg ssoSourceName} \
|
||||||
--secret "$secret" $args
|
--secret "$secret" "''${args[@]}"
|
||||||
echo "added OIDC login source ${ssoSourceName}"
|
echo "added OIDC login source ${ssoSourceName}"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Assert the EFFECT, not the command. The bug this replaced
|
||||||
|
# was a malformed argv: the unit ran the right verb, forgejo
|
||||||
|
# rejected it, and every check that read the rendered script
|
||||||
|
# still passed. A registration that does not appear in the
|
||||||
|
# source list did not happen, whatever the exit code said.
|
||||||
|
if ! forgejo admin auth list | grep -q "[[:space:]]${ssoSourceName}[[:space:]]"; then
|
||||||
|
echo "login source ${ssoSourceName} is absent after registration" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
'';
|
'';
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue