Skip to content
This repository has been archived by the owner on Aug 27, 2024. It is now read-only.

Require less permissions for Zappr #90

Closed
3 tasks done
prayerslayer opened this issue Mar 11, 2016 · 3 comments
Closed
3 tasks done

Require less permissions for Zappr #90

prayerslayer opened this issue Mar 11, 2016 · 3 comments

Comments

@prayerslayer
Copy link
Contributor

prayerslayer commented Mar 11, 2016

It should be possible to give Zappr only a minimal set of permissions initally and then upgrade.

Braindumping how it could work

  • Store consented scopes in a new column in User (first* db migration!)
  • Change config options to accomodate new premium/advanced/god mode.
  • Set cookie to indicate that god mode is used.
  • Dropdown in nav header to change mode.
  • Bonus points: When a check is enabled, it should then show a) who the token belongs to and b) what scopes it has / which mode it was created for and c) if it's valid (see below).
  • Figure out how to obtain consented scopes, should somehow work with the passport plugin: Does not currently work with the passport plugin. Workaround: Make HTTP request to Github API with token, read X-OAuth-Scopes response header.
  • Figure out how we can change consented scopes: Just do another login with more scopes. This won't work for removing scopes though, the users will have to revoke access (effectively invalidating all previously issued tokens) and log in again.
  • Figure out what happens to the old token when a user changes modes: Nothing, stays valid.
@mfellner mfellner modified the milestone: Sprint 3 Mar 20, 2016
@mfellner mfellner modified the milestone: Sprint 3 Apr 15, 2016
@prayerslayer prayerslayer changed the title Do not require public_repo.write initally Require less permissions for Zappr Apr 22, 2016
@ePaul
Copy link
Member

ePaul commented Jul 26, 2016

I agree with this ... I don't really feel comfortable with giving some application write access to all my repositories, or admin access to hooks (for all my repositories). (Even if that application is hosted by Zalando, which I know is trustworthy.)

It should be possible to request the permissions just for those repositories for which they are needed (I'm not installing zappr for all my private repos, just some Zalando ones), and just for the time it is needed. (The admin permissions are only needed while installing/uninstalling, right?)

Do we really need write access to the repo to get the list of contributors? Could I remove that permission when not using this feature in my .zappr.yaml, and not using the branch creation feature? (I don't really see this branch creation thing as valuable – not every issue needs a branch, and often those branches should have a different name anyways, or will be on some forked repository.)

@prayerslayer
Copy link
Contributor Author

prayerslayer commented Jul 26, 2016

It should be possible to request the permissions just for those repositories for which they are needed

Yeah, but the Github API doesn't work that way. Either we get access to all public repos or all public and private repos.

Could I remove that permission when not using this feature in my .zappr.yaml

Yes, that would work.

The main obstacle is that it would take some work to get the UX of all this right.

@ePaul
Copy link
Member

ePaul commented Jul 27, 2016

It should be possible to request the permissions just for those repositories for which they are needed

Yeah, but the Github API doesn't work that way. Either we get access to all public repos or all public and private repos.

Hmm, so Github has a too limited permission model here :-(

(I did have a look to find if there is some public feature request tracker for Github, but didn't find any.)

prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 14, 2016
prayerslayer added a commit that referenced this issue Aug 15, 2016
prayerslayer added a commit that referenced this issue Aug 15, 2016
prayerslayer added a commit that referenced this issue Aug 15, 2016
@mfellner mfellner added this to the GitHub Integration milestone Aug 30, 2016
prayerslayer added a commit that referenced this issue Aug 31, 2016
mfellner pushed a commit that referenced this issue Sep 5, 2016
prayerslayer added a commit that referenced this issue Sep 6, 2016
* #90 save wip

* #90 recover mode via database if necessary

* #90 env reducer for client

* #90 no need to import all env

* #90 make zappr_mode an enum

* #90 remove react-cookie

* #90 switch order with profile

* #90 add some tests

* #90 docs, tests

* #90 fix migrations

* #90 fix migrations again

* #90 remove console.log

* #90 add extended scopes to config

* #90 documentation

* #90 more documentation

* #90 remove unused loggers, add image

* #90 rename mode to access level

* #90 rename upgrade/downgrade link

* #90 new docs image

* #90 do not commit changes of db settings

* #90 rename /change-mode to /change-access-level

* #90 change migrations as model.sync() will go away

* #90 rename migration

* #90 fix: addColumn args in migration
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants