Skip to content

[v14] Fix Assume Roles switch back, don't delete role if access list is using it.#33834

Merged
mdwn merged 2 commits intobranch/v14from
mike.wilson/v14-fix-switch-back
Oct 24, 2023
Merged

[v14] Fix Assume Roles switch back, don't delete role if access list is using it.#33834
mdwn merged 2 commits intobranch/v14from
mike.wilson/v14-fix-switch-back

Conversation

@mdwn
Copy link
Copy Markdown
Contributor

@mdwn mdwn commented Oct 23, 2023

Backport #33746 to branch/v14.

…ng it.

When switching back to the regular user permissions after assuming roles via
an access request, Teleport will now use the user login state to ensure that
access list permissions are taken into account.

Additionally, users will not be able to delete roles if they are in use by an
access list. Finally, when refreshing the user while extending a web session,
the user login state will be regenerated and used for permissions.
Comment thread lib/auth/access.go
}

var nextToken string
for {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there no "forEach" kind of helper method for access lists already somewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, unfortunately. :-(

Comment thread lib/auth/access.go Outdated
}

for _, accessList := range accessLists {
for _, r := range accessList.Spec.Grants.Roles {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: I think we have a "contains" method for string slices somewhere in utils which will make this code a bit cleaner and avoid 3rd nested for loop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've elected to use slices instead here, let me know what you think.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Even better.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just dropping this here: #33851 - this is the equivalent slices change on master that keeps master and branch/v14 in sync.

@mdwn mdwn added this pull request to the merge queue Oct 23, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 23, 2023
@mdwn mdwn added this pull request to the merge queue Oct 24, 2023
Merged via the queue into branch/v14 with commit fb9f15f Oct 24, 2023
@mdwn mdwn deleted the mike.wilson/v14-fix-switch-back branch October 24, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants