RFC: Authentication for read only access to Web UI #2524
Conversation
|
@bendikro, thanks for your PR! By analyzing the history of the files in this pull request, we identified @djmitche, @tardyp and @pieterlexis to be potential reviewers. |
There was a problem hiding this comment.
Beware this is a false sense of security.
You will effectively prevent access to index page, but not to the REST apis.
REST api is what realy matters in term of security as this is our valuable data. The index.html is opensource and public, no need to restrict that.
Then, in term of UX, I think this is a good thing to allow admin to force user login, with a lightwight login UI before sending the full fledge UI (like you did).
So overall I think this is a good PR.
- need documentation on how to setup this (with example)
- Need to put the login template inside buildbot_www module
- Please separate Bitbucket oauth in another PR.
| conf = self.config['auth'].getConfigDict() | ||
| conf.update(user_info) | ||
| conf["message"] = err | ||
| tpl = self.jinja.from_string(""" |
There was a problem hiding this comment.
Oh no! We don't want to hardcode any html here.
We need to get the index template from the buildbot_www module.
There was a problem hiding this comment.
Certainly, this was only a temporary quick fix.
| if isinstance(obj, dict): | ||
| return obj | ||
| return repr(obj) + " not yet IConfigured" | ||
|
|
||
| tpl = self.jinja.get_template('index.html') |
There was a problem hiding this comment.
This is how the template is found from the main www module
|
I believe this has be splitted into several PR alread |
This is most definitely not mergeable as is, and is mainly a request for comments.
Authentication for read only access to Web UI
I have tried to figure out how to enforce authentication for accessing the Web UI. The last trace I found of a discussion on this is in these emails which indicate this is not supported yet.
This PR adds a call to assertUserAllowed() in
www/config.IndexResource.renderIndex()which enforces authentication based on the allowRules. However, this changes the behavior of the current code such that AnyEndpointMatcher now matches any access to the Web UI.
According to the docs, the endpoint matchers are only responsible for the REST endpoints, so
I presume a proper solution would add a separate set of allowRules for viewing the different parts of the Web UI?
oauth2.GitHubAuth
https://api.github.com/users/{username}/orgsonly returns the organizations for the user that are publicly visible. Is that intended?This has been changed to `https://api.github.com/user/orgs' which returns all organziations for the user (requires scope=user)
oauth2.BitbucketAuth
Implemented
Contributor Checklist:
Feedback
Please give feedback on how I can improve this, or even better, make the required changes yourself if you desire.