Code review changes

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 oauth2.authorization_code_grant;
 
 import 'dart:async';
 
 import 'package:http/http.dart' as http;
 
@@ skipping 50 matching lines @@
   /// 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;
 
+  /// Whether to use HTTP Basic authentication for authorizing the client.
+  final bool _basicAuth;
+
   /// The HTTP client used to make HTTP requests.
   http.Client _httpClient;
 
   /// 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.
   _State _state = _State.initial;
 
   /// Creates a new grant.
   ///
+  /// If [basicAuth] is `true` (the default), the client credentials are sent to
+  /// the server using using HTTP Basic authentication as defined in [RFC 2617].
+  /// Otherwise, they're included in the request body. Note that the latter form
+  /// is not recommended by the OAuth 2.0 spec, and should only be used if the
+  /// server doesn't support Basic authentication.
+  ///
+  /// [RFC 2617]: https://tools.ietf.org/html/rfc2617
+  ///
   /// [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.Client httpClient})
-      : _httpClient = httpClient == null ? new http.Client() : httpClient;
+          {this.secret, bool basicAuth: true, http.Client httpClient})
+      : _basicAuth = basicAuth,
+        _httpClient = httpClient == null ? new http.Client() : httpClient;
 
   /// Returns the URL to which the resource owner should be redirected to
   /// authorize this client.
   ///
   /// The resource owner will then be redirected to [redirect], which should
   /// point to a server controlled by the client. This 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.
-  Uri getAuthorizationUrl(Uri redirect,
-      {List<String> scopes: const <String>[], String state}) {
+  Uri getAuthorizationUrl(Uri redirect, {Iterable<String> scopes,
+      String state}) {
     if (_state != _State.initial) {
       throw new StateError('The authorization URL has already been generated.');
     }
     _state = _State.awaitingResponse;
 
+    if (scopes == null) {
+      scopes = [];
+    } else {
+      scopes = scopes.toList();
+    }
+
     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;
@@ skipping 81 matching lines @@
     }
     _state = _State.finished;
 
     return await _handleAuthorizationCode(authorizationCode);
   }
 
   /// This works just like [handleAuthorizationCode], except it doesn't validate
   /// the state beforehand.
   Future<Client> _handleAuthorizationCode(String authorizationCode) async {
     var startTime = new DateTime.now();
-    var response = await _httpClient.post(this.tokenEndpoint, body: {
+
+    var headers = {};
+
+    var body = {
       "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
-    });
+      "redirect_uri": this._redirectEndpoint.toString()
+    };
+
+    if (_basicAuth && secret != null) {
+      headers["Authorization"] = basicAuthHeader(identifier, secret);
+    } else {
+      // The ID is required for this request any time basic auth isn't being
+      // used, even if there's no actual client authentication to be done.
+      body["client_id"] = identifier;
+      if (secret != null) body["client_secret"] = secret;
+    }
+
+    var response = await _httpClient.post(this.tokenEndpoint,
+        headers: headers, body: body);
 
     var credentials = handleAccessTokenResponse(
         response, tokenEndpoint, startTime, _scopes);
     return new Client(
-        this.identifier, this.secret, credentials, httpClient: _httpClient);
+        credentials,
+        identifier: this.identifier,
+        secret: this.secret,
+        basicAuth: _basicAuth,
+        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.
   void close() {
     if (_httpClient != null) _httpClient.close();
     _httpClient = null;
@@ skipping 15 matching lines @@
   // [AuthorizationCodeGrant.handleAuthorizationResponse] or
   // [AuthorizationCodeGrant.handleAuthorizationCode] have been called.
   static const finished = const _State("finished");
 
   final String _name;
 
   const _State(this._name);
 
   String toString() => _name;
 }