Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion lib/src/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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') &&
Expand Down Expand Up @@ -979,3 +990,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');
}();
22 changes: 20 additions & 2 deletions lib/src/oauth2.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense if we move legacy credentials file into new path instead of deleting? It would be backwards-compatible with CI jobs that uses credentials.json for authentication & publishing packages.

Copy link
Contributor Author

@sigurdm sigurdm Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are in the dart pub logout code - it should delete any credentials, so I don't think it makes sense to keep the legacy ones.

In the code for _credentialsFile below we use the credentials file from the legacy location if it exists there. We could consider moving it, but that would make it hard(er) to switch between older and newer versions of pub.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, 😅 haven't noticed it's for logout command.

}
} else {
log.message(
'No existing credentials file $credentialsFile. Cannot log out.');
Expand Down Expand Up @@ -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.
///
Expand Down
62 changes: 49 additions & 13 deletions test/descriptor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -186,26 +186,62 @@ 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 application directory, containing only a pubspec specifying
/// the given [dependencies].
DirectoryDescriptor appDir([Map dependencies]) =>
Expand Down
42 changes: 39 additions & 3 deletions test/oauth2/logout_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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();
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions test/test_pub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -405,6 +409,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',

Expand Down