-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Support access token authorisation for remote files #5239
Comments
This is a show stopper for Deno at many companies who need to use private modules and have a policy of not embedding auth tokens into their software. |
What we need is a decent proposal of what the UI should be for passing it. It should be easy to specify multiple tokens for different hosts, and only when connecting to those hosts, will the header be sent. Maybe
Or:
|
I like it. Some points:
|
We have avoided meta data files. I would loath to add one for this. Using an environment variable like |
I like the way node was going with something like a .npmrc file. Think also about how to build the code in a CI server. Maybe there's a way to put inside the import map file. {
"imports": {
...
},
"authentication": {
"github:denoland": "${GITHUB_ACCESS_TOKEN}",
"github:denoland/deno": "${GITHUB_ACCESS_TOKEN_2}"
}
} Without that feature Deno in unusable for us, because all our libraries are in private GitHub repositories. |
This is a brilliant idea and would really assist when modules are hosted on private registries. I really like the import map file suggestion as to where to place this config, as far as I can tell, the only problem is, this is nonstandard AFAIK from here. I suggest that any programmatic API will also include authentication tokens and then the community can build there own tooling around it. |
Import maps are based on a draft standard. Modifying that will end up in 😢. There is this issue #3179 for that, but I don't think it will go anywhere any time soon. The feature can easily ship without solving that problem too. |
For now everything in this thread is around GitHub, I would strongly suggest to also support other repositories like Azure DevOps, BitBucket, etc. |
@maciejmaciejewski they support auth tokens as well...
GitHub didn't invent the standard. |
@kitsonk What I meant is all of those can handle the Authorization header in a different way - some can use Bearer token, some other Basic one so just want to make sure that all the use cases are covered. |
We could use providers for authentication like |
But where do you end with adding 'providers'? Now there's a magic list of 'providers' who are 'important enough' ? This doesn't seem like a scalable solution. Also +1 for avoiding more dotfiles, most javascript repos have more dotfiles than codefiles these days 😿 |
We need to get it implemented on the command line and with environment variables first, and iterate from there. We don't need to solve every problem at the start. |
One issue I see with the [TOKEN]@[HOST] solution is needing to provide tokens for multiple private github projects at the same host. I don't have this issue right now but I cam imagine someone trying to provide a token for two separate private repositories both on the same host (github). The HOST would need to include some path components to avoid conflict? |
@pomke the tokens are tied to users, not repos... it would be a limitation that the same user has to have read access to all the repos. That is a limitation that seems reasonable. |
As most providers use the |
For GitHub it’s enough to have an environment variable. Deno just needs to Respekt it. |
I'd suggest something like this: "importOptions": {
"headers": {
"authorization": {
"github.com/denoland/deno": "Bearer ${DENO_TOKEN}",
}
}
}
"importOptions": {
"clientCertificate": {
"github.com/denoland/deno": "${CLIENT_CERT}",
},
"clientCertificateKey": {
"github.com/denoland/deno": "${CLIENT_CERT_KEY}",
}
} The data could also be expose to JS using |
My suggestion is to add
In
( Now you can easily: curl -n https://raw.githubusercontent.com/private_org/private_repo/master/foo.ts Wouldn't work if you won't pass curl https://raw.githubusercontent.com/private_org/private_repo/master/foo.ts # will return 404 Just try it with Github and curl, it's very neat. Would be cool if it worked the same way in Deno. |
Thanks for suggestion @jerrygreen but we're very careful about introducing configuration files and we don't pick them up automatically. I think this problem problem should be solved as part of single "metadata" file: |
@bartlomieju We should not put this in the metadata file, because metadata file will be shared between multiple people. This config should be per user. I do like the idea of netrc. It is relatively standard, and existing parsers for it exist in Rust - we wouldnt have to invent anything new. We wouldn't pick up the file automatically though, (instead you have to explicitly specify the path to it with a flag, as environment variable, or as a key in the metadata file). |
Keep in mind |
Adding authentication with bearer tokens for remote imports is a special case of adding headers to fetch requests. The latter can already be done on the web using Service Workers. If we were able to register a Service Worker in the context of a Deno process, then we would be able to not only authenticate with bearer tokens for remote imports, but also support a large variety of other use cases, such as:
That's just from the top of my head; the possibilities are endless. This suggestion is not necessarily the definitive answer for authentication as it still leaves users with the responsibility of setting up a correct Service Worker. Still, it would be a foundation allowing for fast iteration, which sounds safer to me. Indeed, there can be a large variety of authentication schemes and ways to store credentials. The less we assume, the more users we can serve without handling each specific use case directly. @kitsonk What's your opinion on leveraging Service Workers with Deno? |
Looking forward to this feature! |
@scucchiero people could totally use SSH/SSH-forwarding without any changes to Deno if they wish. But to be able to fetch remote modules via |
Ok, I have dusted off this issue as I will be working on it for Deno 1.8. We discussed it as a core team, and I indicated that the first step would be to go back and make a specific proposal. I did some research and it seems pretty much everyone supports the But to solve this, we only want to set the authorization token to sites that we expect, and be able to support multiple sites. Based on all this, I am proposing the following: Configuration via environment variablesWe would use two environment variables:
Would send an If for some reason the environment variables were set, but the sets didn't align, Deno would immediately exit with an error message indicating a problem with the variables. Configuration via JSONThis being complex and potentially difficult to maintain and to provide flexibility with secrets management, I propose we support the {
"github.com": "abc123def456",
"gitlab.com": "2b3c4d5e6g"
} |
Any downside if we use a single variable for both sites and tokens?
The first value always being the site followed by the corresponding token. And delimited by a semi-colon as described above. +1 for the |
What about a
This is how npm works |
@benkeil actually it is more like:
Upon reflection, I think we should start with an environment variable solution and thing about what to do from there. Also, I think we can go to a single env variable, like:
I dislike using the same delimiter for different fields... so a
|
@kitsonk Wouldn't this be a problem if users are providing a port number along with the host? |
|
Ok, the
I still think we should use the square braces for IPv6 literal addresses though (like in the example). |
Currently there is no way to fetch remote modules from private GitHub repositories without exposing your access token.
For example you could do the following currently:
But you could easily "leak" your access token if you then checked that code in and pushed it to a public repo.
GitHub (and I assume other services) all the token to be passed as an authorisation header:
If this was somehow passed on the command line, it would become easier to secure.
The text was updated successfully, but these errors were encountered: