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

apiserver: allow AllWatcher access even without controller login access #6453

Merged

Conversation

rogpeppe
Copy link
Contributor

A watcher is special in that it can't do anything unless a resource
has already been created (with a Watch method). That means
that the important place to check permissions is when the
watcher resource is created, not when the watcher is used.

The permission check removed here was both unnecesssary
(as described above) and insufficient (the user might have login
access to the controller but no access to the model being watched,
for example).

Copy link
Contributor

@mhilton mhilton left a comment

Choose a reason for hiding this comment

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

👍

@rogpeppe rogpeppe force-pushed the 118-allow-allwatcher-model-access branch from 01b2528 to 6955008 Compare October 14, 2016 14:49
Copy link
Member

@frankban frankban left a comment

Choose a reason for hiding this comment

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

Looks good and solves a GUI issue we are seeing.

A watcher is special in that it can't do anything unless a resource
has already been created (with a Watch method). That means
that the important place to check permissions is when the
watcher resource is created, not when the watcher is used.

The permission check removed here was both unnecesssary
(as described above) and insufficient (the user might have login
access to the controller but no access to the model being watched,
for example).
@rogpeppe rogpeppe force-pushed the 118-allow-allwatcher-model-access branch from 6955008 to d6224d1 Compare October 14, 2016 14:54
@rogpeppe
Copy link
Contributor Author

!!build!!

@rogpeppe
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Oct 18, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 2744413 into juju:develop Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants