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

The OAuth integration seems to ask for vast permissions #120

Open
2 tasks done
savetheclocktower opened this issue Nov 29, 2023 · 5 comments
Open
2 tasks done

The OAuth integration seems to ask for vast permissions #120

savetheclocktower opened this issue Nov 29, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@savetheclocktower
Copy link
Contributor

Thanks in advance for your bug report!

  • Have you reproduced this issue in incognito/private browsing?
  • Have you made sure you issue doesn't already exist?

Where is the URL that this occurs?

https://web.pulsar-edit.dev/login

What's your issue?

I haven't seen this because I've already authorized Pulsar, but @steelbrain reported being presented with this screen when they tried to log in:

lots of permissions requested

I'm certain we don't need permissions this broad, and the authorization that exists in my GitHub settings asks for much less:

Screenshot 2023-11-28 at 5 15 31 PM

Do we have any idea what'd explain the disparity here?

Someone who's reluctant to give us this much access (i.e., most people) could instead create a PAT, but it'd be lovely if the documentation explained exactly which permissions are required when creating the token; otherwise it's just a game of trial and error.

Which OS/Browser/Version does this happen on?

No response

Steps to Reproduce/Additional Details:

No response

@savetheclocktower savetheclocktower added the bug Something isn't working label Nov 29, 2023
@savetheclocktower
Copy link
Contributor Author

savetheclocktower commented Nov 29, 2023

OK, the specific scope we ask for is public_repo, which correlates to:

Limits access to public repositories. That includes read/write access to code, commit statuses, repository projects, collaborators, and deployment statuses for public repositories and organizations. Also required for starring public repositories.

The description “Access public repositories” is therefore incredibly vague and misleading.

If I'm reading this page right, we don't actually need this scope if all we're doing is reading the code of a public repo. And I hope that's all that we're doing.

@DeeDeeG
Copy link
Member

DeeDeeG commented Nov 30, 2023

To the best of my knowledge (me and @confused-Techie were the ones discussing this at the time):

We asked for as little permissions to be put on the token as we could (to the best of our understanding), in order to enable all of what ppm (via the token) needs to be able to do. Account creation probably doesn't need it, but we don't store the token ourselves, we just check who owns the token and add that to our list of "people who actually asked to have a Pulsar package registry account". So, the point of the permissions is for ppm to use them. They're tangential to account creation, I suppose. (We don't let arbitrary GitHub users publish packages or auth to our DB/API servers unless they've "signed up" first, which I suppose prevents various kinds of abuse or shenanigans by making legitimate users jump through a hoop to prove that account actually is intending to use our services.)

Important: Only the user stores their token (locally). We read it (you send it) at account creation or at "login" on the web, to check who you are. Also: we read it (you send it) each time an authenticated ppm action is done, we check it (in-memory), and we move on -- we make no attempt to persist the tokens on our servers.

We need to know you have write permissions at a repo before we will let you publish a package for that repo. This is the main reason we ask for public_repo permissions. Likewise, I would presume we need to have read:org to know if you have a relationship to an org-owned repo before publishing a package associated with said org repo. (If we were wrong about needing all these scopes for checking that, then I'd be happy to be proven wrong so we can ask for less scopes.)

It was our intent to ask for the minimum scopes needed, but if someone can prove it all works with less, we'll definitely ask for less. I suppose the way to practically test which scopes are needed is to make a PAT with less and see how much of ppm works with less scopes. I think we already checked this, but there is some chance it was a misunderstanding from just reading the docs at the time.

@DeeDeeG
Copy link
Member

DeeDeeG commented Nov 30, 2023

And yeah, the OAuth tokens (equivalent to "classic" PATs) are pretty disappointing in how coarse-grained the permissions are.

I don't recall if our infrastructure understands the fine-grained tokens, though.

We leaned pretty hard on the notion that we don't hold onto the tokens on the server. And on the user side of things, ppm stores them in the OS's standard encrypted keystore (via keytar), so... We felt that this token hygiene was decent even if we'd have liked to ask for fewer permissions on the tokens.

@savetheclocktower
Copy link
Contributor Author

@steelbrain, I don't know if @DeeDeeG’s explanation quite puts you at ease. But if this is all true, it would at least suggest that we're not asking for anything more than what apm itself would've needed in the Atom days. We'll keep testing to see if we can make do with fewer permissions.

@DeeDeeG
Copy link
Member

DeeDeeG commented Nov 30, 2023

Oh, also, if you "log in" to the Pulsar package website, and view your account webpage, the token is stored locally by your browser storage during the "login," and shown to you from your local browser storage on that page. Per what I recall @confused-Techie saying about this before. (Initial account creation also implies a one-time login, IIRC.)

This is not to say that you never send the token to us, anytime you authenticate to us you send the token, but our server code is all open-source, and we're making a conscious effort not to do anything weird or careless with them. (EDIT to clarify/reiterate: And yes, we are deliberately not persisting them on our servers.)

Essentially we use the scopes to check what kind of permissions you have over on GitHub, and we mirror what we let you do based on what GitHub thinks you should be able to do at the relevant repositories you're trying to do stuff with. We believed it was safer to let GitHub essentially handle the security part and not try to roll our own solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants