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

Users with read permissions are offered to save new dashboard in collections they only have read access to #15281

Closed
nemanjaglumac opened this issue Mar 22, 2021 · 2 comments · Fixed by #15613
Assignees
Labels
Administration/Permissions Collection or Data permissions Organization/Collections Priority:P3 Cosmetic bugs, minor bugs with a clear workaround .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@nemanjaglumac
Copy link
Member

nemanjaglumac commented Mar 22, 2021

Describe the bug
Users with read permissions are offered to save new dashboard in collections they only have read access to. That leads to confusion and an error.
NOTE:

  • The UI is initially correct and such users see only "My personal collection" but the error happens when they use a search functionality (see repro steps below)
  • However, they can use the search and if type the name of a collection that lives inside a root (that they have read access to)
  • The collection will be shown/offered

Logs
403 error in the console.

To Reproduce
Steps to reproduce the behavior:

  1. Click on + icon in the header and choose "New dashboard"
  2. Modal will be shown - enter some name for the dashboard
  3. You should see your personal collection
  4. Click on the selection menu - it will open a dropdown
  5. Click on the "loupe" icon to be able to search
  6. Search for any collection that lives inside the root collection (given that you have read access to it)
  7. You will see that collection offered
  8. Click on it - it will now be selected as a "go to collection"
  9. Click "Create" button
  10. Inline error will be shown

Expected behavior
Search shouldn't return collections users don't have curate access to

Screenshots
Initial UI
image

After searching for a collection user has read access to, and trying to save in it
image

Information about your Metabase Installation:

  • Metabase version: v0.38.3-SNAPSHOT
  • Metabase hosting environment: local dev, release-x.38.x
  • Metabase internal database: H2 (default), sample dataset

Severity
P3

Additional context
Realated to #14052 and to #15280

@flamber flamber added Administration/Permissions Collection or Data permissions Organization/Collections Priority:P3 Cosmetic bugs, minor bugs with a clear workaround and removed .Needs Triage labels Mar 22, 2021
@nemanjaglumac
Copy link
Member Author

nemanjaglumac commented Mar 22, 2021

This gets even more weird with nested collections. We have the following structure In the test snapshot:

  • Our analytics
    • First collection
      • Second collection
        • Third collection

Read only user has READ permissions for ALL these collections.

  1. Notice that initially "First collection" is offered (which is wrong to begin with)
  2. If you click on its name directly, it should be selected but it acts like we clicked > icon
  3. Now the "Second collection" is offered
  4. Click on it, it's still not selected and it again acts like we clicked > icon
  5. But this time "Third collection" is not shown
  6. Go to the search icon and search for "third"
  7. "Third collection" is shown and now when you click on it it IS selected as a go to collection
  8. Click "Create" and you'll see the error

@flamber
Copy link
Contributor

flamber commented Mar 22, 2021

It's some of the missing stuff on #7881

nemanjaglumac added a commit that referenced this issue Mar 22, 2021
@nemanjaglumac nemanjaglumac added this to Backlog in Cypress Testing Mar 22, 2021
nemanjaglumac added a commit that referenced this issue Mar 22, 2021
…hboard in collections they only have read access to (#15282)
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Mar 22, 2021
@kulyk kulyk self-assigned this Apr 9, 2021
kulyk added a commit that referenced this issue Apr 15, 2021
kulyk added a commit that referenced this issue Apr 22, 2021
kulyk added a commit that referenced this issue Apr 26, 2021
kulyk added a commit that referenced this issue May 5, 2021
kulyk added a commit that referenced this issue May 6, 2021
kulyk added a commit that referenced this issue May 6, 2021
…ess to (#15613)

* Test adding question to dashboard

* Test collections filtering when adding a question

When adding a question to dashboard,
we need to display collections a user has "write" access to.
Collection with "read" access have to be hidden

* Fix adding question to dashboard without access

* Add a note about permissions test suite

* Move question permission tests to collection suite

* Revert initial collections filtering

* Fix adding question to dashboard without access

* Remove redundant state field

* Enable #15281 issue repro test

* Remove requireCollectionWritePermission prop

* Filter items user doesn't have `write` access to

* Fix permission tests

* Fix dashboard test

* Fix part of permission tests disabled for nodata user

* Bring back issue reference to Cypress test

* Remove underscore prefixes for component methods

* Test offers saving dashboard to opened collection

* Fix tests nested incorrectly

* Split dashboard permission test

* Fix suggest saving items to read-only collections

* Fix collection permission filtering

See comment:
#15613 (comment)

* Move comment

* Fix test failing due to fixed collection suggestion

* Remove `should("exist")` from Cypress tests

* Merge "adding question to dashboard" tests

* Merge similar permission tests

* Merge similar tests into one

* Use sidebar test ID in permissions test

* Select by .AdminSelect

* Remove redundant search test case

* Revert native query test

* Add React list key to ItemPicker items

* Add collection suggestions tests

* Configure Form's `overwriteOnInitialValuesChange`

* Wrap CollectionSelect with `@Collection.loadList`

When suggesting an initial collection,
we need to check a user has `write` access to it.
For that, collection objects have to be present in Redux store,
so we can retrieve a collection by ID and check the `can_write` flag

* Allow modifying CreateDashboardModal's onSave prop

* Fix collection suggestsions for new dashboard

* Fix collection suggestions when copying dashboards

* Add defaultProps to Form

* Fix SaveQuestionModal unit test

* Fix SaveQuestionModal collection suggestion

* Simplify CollectionSelect wrapper

* Fix dashboard header test selector

* Rename permission tests

* Mock HTTP requests at SaveQuestionModal test

* Pass correct params to initialCollectionId
@flamber flamber added this to the 0.40 milestone May 6, 2021
This was referenced Dec 1, 2022
This was referenced Feb 5, 2024
@replay-io replay-io bot mentioned this issue Apr 24, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Administration/Permissions Collection or Data permissions Organization/Collections Priority:P3 Cosmetic bugs, minor bugs with a clear workaround .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
Development

Successfully merging a pull request may close this issue.

3 participants