[JUJU-2928] fully remove users

[juanmanuel-tirado]

The current Juju logic does not fully remove users from the database in order to maintain relevant info for auditing purposes. This makes impossible to reuse usernames. This is a problem in certain scenarios where by mistake a user is deleted and has to be created again (e.g. Terraform provider). This PR modifies the state package to permit the addition of a previously removed user. We basically, check if a returned doc was already deleted and if so, we update the doc with the new details.

Note: These changes may impact the integration tests on Jenkins.

Checklist

- [ ] Code style: imports ordered, good names, simple structure, etc
- [] Comments saying why design decisions were made

• Go unit tests, with comments saying what you're testing
  - [ ] Integration tests, with comments saying what you're testing
  - [ ] doc.go added or updated in changed packages

QA steps

Verify using the CLI that a user can be added after been removed.

juju add-user user1
add-user user1
User "user1" added
Please send this command to user1:
    juju register MHsTBXVzZXIxMEUTEzEwLjIyMC4xOC4xMjI6MTcwNzAMLltmZDQyOmNmYTE6YWRmNzo2Mjc0OjIxNjozZWZmOmZlMjQ6YzkxOV06MTcwNzAEINqzC2M6xEzIgFkxDvv-hIX8WUmBqd7XJgXYHEXpd_vdEwd0ZXN0ZGV2EwAA

"user1" has not been granted access to any models. You can use "juju grant" to grant access.

List users

juju users
Name    Display name  Access     Date created    Last connection
admin*  admin         superuser  19 hours ago    just now
user1                 login      51 seconds ago  never connected

Remove it

juju remove-user user1 --yes
User "user1" removed

Add it again

juju add-user user1
User "user1" added
Please send this command to user1:
    juju register MHsTBXVzZXIxMEUTEzEwLjIyMC4xOC4xMjI6MTcwNzAMLltmZDQyOmNmYTE6YWRmNzo2Mjc0OjIxNjozZWZmOmZlMjQ6YzkxOV06MTcwNzAEINqzC2M6xEzIgFkxDvv-hIX8WUmBqd7XJgXYHEXpd_vdEwd0ZXN0ZGV2EwAA

"user1" has not been granted access to any models. You can use "juju grant" to grant access.

If we try to add it it will simply fail because the user already exists.

juju add-user user1
ERROR failed to create user: user already exists

[SimonRichardson]

This seems dangerous to me, even if you've got a different password. Ostensibly it means you can pretend to be the previous user and I don't know what all the ramifications are to that. At the very least I'd want a new field (int?) that states if this was a takeover/repurposed account.

[juanmanuel-tirado]

For me the problem there is the concept "user reuse" which is basically what we're doing here but I share your concerns. About indicating if a username was taken over, that can be easily done but probably is not fully informative for auditing purposes. This maybe requires an additional object storing the history of the user entry...

[nvinuesa]

Maybe you could add a user again (like you did in state/user_test.go TestAddDeletedUser) in in api/client/usermanager/client_test.go TestRemoveUser?

[nvinuesa]

LGTM, QA is 👍

[manadart]

@juanmanuel-tirado This needs migration and upgrade steps.

For any already-deleted users, we should add the removal log entry with the current date for deletion and some indicator of upgrade/migration as the user.

If anyone reactivates an already-deleted user after this patch, we'll be missing the log entry.

[juanmanuel-tirado]

Let me address this in another PR.

Thanks!