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

Document connected account permissions #6172

Merged
merged 2 commits into from Sep 26, 2019

Conversation

@davidfischer
Copy link
Contributor

commented Sep 12, 2019

This is a documentation only pull request that just documents why we ask for the permissions from GitHub, Bitbucket, and GitLab. We've had a few questions about this in the past.

There is a meta question of whether this is the right place for this information. Another question is whether we should link to this from the login and signup screens so users are prepared for permission prompts.

@davidfischer davidfischer requested a review from readthedocs/core Sep 12, 2019
@saadmk11

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

Should we talk about the GitLab Event we are accessing (e.g: merge_requests_events) ?

# Optional
'issues_events': False,
'merge_requests_events': True,
'note_events': False,
'job_events': False,
'pipeline_events': False,
'wiki_events': False,

@stsewd

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

@saadmk11 webhooks are part of the API. That is only the list of events we subscribe when creating the webhook (that list of events are in this same page if I remember correctly).

@stsewd
stsewd approved these changes Sep 12, 2019
Copy link
Member

left a comment

There is a meta question of whether this is the right place for this information.

I think it is 👍

we should link to this from the login and signup screens so users are prepared for permission prompts.

I think this is a good idea. We should put this on https://readthedocs.org/accounts/social/connections/ too.

@davidfischer

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

If we're good with this change, let's go forward with it. Once this has "been released" an is available in the stable version, we'll add those links to it.

Copy link
Contributor

left a comment

It's good to have this explicitly stated, there have been questions here in the past. Mostly from commercial accounts wondering why we require admin repo permissions. Hopefully the move to a GitHub App, instead of the OAuth app, will give us more permission granularity and we can avoid asking for admin access.

However, we do need permissions for authorizing your account
so that you can login to Read the Docs with your connected account credentials
and to setup :doc:`webhooks`
which allow us to build your documentation on every change to your repository.

This comment has been minimized.

Copy link
@agjohnson

agjohnson Sep 24, 2019

Contributor

Unfortunately, the commercial side does ask for admin (and therefore write) permissions, as this is the lowest granularity that GitHub has/had in order to add private ssh keys. Moving to a GitHub App instead does probably give us finer permission control, if i remember correctly.

docs/connected-accounts.rst Show resolved Hide resolved
docs/connected-accounts.rst Show resolved Hide resolved
@agjohnson agjohnson merged commit 6905051 into master Sep 26, 2019
3 checks passed
3 checks passed
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details
@agjohnson agjohnson deleted the davidfischer/connected-account-perms branch Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.