Code review changes

lib/src/credentials.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.credentials;
 
 import 'dart:async';
+import 'dart:collection';
 import 'dart:convert';
 
 import 'package:http/http.dart' as http;
 
 import 'handle_access_token_response.dart';
+import 'utils.dart';
 
 /// Credentials that prove that a client is allowed to access a resource on the
 /// resource owner's behalf.
 ///
 /// These credentials are long-lasting and can be safely persisted across
 /// multiple runs of the program.
 ///
 /// Many authorization servers will attach an expiration date to a set of
 /// credentials, along with a token that can be used to refresh the credentials
 /// once they've expired. The [Client] will automatically refresh its
@@ skipping 42 matching lines @@
   /// Whether it's possible to refresh these credentials.
   bool get canRefresh => refreshToken != null && tokenEndpoint != null;
 
   /// Creates a new set of credentials.
   ///
   /// This class is usually not constructed directly; rather, it's accessed via
   /// [Client.credentials] after a [Client] is created by
   /// [AuthorizationCodeGrant]. Alternately, it may be loaded from a serialized
   /// form via [Credentials.fromJson].
   Credentials(
-      this.accessToken,
-      [this.refreshToken,
-       this.tokenEndpoint,
-       this.scopes,
-       this.expiration]);
+          this.accessToken,
+          {this.refreshToken,
+          this.tokenEndpoint,
+          Iterable<String> scopes,
+          this.expiration})
+      : scopes = new UnmodifiableListView(
+            // Explicitly type-annotate the list literal to work around
+            // sdk#24202.
+            scopes == null ? <String>[] : scopes.toList());
 
   /// Loads a set of credentials from a JSON-serialized form.
   ///
   /// Throws a [FormatException] if the JSON is incorrectly formatted.
   factory Credentials.fromJson(String json) {
     validate(condition, message) {
       if (condition) return;
       throw new FormatException(
           "Failed to load credentials: $message.\n\n$json");
     }
@@ skipping 29 matching lines @@
     }
     var expiration = parsed['expiration'];
     if (expiration != null) {
       validate(expiration is int,
           'field "expiration" was not an int, was "$expiration"');
       expiration = new DateTime.fromMillisecondsSinceEpoch(expiration);
     }
 
     return new Credentials(
         parsed['accessToken'],
-        parsed['refreshToken'],
-        tokenEndpoint,
-        scopes,
-        expiration);
+        refreshToken: parsed['refreshToken'],
+        tokenEndpoint: tokenEndpoint,
+        scopes: scopes,
+        expiration: expiration);
   }
 
   /// Serializes a set of credentials to JSON.
   ///
   /// Nothing is guaranteed about the output except that it's valid JSON and
   /// compatible with [Credentials.toJson].
   String toJson() => JSON.encode({
     'accessToken': accessToken,
     'refreshToken': refreshToken,
     'tokenEndpoint': tokenEndpoint == null ? null : tokenEndpoint.toString(),
     'scopes': scopes,
     'expiration': expiration == null ? null : expiration.millisecondsSinceEpoch
   });
 
   /// Returns a new set of refreshed credentials.
   ///
   /// See [Client.identifier] and [Client.secret] for explanations of those
   /// parameters.
   ///
   /// You may request different scopes than the default by passing in
   /// [newScopes]. These must be a subset of [scopes].
   ///
-  /// This will throw a [StateError] if these credentials can't be refreshed, an
+  /// This throws an [ArgumentError] if [secret] is passed without [identifier],
+  /// a [StateError] if these credentials can't be refreshed, an
   /// [AuthorizationException] if refreshing the credentials fails, or a
   /// [FormatError] if the authorization server returns invalid responses.
   Future<Credentials> refresh(
-      String identifier,
+      {String identifier,
       String secret,
-      {List<String> newScopes,
-       http.Client httpClient}) async {
+      Iterable<String> newScopes,
+      bool basicAuth: true,
+      http.Client httpClient}) async {
     var scopes = this.scopes;
-    if (newScopes != null) scopes = newScopes;
-    if (scopes == null) scopes = <String>[];
+    if (newScopes != null) scopes = newScopes.toList();
+    if (scopes == null) scopes = [];
     if (httpClient == null) httpClient = new http.Client();
 
+    if (identifier == null && secret != null) {
+      throw new ArgumentError("secret may not be passed without identifier.");
+    }
+
     var startTime = new DateTime.now();
     if (refreshToken == null) {
       throw new StateError("Can't refresh credentials without a refresh "
           "token.");
     } else if (tokenEndpoint == null) {
       throw new StateError("Can't refresh credentials without a token "
           "endpoint.");
     }
 
-    var fields = {
+    var headers = {};
+
+    var body = {
       "grant_type": "refresh_token",
-      "refresh_token": refreshToken,
-      // 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": identifier,
-      "client_secret": secret
+      "refresh_token": refreshToken
     };
-    if (!scopes.isEmpty) fields["scope"] = scopes.join(' ');
+    if (!scopes.isEmpty) body["scope"] = scopes.join(' ');
 
-    var response = await httpClient.post(tokenEndpoint, body: fields);
+    if (basicAuth && secret != null) {
+      headers["Authorization"] = basicAuthHeader(identifier, secret);
+    } else {
+      if (identifier != null) body["client_id"] = identifier;
+      if (secret != null) body["client_secret"] = secret;
+    }
+
+    var response = await httpClient.post(tokenEndpoint,
+        headers: headers, body: body);
     var credentials = await handleAccessTokenResponse(
-          response, tokenEndpoint, startTime, scopes);
+        response, tokenEndpoint, startTime, scopes);
 
     // The authorization server may issue a new refresh token. If it doesn't,
     // we should re-use the one we already have.
     if (credentials.refreshToken != null) return credentials;
     return new Credentials(
         credentials.accessToken,
-        this.refreshToken,
-        credentials.tokenEndpoint,
-        credentials.scopes,
-        credentials.expiration);
+        refreshToken: this.refreshToken,
+        tokenEndpoint: credentials.tokenEndpoint,
+        scopes: credentials.scopes,
+        expiration: credentials.expiration);
   }
 }