Skip to content

Allow access requests to use user login state.#33317

Merged
mdwn merged 1 commit intomasterfrom
mike.wilson/access-requests-use-user-login-state
Oct 11, 2023
Merged

Allow access requests to use user login state.#33317
mdwn merged 1 commit intomasterfrom
mike.wilson/access-requests-use-user-login-state

Conversation

@mdwn
Copy link
Copy Markdown
Contributor

@mdwn mdwn commented Oct 11, 2023

Access requests are now able to use the user login state as opposed to just the static user definition. This will allow access lists to influence who can review access requests.

Access requests are now able to use the user login state as opposed to just
the static user definition. This will allow access lists to influence who can
review access requests.
@mdwn mdwn force-pushed the mike.wilson/access-requests-use-user-login-state branch from c37ed0e to 1c09f9d Compare October 11, 2023 19:47
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix lgtm but I think we should rethink this weird User vs UserState "dichotomy" we ended up with. It's very hard to reason about which one to use in which case now, plus how many other places are there we missed that still use User and won't get access list roles?

What can we do to fix this problem? Why can't we update the existing User object with something like "Status" calculated field that would be updated dynamically on login and contain user state? Then callers wouldn't need to worry about using wrong API.

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from strideynet October 11, 2023 20:44
@mdwn mdwn added this pull request to the merge queue Oct 11, 2023
Merged via the queue into master with commit 82522ac Oct 11, 2023
@mdwn mdwn deleted the mike.wilson/access-requests-use-user-login-state branch October 11, 2023 22:22
@public-teleport-github-review-bot
Copy link
Copy Markdown

@mdwn See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants