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

[branch/v7] Ensure ListResources is accurate with denied resources (#14544) #14547

Merged
merged 3 commits into from
Jul 18, 2022

Conversation

rosstimothy
Copy link
Contributor

Backport

This will backport the following commits from branch/v8 to branch/v7:

NOTE: this is only backporting a subset of the backport to v8 because ListResources doesn't exist on v7

rosstimothy and others added 3 commits July 15, 2022 18:05
* Ensure ListResources is accurate with denied resources

Prior to this tsh ls and the web UI were reporting different number
of resources when a user had role/roles that restricted access to
certain resources. This was caused by returning an incorrect
`NextKey` in the `ListResourcesResponse`.

In the event that `IterateResourcePages` was returning early due to
the callback requesting to stop iteration, the `NextKey` being returned
was the last key for the **current page** instead of the key for
the **next resource** in the page. This would cause N resources between
the last resource iterated on and the end of the page to be skipped.

To remedy this `IterateResourcePages` has been refactored to
`IterateResources`. This shifts the responsiblity of building the
`ListResourcesResponse` onto the caller and not the iterator. There
are no more pages to worry about, so the logic about what the next
key should be becomes simpler.

`TestListResources_WithRoles` was added to ensure that a user with
a set of roles that denies access is always returned the correct
resources from `ListResources` from both tsh and the web ui.

(cherry picked from commit ac69da9)
@rosstimothy rosstimothy marked this pull request as ready for review July 18, 2022 16:19
@github-actions github-actions bot removed the request for review from zmb3 July 18, 2022 16:21
@rosstimothy rosstimothy merged commit 018b4e9 into branch/v7 Jul 18, 2022
@zmb3 zmb3 deleted the tross/v7/ls_discrepancy branch September 9, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants