fix: don't restore revoked access from the gpg-id files - #15
Conversation
When a user's access was revoked, the next reload imported the keys back from the gpg-id file, undoing the revocation. Now an already imported gpg-id file is not merged again, only warned about if it holds keys the access store doesn't know. Also revoke users matching by key or email, not just by name, and sort the keys when writing the gpg-id files so the output is stable.
lyz-code
left a comment
lyz-code
left a comment
There was a problem hiding this comment.
if i'm understanding well your reasoning, i think you are trying to solve by code a situation created by a bad operation.
if you revoke the keys of a user, the .gpg-id files should be updated, so you should not find yourself needing these changes.
in fact, the solution should be to revoke the user again.
if the above operations don't do what i just said, then the pr should go in that direction
| assert user == result | ||
|
|
||
|
|
||
| def test_auth_loads_gpg_id_even_if_entry_exists( |
There was a problem hiding this comment.
I like this behaviour, why would you want to remove it?
| Given: an auth store with a key in the access property | ||
| Then: The missing keys are loaded | ||
| Given: an auth store with an already imported gpg-id file | ||
| When: the gpg-id file has keys that the access store doesn't have |
There was a problem hiding this comment.
the test you made below doesn't reflect this case
| # therefore the access to the directory doesn't change as it's | ||
| # binded to the group | ||
| if isinstance(revoke, User): | ||
| # An access entry can also be the key or the email of the user. |
There was a problem hiding this comment.
that's why we use self.get_identifier in line 466.
so we shouldn't need this block
| """Import the gpg-id files that are not yet in the access store. | ||
|
|
||
| Merging the keys of an already imported gpg-id file would undo any | ||
| revocation done on the access store, so they are only warned about. |
There was a problem hiding this comment.
no they wouldn't as the revocation takes care of this
2 participants
When a user's access was revoked, the next reload imported the keys back from the gpg-id file, undoing the revocation. Now an already imported gpg-id file is not merged again, only warned about if it holds keys the access store doesn't know.
Also revoke users matching by key or email, not just by name, and sort the keys when writing the gpg-id files so the output is stable.
Checklist