Store credentials in a config directory

lib/src/authentication/credential.dart

@@ -59,7 +59,7 @@ class Credential {
   /// Authentication token value
   final String? token;
 
-  /// Unknown fields found in tokens.json. The fields might be created by the
+  /// Unknown fields found in pub-tokens.json. The fields might be created by the
   /// future version of pub tool. We don't want to override them when using the
   /// old SDK.
   final Map<String, dynamic> unknownFields;
@@ -99,7 +99,7 @@ class Credential {
 
   /// Returns boolean indicates whether or not the credentials is valid.
   ///
-  /// This method might return `false` when a `tokens.json` file created by
+  /// This method might return `false` when a `pub-tokens.json` file created by
   /// future SDK used by pub tool from old SDK.
   bool isValid() => token != null;
 

lib/src/authentication/token_store.dart

@@ -14,18 +14,18 @@ import 'credential.dart';
 
 /// Stores and manages authentication credentials.
 class TokenStore {
-  TokenStore(this.cacheRootDir);
+  TokenStore(this.configDir);
 
   /// Cache directory.
-  final String cacheRootDir;
+  final String configDir;
 
   /// List of saved authentication tokens.
   ///
   /// Modifying this field will not write changes to the disk. You have to call
   /// [flush] to save changes.
   Iterable<Credential> get credentials => _loadCredentials();
 
-  /// Reads "tokens.json" and parses / deserializes it into list of
+  /// Reads "pub-tokens.json" and parses / deserializes it into list of
   /// [Credential].
   List<Credential> _loadCredentials() {
     final result = List<Credential>.empty(growable: true);
@@ -84,14 +84,15 @@ class TokenStore {
         }
       }
     } on FormatException catch (e) {
-      log.warning('Failed to load tokens.json: ${e.message}');
+      log.warning('Failed to load pub-tokens.json: ${e.message}');
     }
 
     return result;
   }
 
-  /// Writes [credentials] into "tokens.json".
+  /// Writes [credentials] into "pub-tokens.json".
   void _saveCredentials(List<Credential> credentials) {
+    ensureDir(path.dirname(_tokensFile));
     writeTextFile(
         _tokensFile,
         jsonEncode(<String, dynamic>{
@@ -158,12 +159,12 @@ class TokenStore {
     return credentials.any((it) => it.url == url && it.isValid());
   }
 
-  /// Deletes tokens.json file from the disk.
+  /// Deletes pub-tokens.json file from the disk.
   void deleteTokensFile() {
     deleteEntry(_tokensFile);
-    log.message('tokens.json is deleted.');
+    log.message('pub-tokens.json is deleted.');
   }
 
-  /// Full path to the "tokens.json" file.
-  String get _tokensFile => path.join(cacheRootDir, 'tokens.json');
+  /// Full path to the "pub-tokens.json" file.
+  String get _tokensFile => path.join(configDir, 'pub-tokens.json');
 }

lib/src/io.dart

@@ -509,7 +509,18 @@ void createPackageSymlink(String name, String target, String symlink,
 ///
 /// The "_PUB_TESTING" variable is automatically set for all the test code's
 /// invocations of pub.
-final bool runningFromTest = Platform.environment.containsKey('_PUB_TESTING');
+final bool runningFromTest =
+    Platform.environment.containsKey('_PUB_TESTING') && _assertionsEnabled;
+
+final bool _assertionsEnabled = () {
+  try {
+    assert(false);
+    // ignore: avoid_catching_errors
+  } on AssertionError {
+    return true;
+  }
+  return false;
+}();
 
 final bool runningFromFlutter =
     Platform.environment.containsKey('PUB_ENVIRONMENT') &&
@@ -984,3 +995,23 @@ class PubProcessResult {
 
   bool get success => exitCode == exit_codes.SUCCESS;
 }
+
+/// The location for dart-specific configuration.
+final String dartConfigDir = () {
+  if (runningFromTest) {
+    return Platform.environment['_PUB_TEST_CONFIG_DIR'];
+  }
+  String configDir;
+  if (Platform.isLinux) {
+    configDir = Platform.environment['XDG_CONFIG_HOME'] ??
+        path.join(Platform.environment['HOME'], '.config');
+  } else if (Platform.isWindows) {
+    configDir = Platform.environment['APPDATA'];
+  } else if (Platform.isMacOS) {
+    configDir = path.join(
+        Platform.environment['HOME'], 'Library', 'Application Support');
+  } else {
+    configDir = path.join(Platform.environment['HOME'], '.config');
+  }
+  return path.join(configDir, 'dart');
+}();

lib/src/oauth2.dart

@@ -80,6 +80,12 @@ void logout(SystemCache cache) {
     log.message('Logging out of pub.dartlang.org.');
     log.message('Deleting $credentialsFile');
     _clearCredentials(cache);
+    // Test if we also have a legacy credentials file.
+    final legacyCredentialsFile = _legacyCredentialsFile(cache);
+    if (entryExists(legacyCredentialsFile)) {
+      log.message('Also deleting legacy credentials at $legacyCredentialsFile');
+      deleteEntry(legacyCredentialsFile);
+    }
   } else {
     log.message(
         'No existing credentials file $credentialsFile. Cannot log out.');
@@ -180,8 +186,20 @@ void _saveCredentials(SystemCache cache, Credentials credentials) {
 }
 
 /// The path to the file in which the user's OAuth2 credentials are stored.
-String _credentialsFile(SystemCache cache) =>
-    path.join(cache.rootDir, 'credentials.json');
+///
+/// This used to be PUB_CACHE/credentials.json. But the pub cache is not the
+/// best place for storing secrets, as it might be shared.
+///
+/// To provide backwards compatibility we use the legacy file if only it exists.
+String _credentialsFile(SystemCache cache) {
+  final newCredentialsFile = path.join(dartConfigDir, 'pub-credentials.json');
+  return [newCredentialsFile, _legacyCredentialsFile(cache)]
+      .firstWhere(fileExists, orElse: () => newCredentialsFile);
+}
+
+String _legacyCredentialsFile(SystemCache cache) {
+  return path.join(cache.rootDir, 'credentials.json');
+}
 
 /// Gets the user to authorize pub as a client of pub.dartlang.org via oauth2.
 ///

lib/src/system_cache.dart

@@ -89,7 +89,7 @@ class SystemCache {
   /// Defaults to `false`.
   SystemCache({String rootDir, bool isOffline = false})
       : rootDir = rootDir ?? SystemCache.defaultDir,
-        tokenStore = TokenStore(rootDir ?? SystemCache.defaultDir) {
+        tokenStore = TokenStore(dartConfigDir) {
     for (var source in sources.all) {
       if (source is HostedSource) {
         _boundSources[source] = source.bind(this, isOffline: isOffline);

test/descriptor.dart

@@ -186,31 +186,68 @@ Descriptor hostedCache(Iterable<Descriptor> contents, {int port}) {
   ]);
 }
 
-/// Describes the file in the system cache that contains the client's OAuth2
+/// Describes the file that contains the client's OAuth2
 /// credentials. The URL "/token" on [server] will be used as the token
 /// endpoint for refreshing the access token.
 Descriptor credentialsFile(PackageServer server, String accessToken,
     {String refreshToken, DateTime expiration}) {
-  return dir(cachePath, [
-    file(
+  return dir(
+    configPath,
+    [
+      file(
+        'pub-credentials.json',
+        _credentialsFileContent(
+          server,
+          accessToken,
+          refreshToken: refreshToken,
+          expiration: expiration,
+        ),
+      ),
+    ],
+  );
+}
+
+Descriptor legacyCredentialsFile(PackageServer server, String accessToken,
+    {String refreshToken, DateTime expiration}) {
+  return dir(
+    cachePath,
+    [
+      file(
         'credentials.json',
-        oauth2.Credentials(accessToken,
-                refreshToken: refreshToken,
-                tokenEndpoint: Uri.parse(server.url).resolve('/token'),
-                scopes: [
-                  'openid',
-                  'https://www.googleapis.com/auth/userinfo.email',
-                ],
-                expiration: expiration)
-            .toJson())
-  ]);
+        _credentialsFileContent(
+          server,
+          accessToken,
+          refreshToken: refreshToken,
+          expiration: expiration,
+        ),
+      ),
+    ],
+  );
 }
 
+String _credentialsFileContent(
+  PackageServer server,
+  String accessToken, {
+  String refreshToken,
+  DateTime expiration,
+}) =>
+    oauth2.Credentials(
+      accessToken,
+      refreshToken: refreshToken,
+      tokenEndpoint: Uri.parse(server.url).resolve('/token'),
+      scopes: [
+        'openid',
+        'https://www.googleapis.com/auth/userinfo.email',
+      ],
+      expiration: expiration,
+    ).toJson();
+
 /// Describes the file in the system cache that contains credentials for
 /// third party hosted pub servers.
 Descriptor tokensFile([Map<String, dynamic> contents = const {}]) {
-  return dir(cachePath,
-      [file('tokens.json', contents != null ? jsonEncode(contents) : null)]);
+  return dir(configPath, [
+    file('pub-tokens.json', contents != null ? jsonEncode(contents) : null)
+  ]);
 }
 
 /// Describes the application directory, containing only a pubspec specifying

test/oauth2/logout_test.dart

@@ -10,7 +10,7 @@ import '../descriptor.dart' as d;
 import '../test_pub.dart';
 
 void main() {
-  test('with an existing credentials.json, deletes it.', () async {
+  test('with an existing credentials file, deletes it.', () async {
     await servePackages();
     await d
         .credentialsFile(globalPackageServer, 'access token',
@@ -21,13 +21,49 @@ void main() {
     await runPub(
         args: ['logout'], output: contains('Logging out of pub.dartlang.org.'));
 
+    await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate();
+  });
+
+  test(
+      'with an existing credentials file stored in the legacy location, deletes both.',
+      () async {
+    await servePackages();
+    await d
+        .credentialsFile(
+          globalPackageServer,
+          'access token',
+          refreshToken: 'refresh token',
+          expiration: DateTime.now().add(Duration(hours: 1)),
+        )
+        .create();
+
+    await d
+        .legacyCredentialsFile(
+          globalPackageServer,
+          'access token',
+          refreshToken: 'refresh token',
+          expiration: DateTime.now().add(Duration(hours: 1)),
+        )
+        .create();
+
+    await runPub(
+      args: ['logout'],
+      output: allOf(
+        [
+          contains('Logging out of pub.dartlang.org.'),
+          contains('Also deleting legacy credentials at ')
+        ],
+      ),
+    );
+
     await d.dir(cachePath, [d.nothing('credentials.json')]).validate();
+    await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate();
   });
   test('with no existing credentials.json, notifies.', () async {
-    await d.dir(cachePath, [d.nothing('credentials.json')]).create();
+    await d.dir(configPath, [d.nothing('pub-credentials.json')]).create();
     await runPub(
         args: ['logout'], output: contains('No existing credentials file'));
 
-    await d.dir(cachePath, [d.nothing('credentials.json')]).validate();
+    await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate();
   });
 }

test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart

@@ -18,7 +18,8 @@ void main() {
     await d.validPackage.create();
 
     await servePackages();
-    await d.dir(cachePath, [d.file('credentials.json', '{bad json')]).create();
+    await d.dir(
+        configPath, [d.file('pub-credentials.json', '{bad json')]).create();
 
     var pub = await startPublish(globalPackageServer);
     await confirmPublish(pub);

test/test_pub.dart

@@ -62,6 +62,10 @@ String yaml(value) => jsonEncode(value);
 /// sandbox directory.
 const String cachePath = 'cache';
 
+/// The path of the config directory used for tests, relative to the
+/// sandbox directory.
+const String configPath = '.config';
+
 /// The path of the mock app directory used for tests, relative to the sandbox
 /// directory.
 const String appPath = 'myapp';
@@ -326,16 +330,17 @@ void symlinkInSandbox(String target, String symlink) {
 ///
 /// If [environment] is given, any keys in it will override the environment
 /// variables passed to the spawned process.
-Future<void> runPub(
-    {List<String> args,
-    output,
-    error,
-    outputJson,
-    silent,
-    int exitCode,
-    String workingDirectory,
-    Map<String, String> environment,
-    List<String> input}) async {
+Future<void> runPub({
+  List<String> args,
+  output,
+  error,
+  outputJson,
+  silent,
+  int exitCode,
+  String workingDirectory,
+  Map<String, String> environment,
+  List<String> input,
+}) async {
   exitCode ??= exit_codes.SUCCESS;
   // Cannot pass both output and outputJson.
   assert(output == null || outputJson == null);
@@ -415,6 +420,7 @@ Map<String, String> getPubTestEnvironment([String tokenEndpoint]) {
   var environment = {
     'CI': 'false', // unless explicitly given tests don't run pub in CI mode
     '_PUB_TESTING': 'true',
+    '_PUB_TEST_CONFIG_DIR': _pathInSandbox(configPath),
     'PUB_CACHE': _pathInSandbox(cachePath),
     'PUB_ENVIRONMENT': 'test-environment',