Skip to content

Support 3rd-party authentication via bearer token, also other patches #2167

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

Closed
wants to merge 14 commits into from
Closed

Support 3rd-party authentication via bearer token, also other patches #2167

wants to merge 14 commits into from

Conversation

thosakwe
Copy link

@thosakwe thosakwe commented Jul 9, 2019

Hello! This pull request is an implementation of the features discussed in the issue #1381.

Changes to Existing Functionality

  • Use withClient downloading hosted packages. This allows bearer tokens to be sent to 3rd-party servers when downloading packages. Otherwise, any Pub server would need to make all downloads publicly-accessible, without a token. However, this effectively requires authentication even when downloading from pub.dartlang.org/pub.dev. (If this is unacceptable, the logic can be modified to only send a token when using a 3rd-party server, and use the regular httpClient.)
  • Resolve the URLs for api/versions/new, api/packages/:package/uploaders, and api/packages/:package/uploaders/:uploader relative to the $PUB_HOSTED_URL. In my testing, I noticed that calls to pub get, pub upgrade, pub global activate, (basically any command that involved a download) worked fine when the $PUB_HOSTED_URL included a path, because their implementations append the desired URI to $PUB_HOSTED_URL (ex. https://foo.com/my/path -> https://foo.com/my/path/api/packages/string_scanner). However, since pub uploaders and pub lish were using Uri.resolve with absolute URIs, whatever path was in $PUB_HOSTED_URL would be obliterated.

New Functionality

From the discussion on #1381:

I've discussed this IRL with a few people and consensus is that:

  • pub should use an opaque token for authentication with 3rd party servers.
  • The opaque token is attached as authorization: bearer <token> in authenticated requests.
  • An opaque token is stored for each hostname in $PUB_CACHE/tokens.json (other than pub.dartlang.org and pub.dev, which will use credentials.json).
  • pub should prompt users for this token on the command line interface, if not present in $PUB_CACHE/tokens.json.

This means that pub.dartlang.org and pub.dev will have special treatment, where as for all other pub servers the flow will be as follows:

  1. User starts command that requires authentication (e.g. pub publish).

  2. If $PUB_CACHE/tokens.json doesn't contain a token for the pub server (identified by hostname), then:

    • Prompt user for a token (asking the user to obtain this from the server out of band).
    • Associated token with hostname for server in $PUB_CACHE/tokens.json.
  3. Use token associated with hostname for server from $PUB_CACHE/tokens.json.

  4. Send request associating token using authorization: bearer <token>.

@thosakwe, if you're interested in working on this let me know, I'll be happy to answer questions around the design, and help with reviews. This is definitely a contributable features and I think we want it :)

Consider: attempting without a token first, and when receiving a 401 ask the user for a token, with a message from the server's reply.
This allows a custom server to guide the user to obtain a token.

What's Missing

  • There are currently no unit tests for the new functionality. This will change.

Other

Resolves #1381.

@thosakwe
Copy link
Author

thosakwe commented Jul 9, 2019

My current implementation has a known bug. When the server receives a 401, after the user is prompted for a token, pub will attempt to resend the same Request instance, which is not allowed, because it has already been finalized.

A possible solution is sending a "preflight" request to the 3rd-party server if there is no token, perhaps to $PUB_HOSTED_URL/api/packages/version/new, or another static endpoint. If the server returns a 401, then prompt the user for the token. Finally, send the original Request.

@jonasfj jonasfj self-requested a review July 9, 2019 14:07
@jonasfj
Copy link
Member

jonasfj commented Jul 11, 2019

this effectively requires authentication even when downloading from pub.dartlang.org/pub.dev...

As you've anticipate that's not going to work. Lots of developers never publish packages.
Moreover, it's going to be a real problem in CI systems like travis :)
Allowing users to fetch packages without authentication is probably critical, even for 3rd party servers.

But we could say that if the GET request responds 401 with a header:

WWW-Authenticate: Bearer realm="<some message>"

Then the client will:

  1. Print <some message> (only allowing human readable text, so sanitizing for fancy stuff, max 2kb)
  2. Ask the client to authenticate, if talking to pub.dartlang.org then initiate oauth2 flow, if talking to third-party server ask the user to enter a token.

This way servers can put information about how to obtain the authentication token into <some-message> in the WWW-Authenticate header outlined above.

Side note: I think this would also be in line with how www-authenticate is supposed to be used :)


I'm super excited about this PR! But I haven't had time to look through all the code. Don't worry, I'll get back to taking a deeper look.

But perhaps we should split this in two PRs. Moving all the URLs for api/versions/new, api/packages/:package/uploaders, etc. to be relative to $PUB_HOSTED_URL is awesome. But we could do that in a separate PR.
These probably have to relative to the publish_to key from the current pubspec.yaml (if any), but be overwritten by $PUB_HOSTED_URL if that is provided.
(We should mirror the precedence rules already used for publish_to when publishing).

Similarly, when authenticating, we probably look at the URL rather than $PUB_HOSTED_URL. Since you can have a single project that uses packages from multiple package repositories. You can simply specify the url for your hosted dependencies in the pubspec.yaml, see:
https://dart.dev/tools/pub/dependencies#hosted-packages

This also makes sense. It's nice that third parties with their own pub servers can still fetch packages from pub.dev.

@thosakwe
Copy link
Author

thosakwe commented Aug 8, 2019

SGTM. I'll first send the PR for moving URL's relative to the hosted URL. Later, I'll send the patch to support auth via third party hosts. In the meantime, I will close this PR.

I like the idea of sending a WWW-Authenticate header too, I think that's a very low-friction, easy-to-implement solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support authenticating pub against 3rd-party servers
3 participants