Add a package for authenticating via OAuth2.

pkg/oauth2/lib/src/authorization_code_grant.dart

+// Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library authorization_code_grant;
+
+import 'dart:uri';
+
+import '../../../http/lib/http.dart' as http;

[Bob Nystrom, 2012/11/16 19:53:30]

Add TODO to use "package:" here. Mention bug 6745.

[nweiz, 2012/11/17 01:06:27]

Done.

+
+import 'client.dart';
+import 'authorization_exception.dart';
+import 'handle_access_token_response.dart';
+import 'utils.dart';
+
+/// A class for obtaining credentials via an [authorization code grant][]. This
+/// method of authorization involves sending the resource owner to the
+/// authorization server where they will authorize the client. They're then
+/// redirected back to your server, along with an authorization code. This is
+/// used to obtain [Credentials] and create a fully-authorized [Client].
+///
+/// To use this class, you must first call [getAuthorizationUrl] to get the URL
+/// to which to redirect the resource owner. Then once they've been redirected
+/// back to your application, call [handleAuthorizationResponse] or
+/// [handleAuthorizationCode] to process the authorization server's response and
+/// construct a [Client].
+///
+/// [authorization code grant]: http://tools.ietf.org/html/draft-ietf-oauth-v2-31#section-4.1
+class AuthorizationCodeGrant {

[Bob Nystrom, 2012/11/16 19:53:30]

The state enums and all of the StateErrors here are a bit of a code smell to me.
It feels like this is actually two classes masquerading as one. You could split
it into:

AuthorizationCodeGrant (_INITIAL_STATE)
- has getAuthorizationUrl() which returns a AuthorizationResponseHandler

AuthorizationResponseHandler (_AWAITING_RESPONSE_STATE)
- has handleResponse()

This might make things a little less error-prone. Of course, fewer types is good
too though. Thoughts?

[nweiz, 2012/11/17 01:06:27]

When I was designing this, I tried to strike a balance between statically
guiding users along the right path and having classes that represented
reasonable things. I think splitting up this class goes too far in the direction
of the former; there's no longer one class you can point to that's in charge of
the entire flow.

[Bob Nystrom, 2012/11/19 21:37:10]

SGTM.

+  /// An enum value for [_state] indicating that [getAuthorizationUrl] has

[Bob Nystrom, 2012/11/16 19:53:30]

Nittiest of nits, but it looks like most of these comments could be word-wrapped
at 80 columns more precisely. Do that if you want, but it's fine if you don't
too.

[nweiz, 2012/11/17 01:06:27]

Done.

+  /// not yet been called for this grant.
+  static final int _INITIAL_STATE = 0;

[Bob Nystrom, 2012/11/16 19:53:30]

"static final int" -> "static const". Here and elsewhere.

[nweiz, 2012/11/17 01:06:27]

Done.

What's the difference between "static const" and "static final"?

[Bob Nystrom, 2012/11/19 21:37:10]

"const" declares an actual Dart constant: something you can use in a const
expression. "final" declares a lazy-initialized single-assignment variable.

So this is fine:

const FOO = 1;
bar([foo = FOO]) { ... } // can reference FOO in a const expression

but this isn't:

final FOO = 1;
bar([foo = FOO]) { ... }

since parameter default values have to be const expressions. There are a couple
of other places where const expressions are required, but they're fairly rare.
In general, it seems like we are using const less and less in the language. (For
example, the recent change to RegExp to remove its const constructor.)

+
+  // An enum value for [_state] indicating that [getAuthorizationUrl] has
+  // been called but neither [handleAuthorizationResponse] nor
+  // [handleAuthorizationCode] has been called.
+  static final int _AWAITING_RESPONSE_STATE = 1;
+
+  // An enum value for [_state] indicating that [getAuthorizationUrl] and
+  // either [handleAuthorizationResponse] or [handleAuthorizationCode] have been
+  // called.
+  static final int _FINISHED_STATE = 2;
+
+  /// The client identifier for this client. The authorization server will issue
+  /// each client a separate client identifier and secret, which allows the
+  /// server to tell which client is accessing it. Some servers may also have an
+  /// anonymous identifier/secret pair that any client may use.
+  ///
+  /// This is usually global to the program using this library.
+  final String identifier;
+
+  /// The client secret for this client. The authorization server will issue
+  /// each client a separate client identifier and secret, which allows the
+  /// server to tell which client is accessing it. Some servers may also have an
+  /// anonymous identifier/secret pair that any client may use.
+  ///
+  /// This is usually global to the program using this library.
+  ///
+  /// Note that clients whose source code or binary executable is readily
+  /// available may not be able to make sure the client secret is kept a secret.
+  /// This is fine; OAuth2 servers generally won't rely on knowing with
+  /// certainty that a client is who it claims to be.
+  final String secret;
+
+  /// A URL provided by the authorization server that serves as the base for the
+  /// URL that the resource owner will be redirected to to authorize this
+  /// client. This will usually be listed in the authorization server's
+  /// OAuth2 API documentation.
+  final Uri authorizationEndpoint;
+
+  /// A URL provided by the authorization server that this library uses to
+  /// obtain long-lasting credentials. This will usually be listed in the
+  /// authorization server's OAuth2 API documentation.
+  final Uri tokenEndpoint;
+
+  /// The HTTP client used to make HTTP requests.
+  final http.BaseClient _httpClient;

[Bob Nystrom, 2012/11/16 19:53:30]

Using BaseClient as a type annotation here and elsewhere makes sense, but it
makes me wonder if the type names could be better in http. It seems like you'll
mention clients in type annotations more often than in constructor calls, so I
wonder if it should be something like:

  BaseClient -> Client
  Client -> DefaultClient (or some other name here)

I worry that otherwise people will just annotate to take Client and not realize
they've made their type annotation incorrectly too specific. Thoughts?

[nweiz, 2012/11/17 01:06:27]

I think this code gives a misleading impression about the degree to which people
will take clients as parameters versus constructing their own. I think in the
vast majority of cases, users will just want to construct a Client and make a
few HTTP requests with it. Creating wrappers like this is a much less common
case.

It's also worth noting that if you really need to pass a non-Client BaseClient
to a method annotated with "Client", you can have your client implement Client
as an interface.

One possibility would be to expose http.Client as an interface with a factory
constructor. BaseClient could then implement this interface. The downside to
that is that all of the BaseClient method signatures would have to be
duplicated, but maybe that's worthwhile.

[Bob Nystrom, 2012/11/19 21:37:10]

Yeah, I like this.

+
+  /// The URL to which the resource owner will be redirected after they
+  /// authorize this client with the authorization server.
+  Uri _redirectEndpoint;
+
+  /// The scopes that the client is requesting access to.
+  List<String> _scopes;
+
+  /// An opaque string that users of this library may specify that will be
+  /// included in the response query parameters.
+  String _stateString;
+
+  /// The current state of the grant object. One of [_INITIAL_STATE],
+  /// [_AWAITING_RESPONSE_STATE], or [_FINISHED_STATE].
+  int _state = _INITIAL_STATE;
+
+  /// Creates a new grant.
+  ///
+  /// `httpClient` is used for all HTTP requests made by this grant, as well as
+  /// those of the [Client] is constructs.
+  AuthorizationCodeGrant(
+      this.identifier,
+      this.secret,
+      this.authorizationEndpoint,
+      this.tokenEndpoint,
+      {http.BaseClient httpClient})
+    : _httpClient = httpClient == null ? new http.Client() : httpClient;
+
+  /// Returns the URL to which the resource owner should be redirected to

[Bob Nystrom, 2012/11/16 19:53:30]

"redirected" -> "directed"?

[nweiz, 2012/11/17 01:06:27]

In most cases this will be run on a server, where the resource owner's browser
will actually be redirected.

+  /// authorize this client. The resource owner will then be redirected to
+  /// `redirect`, which should point to a server controlled by the client. This

[Bob Nystrom, 2012/11/16 19:53:30]

backquotes -> square brackets. You can (and should) use [foo] for parameters
too. Here and elsewhere.

[nweiz, 2012/11/17 01:06:27]

Done.

+  /// redirect will have additional query parameters that should be passed to
+  /// [handleAuthorizationResponse].
+  ///
+  /// The specific permissions being requested from the authorization server may
+  /// be specified via `scopes`. The scope strings are specific to the
+  /// authorization server and may be found in its documentation. Note that you
+  /// may not be granted access to every scope you request; you may check the
+  /// [Credentials.scopes] field of [Client.credentials] to see which scopes you
+  /// were granted.
+  ///
+  /// An opaque `state` string may also be passed that will be present in the
+  /// query parameters provided to the redirect URL.
+  ///
+  /// It is a [StateError] to call this more than once for a given instance of

[Bob Nystrom, 2012/11/16 19:53:30]

Remove the "for a given..." part. That should be implied since this is an
instance method. :)

[nweiz, 2012/11/17 01:06:27]

Done.

+  /// [AuthorizationCodeGrant].
+  Uri getAuthorizationUrl(Uri redirect,
+      {List<String> scopes: const <String>[], String state}) {
+    if (_state != _INITIAL_STATE) {
+      throw new StateError('The authorization URL has already been generated.');
+    }
+    _state = _AWAITING_RESPONSE_STATE;
+
+    this._redirectEndpoint = redirect;
+    this._scopes = scopes;
+    this._stateString = state;
+    var parameters = {
+      "response_type": "code",
+      "client_id": this.identifier,
+      "redirect_uri": redirect.toString()
+    };
+
+    if (state != null) parameters['state'] = state;
+    if (!scopes.isEmpty) parameters['scope'] = Strings.join(scopes, ' ');
+
+    return addQueryParameters(this.authorizationEndpoint, parameters);
+  }
+
+  /// Processes the query parameters added to a redirect from the authorization
+  /// server. Note that this "response" is not an HTTP response, but rather the

[Bob Nystrom, 2012/11/16 19:53:30]

Maybe: "...rather the data passed to the redirect server as query parameters on
the redirect URL."

?

[nweiz, 2012/11/17 01:06:27]

Done.

+  /// query parameters added to the redirect URL.
+  ///
+  /// It is a [StateError] to call this more than once, to call it before
+  /// [getAuthorizationUrl] is called, or to call it after
+  /// [handleAuthorizationCode] is called.
+  ///
+  /// Throws [FormatError] if `parameters` is invalid according to the OAuth2
+  /// spec or if the authorization server otherwise provides invalid responses.
+  /// If `state` was passed to [getAuthorizationUrl], this will throw a
+  /// [FormatError] if the `state` parameter doesn't match the original value.
+  ///
+  /// Throws [AuthorizationException] if the authorization fails.
+  Future<Client> handleAuthorizationResponse(Map<String, String> parameters) {
+    return async.chain((_) {
+      if (_state == _INITIAL_STATE) {
+        throw new StateError('The authorization URL has not yet been generated.');

[Bob Nystrom, 2012/11/16 19:53:30]

Long line.

[nweiz, 2012/11/17 01:06:27]

Done.

+      } else if (_state == _FINISHED_STATE) {
+        throw new StateError('The authorization code has already been received.');

[Bob Nystrom, 2012/11/16 19:53:30]

Ditto.

[nweiz, 2012/11/17 01:06:27]

Done.

+      }
+      _state = _FINISHED_STATE;
+
+      if (this._stateString != null) {

[Bob Nystrom, 2012/11/16 19:53:30]

Is the "this." needed here?

[nweiz, 2012/11/17 01:06:27]

No. Removed.

+        if (!parameters.containsKey('state')) {
+          throw new FormatException('Invalid OAuth response for '
+              '"$authorizationEndpoint": did not contain required parameter '
+              '"state".');

[Bob Nystrom, 2012/11/16 19:53:30]

How about:

               '"$authorizationEndpoint": parameter "state" expected to be '
               '"$_stateString", was missing.');

[nweiz, 2012/11/17 01:06:27]

Done.

+        } else if (parameters['state'] != this._stateString) {

[Bob Nystrom, 2012/11/16 19:53:30]

"this."?

[nweiz, 2012/11/17 01:06:27]

Done.

+          throw new FormatException('Invalid OAuth response for '
+              '"$authorizationEndpoint": parameter "state" expected to be '
+              '"$_stateString", was "${parameters['state']}".');
+        }
+      }
+
+      if (parameters.containsKey('error')) {
+        var description = parameters['error_description'];
+        var uriString = parameters['error_uri'];
+        var uri = uriString == null ? null : new Uri.fromString(uriString);
+        throw new AuthorizationException(parameters['error'], description, uri);
+      } else if (!parameters.containsKey('code')) {
+        throw new FormatException('Invalid OAuth response for '
+            '"$authorizationEndpoint": did not contain required parameter '
+            '"code".');
+      }
+
+      return _handleAuthorizationCode(parameters['code']);
+    });
+  }
+
+  /// Processes an authorization code directly. Usually
+  /// [handleAuthorizationResponse] is preferable to this method, since it
+  /// validates all of the query parameters. However, some authorization servers
+  /// allow the user to copy and paste an authorization code into a command-line
+  /// application, in which case this method must be used.
+  ///
+  /// It is a [StateError] to call this more than once, to call it before
+  /// [getAuthorizationUrl] is called, or to call it after
+  /// [handleAuthorizationCode] is called.
+  ///
+  /// Throws [FormatError] if the authorization server provides invalid
+  /// responses while retrieving credentials.
+  ///
+  /// Throws [AuthorizationException] if the authorization fails.
+  Future<Client> handleAuthorizationCode(String authorizationCode) {
+    return async.chain((_) {
+      if (_state == _INITIAL_STATE) {
+        throw new StateError('The authorization URL has not yet been generated.');

[Bob Nystrom, 2012/11/16 19:53:30]

Long lines.

[nweiz, 2012/11/17 01:06:27]

Done.

+      } else if (_state == _FINISHED_STATE) {
+        throw new StateError('The authorization code has already been received.');
+      }
+      _state = _FINISHED_STATE;

[Bob Nystrom, 2012/11/16 19:53:30]

Move this into _handleAuthorizationCode so it's not duplicated in both
codepaths?

[nweiz, 2012/11/17 01:06:27]

_handleAuthorizationCode exists explicitly to avoid this code.
_handleAuthorizationResponse sets _state to _FINISHED_STATE, so if it runs this
code when it enters _handleAuthorizationCode, it'll always throw an error.

+
+      return _handleAuthorizationCode(authorizationCode);
+    });
+  }
+
+  /// This works just like [handleAuthorizationCode], except it doesn't validate
+  /// the state beforehand.
+  Future<Client> _handleAuthorizationCode(String authorizationCode) {
+    var startTime = new Date.now();
+    return _httpClient.post(this.tokenEndpoint, fields: {
+      "grant_type": "authorization_code",
+      "code": authorizationCode,
+      "redirect_uri": this._redirectEndpoint.toString(),
+      // TODO(nweiz): the spec recommends that HTTP basic auth be used in
+      // preference to form parameters, but Google doesn't support that. Should
+      // it be configurable?
+      "client_id": this.identifier,
+      "client_secret": this.secret
+    }).transform((response) {
+      var credentials = handleAccessTokenResponse(
+          response, tokenEndpoint, startTime, _scopes);
+      return new Client(
+          this.identifier, this.secret, credentials, httpClient: _httpClient);
+    });
+  }
+
+  /// Closes the grant and frees its resources.
+  ///
+  /// This will close the underlying HTTP client, which is shared by the
+  /// [Client] created by this grant, so it's not safe to close the grant and
+  /// continue using the client.

[Bob Nystrom, 2012/11/16 19:53:30]

Ugh, this makes me wonder if we should be ref-counting here, but that just makes
me feel dirty.

Maybe check the state and only close the client if you're not in
_FINISHED_STATE? And then document that?

[nweiz, 2012/11/17 01:06:27]

I think it's fine as-is. This method isn't expected to be used except in
exceptional circumstances, when you want to close the AuthorizationCodeGrant
in-flight but you don't want to completely exit the program. If it's used
incorrectly, it'll already cause reasonably obvious bugs pretty quickly.

[Adam, 2012/11/17 07:29:21]

_httpClient is final so setting to null should be an error?

[nweiz, 2012/11/19 20:55:26]

Good catch. This was also an issue in oauth2.Client.

+  void close() {
+    if (_httpClient != null) _httpClient.close();
+    _httpClient = null;
+  }
+}