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

[Web] Hide access request promoted notification quick action button if the user is already part of the access list #51276

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

rudream
Copy link
Contributor

@rudream rudream commented Jan 21, 2025

Purpose

e counterpart: https://github.com/gravitational/teleport.e/pull/5893

This PR fixes a UX bug which caused users to still see the "Log in again to gain access" button on their access request promoted notification even if they had already done so.

The logic simply works by keeping track of when the user logged in, if they logged in before the access request was promoted, then they will be shown the button to log in again.

Demo

fix.access.list.ux.mov

@rudream rudream added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 labels Jan 21, 2025
@github-actions github-actions bot requested review from gzdunek and kimlisa January 21, 2025 01:29
Copy link
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

This fix is a bit hacky, but I guess we don't have a way to tell if a user gained the access, right?

@rudream
Copy link
Contributor Author

rudream commented Jan 21, 2025

This fix is a bit hacky, but I guess we don't have a way to tell if a user gained the access, right?

@gzdunek Yes, I had explored other ways to try to actually determine whether the access list was active, but there was no way to without surfacing lots of information about the session and touching lots of related code, which I figured was not worth it for a small UX bug. The issue is that while it's possible and quite easy to know when the user has been added to the access list (for example by making a request to fetch access lists), it's not possible to know whether or not those new permissions have been applied to their current web session and certificate. Keeping track of login time seemed the most straightforward way which works as intended and doesn't introduce any risk for regressions.

@rudream rudream requested a review from gzdunek January 21, 2025 14:08
@rudream rudream force-pushed the yassine/fix-access-list-notification branch from db0fec0 to 49adc20 Compare January 21, 2025 21:15
@rudream rudream enabled auto-merge January 21, 2025 21:16
@rudream rudream added this pull request to the merge queue Jan 21, 2025
Merged via the queue into master with commit 0cf3542 Jan 21, 2025
41 checks passed
@rudream rudream deleted the yassine/fix-access-list-notification branch January 21, 2025 21:32
@public-teleport-github-review-bot

@rudream See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v16 backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants