fix team member deprecation #2072
Conversation
FxKu
requested review from
sdudoladov,
Jan-M,
jopadi and
idanovinda
as code owners
| } else { | ||
| // user found in database and not wanted in newUsers - replace LOGIN flag with NOLOGIN | ||
| dbUser.Flags = util.StringSliceReplaceElement(dbUser.Flags, constants.RoleFlagLogin, constants.RoleFlagNoLogin) |
There was a problem hiding this comment.
When a member leaves the team revoking the LOGIN right will still succeed but renaming will fail because the deprecated user already exists.
| _, originalUserWanted := newUsers[originalName] | ||
| _, originalUserAlreadyExists := dbUsers[originalName] | ||
| if !originalUserWanted || originalUserAlreadyExists { | ||
| continue |
There was a problem hiding this comment.
When a member joins again but original role already exist the operator will not even try to ALTER the deprecated user.
| originalUsername, foundDeletedUser := deletedUsers[dbUser.Name] | ||
| // check if original user does not exist in dbUsers | ||
| _, originalUserAlreadyExists := dbUsers[originalUsername] | ||
| if foundDeletedUser && !originalUserAlreadyExists { |
There was a problem hiding this comment.
With this PR the operator can revoke LOGIN from users but is allowed to fail the renaming. This means, both users with and without suffix cannot login anymore. When the member joins the team again altering the deprecated user is skipped, but LOGIN should be granted back to the role without the suffix. ProduceSyncRequests can do this, but only if we do not mark the user as deleted here. Otherwise, it would be skipped.
|
👍 |
1 similar comment
|
👍 |
#1953 introduced a bug in the user sync code. An extra pgUserMap newUsers was added to include systemUsers when passed to ProduceSyncRequests function. The info if a user is deprecated is overridden after newUsers variable is created, but only stored in the cluster's pgUsers variable.
Thus, ProduceSyncRequests does not know which users can be considered deprecated. The following bug can now happen:
2.1. readPgUsersFromDatabase finds the deprecated user counterpart to a user that is desired in c.pgUsers.
2.2. c.pgUsers is updated based on this finding, but not newUsers variable which is passed to ProduceSyncRequests.
2.3. ProduceSyncRequests skips users marked as deleted, but the info isn't found.
2.4. Since the new desired user does not exist in the database it is created.
Both the LOGIN team user and its deprecated NOLOGIN counterpart now exist in the database. The next time the member moves the operator will fail to rename the user because either of its versions already exist.
This PR fixes the bug and also enables the operator to deal with the erroneous setup of "redundant" roles. If rename requests fails the operator will only log the error but will not fail the sync.