refactor(#3354): make the derived hive clients a definition of the option
The client list was `cfg.oidc.clients ++ hiveClients`, where the first half comes through the submodule and the second was a raw attrset from this module's `let` block. That list is only half-typed: a field added to the submodule exists on the declared entries and not on the derived ones, so reading it plainly is an eval error the moment hive identities are on. The operator asked whether it should be uniformly typed instead of guarding each read, and it should. The hive identities are now declared the same way an operator declares a client, so the module system applies the submodule to them and every option's default is present. Downstream reads one uniformly-typed list and the guard added for the field that broke is gone with it.
This commit is contained in:
parent
f5fdbaae1a
commit
fd4b79f04a
1 changed files with 35 additions and 27 deletions
|
|
@ -79,6 +79,14 @@ let
|
||||||
# per service from it. Were the queue to declare this list, the next
|
# per service from it. Were the queue to declare this list, the next
|
||||||
# consumer would collide on the same client id — and only at the
|
# consumer would collide on the same client id — and only at the
|
||||||
# moment it landed.
|
# moment it landed.
|
||||||
|
#
|
||||||
|
# Fed to the option as a DEFINITION in the config block below, rather
|
||||||
|
# than appended to the declared list downstream. That is what puts it
|
||||||
|
# through the submodule: one list, one type, every option's default
|
||||||
|
# present. Appending a raw attrset instead left the list half-typed —
|
||||||
|
# and a field later added to the submodule then existed on the
|
||||||
|
# declared entries and not on these, which is an eval error reachable
|
||||||
|
# only once hive identities are on.
|
||||||
hiveClients = lib.mapAttrsToList (name: _: {
|
hiveClients = lib.mapAttrsToList (name: _: {
|
||||||
id = "hive-${name}";
|
id = "hive-${name}";
|
||||||
description = "HyperHive hive ${name}";
|
description = "HyperHive hive ${name}";
|
||||||
|
|
@ -86,21 +94,18 @@ let
|
||||||
redirectUris = [ ];
|
redirectUris = [ ];
|
||||||
}) hyperhiveCfg.swarm.hives;
|
}) hyperhiveCfg.swarm.hives;
|
||||||
|
|
||||||
# Everything downstream renders and mints from this, not from the
|
|
||||||
# declared list alone.
|
|
||||||
allClients = cfg.oidc.clients ++ lib.optionals cfg.oidc.hiveIdentities hiveClients;
|
|
||||||
|
|
||||||
# authelia refuses to start with an OIDC provider that has no clients,
|
# authelia refuses to start with an OIDC provider that has no clients,
|
||||||
# so the provider is derived from the client list rather than carrying
|
# so the provider is derived from the client list rather than carrying
|
||||||
# its own `enable`: one fact, and it cannot contradict itself. An empty
|
# its own `enable`: one fact, and it cannot contradict itself. An empty
|
||||||
# list is the default, which makes every hive that has not opted in
|
# list is the default, which makes every hive that has not opted in
|
||||||
# byte-identical to before.
|
# byte-identical to before.
|
||||||
#
|
#
|
||||||
# ⚠️ Derived from `allClients`, so hive identities can turn the provider
|
# ⚠️ The derived hive identities are definitions of this same option,
|
||||||
# on by themselves. That is only reachable where the queue is already
|
# so they can turn the provider on by themselves. That is only
|
||||||
# enabled (`hiveIdentities` defaults to it) — and a queue-enabled hive
|
# reachable where the queue is already enabled (`hiveIdentities`
|
||||||
# already contributes a client, so no existing deployment flips.
|
# defaults to it) — and a queue-enabled hive already contributes a
|
||||||
oidcEnabled = allClients != [ ];
|
# client, so no existing deployment flips.
|
||||||
|
oidcEnabled = cfg.oidc.clients != [ ];
|
||||||
|
|
||||||
# Secrets that are 64 random bytes of hex and nothing more. The OIDC
|
# Secrets that are 64 random bytes of hex and nothing more. The OIDC
|
||||||
# hmac key joins them; the issuer key does not (see below — it is RSA).
|
# hmac key joins them; the issuer key does not (see below — it is RSA).
|
||||||
|
|
@ -144,19 +149,12 @@ let
|
||||||
printf -- " client_secret: '%s'\n" "$(cat ${lib.escapeShellArg "${clientsDir}/${c.id}.digest"})"
|
printf -- " client_secret: '%s'\n" "$(cat ${lib.escapeShellArg "${clientsDir}/${c.id}.digest"})"
|
||||||
printf -- ' authorization_policy: one_factor\n'
|
printf -- ' authorization_policy: one_factor\n'
|
||||||
''
|
''
|
||||||
# ⚠️ `or null`, and it is not defensive noise: `allClients` is NOT
|
# Read plainly, and that is a property of the list rather than of
|
||||||
# uniformly submodule-typed. `cfg.oidc.clients` entries come through
|
# this line: every entry reaching here is a definition of
|
||||||
# the submodule and carry every option's default; `hiveClients` is a
|
# `oidc.clients`, so the module system has applied the submodule and
|
||||||
# raw attrset built in this file's `let` block and carries only the
|
# each option's default is present. A derived entry that names only
|
||||||
# four fields written there. So a field added to the submodule exists
|
# the fields it cares about still arrives with the rest filled in.
|
||||||
# on one half of this list and not the other, and reading it plainly
|
+ lib.optionalString (c.tokenEndpointAuthMethod != null) ''
|
||||||
# is an eval error the moment `hiveIdentities` is on.
|
|
||||||
#
|
|
||||||
# That is exactly how this broke: `tokenEndpointAuthMethod` was added
|
|
||||||
# to the submodule and read here, which is fine for a declared client
|
|
||||||
# and fatal for a derived one. Anything read here needs `or` unless
|
|
||||||
# every producer is known to set it.
|
|
||||||
+ lib.optionalString ((c.tokenEndpointAuthMethod or null) != null) ''
|
|
||||||
printf -- ' token_endpoint_auth_method: %s\n' ${lib.escapeShellArg c.tokenEndpointAuthMethod}
|
printf -- ' token_endpoint_auth_method: %s\n' ${lib.escapeShellArg c.tokenEndpointAuthMethod}
|
||||||
''
|
''
|
||||||
+ (
|
+ (
|
||||||
|
|
@ -231,7 +229,7 @@ let
|
||||||
|
|
||||||
${lib.concatMapStrings (c: ''
|
${lib.concatMapStrings (c: ''
|
||||||
mint ${lib.escapeShellArg c.id}
|
mint ${lib.escapeShellArg c.id}
|
||||||
'') allClients}
|
'') cfg.oidc.clients}
|
||||||
|
|
||||||
# Re-rendered every boot, deliberately: the secret is minted once,
|
# Re-rendered every boot, deliberately: the secret is minted once,
|
||||||
# but the metadata around it (a new redirect URI, a renamed client)
|
# but the metadata around it (a new redirect URI, a renamed client)
|
||||||
|
|
@ -242,7 +240,7 @@ let
|
||||||
printf -- 'identity_providers:\n'
|
printf -- 'identity_providers:\n'
|
||||||
printf -- ' oidc:\n'
|
printf -- ' oidc:\n'
|
||||||
printf -- ' clients:\n'
|
printf -- ' clients:\n'
|
||||||
${lib.concatMapStrings renderClient allClients}
|
${lib.concatMapStrings renderClient cfg.oidc.clients}
|
||||||
} > ${lib.escapeShellArg "${clientsFile}.tmp"}
|
} > ${lib.escapeShellArg "${clientsFile}.tmp"}
|
||||||
chmod 0600 ${lib.escapeShellArg "${clientsFile}.tmp"}
|
chmod 0600 ${lib.escapeShellArg "${clientsFile}.tmp"}
|
||||||
mv ${lib.escapeShellArg "${clientsFile}.tmp"} ${lib.escapeShellArg clientsFile}
|
mv ${lib.escapeShellArg "${clientsFile}.tmp"} ${lib.escapeShellArg clientsFile}
|
||||||
|
|
@ -550,6 +548,14 @@ in
|
||||||
};
|
};
|
||||||
|
|
||||||
config = lib.mkIf (hyperhiveCfg.enable && cfg.enable) {
|
config = lib.mkIf (hyperhiveCfg.enable && cfg.enable) {
|
||||||
|
# The derived half of the client list, declared the same way an
|
||||||
|
# operator declares one. Everything downstream then reads a single
|
||||||
|
# uniformly-typed `cfg.oidc.clients` and cannot tell the two apart —
|
||||||
|
# including the assertions below, which is why a hive named `x`
|
||||||
|
# colliding with a declared `hive-x` is caught rather than rendered
|
||||||
|
# twice.
|
||||||
|
services.hyperhive.swarm.authelia.oidc.clients = lib.mkIf cfg.oidc.hiveIdentities hiveClients;
|
||||||
|
|
||||||
# A redirect URI on a machine client is not harmless-but-unused: it
|
# A redirect URI on a machine client is not harmless-but-unused: it
|
||||||
# means whoever wrote it believes a browser is involved. Failing here
|
# means whoever wrote it believes a browser is involved. Failing here
|
||||||
# is how that belief gets corrected at the point it was expressed,
|
# is how that belief gets corrected at the point it was expressed,
|
||||||
|
|
@ -562,7 +568,7 @@ in
|
||||||
+ "kind = \"machine\" but declares redirectUris. A client_credentials "
|
+ "kind = \"machine\" but declares redirectUris. A client_credentials "
|
||||||
+ "client has nobody to redirect; drop the URIs or make it "
|
+ "client has nobody to redirect; drop the URIs or make it "
|
||||||
+ "kind = \"interactive\".";
|
+ "kind = \"interactive\".";
|
||||||
}) allClients
|
}) cfg.oidc.clients
|
||||||
# Two clients sharing an id renders two YAML entries under one name.
|
# Two clients sharing an id renders two YAML entries under one name.
|
||||||
# Newly reachable now that part of the list is DERIVED: a hive called
|
# Newly reachable now that part of the list is DERIVED: a hive called
|
||||||
# `x` and a service client called `hive-x` never met before. Authelia
|
# `x` and a service client called `hive-x` never met before. Authelia
|
||||||
|
|
@ -570,12 +576,14 @@ in
|
||||||
# sources here is the cheaper failure.
|
# sources here is the cheaper failure.
|
||||||
++ [
|
++ [
|
||||||
{
|
{
|
||||||
assertion = lib.length (lib.unique (map (c: c.id) allClients)) == lib.length allClients;
|
assertion = lib.length (lib.unique (map (c: c.id) cfg.oidc.clients)) == lib.length cfg.oidc.clients;
|
||||||
message =
|
message =
|
||||||
"services.hyperhive.swarm.authelia: duplicate OIDC client id(s): "
|
"services.hyperhive.swarm.authelia: duplicate OIDC client id(s): "
|
||||||
+ lib.concatStringsSep ", " (
|
+ lib.concatStringsSep ", " (
|
||||||
lib.unique (
|
lib.unique (
|
||||||
lib.filter (id: lib.count (x: x == id) (map (c: c.id) allClients) > 1) (map (c: c.id) allClients)
|
lib.filter (id: lib.count (x: x == id) (map (c: c.id) cfg.oidc.clients) > 1) (
|
||||||
|
map (c: c.id) cfg.oidc.clients
|
||||||
|
)
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
+ ". Hive identities are named `hive-<name>` from "
|
+ ". Hive identities are named `hive-<name>` from "
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue