From 856f67a6b439ce7e0094a542276730f73133a517 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 2 Sep 2021 12:33:37 +0200 Subject: [PATCH 1/9] Store credentials in a config directory --- lib/src/io.dart | 33 +++++++++- lib/src/oauth2.dart | 22 ++++++- test/descriptor.dart | 62 +++++++++++++++---- test/oauth2/logout_test.dart | 42 ++++++++++++- ..._credentials_authenticates_again_test.dart | 2 +- test/test_pub.dart | 5 ++ 6 files changed, 146 insertions(+), 20 deletions(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index 2fc38189e..9acc367e3 100644 --- a/lib/src/io.dart +++ b/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') && @@ -979,3 +990,23 @@ class PubProcessResult { bool get success => exitCode == exit_codes.SUCCESS; } + +/// The location for dart-specific configuration. +final String dartConfigDir = () { + String configDir; + if (runningFromTest) { + return Platform.environment['_PUB_TEST_CONFIG_DIR']; + } + 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'); +}(); diff --git a/lib/src/oauth2.dart b/lib/src/oauth2.dart index 16f48a0dc..c10bc9f7e 100644 --- a/lib/src/oauth2.dart +++ b/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. /// diff --git a/test/descriptor.dart b/test/descriptor.dart index 6c0e8664d..bc7970c09 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -186,26 +186,62 @@ Descriptor hostedCache(Iterable 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 application directory, containing only a pubspec specifying /// the given [dependencies]. DirectoryDescriptor appDir([Map dependencies]) => diff --git a/test/oauth2/logout_test.dart b/test/oauth2/logout_test.dart index f7bceab90..55b9ae37a 100644 --- a/test/oauth2/logout_test.dart +++ b/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(); }); } diff --git a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart b/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart index 793bd9364..2b4d26b9e 100644 --- a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart +++ b/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart @@ -18,7 +18,7 @@ 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); diff --git a/test/test_pub.dart b/test/test_pub.dart index 9c764ab0c..bd54fe18a 100644 --- a/test/test_pub.dart +++ b/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'; @@ -405,6 +409,7 @@ Map 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', From 5436270971a5aa4a5b3396a1185c959a8e66c25b Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 2 Sep 2021 12:37:55 +0200 Subject: [PATCH 2/9] reorder statements --- lib/src/io.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index 9acc367e3..627411ccb 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -993,10 +993,10 @@ class PubProcessResult { /// The location for dart-specific configuration. final String dartConfigDir = () { - String configDir; 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'); From 474121745895eb017b60e1fdd62bd3c7666d7073 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 2 Sep 2021 12:40:40 +0200 Subject: [PATCH 3/9] format --- .../with_a_malformed_credentials_authenticates_again_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart b/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart index 2b4d26b9e..80b961331 100644 --- a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart +++ b/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(configPath, [d.file('pub_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); From 86bbd1da8c0946db57b66468c6a2c776ac63ecf9 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 11:46:30 +0200 Subject: [PATCH 4/9] format --- test/test_pub.dart | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/test/test_pub.dart b/test/test_pub.dart index 897e1c86e..356b9597e 100644 --- a/test/test_pub.dart +++ b/test/test_pub.dart @@ -330,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 runPub( - {List args, - output, - error, - outputJson, - silent, - int exitCode, - String workingDirectory, - Map environment, - List input,}) async { +Future runPub({ + List args, + output, + error, + outputJson, + silent, + int exitCode, + String workingDirectory, + Map environment, + List input, +}) async { exitCode ??= exit_codes.SUCCESS; // Cannot pass both output and outputJson. assert(output == null || outputJson == null); From 3022fd98a1fd93ededbb804411481be74163e85e Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 11:51:58 +0000 Subject: [PATCH 5/9] Rename files: pub-credentials.json and pub-tokens.json --- lib/src/authentication/token_store.dart | 2 +- lib/src/oauth2.dart | 2 +- test/descriptor.dart | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index aa5ae72cf..b4645c5b8 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -166,5 +166,5 @@ class TokenStore { } /// Full path to the "tokens.json" file. - String get _tokensFile => path.join(configDir, 'tokens.json'); + String get _tokensFile => path.join(configDir, 'pub-tokens.json'); } diff --git a/lib/src/oauth2.dart b/lib/src/oauth2.dart index c10bc9f7e..c1284e9d9 100644 --- a/lib/src/oauth2.dart +++ b/lib/src/oauth2.dart @@ -192,7 +192,7 @@ void _saveCredentials(SystemCache cache, Credentials credentials) { /// /// 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'); + final newCredentialsFile = path.join(dartConfigDir, 'pub-credentials.json'); return [newCredentialsFile, _legacyCredentialsFile(cache)] .firstWhere(fileExists, orElse: () => newCredentialsFile); } diff --git a/test/descriptor.dart b/test/descriptor.dart index 7e6663da0..dfefab724 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -195,7 +195,7 @@ Descriptor credentialsFile(PackageServer server, String accessToken, configPath, [ file( - 'pub_credentials.json', + 'pub-credentials.json', _credentialsFileContent( server, accessToken, From 4165899aad99dd3647e9ee9aac375d15054c5ed2 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 11:54:34 +0000 Subject: [PATCH 6/9] Update references to tokens.json -> pub-tokens.json --- lib/src/authentication/credential.dart | 4 ++-- lib/src/authentication/token_store.dart | 12 ++++++------ test/token/add_token_test.dart | 6 +++--- test/token/remove_token_test.dart | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 1798eb66a..db3b30887 100644 --- a/lib/src/authentication/credential.dart +++ b/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 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; diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index b4645c5b8..6f68fe625 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -25,7 +25,7 @@ class TokenStore { /// [flush] to save changes. Iterable 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 _loadCredentials() { final result = List.empty(growable: true); @@ -84,13 +84,13 @@ 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 credentials) { ensureDir(path.dirname(_tokensFile)); writeTextFile( @@ -159,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. + /// Full path to the "pub-tokens.json" file. String get _tokensFile => path.join(configDir, 'pub-tokens.json'); } diff --git a/test/token/add_token_test.dart b/test/token/add_token_test.dart index 2197bf374..56250fe7d 100644 --- a/test/token/add_token_test.dart +++ b/test/token/add_token_test.dart @@ -11,7 +11,7 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; void main() { - test('with correct server url creates tokens.json that contains token', + test('with correct server url creates pub-tokens.json that contains token', () async { await d.tokensFile({ 'version': 1, @@ -82,7 +82,7 @@ void main() { exitCode: exit_codes.USAGE, ); - await d.dir(configPath, [d.nothing('tokens.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-tokens.json')]).validate(); }); test('with non-secure server url returns error', () async { @@ -93,6 +93,6 @@ void main() { exitCode: exit_codes.DATA, ); - await d.dir(configPath, [d.nothing('tokens.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-tokens.json')]).validate(); }); } diff --git a/test/token/remove_token_test.dart b/test/token/remove_token_test.dart index 9157f7df1..f9ce67d92 100644 --- a/test/token/remove_token_test.dart +++ b/test/token/remove_token_test.dart @@ -59,6 +59,6 @@ void main() { await runPub(args: ['token', 'remove', '--all']); - await d.dir(configPath, [d.nothing('tokens.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-tokens.json')]).validate(); }); } From 6a1d2d0c2d880eddae049ae536f3057402c6d1d0 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 12:10:01 +0000 Subject: [PATCH 7/9] Update references to tokens.json -> pub-tokens.json 2 --- test/descriptor.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/descriptor.dart b/test/descriptor.dart index dfefab724..3eee5d19c 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -245,8 +245,9 @@ String _credentialsFileContent( /// Describes the file in the system cache that contains credentials for /// third party hosted pub servers. Descriptor tokensFile([Map contents = const {}]) { - return dir(configPath, - [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 From 0b5dbb3811208106c9fa96bfcdb504654645777b Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 12:13:29 +0000 Subject: [PATCH 8/9] Another fix --- test/oauth2/logout_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/oauth2/logout_test.dart b/test/oauth2/logout_test.dart index 55b9ae37a..81d412528 100644 --- a/test/oauth2/logout_test.dart +++ b/test/oauth2/logout_test.dart @@ -21,7 +21,7 @@ void main() { await runPub( args: ['logout'], output: contains('Logging out of pub.dartlang.org.')); - await d.dir(configPath, [d.nothing('pub_credentials.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate(); }); test( @@ -57,13 +57,13 @@ void main() { ); await d.dir(cachePath, [d.nothing('credentials.json')]).validate(); - await d.dir(configPath, [d.nothing('pub_credentials.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate(); }); test('with no existing credentials.json, notifies.', () async { - await d.dir(configPath, [d.nothing('pub_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(configPath, [d.nothing('pub_credentials.json')]).validate(); + await d.dir(configPath, [d.nothing('pub-credentials.json')]).validate(); }); } From 04978faf6fc6cc5e7d2d23cef11970977e5b3614 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 14 Sep 2021 12:13:41 +0000 Subject: [PATCH 9/9] Another fix --- .../with_a_malformed_credentials_authenticates_again_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart b/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart index 80b961331..558552648 100644 --- a/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart +++ b/test/oauth2/with_a_malformed_credentials_authenticates_again_test.dart @@ -19,7 +19,7 @@ void main() { await servePackages(); await d.dir( - configPath, [d.file('pub_credentials.json', '{bad json')]).create(); + configPath, [d.file('pub-credentials.json', '{bad json')]).create(); var pub = await startPublish(globalPackageServer); await confirmPublish(pub);