feat(3201): swarmctl user update — change an existing subject's attributes

`user add` refuses on an existing name, so the `--group` flag it takes at
creation time could not be added afterwards at all: repairing an account
meant hand-editing both users.json and the rendered users.yml as root.
mara, on #3167: "i will not edit those files by hand, we will have the
same issues elsewhere".

The merge rules live in users.rs as a pure function over a UserUpdate, so
they are testable without a command line, a container or a running
authelia — main.rs's arm only loads, applies, publishes and prints.

Removals are strict and everything else is idempotent, which is the one
asymmetry here and is deliberate: a --remove-group naming a group the
user does not have fails, because a revocation that reports success
without revoking is the outcome nobody re-checks; while refusing an
already-satisfied set would make the multi-attribute call this verb
exists for break whenever one of the values was already right.

A command that changes nothing at all still fails — it would otherwise
rewrite both files and restart the SSO provider to no effect.

Passwords are out of scope: regenerating a credential is a different
intent from editing an attribute, and folded together an attribute edit
can invalidate a login by accident.

Extracts publish() from user_add so both verbs share the
render -> store -> users.yml -> restart ordering and the comment that
explains why that order, rather than the second verb copying it.

docs/swarm/sso.md

@@ -57,6 +57,37 @@ non-interactively means a secret arriving from somewhere — a file, an
 env var, a nix expression — and every one of those is worse than an
 operator typing one command once.
 
+### Changing a subject afterwards
+
+`user add` only ever adds: on a name that already exists it refuses,
+rather than resurfacing as a second account or a silent overwrite.
+Editing an existing subject is `user update`, and the flags compose, so
+one call can change several things:
+
+```console
+# swarmctl user update mara --add-group admins --email mara@example.com
+added to group "admins"
+email: unset -> "mara@example.com"
+mara is now in groups: admins
+```
+
+Two behaviours worth knowing before you rely on them:
+
+- **`--remove-group` fails if the user is not in that group.** Every
+  other flag is idempotent — setting what is already set is fine, so a
+  "make these four things true" call does not break when one of them
+  already was. Revocation is the exception on purpose: a typo'd group
+  name that reported success would leave an account holding access you
+  believe you took away, and that is the one outcome nobody re-checks.
+- **The resulting group list is printed** because group names have no
+  registry anywhere. A misspelled `--add-group` creates a real group that
+  no access-control rule mentions, so the user gains nothing and no error
+  is possible — reading the line back is the only check there is.
+
+Passwords are deliberately out of scope here: regenerating a credential
+is a different intent from editing an attribute, and folding them means
+an attribute edit can invalidate a login by accident.
+
 ## What secrets exist, and where each one lives
 
 | secret | generated by | rests in | read by |

docs/tools/swarmctl-cli.md

@@ -7,6 +7,7 @@ This document contains the help content for the `swarmctl` command-line program.
 * [`swarmctl`↴](#swarmctl)
 * [`swarmctl user`↴](#swarmctl-user)
 * [`swarmctl user add`↴](#swarmctl-user-add)
+* [`swarmctl user update`↴](#swarmctl-user-update)
 
 ## `swarmctl`
 
@@ -37,6 +38,7 @@ Manage subjects in the swarm's SSO provider
 ###### **Subcommands:**
 
 * `add` — Add a user, generating a password for them
+* `update` — Change an existing user's attributes
 
 
 
@@ -58,6 +60,27 @@ Add a user, generating a password for them
 
 
 
+## `swarmctl user update`
+
+Change an existing user's attributes.
+
+Every flag is optional and they compose, so one call can set several things at once. Deliberately does **not** touch the password: regenerating a credential is a different intent from editing an attribute, and folded together an attribute edit can invalidate a login by accident.
+
+**Usage:** `swarmctl user update [OPTIONS] <USERNAME>`
+
+###### **Arguments:**
+
+* `<USERNAME>` — Login name of an existing user
+
+###### **Options:**
+
+* `--display-name <TEXT>` — Name shown in the SSO UI
+* `--email <ADDRESS>`
+* `--add-group <GROUP>` — Repeatable. Adding a group the user is already in is not an error
+* `--remove-group <GROUP>` — Repeatable. Fails if the user is not in the group — a revocation that reports success without revoking is the failure nobody re-checks
+
+
+
 <hr/>
 
 <small><i>

swarmctl/src/main.rs

@@ -149,6 +149,14 @@ enum Verb {
 enum UserVerb {
     /// Add a user, generating a password for them.
     Add(AddArgs),
+    /// Change an existing user's attributes.
+    ///
+    /// Every flag is optional and they compose, so one call can set
+    /// several things at once. Deliberately does **not** touch the
+    /// password: regenerating a credential is a different intent from
+    /// editing an attribute, and folded together an attribute edit can
+    /// invalidate a login by accident.
+    Update(UpdateArgs),
 }
 
 #[derive(Args)]
@@ -166,6 +174,25 @@ struct AddArgs {
     groups: Vec<String>,
 }
 
+#[derive(Args)]
+struct UpdateArgs {
+    /// Login name of an existing user.
+    username: String,
+    /// Name shown in the SSO UI.
+    #[arg(long, value_name = "TEXT")]
+    display_name: Option<String>,
+    #[arg(long, value_name = "ADDRESS")]
+    email: Option<String>,
+    /// Repeatable. Adding a group the user is already in is not an error.
+    #[arg(long = "add-group", value_name = "GROUP")]
+    add_groups: Vec<String>,
+    /// Repeatable. Fails if the user is not in the group — a revocation
+    /// that reports success without revoking is the failure nobody
+    /// re-checks.
+    #[arg(long = "remove-group", value_name = "GROUP")]
+    remove_groups: Vec<String>,
+}
+
 fn main() -> Result<()> {
     let Cli { paths, command } = Cli::parse();
     match command {
@@ -175,6 +202,9 @@ fn main() -> Result<()> {
         Verb::User {
             command: UserVerb::Add(args),
         } => user_add(&paths.resolve()?, args),
+        Verb::User {
+            command: UserVerb::Update(args),
+        } => user_update(&paths.resolve()?, args),
         Verb::MarkdownDocs => {
             print!("{}", clap_markdown::help_markdown::<Cli>());
             Ok(())
@@ -206,11 +236,59 @@ fn user_add(paths: &Paths, args: AddArgs) -> Result<()> {
         },
     );
 
+    publish(paths, &store)?;
+
+    println!("added {} to {}", args.username, paths.users_file.display());
+    println!("password: {}", generated.password);
+    println!("this password is stored nowhere — record it now");
+    Ok(())
+}
+
+fn user_update(paths: &Paths, args: UpdateArgs) -> Result<()> {
+    let mut store = load_store(&paths.store, &paths.users_file)?;
+    let Some(user) = store.users.get_mut(&args.username) else {
+        bail!(
+            "no user {:?} in {} — `swarmctl user add` creates one",
+            args.username,
+            paths.store.display()
+        );
+    };
+
+    let changes = users::apply_update(
+        user,
+        &users::UserUpdate {
+            displayname: args.display_name,
+            email: args.email,
+            add_groups: args.add_groups,
+            remove_groups: args.remove_groups,
+        },
+    )?;
+    // Read back before the borrow ends: this is what the operator gets
+    // instead of a group registry we don't have — a typo'd `--add-group`
+    // is a real group with nobody reading it, and seeing the resulting
+    // list is the only way to notice.
+    let groups = users::fmt_groups(&user.groups);
+
+    publish(paths, &store)?;
+
+    for change in &changes {
+        println!("{change}");
+    }
+    println!("{} is now in groups: {groups}", args.username);
+    Ok(())
+}
+
+/// Write the store + the rendered users file, then restart authelia.
+///
+/// Shared by every verb that mutates the store, so the ordering rules
+/// below hold for all of them rather than for whichever one was written
+/// first.
+fn publish(paths: &Paths, store: &UserStore) -> Result<()> {
     // Render before writing anything: a value this refuses to emit should
     // stop the whole operation, not leave the canonical store one user
     // ahead of the file authelia reads.
-    let rendered = users::render_yaml(&store)?;
-    let store_json = serde_json::to_string_pretty(&store).context("serialising the user store")?;
+    let rendered = users::render_yaml(store)?;
+    let store_json = serde_json::to_string_pretty(store).context("serialising the user store")?;
 
     // Store first, and the order matters. If the store lands and the
     // users file does not, the next run re-renders and repairs it. The
@@ -219,12 +297,7 @@ fn user_add(paths: &Paths, args: AddArgs) -> Result<()> {
     write_atomic(&paths.store, &format!("{store_json}\n"))?;
     write_atomic(&paths.users_file, &rendered)?;
 
-    restart_authelia(&paths.machine, &paths.unit)?;
-
-    println!("added {} to {}", args.username, paths.users_file.display());
-    println!("password: {}", generated.password);
-    println!("this password is stored nowhere — record it now");
-    Ok(())
+    restart_authelia(&paths.machine, &paths.unit)
 }
 
 /// Load the canonical store, or start an empty one if this deployment has

swarmctl/src/users.rs

@@ -157,6 +157,117 @@ pub fn render_yaml(store: &UserStore) -> Result<String> {
     Ok(out)
 }
 
+/// A requested change to an existing user.
+///
+/// A plain struct rather than the CLI's `Args` type, so the merge rules in
+/// [`apply_update`] can be tested without building a command line — and so
+/// the single place that decides what *update* means has no opinion about
+/// how it is spelled.
+#[derive(Debug, Default)]
+pub struct UserUpdate {
+    pub displayname: Option<String>,
+    pub email: Option<String>,
+    pub add_groups: Vec<String>,
+    pub remove_groups: Vec<String>,
+}
+
+/// Apply `update` to `user`, returning one line per change actually made.
+///
+/// **Removals are strict; everything else is idempotent.** That asymmetry
+/// is the whole safety argument of this function, so it is deliberate
+/// rather than an oversight:
+///
+/// - `--remove-group` on a group the user does not have **fails**. A
+///   revocation that reports success without revoking is the one outcome
+///   here nobody re-checks — you typo the group, the command says ok, and
+///   the account keeps the access you believe you took away.
+/// - Setting an attribute to the value it already holds, or adding a group
+///   the user is already in, is **not** an error: the end state matches the
+///   intent, and refusing would make the multi-attribute call this verb
+///   exists for brittle — "set these four things" should not fail because
+///   one of them was already right.
+///
+/// A command that changes *nothing at all* still fails, because it would
+/// otherwise rewrite both files and restart the SSO provider to no effect.
+///
+/// On failure the `user` it was handed may be **partially mutated** — the
+/// guarantee is not in-place atomicity but that the caller publishes
+/// nothing on an error, so neither file and neither process ever sees a
+/// half-applied update. Say it plainly rather than implying a rollback
+/// this doesn't do.
+///
+/// Note what cannot be validated here: group names are free-form strings
+/// with no registry, so a typo'd `--add-group` creates a group nothing
+/// references, and the user silently gains no access. The caller prints the
+/// resulting group list for exactly that reason — it is the only signal
+/// available.
+pub fn apply_update(user: &mut User, update: &UserUpdate) -> Result<Vec<String>> {
+    if let Some(dup) = update
+        .add_groups
+        .iter()
+        .find(|g| update.remove_groups.contains(g))
+    {
+        bail!("group {dup:?} is both added and removed; refusing to guess an order");
+    }
+
+    let mut changes = Vec::new();
+
+    if let Some(name) = &update.displayname
+        && *name != user.displayname
+    {
+        reject_control_chars("displayname", name)?;
+        changes.push(format!("displayname: {:?} -> {name:?}", user.displayname));
+        user.displayname.clone_from(name);
+    }
+
+    if let Some(email) = &update.email
+        && user.email.as_deref() != Some(email.as_str())
+    {
+        reject_control_chars("email", email)?;
+        changes.push(match &user.email {
+            Some(old) => format!("email: {old:?} -> {email:?}"),
+            None => format!("email: unset -> {email:?}"),
+        });
+        user.email = Some(email.clone());
+    }
+
+    for group in &update.remove_groups {
+        let Some(at) = user.groups.iter().position(|g| g == group) else {
+            bail!(
+                "user is not in group {group:?}, so there is nothing to revoke \
+                 (groups: {})",
+                fmt_groups(&user.groups)
+            );
+        };
+        user.groups.remove(at);
+        changes.push(format!("removed from group {group:?}"));
+    }
+
+    for group in &update.add_groups {
+        if user.groups.iter().any(|g| g == group) {
+            continue;
+        }
+        reject_control_chars("group", group)?;
+        user.groups.push(group.clone());
+        changes.push(format!("added to group {group:?}"));
+    }
+
+    if changes.is_empty() {
+        bail!("nothing to change — every requested value is already set");
+    }
+    Ok(changes)
+}
+
+/// Group list for a message, so an empty one reads as a word rather than
+/// as a missing value.
+pub fn fmt_groups(groups: &[String]) -> String {
+    if groups.is_empty() {
+        "none".to_owned()
+    } else {
+        groups.join(", ")
+    }
+}
+
 /// Whether an existing `users.yml` is safe to take over when no canonical
 /// store exists yet — i.e. it is the untouched first-boot seed.
 ///
@@ -297,6 +408,130 @@ mod tests {
         }
     }
 
+    fn with_groups(groups: &[&str]) -> User {
+        let mut u = user("$argon2id$x");
+        u.groups = groups.iter().map(|g| (*g).to_owned()).collect();
+        u
+    }
+
+    /// The verb exists to change several things in one call, so the
+    /// composed case is the one that has to work.
+    #[test]
+    fn one_update_can_change_several_attributes() {
+        let mut u = with_groups(&["users"]);
+        let changes = apply_update(
+            &mut u,
+            &UserUpdate {
+                displayname: Some("Mara".to_owned()),
+                email: Some("mara@example.com".to_owned()),
+                add_groups: vec!["admins".to_owned()],
+                remove_groups: vec!["users".to_owned()],
+            },
+        )
+        .expect("applies");
+
+        assert_eq!(u.displayname, "Mara");
+        assert_eq!(u.email.as_deref(), Some("mara@example.com"));
+        assert_eq!(u.groups, ["admins"]);
+        assert_eq!(changes.len(), 4, "every change is reported: {changes:?}");
+    }
+
+    /// ⭐ The asymmetry that is the point of this function. A revocation
+    /// that reports success without revoking is the failure nobody
+    /// re-checks — so a `--remove-group` naming a group the user is not
+    /// in must fail, and must leave the user untouched.
+    #[test]
+    fn removing_a_group_the_user_lacks_fails_and_changes_nothing() {
+        let mut u = with_groups(&["admins"]);
+        let err = apply_update(
+            &mut u,
+            &UserUpdate {
+                // The realistic shape: a typo for `admins`.
+                remove_groups: vec!["admin".to_owned()],
+                ..UserUpdate::default()
+            },
+        )
+        .expect_err("a no-op revocation must not report success");
+
+        assert!(err.to_string().contains("nothing to revoke"), "{err}");
+        assert_eq!(u.groups, ["admins"], "the user must be untouched");
+    }
+
+    /// The other half of the asymmetry: setting what is already set is
+    /// fine, because the end state matches the intent. Refusing would
+    /// make "set these four things" fail when one was already right.
+    #[test]
+    fn already_satisfied_additions_are_not_errors() {
+        let mut u = with_groups(&["admins"]);
+        u.displayname = "Mara".to_owned();
+
+        let changes = apply_update(
+            &mut u,
+            &UserUpdate {
+                displayname: Some("Mara".to_owned()),
+                add_groups: vec!["admins".to_owned(), "ops".to_owned()],
+                ..UserUpdate::default()
+            },
+        )
+        .expect("a partially-satisfied update still applies the rest");
+
+        assert_eq!(u.groups, ["admins", "ops"], "no duplicate `admins`");
+        assert_eq!(
+            changes.len(),
+            1,
+            "only the real change reports: {changes:?}"
+        );
+    }
+
+    /// A command that changes nothing would still rewrite both files and
+    /// restart the SSO provider, so it is an error rather than a no-op.
+    #[test]
+    fn an_update_that_changes_nothing_fails() {
+        let mut u = with_groups(&["admins"]);
+        let err = apply_update(
+            &mut u,
+            &UserUpdate {
+                add_groups: vec!["admins".to_owned()],
+                ..UserUpdate::default()
+            },
+        )
+        .expect_err("a no-op must not restart authelia");
+        assert!(err.to_string().contains("nothing to change"), "{err}");
+    }
+
+    #[test]
+    fn adding_and_removing_the_same_group_is_refused() {
+        let mut u = with_groups(&["admins"]);
+        let err = apply_update(
+            &mut u,
+            &UserUpdate {
+                add_groups: vec!["admins".to_owned()],
+                remove_groups: vec!["admins".to_owned()],
+                ..UserUpdate::default()
+            },
+        )
+        .expect_err("contradictory flags must not pick a winner silently");
+        assert!(err.to_string().contains("both added and removed"), "{err}");
+    }
+
+    /// The update path writes the same file `render_yaml` validates, so a
+    /// control character has to be refused *before* it reaches the store —
+    /// not at render time, with the store already mutated.
+    #[test]
+    fn a_control_character_is_refused_by_the_update_path_too() {
+        let mut u = with_groups(&[]);
+        let err = apply_update(
+            &mut u,
+            &UserUpdate {
+                displayname: Some("bad\nname".to_owned()),
+                ..UserUpdate::default()
+            },
+        )
+        .expect_err("must refuse");
+        assert!(err.to_string().contains("control character"), "{err}");
+        assert_eq!(u.displayname, "Test User", "the user must be untouched");
+    }
+
     /// The seed check gates an irreversible overwrite, so it has to be
     /// tolerant of trailing whitespace and intolerant of everything else.
     #[test]