Skip to content

Admins cannot view /package/:package/maintain #124

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

Closed
hesselink opened this issue Oct 7, 2013 · 6 comments · Fixed by #1045
Closed

Admins cannot view /package/:package/maintain #124

hesselink opened this issue Oct 7, 2013 · 6 comments · Fixed by #1045

Comments

@hesselink
Copy link
Contributor

Users in the admins group cannot view /package/:package/maintain, but they can access all the links on that page. I think they should also be able to view that page.

@dcoutts
Copy link
Contributor

dcoutts commented Oct 22, 2013

Should be easy to fix. There's an auth check that uses a list of groups.

@gracenotes
Copy link
Contributor

Admins (who administer users/infrastructure) shouldn't need to do this, right? This should be in the realm of trustees (who administrate packages). If there are admins who are also de facto trustees, they should be added to the trustees group.

(All of the links on the maintain page should probably be changed likewise.)

@hesselink
Copy link
Contributor Author

I've looked into this a bit further. All html pages linked from the 'maintain' page have no security checks, but the actual actions all call guardAuthorisedAsMaintainerOrTrustee, which means admins cannot perform them. The exception is editing the maintainers group, which is also editable for admins.

So the question here really is what we want. Adding a maintainer (usually for a package takeover) is something we now do regularly as admins, so it would be convenient if the index page is viewable. If we decide that this is not a task for admins but only for trustees, then the permissions for the editing of maintainers should be tightened.

@dcoutts
Copy link
Contributor

dcoutts commented Apr 23, 2014

I would suggest we eliminate guardAuthorisedAsMaintainer and guardAuthorisedAsMaintainerOrTrustee, replacing them with their expansions. Their expansions are simpler, more uniform, and easier to audit and edit. In particular, this change would be trivial: just adding the admins group to the list.

@hesselink
Copy link
Contributor Author

So should we make this change?

@gbaz
Copy link
Contributor

gbaz commented Apr 24, 2015

i think we should, yes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants