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

Support access token authorisation for remote files #5239

Closed
kitsonk opened this issue May 12, 2020 · 32 comments · Fixed by #9508
Closed

Support access token authorisation for remote files #5239

kitsonk opened this issue May 12, 2020 · 32 comments · Fixed by #9508
Assignees
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2020

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:

import * as foo from "https://$TOKEN@raw.githubusercontent.com/private_org/private_repo/master/foo.ts";

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:

Authorization: token $TOKEN

If this was somehow passed on the command line, it would become easier to secure.

@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels May 21, 2020
@tjstebbing
Copy link

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.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 17, 2020

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 [TOKEN]@[HOST] would work? So on the CLI it something like:

$ deno run --auth-token [TOKEN]@raw.githubusercontent.com https://raw.githubusercontent.com/private_org/repo/master/main.ts

Or:

$ deno run --auth-token=[TOKEN]@raw.githubusercontent.com,[TOKEN]@example.com https://raw.githubusercontent.com/private_org/repo/master/main.ts

@Spoonbender
Copy link
Contributor

I like it.

Some points:

  • Tokens can be quite long, and command lines do have a length limit, so i think there should also be an option to set them in a file (resembles .npmrc)
  • Tokens should also be settable via environment variables, to make life easier ehen integrating with build pipelines. This could be trivial for command line, but requires some more effort for files (finding and replacing placeholders)
  • What kind of credentials to support? Just bearer tokens, or also others like basic auth etc.?

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 19, 2020

We have avoided meta data files. I would loath to add one for this. Using an environment variable like DENO_AUTH_TOKEN with the same comma separated format should address the length problem. As far as other auth, personal opinion is we keep it to tokens. Basic Auth encourages bad security practices and I am not aware of something professional grade that doesn't support tokens for things like this. It could be an enhancement down the road if there is a legitimate use case.

@benkeil
Copy link

benkeil commented Jul 20, 2020

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.

@qballer
Copy link

qballer commented Jul 20, 2020

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.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 20, 2020

a way to put inside the import map file

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.

@maciejmaciejewski
Copy link

maciejmaciejewski commented Jul 20, 2020

For now everything in this thread is around GitHub, I would strongly suggest to also support other repositories like Azure DevOps, BitBucket, etc.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 20, 2020

@maciejmaciejewski they support auth tokens as well...

GitHub didn't invent the standard.

@maciejmaciejewski
Copy link

@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.

@benkeil
Copy link

benkeil commented Jul 20, 2020

We could use providers for authentication like github:denoland or azure:denoland. Deno needs to recognize that an import from e.g. https://raw.githubusercontent.com/private_org/private_repo/master/foo.ts is handled by the corresponding provider.

@tjstebbing
Copy link

tjstebbing commented Jul 30, 2020

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 😿

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 30, 2020

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.

@tjstebbing
Copy link

tjstebbing commented Jul 30, 2020

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?

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 30, 2020

@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.

@keithamus
Copy link

As most providers use the Authorization header, one option is to simply provide the raw header value as an environment variable i.e. the env var would include the Bearer or Basic portion of the string.

@benkeil
Copy link

benkeil commented Jul 31, 2020

For GitHub it’s enough to have an environment variable. Deno just needs to Respekt it.

@silverwind
Copy link

silverwind commented Sep 20, 2020

I'd suggest something like this:

"importOptions": {
  "headers": {
    "authorization": {
      "github.com/denoland/deno": "Bearer ${DENO_TOKEN}",
    }
  }
}

authorization can be replaced with any custom header name. The keys inside authorization could be tested against each import URL and the first partial substring match should be used. In the same vein, one could implement TLS client certificates for import:

"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 import.meta.options.

@kitsonk kitsonk self-assigned this Oct 22, 2020
@jerrygreen
Copy link

jerrygreen commented Nov 13, 2020

My suggestion is to add -n or --netrc (or similarly named) option, which will handle credentials placed in ~/.netrc, just like curl:

Usage: curl [options...] <url>
 -n, --netrc         Must read .netrc for user name and password
     --netrc-file <filename> Specify FILE for netrc

In ~/.netrc you'll have this:

machine   raw.githubusercontent.com
login     YOUR_TOKEN
password  x-oauth-basic

(YOUR_TOKEN is a personal Github token with "Full control of private repositories")

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 -n option though:

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.

@bartlomieju
Copy link
Member

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:

@lucacasonato
Copy link
Member

@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).

@silverwind
Copy link

silverwind commented Nov 13, 2020

Keep in mind .netrc will probably not be able to support other forms of authorization, like client certificates. Also, storing passwords in plaintext on the disk may be a security risk.

@yacinehmito
Copy link
Contributor

yacinehmito commented Jan 2, 2021

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:

  • Basic authentication for remote imports, as some have requested in this issue
  • Proxying all imports; for example, rerouting imports from GitHub to a local server that handles authentication
  • Customising Deno's caching behaviour of modules (if desirable)
  • Developing client applications with Deno that can work with a spotty network (the original use case for Service Workers)

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?

@scucchiero
Copy link

Looking forward to this feature!
Is there a reason why token auth is preferred over SSH/SSH-forwarding? I feel like SSH auth would perhaps work better with CI/CD.

@kitsonk kitsonk added this to the 1.8.0 milestone Jan 21, 2021
@kitsonk
Copy link
Contributor Author

kitsonk commented Jan 25, 2021

@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 https some sort of token would require the least amount of configuration and the most straight forward way to provide it.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jan 25, 2021

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 Authorization: Bearer [token] header.

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 variables

We would use two environment variables: DENO_AUTH_SITES and DENO_AUTH_TOKENS where the value of each would contain a semi-colon delimitated set of sites and tokens. For example:

$ DENO_AUTH_SITES=github.com;gitlab.com DENO_AUTH_TOKENS=abc123def456;2b3c4d5e6g deno run main.ts

Would send an Authorization: Bearer abc123def456 for any remote modules where the host ends with .github.com and Authorization: Bearer 2b3c4d5e6g for any remote modules where the host ends with .gitlab.com.

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 JSON

This being complex and potentially difficult to maintain and to provide flexibility with secrets management, I propose we support the --tokens argument on the command line which would allow this to be configured to via a JSON file, where the format would look something like this:

{
  "github.com": "abc123def456",
  "gitlab.com": "2b3c4d5e6g"
}

@satyarohith
Copy link
Member

satyarohith commented Jan 25, 2021

Any downside if we use a single variable for both sites and tokens?
Example:

DENO_AUTH_CONFIG=github.com;abc123def456;gitlab.com;2b3c4d5e6g

The first value always being the site followed by the corresponding token. And delimited by a semi-colon as described above.

+1 for the --tokens flag.

@benkeil
Copy link

benkeil commented Jan 25, 2021

What about a .denorc in the home directory with

github.com:123455abcde
gitlab.com:agkdhg174726

This is how npm works

@kitsonk
Copy link
Contributor Author

kitsonk commented Jan 27, 2021

@benkeil actually it is more like:

//github.com/:_authToken=123455abcde
//gitlab.com/:_authToken=agkdhg174726

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:

DENO_AUTH_TOKENS=github.com:abc123def456;gitlab.com:2b3c4d5e6g

I dislike using the same delimiter for different fields... so a : to delimit a host (or IP) and ; to deliminate tokens. IPv6 should be supportable using the square bracket:

DENO_AUTH_TOKENS=[::1/128]:2b3c4d5e6g

@satyarohith
Copy link
Member

so a : to delimit a host (or IP)

@kitsonk Wouldn't this be a problem if users are providing a port number along with the host?

@Spoonbender
Copy link
Contributor

: could get annoying if we have to escape it for ports and for IPv6. Perhaps use @, like they often do on URL schemes that contain both addresses and credentials (FTP, SMB, SSH etc.)

@kitsonk
Copy link
Contributor Author

kitsonk commented Feb 8, 2021

Ok, the @ is a better separator for the host and the token. So:

DENO_AUTH_TOKENS=abc123def456@github.com;2b3c4d5e6g@gitlab.com;2b3c4d5e6g@[::1/128];g00dg05h@localhost:8080

I still think we should use the square braces for IPv6 literal addresses though (like in the example).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging a pull request may close this issue.