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

Requests too excessive authorization #2

Closed
gitster opened this issue May 23, 2015 · 3 comments
Closed

Requests too excessive authorization #2

gitster opened this issue May 23, 2015 · 3 comments

Comments

@gitster
Copy link

gitster commented May 23, 2015

Maybe it is just me, but it feels that requesting read & write authorization for all public repositories of the user is way too excessive. If a user can live without the "I sent email using submitgit" breadcrumb left in his or her pull request, can't this be done only with read authorization on pull requests and nothing else?

Not that I'd be afraid that Roberto may go evil; more realistic and serious worry would be the service can be taken over by malicious crackers, whose evil deeds Roberto would not have any control over.

@gitster gitster changed the title Requests authorization too excessively Requests too excessive authorization May 23, 2015
@rtyley
Copy link
Owner

rtyley commented May 23, 2015

That's a totally fair concern - at the moment, submitGit uses the
user's credentials for all things, including reading the PR from the
repo, so it needs 'repo' access - but there is no reason why it can't
read those public details with it's own credentials, and comment on
the PR with it's own account. I think the only required auth is the
remaining one to read the user's email details.

I think the one thing it won't be able to do is close the PR (which it
doesn't do at the moment anyway).

I'm on holiday at the moment, without a laptop, so will start work on
this stuff mid-next week. As it happens, I've found that the Gmail
android app on my phone always sends HTML, so can't easily reply to
the Git mailing list either at the moment!

On 23/05/2015, Junio C Hamano notifications@github.com wrote:

Maybe it is just me, but it feels that requesting read & write authorization
for all public repositories of the user is way too excessive. If a user can
live without the "I sent email using submitgit" breadcrumb left in his or
her pull request, can't this be done only with read authorization on pull
requests and nothing else?

Not that I'd be afraid that Roberto may go evil; more realistic and serious
worry would be the service can be taken over by malicious crackers, whose
evil deeds Roberto would not have any control over.


Reply to this email directly or view it on GitHub:
#2

@mhagger
Copy link
Contributor

mhagger commented May 24, 2015

@rtyley I love this tool! Thanks for working on it!

Would there be advantages/disadvantages to creating a submitgit/git repo that is forked from git/git and owned by a submitgit org? Then you could have users submit PRs against that repo instead of directly against git/git?

The repo would be owned by a submitgit org with its own creds, making it possible to close PRs under its own authorization. It would also avoid the need for users to create PRs against git/git. Such PRs would be "dead" in the sense that no discussion is taking place there, and if discussion were added there it would usually be ignored, so having the PRs a little bit distant from git/git might be an advantage. On the other hand, the submitgit/git repo would be harder to find than git/git and people would have to select it as their PR base repository explicitly in most circumstances, adding a bit of friction.

I don't know...it was just a thought.

rtyley added a commit that referenced this issue May 28, 2015
The `public_repo` requirement is dropped, with users now only required
to give the `user:email` scope so we can get their email address.

https://developer.github.com/v3/oauth/#scopes

Note that existing users will get a slightly alarming-looking notification
from GitHub to tell them that the required permissions have been reduced:

!['Removed permissions' is highlighted in red](https://cloud.githubusercontent.com/assets/52038/7860692/2ae50980-0543-11e5-9f6d-01d001ef0fab.png)

The PR comment generated when a user sends mail to the list is now made
by the @submitgit account.

Conditional response caching with OkHttp is now also enabled so that
there's less risk of running out of GitHub API quota - given that now we
don't take advantage of the user's quota.

http://thread.gmane.org/gmane.comp.version-control.git/269699/focus=269733
rtyley added a commit that referenced this issue May 29, 2015
Fix issue #2, reducing the oauth scope permissions required from user
@rtyley
Copy link
Owner

rtyley commented Jun 10, 2015

Closing, because I think #3 addressed this.

@rtyley rtyley closed this as completed Jun 10, 2015
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

No branches or pull requests

3 participants