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

App Repository UI does not properly handle cases where the user does not have read/write to app repos in ns #3679

Closed
dlaloue-vmware opened this issue Oct 27, 2021 · 8 comments · Fixed by #5480
Assignees
Labels
component/apprepository Issue related to kubeapps apprepository component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature

Comments

@dlaloue-vmware
Copy link
Collaborator

I created a user that only has access to his/her own namespace, and that does not have the right to view/create private repositories, but only use the global repositories (e.g. because of company policy).

The user can login to Kubeapps, view applications and view the catalog fine.

But when looking at the repository page, the UI shows an error message:
Screen Shot 2021-10-27 at 10 58 29 AM

I would expect that instead, the page should render the global repositories and then have a message in the namespaced section that indicates the user does not have the priviledges to manage repositories.
something like this:
Screen Shot 2021-10-27 at 10 55 36 AM

The screenshot above was taken after granting "read" to repositories in the user's namespace.
We can see that the UI can properly handle disabling the edit/refresh/delete buttons, so it should also be able to handle when the user does not even have read access.

Last, though the repositories edit/refresh/delete are disabled, the "add app repository" and "refresh all" buttons are enabled instead of being disabled.

@dlaloue-vmware
Copy link
Collaborator Author

there is another case where the user only has the default view permissions across the cluster.
in that case, for some reason, when the context is set to a given namespace, the UI reports an error that the user does not have access to the namespace, while it does have access when using kubectl. There is no error if the context is set to the kubeapps namespace.
Screen Shot 2021-10-27 at 10 46 57 AM

@absoludity
Copy link
Contributor

The user can login to Kubeapps, view applications and view the catalog fine.

But when looking at the repository page, the UI shows an error message:

Note that this page is accessed via the Administration->AppRepositories menu. As you mention, the user can login, view apps and view the configured catalog, but Kubeapps doesn't consider that this user has access to administer app repositories in the current context, which they don't. Early on the Kubeapps project adopted the approach of letting the user know the RBAC that is required for functionality if they cannot access it (we should definitely fix the end of the message though - users don't need to know about pinniped proxy).

I'm assuming you didn't expect this user to be able to administer repositories, but rather, are thinking of this as just a view of repositories currently available? Let me know what you think. I'd be keen to keep this under "Administration->AppRepositories", but open to ideas.

@absoludity
Copy link
Contributor

there is another case where the user only has the default view permissions across the cluster.

We mentioned in another issue (#3655) that we don't support this use-case currently because Helm stores the release info as a secret, so reading secrets is required to view.

in that case, for some reason, when the context is set to a given namespace, the UI reports an error that the user does not have access to the namespace, while it does have access when using kubectl.

That's because the test we use is to check if the user can read secrets.

There is no error if the context is set to the kubeapps namespace.

Haven't yet checked, but guessing when requesting the catalog in the kubeapps namespace we skip the check (could be a bug, need to look more closely).

@dlaloue-vmware
Copy link
Collaborator Author

Note that this page is accessed via the Administration->AppRepositories menu

One option would be to disable this link if the user does not have the proper permissions. I am not sure how easy it is to handle.
But i would also disagree that this is only for administration. The goal is for a namespace user to be able to create his own repository. And that user will only have edit on the namespace and write on app repositories.

I'm assuming you didn't expect this user to be able to administer repositories, but rather, are thinking of this as just a view of repositories currently available?

If the namespace user had write on app repositories (within the namespace), as mentioned above, i expect the user to be able to add his own repository - i thought this was the goal of namespace scoped repositories.
If the namespace user has read on app repositories (within the namespace), i expect the user to be able to view the namespace repositories, though not manage them.
If the namespace user has neither read/write on app repositories (within the namespace), i would still think he should be able to navigate to the page and view the global repositories (not manage them) as he is able to see their name and charts in the catalog.

@dlaloue-vmware
Copy link
Collaborator Author

in that case, for some reason, when the context is set to a given namespace, the UI reports an error that the user does not have access to the namespace, while it does have access when using kubectl.

That's because the test we use is to check if the user can read secrets.

that was true and understandable for applications, but why would that be the case for app repositories?
and the error message is misleading as the user does have access to the namespace.

@absoludity
Copy link
Contributor

Note that this page is accessed via the Administration->AppRepositories menu

One option would be to disable this link if the user does not have the proper permissions. I am not sure how easy it is to handle.

Yeah, I was wondering the same, but giving the user feedback as to why they can't administrate repos is also important. Could be a tool-tip on the disabled link, not sure.

But i would also disagree that this is only for administration. The goal is for a namespace user to be able to create his own repository. And that user will only have edit on the namespace and write on app repositories.

If a user can administrate AppRepositories in that namespace, then yes, they should be able to without error, but that's different to what you're reporting here isn't it? I thought this issue is about a user who does not have perms to do so in the namespace?

If the namespace user had write on app repositories (within the namespace), as mentioned above, i expect the user to be able to add his own repository - i thought this was the goal of namespace scoped repositories.

Same here, but that's different to the issue in the description or I'm lost :) "I created a user that only has access to his/her own namespace, and that does not have the right to view/create private repositories"

If the namespace user has read on app repositories (within the namespace), i expect the user to be able to view the namespace repositories, though not manage them.

Yep, we could change to support that, but currently it's meant to be an administration function so that admins (of the namespace or cluster) can setup the catalog that can be used in that cluster.

If the namespace user has neither read/write on app repositories (within the namespace), i would still think he should be able to navigate to the page and view the global repositories (not manage them) as he is able to see their name and charts in the catalog.

Not sure why I'd want to do that as a non-admin user? I can already see what repos are available in the namespace from the catalog. But yes, if we want to update this to be a non-admin page, we can of course consider that. As it is, it's currently meant for administration (and labeled as such).

That's because the test we use is to check if the user can read secrets.

that was true and understandable for applications, but why would that be the case for app repositories? and the error message is misleading as the user does have access to the namespace.

Much the same reason (secrets). If a user clicks on a repository to see the details, do we show the secret name? Do we allow them to update the secret. It's been designed for admins to create and configure the catalog. We could update this administration function so that we have a view-only version. I'd personally wait to see if people are asking for this functionality and work to understand why they are expecting to be able to view administration pages and what they would do with the information etc. before investing too much into it.

@ppbaena ppbaena added the component/apprepository Issue related to kubeapps apprepository label Oct 28, 2021
@antgamdia antgamdia added the component/ui Issue related to kubeapps UI label Oct 28, 2021
@dlaloue-vmware
Copy link
Collaborator Author

I was about to write something long, but i guess I can make it shorter.

The main issue i guess that confused me is that the "app repository" navigation link is always enabled, and then when you navigate you get different kind of errors depending on how the user is configured (which is confusing).

The second issue is that in the case of "view", only parts of the buttons are properly disabled. The global repositories edit/remove/refresh buttons are properly handled, but all other buttons are not - i.e. the global buttons (add, refresh all) and the namespace repository buttons.

So:

  • we should disable the "app repository" if the user does not have read/write on global repositories or namespace repositories.
  • we should fix all buttons to be disabled if write access is not allowed (same as what is done for global repositories)
    Does this sound ok?

Last, regarding your last comments on secrets, if i have view on pods, i can do a get pod and see the secrets the pod points too. I will not be able to do a get secret and extract its sensitive data though.
I would assume this is the same for app repositories. If i have view on app repositories, i can see the name of the secrets it references, but i will not be able to view the secret itself or act on it.

@dlaloue-vmware
Copy link
Collaborator Author

I just realized that this was true when we only had app repositories but will most likely be different with the pluggable APIs.
How are we going to manage read/write roles (and UI behavior) across the 3 different sets of repositories?

@antgamdia antgamdia added this to the App repository refactor milestone Nov 10, 2021
@ppbaena ppbaena added kind/enhancement An issue that reports an enhancement for an implemented feature size/M labels Nov 22, 2021
@ppbaena ppbaena added this to Kubeapps Mar 9, 2022
@ppbaena ppbaena moved this to 🗂. Backlog in Kubeapps Mar 9, 2022
@ppbaena ppbaena added this to Kubeapps Mar 31, 2022
@ppbaena ppbaena moved this to 🗂 Backlog in Kubeapps Mar 31, 2022
@ppbaena ppbaena moved this from 🗂 Backlog to 🗒 Todo in Kubeapps Oct 10, 2022
@castelblanque castelblanque moved this from 🗒 Todo to 🏗 In Progress in Kubeapps Oct 17, 2022
@castelblanque castelblanque moved this from 🏗 In Progress to 🔎 In Review in Kubeapps Oct 18, 2022
Repository owner moved this from 🗂. Backlog to ✅. Done in Kubeapps Oct 20, 2022
Repository owner moved this from 🔎 In Review to ✅ Done in Kubeapps Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/apprepository Issue related to kubeapps apprepository component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants