RBAC: Removing a user from a role doesn't automatically revoke their SSH access where expected

When i removed @groente from the foundations-team-members RBAC role (that's puppet-code@571a2128 for tpo/tpa/tails-sysadmin#18184), I expected its SSH key to be automatically removed from apt.lizard:~reprepro-time-based-snapshots/.ssh/authorized_keys), but that didn't happen and I had to manually remove the key from that file.

I think this is not related to #18192 because (1) that bug doesn't manifest when puppet is run by systemd and (2) i manually removed one key to test and puppet correctly added it back (when run by systemd).

/cc @groente

added Tails label

mentioned in issue #18192

ah, purge_ssh_keys is not set to true for that user. i'm not sure whether puppet-rbac should force that or whether we should fix this in puppet-tails..

puppet-rbac already handles purge_ssh_keys for users created with rbac::user, so maybe we should have something similar that we can use to replace the user declaration in tails::profile::reprepro::snapshots::base?

yeah, that's what i was thinking as well. something like

User <| name = $user |> {
  purge_ssh_keys => true,
}

should we make this conditional? i'm not sure if there's any valid reason no to want this...

yeah, that's what i was thinking as well. something like

User <| name = $user |> {
  purge_ssh_keys => true,
}

should we make this conditional? i'm not sure if there's any valid reason no to want this...

I've had so much trouble with that codepath in Puppet... In team#41890, there's a glimpse of this, but basically, the ssh authorized_keys code in core puppet is super buggy. So landmines there.

i'm not sure whether puppet-rbac should force that

FTR, i do think this should be a feature of puppet-rbac, otherwise revoking access will always need a manual lookup and delete, which is slow and prone to errors.

FTR, i do think this should be a feature of puppet-rbac, otherwise revoking access will always need a manual lookup and delete, which is slow and prone to errors.

yeah, makes sense, i guess i was a bit skeptical because the creation of the user itself is out of scope for puppet-rbac, but it's probably better to make sure it's done right, even if out of scope.

assigned to @groente

added Needs Information label

removed Needs Information label

added Next label

this is fixed in v0.1.2

closed