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

Add optional configuration for restricting access to admin functionality by username #5

Merged
merged 3 commits into from
Jun 9, 2014

Conversation

junkafarian
Copy link
Contributor

No description provided.

if not admin_users:
return is_authenticated()

return is_authenticated() and session['gh-username'] in admin_users
Copy link
Contributor

Choose a reason for hiding this comment

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

will session always have the 'gh-username' (once you're authenticated)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unless we go with some other authentication method (other than github)

Copy link
Contributor

Choose a reason for hiding this comment

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

think gh-only is a fair assumption for now

did we add some way to restrict to an "allowed set" of gh users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In terms of user facing, #8 should prevent users from seeing pull requests from repos they have no access to. Do you think we need another authorisation layer?

@davidszotten
Copy link
Contributor

this is ok for now, but possibly not a great pattern since missing/bad config lets more people in

@junkafarian
Copy link
Contributor Author

can switch the condition to require an admin list to be configured if you'd prefer?

@davidszotten
Copy link
Contributor

yes, let's

@davidszotten
Copy link
Contributor

👍

junkafarian added a commit that referenced this pull request Jun 9, 2014
Add optional configuration for restricting access to admin functionality by username
@junkafarian junkafarian merged commit 0cc172d into master Jun 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants