Replies: 6 comments 1 reply
-
|
Hi @alamothe, I am not sure what security issue you are reporting. Is there a bug or are you in disagreement with the way the plugin works? Most production software involves secrets that need to be managed securely. Requests to the GitHub API will inevitably involve secrets. I might be misunderstanding you but I am not sure what you expect from the plugin in this regard. If you're planning to share the token with coworkers, I would follow the advice here (blue tip bubble) and create a machine account for company-related automation. Does that address the issue? If not, could you be more specific about your concern and how you expect the plugin to handle it? |
Beta Was this translation helpful? Give feedback.
-
|
Hello,
Not only we need to set up a secrets management in order to use it, but it requires a big leap of faith to put the token there, as that means trusting this plugin does not have exploits, and same goes for Verdaccio, which is an unrealistic expectation. My suggestion is to make the token optional, and allow the plugin to operate without it. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the update. 👍
You will need a token either from the registry owner or from every user. For every user accessing the registry, you'll need to be able to make decisions whether that user can access, publish, unpublish, or view a package based on the org, repo, or team memberships configured in your registry config. In order to be able to verify those memberships, you need to query the GitHub API using a token. This token can come from either the registry owner or the registry user. For example, in order to decide whether a user has access to a private repo, you can either request the list of repo members from the registry owner's perspective, or a list of accessible repos from the user's perspective. For private repos this means: if we were using user tokens, we would need to ask every registry user to grant full private repository access (including write permissions) to all their repositories. This seems like asking for a lot and seems more insecure than asking just the registry owners to configure a token, who are in control of the registry and in control of their own secrets management. From a technical perspective, there is no way of making the necessary GitHub API requests required to verify user access without the corresponding tokens involved from someone.
You need to set up secrets management anyway since you need to configure the OAuth client id and secret. The GitHub token is just one more environment variable.
If the token was optional and we continue not to ask users for access to any of their data, all you would be able to verify is that someone has a GitHub account, which every person can have for free. But the point of the plugin is to limit access to the registry based on the users' access to a certain GitHub organization, repository, etc.
I'm not familiar with that plugin, but either it's requesting access to personal information from all its users (even information that the registry/plugin doesn't care about), which in my opinion is worse. Or it's not able to make those decisions and doesn't support registry access by repository, for example. Note that the registry owner token only needs visibility to what it actually needs to verify. If you're not using private repos in your registry config, your token also does not need private repo visibility. If all you need is public information, you can create a personal access token with all permissions unchecked. A token is still necessary due to GitHub API rate limiting on anonymous requests. |
Beta Was this translation helpful? Give feedback.
-
But that's a very different scope. Client ID and secret can't do much if anything (you can just pretend to be that app), while the other one grants access to the whole repo.
It's claiming that it can check if the user belongs to an org, which is the majority of use cases. So my suggestion is to support a "degraded" mode where the token would be optional, and only support org level membership.
That's a fair compromise, if it supports org membership (majority of use cases) |
Beta Was this translation helpful? Give feedback.
-
|
Hi @alamothe, As said, the technical options for implementing such checks are:
As said previously, I believe the second option is the better and safer choice, especially for registry users. I believe the "degraded" mode you would like to see is already built into GitHub tokens. You can already limit a token's scopes to what you need it to be able to access, to the granularity GitHub provides. In your case, read:org should suffice. The plugin doesn't need any change for that to work. Thanks for bringing this up. I am failing to see the need or benefit of changing the way the plugin authorizes with GitHub. Requesting GitHub information through a registry owner PAT is an intentional choice over doing the same through registry user access tokens. From here on, I'm afraid I will mostly repeat myself. If you have any new perspectives to share, feel free to reply and I will consider them. I hope it was helpful. |
Beta Was this translation helpful? Give feedback.
-
|
In our case, we only use this plugin for verifying organization membership so the previous "request read:org scope" was sufficient. So requiring a token here just seems unnecessary. The problem with tokens is that a user has to own them. While machine accounts work, they cost money (because they're an actual user) and you can't easily scope personal access tokens (the new fine-grained tokens work around this but are in beta and don't support multiple orgs). Have you considered using Github app authentication instead? That allows you to have an actual "machine account" access token without requiring an actual Github account. It also has the strict controls that fine-grained tokens have so you can restrict which organizations and repos are accessible. |
Beta Was this translation helpful? Give feedback.
-
Maybe we can support both mechanisms as alternatives. You're welcome to create a PR for it.
It might not be the ideal solution for everyone but it seemed as the best tradeoff so far.
Looks like that would be a good addition. Maybe we could offer this as an alternative. You're welcome to create a PR for it. |
Beta Was this translation helpful? Give feedback.
-
Bug Report
This plugin requires
tokento work. It simply says:No matter how you deploy this, it is a giant security hole as other people in my company will be exposed to the token.
Versions
Expected behavior
Allow this plugin to operate without token.
Beta Was this translation helpful? Give feedback.
All reactions