Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate duplicate code paths for deleting users #2838

Closed
dnephin opened this issue Aug 4, 2022 · 1 comment · Fixed by #3398
Closed

Consolidate duplicate code paths for deleting users #2838

dnephin opened this issue Aug 4, 2022 · 1 comment · Fixed by #3398
Assignees
Labels
kind/improvement A report of a quality problem, or a change that addresses a quality problem.

Comments

@dnephin
Copy link
Contributor

dnephin commented Aug 4, 2022

@j-sneh noticed this while working on #2767

Today we have the following:

  • access.DeleteIdentity - handles removing credentials, access keys, and grants associated with the identity, and fix: remove deleted identities from identities_groups #2767 adds removal of identities_group references
  • data.DeleteIdentity - only deletes the one row from identities table
  • data.DeleteIdentities - handles removing grants, and fix: remove deleted identities from identities_groups #2767 adds removal of identities_group references
  • Server.loadUsers (from server config) - calls data.DeleteIdentities but does not remove credentials, or access keys for the users
  • access.DeleteProviders - calls data.DeleteIdentities and handles deleting access keys (doesn't need to delete credentials)

With #2767 merged it seems like the only gap may be loadUsers not deleting of credentials or access keys. I'm not sure if that is by design.

The problem now is that there are two ways to delete users, in one code path the logic to delete associations is handled in access, and in the other code path it's handled in data.

This issue is to clean this up so that at the very least the two flows are consistent about where associations are deleted, either access or data. Ideally we would also remove the need to have separate code paths for deleting one identity and deleting many identities, but I'm not sure if that's possible.

@dnephin dnephin added the kind/improvement A report of a quality problem, or a change that addresses a quality problem. label Aug 4, 2022
@stale
Copy link

stale bot commented Oct 4, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the status/stale Used by actions/stale to mark an issue or PR as stale. label Oct 4, 2022
@dnephin dnephin removed the status/stale Used by actions/stale to mark an issue or PR as stale. label Oct 4, 2022
@BruceMacD BruceMacD self-assigned this Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/improvement A report of a quality problem, or a change that addresses a quality problem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants