From 218d1146f82010a503acdf05dd6ee4764f8d81f1 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 19 May 2021 03:09:13 +0400 Subject: [PATCH 01/77] wip: Prepare authentication proposal --- doc/authentication-proposal.md | 103 +++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 doc/authentication-proposal.md diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md new file mode 100644 index 000000000..8b30635bf --- /dev/null +++ b/doc/authentication-proposal.md @@ -0,0 +1,103 @@ +Authenticating with Hosted Pub Repositories Proposal +==================================================== + +This document specifies how the REST API could be authenticated with the pub CLI +tool. + +## Requesting authentication + +If the repository requires special authentication to access resources or upload +new packages, it has to respond with `401 Unauthorized` status code with `WWW-Authenticate` header. This header should specify authentication method that should be used to gain access to the resource. + +WWW-Authenticate header syntax: + +```plain +WWW-Authenticate: realm=[, charset="UTF-8"] +``` + +Pub CLI will only support **Basic** and **Bearer** authentication methods by +default. + +## Authentication flow + +After receiving `WWW-Authenticate` header, pub CLI will prompt user for +the credentials like this: + +```plain +user@machine$ pub get +Please enter required credentials to authenticate with "https://myserver.com" +hosted repository. + +Username: bob +Password: password + +Please enter required credentials to authenticate with "https://pub.example.com" +hosted repository. + +Bearer token: 8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r +``` + +After providing credentials, the client will send those credentials to the +server in `Authorization` header. + + +```dart +// Bearer authentication +{ 'Autorization': 'Bearer $token' } + +// Basic authentication +{ 'Autorization': 'Basic ' + base64('$username:$password') } +``` + +### Explicit authentication + +Users can also explicitly define authentication credentials even before server +asks for it using a modified **pub login** command. By default if no argument is +presented, **pub login** will behave as previously: authenticate with Google. +But if you you specify server to login, it will try to authenticate with the +given server instead. + +```plain +pub login https://myspuberver.dev +``` + +To discover authentication method, client will send **GET** request to `/login` +endpoint. This endpoint should be authenticated by the server as well as other +endpoints. The client might use this endpoint to: + +1. Validate cached credentials when needed +2. Discover authentication method by sending unauthenticated requests + +## Storing credentials + +Hosted Pub Repository authentication credentials will be stored on json file +named `hosted.credentials.json` located in cache root directory. Authentication +details will be stored at this file as json values while their keys will be URLs +of the server (`PUB_HOSTED_URL`). + +```json +{ + "https://myserver.com": { + "method": "Basic", + "credentials": { + "username": "bob", + "password": "password" + } + }, + "https://pub.example.com": { + "method": "Bearer", + "credentials": { + "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" + } + } +} +``` + +This model of storing credentials allows us to extend support to new +authentication methods in future. + +## References + +- [RFC7235 - 401 Unauthorized](https://datatracker.ietf.org/doc/html/rfc7235#section-3.1) +- [RFC7235 - WWW-Authenticate](https://datatracker.ietf.org/doc/html/rfc7235#section-4.1) +- [MDN - WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) \ No newline at end of file From da1549ffa99d1da0e6b5ed7acd87b375d296b355 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 19 May 2021 13:09:16 +0400 Subject: [PATCH 02/77] doc: Add details about 'realm' parameter --- doc/authentication-proposal.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md index 8b30635bf..73d542026 100644 --- a/doc/authentication-proposal.md +++ b/doc/authentication-proposal.md @@ -12,9 +12,13 @@ new packages, it has to respond with `401 Unauthorized` status code with `WWW-Au WWW-Authenticate header syntax: ```plain -WWW-Authenticate: realm=[, charset="UTF-8"] +WWW-Authenticate: [realm=][, charset="UTF-8"] ``` +> `realm` parameter is completely optional, and is not used in this proposal. +> You can read more about this parameter here: +> https://datatracker.ietf.org/doc/html/rfc7235#section-2.2 + Pub CLI will only support **Basic** and **Bearer** authentication methods by default. @@ -100,4 +104,4 @@ authentication methods in future. - [RFC7235 - 401 Unauthorized](https://datatracker.ietf.org/doc/html/rfc7235#section-3.1) - [RFC7235 - WWW-Authenticate](https://datatracker.ietf.org/doc/html/rfc7235#section-4.1) -- [MDN - WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) \ No newline at end of file +- [MDN - WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) From becf34f50b87a3f09a24a8fdfe100531cf753bb3 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 19 May 2021 13:20:28 +0400 Subject: [PATCH 03/77] Update authentication-proposal.md - Modified references - Added description about specs is based on RFC7235 --- doc/authentication-proposal.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md index 73d542026..1c9a2d73d 100644 --- a/doc/authentication-proposal.md +++ b/doc/authentication-proposal.md @@ -4,10 +4,18 @@ Authenticating with Hosted Pub Repositories Proposal This document specifies how the REST API could be authenticated with the pub CLI tool. +This proposal is mostly based on +[RFC7235](https://datatracker.ietf.org/doc/html/rfc7235) - HTTP Authentication +specifications. This ensures exists un-protected endpoints could be protected +just by setting up reverse proxies like NGINX, Apache, Traefik, which already +supports this specifications. + ## Requesting authentication If the repository requires special authentication to access resources or upload -new packages, it has to respond with `401 Unauthorized` status code with `WWW-Authenticate` header. This header should specify authentication method that should be used to gain access to the resource. +new packages, it has to respond with `401 Unauthorized` status code with +`WWW-Authenticate` header. This header should specify authentication method that +should be used to gain access to the resource. WWW-Authenticate header syntax: @@ -102,6 +110,5 @@ authentication methods in future. ## References -- [RFC7235 - 401 Unauthorized](https://datatracker.ietf.org/doc/html/rfc7235#section-3.1) -- [RFC7235 - WWW-Authenticate](https://datatracker.ietf.org/doc/html/rfc7235#section-4.1) +- [RFC7235](https://datatracker.ietf.org/doc/html/rfc7235) - [MDN - WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) From ac8b70f13a2af52959cc4c37a83f030c8faee9e4 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 20 May 2021 03:10:47 +0400 Subject: [PATCH 04/77] Implement CredentialStore and Credential for bearer method --- lib/src/authentication/bearer.dart | 46 +++++++++++++++ lib/src/authentication/credential.dart | 42 +++++++++++++ lib/src/authentication/credential_store.dart | 62 ++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 lib/src/authentication/bearer.dart create mode 100644 lib/src/authentication/credential.dart create mode 100644 lib/src/authentication/credential_store.dart diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart new file mode 100644 index 000000000..86662fe0f --- /dev/null +++ b/lib/src/authentication/bearer.dart @@ -0,0 +1,46 @@ +import 'dart:io'; + +import 'package:http/http.dart'; + +import 'credential.dart'; + +class BearerCredential extends Credential { + BearerCredential(this.token); + + static BearerCredential fromMap(Map map) => + BearerCredential(map['token'] as String); + + final String token; + + @override + String get authenticationType => 'Bearer'; + + @override + Future createClient() { + return Future.value(_Client(token)); + } + + @override + Map toMapInternal() { + return {'token': token}; + } +} + +class _Client extends BaseClient { + _Client(this._token); + + final Client _client = Client(); + final String _token; + + @override + Future send(BaseRequest request) { + request.headers[HttpHeaders.authorizationHeader] = 'Bearer $_token'; + return _client.send(request); + } + + @override + void close() { + _client.close(); + super.close(); + } +} diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart new file mode 100644 index 000000000..44c165b5e --- /dev/null +++ b/lib/src/authentication/credential.dart @@ -0,0 +1,42 @@ +import 'package:http/http.dart'; +import 'package:meta/meta.dart'; + +import 'bearer.dart'; + +typedef CredentialDeserializer = Credential Function(Map); + +final Map _supportedMethods = { + 'Bearer': BearerCredential.fromMap, +}; + +abstract class Credential { + static Credential? fromMap(Map map) { + final authMethod = map['method'] as String?; + final credentials = map['credentials'] as Map?; + + if (credentials != null && + authMethod?.isNotEmpty == true && + _supportedMethods.containsKey(authMethod)) { + return _supportedMethods[authMethod]!(credentials); + } + + return null; + } + + Map toMap() { + return { + 'method': authenticationType, + 'credentials': toMapInternal(), + }; + } + + /// Authentication type of this credential. + String get authenticationType; + + /// Creates authenticated client using this credential. + Future createClient(); + + /// Converts credential data into [Map]. + @protected + Map toMapInternal(); +} diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart new file mode 100644 index 000000000..74810fde3 --- /dev/null +++ b/lib/src/authentication/credential_store.dart @@ -0,0 +1,62 @@ +// @dart=2.10 + +import 'dart:convert'; + +import 'package:path/path.dart' as path; + +import '../io.dart'; +import '../system_cache.dart'; +import 'credential.dart'; + +class CredentialStore { + CredentialStore(this.cache); + + final SystemCache cache; + + /// Adds [credentials] for [server] into store. + void addCredentials(String server, Credential credentials) { + serverCredentials[server] = credentials; + _save(); + } + + /// Removes credentials for [server]. + void removeCredentials(String server) { + serverCredentials.removeWhere((key, value) => key == server); + _save(); + } + + /// Returns credentials for [server] if any exists, otherwise returns null. + Credential getCredential(String server) { + return serverCredentials[server]; + } + + void _save() { + _saveCredentials(_serverCredentials); + } + + Map _serverCredentials; + Map get serverCredentials => + _serverCredentials ??= _loadCredentials(); + + String get _tokensFile => path.join(cache.rootDir, 'tokens.json'); + + Map _loadCredentials() { + final path = _tokensFile; + if (!fileExists(path)) return {}; + + final parsed = jsonDecode(readTextFile(path)) as Map; + final result = parsed + .map((key, value) => MapEntry(key, Credential.fromMap(value))) + ..removeWhere((key, value) => value == null); + + return result.cast(); + } + + void _saveCredentials(Map credentials) { + final path = _tokensFile; + writeTextFile( + path, + jsonEncode( + credentials.map((key, value) => MapEntry(key, value.toMap())))); + } +} From fc12fa8a9539c19098463c3ddb67f3f8d54400e2 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 20 May 2021 03:11:33 +0400 Subject: [PATCH 05/77] Login and logout commands with hosted pub repo --- lib/src/command.dart | 6 ++++++ lib/src/command/login.dart | 40 +++++++++++++++++++++++++++++++++---- lib/src/command/logout.dart | 8 ++++++-- lib/src/io.dart | 5 +++++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/lib/src/command.dart b/lib/src/command.dart index bd0178257..a3488de77 100644 --- a/lib/src/command.dart +++ b/lib/src/command.dart @@ -12,6 +12,7 @@ import 'package:args/command_runner.dart'; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; +import 'authentication/credential_store.dart'; import 'command_runner.dart'; import 'entrypoint.dart'; import 'exceptions.dart'; @@ -55,6 +56,11 @@ abstract class PubCommand extends Command { GlobalPackages _globals; + CredentialStore get credentialStore => + _credentialStore ?? CredentialStore(cache); + + CredentialStore _credentialStore; + /// Gets the [Entrypoint] package for the current working directory. /// /// This will load the pubspec and fail with an error if the current directory diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 72a5f4ab3..9e761ac70 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -7,8 +7,11 @@ import 'dart:async'; import 'dart:convert'; +import '../authentication/bearer.dart'; + import '../command.dart'; import '../http.dart'; +import '../io.dart'; import '../log.dart' as log; import '../oauth2.dart' as oauth2; @@ -21,16 +24,45 @@ class LoginCommand extends PubCommand { @override String get invocation => 'pub login'; - LoginCommand(); + String get token => argResults['token']; + bool get tokenStdin => argResults['token-stdin']; + + LoginCommand() { + argParser.addOption('token', help: 'Authorization token for the server'); + + argParser.addFlag('token-stdin', + help: 'Read authorization token from stdin stream'); + } @override Future runProtected() async { + if (argResults.rest.isEmpty) { + await _loginToPubDev(); + } else { + if (token?.isNotEmpty != true && !tokenStdin) { + usageException('Must specify a token.'); + } + await _loginToServer(argResults.rest.first); + } + } + + Future _loginToServer(String server) async { + if (Uri.tryParse(server) == null) { + usageException('Invalid or malformed server URL provided.'); + } + + final _token = tokenStdin ? await readLine() : token; + credentialStore.addCredentials(server, BearerCredential(_token)); + log.message('You are now logged in to $server using bearer token'); + } + + Future _loginToPubDev() async { final credentials = oauth2.loadCredentials(cache); if (credentials == null) { - final userInfo = await retrieveUserInfo(); + final userInfo = await _retrieveUserInfo(); log.message('You are now logged in as $userInfo'); } else { - final userInfo = await retrieveUserInfo(); + final userInfo = await _retrieveUserInfo(); if (userInfo == null) { log.warning('Your credentials seems broken.\n' 'Run `pub logout` to delete your credentials and try again.'); @@ -40,7 +72,7 @@ class LoginCommand extends PubCommand { } } - Future<_UserInfo> retrieveUserInfo() async { + Future<_UserInfo> _retrieveUserInfo() async { return await oauth2.withClient(cache, (client) async { final discovery = await httpClient.get(Uri.https( 'accounts.google.com', '/.well-known/openid-configuration')); diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 071e8c06c..0d7871097 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -16,12 +16,16 @@ class LogoutCommand extends PubCommand { @override String get description => 'Log out of pub.dev.'; @override - bool get takesArguments => false; + bool get takesArguments => true; LogoutCommand(); @override Future runProtected() async { - oauth2.logout(cache); + if (argResults.rest.isEmpty) { + oauth2.logout(cache); + } else { + credentialStore.removeCredentials(argResults.rest.first); + } } } diff --git a/lib/src/io.dart b/lib/src/io.dart index b73312a37..4dfc1871e 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -568,6 +568,11 @@ Future confirm(String message) { return _stdinLines.first.then(RegExp(r'^[yY]').hasMatch); } +/// Reads a line from stdin stream. +Future readLine() { + return _stdinLines.first; +} + /// Flushes the stdout and stderr streams, then exits the program with the given /// status code. /// From 5a8d6f353a9ab4dbf32d9737bc12f90f81d8e1de Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 20 May 2021 03:16:09 +0400 Subject: [PATCH 06/77] formatting: remove space --- lib/src/command/login.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 9e761ac70..a42b8a2cf 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -8,7 +8,6 @@ import 'dart:async'; import 'dart:convert'; import '../authentication/bearer.dart'; - import '../command.dart'; import '../http.dart'; import '../io.dart'; From 427a7d956e01ecc83514cddb062e6c3f2d84c042 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 20 May 2021 03:28:07 +0400 Subject: [PATCH 07/77] Rename add/removeCredentials to add/removeServer --- lib/src/authentication/credential_store.dart | 4 ++-- lib/src/command/login.dart | 2 +- lib/src/command/logout.dart | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 74810fde3..e69a95ea3 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -14,13 +14,13 @@ class CredentialStore { final SystemCache cache; /// Adds [credentials] for [server] into store. - void addCredentials(String server, Credential credentials) { + void addServer(String server, Credential credentials) { serverCredentials[server] = credentials; _save(); } /// Removes credentials for [server]. - void removeCredentials(String server) { + void removeServer(String server) { serverCredentials.removeWhere((key, value) => key == server); _save(); } diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index a42b8a2cf..e6da9198a 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -51,7 +51,7 @@ class LoginCommand extends PubCommand { } final _token = tokenStdin ? await readLine() : token; - credentialStore.addCredentials(server, BearerCredential(_token)); + credentialStore.addServer(server, BearerCredential(_token)); log.message('You are now logged in to $server using bearer token'); } diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 0d7871097..4a76e20cf 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -25,7 +25,7 @@ class LogoutCommand extends PubCommand { if (argResults.rest.isEmpty) { oauth2.logout(cache); } else { - credentialStore.removeCredentials(argResults.rest.first); + credentialStore.removeServer(argResults.rest.first); } } } From fd45e4aeef52da3138b3da48deb947b8a529c8ee Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 20 May 2021 15:54:04 +0400 Subject: [PATCH 08/77] Allow wrapping http clients --- lib/src/authentication/bearer.dart | 23 ++++++++++++----------- lib/src/authentication/credential.dart | 4 ++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index 86662fe0f..40bc35c15 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -1,6 +1,6 @@ import 'dart:io'; -import 'package:http/http.dart'; +import 'package:http/http.dart' as http; import 'credential.dart'; @@ -16,8 +16,8 @@ class BearerCredential extends Credential { String get authenticationType => 'Bearer'; @override - Future createClient() { - return Future.value(_Client(token)); + Future createClient([http.Client? inner]) { + return Future.value(_Client(token: token, inner: inner)); } @override @@ -26,21 +26,22 @@ class BearerCredential extends Credential { } } -class _Client extends BaseClient { - _Client(this._token); +class _Client extends http.BaseClient { + _Client({required this.token, http.Client? inner}) + : _inner = inner ?? http.Client(); - final Client _client = Client(); - final String _token; + final http.Client _inner; + final String token; @override - Future send(BaseRequest request) { - request.headers[HttpHeaders.authorizationHeader] = 'Bearer $_token'; - return _client.send(request); + Future send(http.BaseRequest request) { + request.headers[HttpHeaders.authorizationHeader] = 'Bearer $token'; + return _inner.send(request); } @override void close() { - _client.close(); + _inner.close(); super.close(); } } diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 44c165b5e..715846fa9 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -1,4 +1,4 @@ -import 'package:http/http.dart'; +import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; import 'bearer.dart'; @@ -34,7 +34,7 @@ abstract class Credential { String get authenticationType; /// Creates authenticated client using this credential. - Future createClient(); + Future createClient(); /// Converts credential data into [Map]. @protected From 7955153b7f94cddc85352f80fd86cfa6277cb81d Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 02:06:08 +0400 Subject: [PATCH 09/77] Implemented AuthenticationClient --- .../authentication/authentication_client.dart | 24 ++++++++++++ lib/src/authentication/bearer.dart | 24 +----------- lib/src/authentication/credential.dart | 4 +- lib/src/authentication/credential_store.dart | 37 ++++++++++++++----- 4 files changed, 55 insertions(+), 34 deletions(-) create mode 100644 lib/src/authentication/authentication_client.dart diff --git a/lib/src/authentication/authentication_client.dart b/lib/src/authentication/authentication_client.dart new file mode 100644 index 000000000..b0d85151a --- /dev/null +++ b/lib/src/authentication/authentication_client.dart @@ -0,0 +1,24 @@ +import 'package:http/http.dart' as http; +import 'credential_store.dart'; + +class AuthenticationClient extends http.BaseClient { + AuthenticationClient(this._inner, {required this.credentialStore}); + + final http.BaseClient _inner; + final CredentialStore credentialStore; + + @override + Future send(http.BaseRequest request) async { + var creds = credentialStore.getCredential(request.url.toString()); + if (creds != null) { + await creds.beforeRequest(request); + } + return _inner.send(request); + } + + @override + void close() { + _inner.close(); + super.close(); + } +} diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index 40bc35c15..d89b70a45 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -16,8 +16,8 @@ class BearerCredential extends Credential { String get authenticationType => 'Bearer'; @override - Future createClient([http.Client? inner]) { - return Future.value(_Client(token: token, inner: inner)); + Future beforeRequest(http.BaseRequest request) async { + request.headers[HttpHeaders.authorizationHeader] = 'Bearer $token'; } @override @@ -25,23 +25,3 @@ class BearerCredential extends Credential { return {'token': token}; } } - -class _Client extends http.BaseClient { - _Client({required this.token, http.Client? inner}) - : _inner = inner ?? http.Client(); - - final http.Client _inner; - final String token; - - @override - Future send(http.BaseRequest request) { - request.headers[HttpHeaders.authorizationHeader] = 'Bearer $token'; - return _inner.send(request); - } - - @override - void close() { - _inner.close(); - super.close(); - } -} diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 715846fa9..b141700a9 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -33,8 +33,8 @@ abstract class Credential { /// Authentication type of this credential. String get authenticationType; - /// Creates authenticated client using this credential. - Future createClient(); + /// Add required details for authentication to [request]. + Future beforeRequest(http.BaseRequest request); /// Converts credential data into [Map]. @protected diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index e69a95ea3..06d7b5ed9 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -1,4 +1,4 @@ -// @dart=2.10 +// ignore_for_file: import_of_legacy_library_into_null_safe import 'dart:convert'; @@ -15,26 +15,39 @@ class CredentialStore { /// Adds [credentials] for [server] into store. void addServer(String server, Credential credentials) { - serverCredentials[server] = credentials; + var key = server.toLowerCase(); + // Make sure server name ends with a backslash. It's here to deny possible + // credential thief attach vectors where victim can add credential for + // server 'https://safesite.com' and attacker could steal credentials by + // requesting credentials for 'https://safesite.com.attacher.com', because + // URL matcher (_serverMatches method) matches credential keys with the + // beginning of the URL. + if (!key.endsWith('/')) key += '/'; + serverCredentials[key] = credentials; _save(); } - /// Removes credentials for [server]. - void removeServer(String server) { - serverCredentials.removeWhere((key, value) => key == server); + /// Removes credentials for servers that [url] matches with. + void removeServer(String url) { + serverCredentials.removeWhere((key, value) => _serverMatches(key, url)); _save(); } - /// Returns credentials for [server] if any exists, otherwise returns null. - Credential getCredential(String server) { - return serverCredentials[server]; + /// Returns credentials for server that [url] matches if any exists, otherwise + /// returns null. + Credential? getCredential(String url) { + for (final key in serverCredentials.keys) { + if (_serverMatches(key, url)) { + return serverCredentials[key]; + } + } } void _save() { - _saveCredentials(_serverCredentials); + _saveCredentials(serverCredentials); } - Map _serverCredentials; + Map? _serverCredentials; Map get serverCredentials => _serverCredentials ??= _loadCredentials(); @@ -59,4 +72,8 @@ class CredentialStore { jsonEncode( credentials.map((key, value) => MapEntry(key, value.toMap())))); } + + bool _serverMatches(String server, String url) { + return server.startsWith(url.toLowerCase()); + } } From 719005db6d83217867d3721fce0d0e8981aad44b Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 02:25:32 +0400 Subject: [PATCH 10/77] Authentication client added to stack of other clients --- lib/src/http.dart | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/src/http.dart b/lib/src/http.dart index 541931fd0..b350deeeb 100644 --- a/lib/src/http.dart +++ b/lib/src/http.dart @@ -16,12 +16,15 @@ import 'package:pedantic/pedantic.dart'; import 'package:pool/pool.dart'; import 'package:stack_trace/stack_trace.dart'; +import 'authentication/authentication_client.dart'; +import 'authentication/credential_store.dart'; import 'command.dart'; import 'io.dart'; import 'log.dart' as log; import 'oauth2.dart' as oauth2; import 'package.dart'; import 'sdk.dart'; +import 'system_cache.dart'; import 'utils.dart'; /// Headers and field names that should be censored in the log output. @@ -241,7 +244,13 @@ class _ThrowingClient extends http.BaseClient { /// The HTTP client to use for all HTTP requests. final httpClient = _ThrottleClient( 16, - _ThrowingClient(RetryClient(_pubClient, + _ThrowingClient(RetryClient( + AuthenticationClient( + _pubClient, + // TODO(themisir): Figure out a way to resolve system cache from + // somewhere else + credentialStore: CredentialStore(SystemCache()), + ), retries: math.max( 1, // Having less than 1 retry is **always** wrong. int.tryParse(Platform.environment['PUB_MAX_HTTP_RETRIES'] ?? '') ?? 7, From 383122d0c7ee02bcb23060faedd728acbbb834ed Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 03:59:24 +0400 Subject: [PATCH 11/77] Throw usage exception for more than one argument login and logout accepts only one argument at most. --- lib/src/command/login.dart | 2 ++ lib/src/command/logout.dart | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index e6da9198a..d035672b7 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -37,6 +37,8 @@ class LoginCommand extends PubCommand { Future runProtected() async { if (argResults.rest.isEmpty) { await _loginToPubDev(); + } else if (argResults.rest.length > 1) { + usageException('Takes only a single argument.'); } else { if (token?.isNotEmpty != true && !tokenStdin) { usageException('Must specify a token.'); diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 4a76e20cf..26b76d556 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -24,6 +24,8 @@ class LogoutCommand extends PubCommand { Future runProtected() async { if (argResults.rest.isEmpty) { oauth2.logout(cache); + } else if (argResults.rest.length > 1) { + usageException('Takes only a single argument.'); } else { credentialStore.removeServer(argResults.rest.first); } From 15e623849e46fe1309446db9deadaf99f6a02f16 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 04:22:09 +0400 Subject: [PATCH 12/77] Implemented publishing with saved credential support --- lib/src/authentication/credential_store.dart | 19 ++++- lib/src/command/lish.dart | 88 +++++++++++++------- 2 files changed, 72 insertions(+), 35 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 06d7b5ed9..b965823a1 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -29,7 +29,7 @@ class CredentialStore { /// Removes credentials for servers that [url] matches with. void removeServer(String url) { - serverCredentials.removeWhere((key, value) => _serverMatches(key, url)); + serverCredentials.removeWhere((key, value) => _serverKeyMatches(key, url)); _save(); } @@ -37,12 +37,23 @@ class CredentialStore { /// returns null. Credential? getCredential(String url) { for (final key in serverCredentials.keys) { - if (_serverMatches(key, url)) { + if (_serverKeyMatches(key, url)) { return serverCredentials[key]; } } } + /// Returns whether or not store has a credential for server that [url] + /// matches to. + bool hasCredential(String url) { + for (final key in serverCredentials.keys) { + if (_serverKeyMatches(key, url)) { + return true; + } + } + return false; + } + void _save() { _saveCredentials(serverCredentials); } @@ -73,7 +84,7 @@ class CredentialStore { credentials.map((key, value) => MapEntry(key, value.toMap())))); } - bool _serverMatches(String server, String url) { - return server.startsWith(url.toLowerCase()); + bool _serverKeyMatches(String serverKey, String url) { + return serverKey.startsWith(url.toLowerCase()); } } diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index eb57e0f90..eec25fc8c 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -70,40 +70,42 @@ class LishCommand extends PubCommand { abbr: 'C', help: 'Run this in the directory.', valueHelp: 'dir'); } - Future _publish(List packageBytes) async { + Future _publishUsingClient( + List packageBytes, + http.BaseClient client, + ) async { Uri cloudStorageUrl; + try { - await oauth2.withClient(cache, (client) { - return log.progress('Uploading', () async { - // TODO(nweiz): Cloud Storage can provide an XML-formatted error. We - // should report that error and exit. - var newUri = server.resolve('/api/packages/versions/new'); - var response = await client.get(newUri, headers: pubApiHeaders); - var parameters = parseJsonResponse(response); - - var url = _expectField(parameters, 'url', response); - if (url is! String) invalidServerResponse(response); - cloudStorageUrl = Uri.parse(url); - var request = http.MultipartRequest('POST', cloudStorageUrl); - - var fields = _expectField(parameters, 'fields', response); - if (fields is! Map) invalidServerResponse(response); - fields.forEach((key, value) { - if (value is! String) invalidServerResponse(response); - request.fields[key] = value; - }); - - request.followRedirects = false; - request.files.add(http.MultipartFile.fromBytes('file', packageBytes, - filename: 'package.tar.gz')); - var postResponse = - await http.Response.fromStream(await client.send(request)); - - var location = postResponse.headers['location']; - if (location == null) throw PubHttpException(postResponse); - handleJsonSuccess( - await client.get(Uri.parse(location), headers: pubApiHeaders)); + await log.progress('Uploading', () async { + // TODO(nweiz): Cloud Storage can provide an XML-formatted error. We + // should report that error and exit. + var newUri = server.resolve('/api/packages/versions/new'); + var response = await client.get(newUri, headers: pubApiHeaders); + var parameters = parseJsonResponse(response); + + var url = _expectField(parameters, 'url', response); + if (url is! String) invalidServerResponse(response); + cloudStorageUrl = Uri.parse(url); + var request = http.MultipartRequest('POST', cloudStorageUrl); + + var fields = _expectField(parameters, 'fields', response); + if (fields is! Map) invalidServerResponse(response); + fields.forEach((key, value) { + if (value is! String) invalidServerResponse(response); + request.fields[key] = value; }); + + request.followRedirects = false; + request.files.add(http.MultipartFile.fromBytes('file', packageBytes, + filename: 'package.tar.gz')); + var postResponse = + await http.Response.fromStream(await client.send(request)); + + var location = postResponse.headers['location']; + if (location == null) throw PubHttpException(postResponse); + handleJsonSuccess( + await client.get(Uri.parse(location), headers: pubApiHeaders)); }); } on PubHttpException catch (error) { var url = error.response.request.url; @@ -120,6 +122,30 @@ class LishCommand extends PubCommand { } } + Future _publish(List packageBytes) async { + try { + if (credentialStore.hasCredential(server.toString())) { + // If there's a saved credential for the server, publish using + // httpClient which should authenticate with the server using + // AuthenticationClient. + await _publishUsingClient(packageBytes, httpClient); + } else { + // If user had not yet logged in into the server, use OAuth2 credentials + // for authentication. + await oauth2.withClient(cache, (client) { + return _publishUsingClient(packageBytes, client); + }); + } + } on PubHttpException catch (error) { + var url = error.response.request.url; + if (Uri.parse(url.origin) == Uri.parse(server.origin)) { + handleJsonError(error.response); + } else { + rethrow; + } + } + } + @override Future runProtected() async { if (argResults.wasParsed('server')) { From 402154d995456ff571f339eeb0f8c35ef917d9c6 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 19:11:38 +0400 Subject: [PATCH 13/77] Modified authentication client --- lib/src/authentication/authentication_client.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/src/authentication/authentication_client.dart b/lib/src/authentication/authentication_client.dart index b0d85151a..7044855c5 100644 --- a/lib/src/authentication/authentication_client.dart +++ b/lib/src/authentication/authentication_client.dart @@ -17,8 +17,5 @@ class AuthenticationClient extends http.BaseClient { } @override - void close() { - _inner.close(); - super.close(); - } + void close() => _inner.close(); } From bf675a34c53aacc405d24f0226fd890a16eaa802 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 19:15:45 +0400 Subject: [PATCH 14/77] Add comment documentation to AuthenticationClient --- lib/src/authentication/authentication_client.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/src/authentication/authentication_client.dart b/lib/src/authentication/authentication_client.dart index 7044855c5..4b720a1b5 100644 --- a/lib/src/authentication/authentication_client.dart +++ b/lib/src/authentication/authentication_client.dart @@ -1,6 +1,10 @@ import 'package:http/http.dart' as http; + import 'credential_store.dart'; +/// This client automatically modifies request to contain required credentials +/// in request. For example some credentials might add `Authentication` header +/// to request. class AuthenticationClient extends http.BaseClient { AuthenticationClient(this._inner, {required this.credentialStore}); From 13f0fc0bb556b0f4c17eabd2d400b144fe097026 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 21 May 2021 19:16:41 +0400 Subject: [PATCH 15/77] Rename to/fromMap to to/fromJson --- lib/src/authentication/credential.dart | 5 +++-- lib/src/authentication/credential_store.dart | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index b141700a9..7980bb6f7 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -10,7 +10,7 @@ final Map _supportedMethods = { }; abstract class Credential { - static Credential? fromMap(Map map) { + static Credential? fromJson(Map map) { final authMethod = map['method'] as String?; final credentials = map['credentials'] as Map?; @@ -23,7 +23,8 @@ abstract class Credential { return null; } - Map toMap() { + /// Converts this instance into Json map. + Map toJson() { return { 'method': authenticationType, 'credentials': toMapInternal(), diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index b965823a1..db2cef36a 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -70,7 +70,7 @@ class CredentialStore { final parsed = jsonDecode(readTextFile(path)) as Map; final result = parsed - .map((key, value) => MapEntry(key, Credential.fromMap(value))) + .map((key, value) => MapEntry(key, Credential.fromJson(value))) ..removeWhere((key, value) => value == null); return result.cast(); @@ -81,7 +81,7 @@ class CredentialStore { writeTextFile( path, jsonEncode( - credentials.map((key, value) => MapEntry(key, value.toMap())))); + credentials.map((key, value) => MapEntry(key, value.toJson())))); } bool _serverKeyMatches(String serverKey, String url) { From 8e7fd96ca6d7eee08bb1dd6176cd409c283cfc6d Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 00:34:20 +0400 Subject: [PATCH 16/77] Login / logout now uses option parameter instead of argument --- lib/src/command/login.dart | 13 +++++++++---- lib/src/command/logout.dart | 13 ++++++++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index d035672b7..d076ca740 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -25,8 +25,12 @@ class LoginCommand extends PubCommand { String get token => argResults['token']; bool get tokenStdin => argResults['token-stdin']; + String get server => argResults['server']; LoginCommand() { + argParser.addOption('server', + help: 'The package server to which needs to be authenticated.'); + argParser.addOption('token', help: 'Authorization token for the server'); argParser.addFlag('token-stdin', @@ -35,15 +39,16 @@ class LoginCommand extends PubCommand { @override Future runProtected() async { - if (argResults.rest.isEmpty) { + if (server == null) { await _loginToPubDev(); - } else if (argResults.rest.length > 1) { - usageException('Takes only a single argument.'); } else { + if (Uri.tryParse(server) == null) { + usageException('Invalid or malformed server URL provided.'); + } if (token?.isNotEmpty != true && !tokenStdin) { usageException('Must specify a token.'); } - await _loginToServer(argResults.rest.first); + await _loginToServer(server); } } diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 26b76d556..8ede07edb 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -18,16 +18,19 @@ class LogoutCommand extends PubCommand { @override bool get takesArguments => true; - LogoutCommand(); + String get server => argResults['server']; + + LogoutCommand() { + argParser.addOption('server', + help: 'The package server to which needs to be authenticated.'); + } @override Future runProtected() async { - if (argResults.rest.isEmpty) { + if (server == null) { oauth2.logout(cache); - } else if (argResults.rest.length > 1) { - usageException('Takes only a single argument.'); } else { - credentialStore.removeServer(argResults.rest.first); + credentialStore.removeServer(server); } } } From 50a6b5f1c7bcfb449851d4db1aced4f58b5b0993 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 01:40:47 +0400 Subject: [PATCH 17/77] Authentication now needs to be used explicitly when needed --- .../authentication/authentication_client.dart | 25 ---------- lib/src/authentication/client.dart | 49 +++++++++++++++++++ lib/src/authentication/credential_store.dart | 13 ++++- lib/src/http.dart | 11 +---- lib/src/source/hosted.dart | 31 ++++++++---- 5 files changed, 83 insertions(+), 46 deletions(-) delete mode 100644 lib/src/authentication/authentication_client.dart create mode 100644 lib/src/authentication/client.dart diff --git a/lib/src/authentication/authentication_client.dart b/lib/src/authentication/authentication_client.dart deleted file mode 100644 index 4b720a1b5..000000000 --- a/lib/src/authentication/authentication_client.dart +++ /dev/null @@ -1,25 +0,0 @@ -import 'package:http/http.dart' as http; - -import 'credential_store.dart'; - -/// This client automatically modifies request to contain required credentials -/// in request. For example some credentials might add `Authentication` header -/// to request. -class AuthenticationClient extends http.BaseClient { - AuthenticationClient(this._inner, {required this.credentialStore}); - - final http.BaseClient _inner; - final CredentialStore credentialStore; - - @override - Future send(http.BaseRequest request) async { - var creds = credentialStore.getCredential(request.url.toString()); - if (creds != null) { - await creds.beforeRequest(request); - } - return _inner.send(request); - } - - @override - void close() => _inner.close(); -} diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart new file mode 100644 index 000000000..cc2c079eb --- /dev/null +++ b/lib/src/authentication/client.dart @@ -0,0 +1,49 @@ +// ignore_for_file: import_of_legacy_library_into_null_safe + +import 'package:http/http.dart' as http; + +import '../http.dart'; +import '../system_cache.dart'; +import 'credential.dart'; +import 'credential_store.dart'; + +/// This client automatically modifies request to contain required credentials +/// in request. For example some credentials might add `Authentication` header +/// to request. +class _AuthenticationClient extends http.BaseClient { + _AuthenticationClient(this._inner, {required this.credential}); + + final http.BaseClient _inner; + final Credential credential; + + @override + Future send(http.BaseRequest request) async { + await credential.beforeRequest(request); + return _inner.send(request); + } + + @override + void close() => _inner.close(); +} + +Future withAuthenticatedClient( + SystemCache systemCache, + String server, + Future Function(http.Client) fn, { + List? alsoMatches, +}) { + final store = CredentialStore(systemCache); + final credential = store.getCredential(server, alsoMatches: alsoMatches); + final http.Client client = credential == null + ? httpClient + : _AuthenticationClient(httpClient, credential: credential); + + return fn(client).catchError((error) { + if (error is PubHttpException) { + if (error.response.statusCode == 401) { + // TODO(themisir): authentication is required for the server or + // credential might be invalid. + } + } + }); +} diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index db2cef36a..8b034aa68 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -34,10 +34,19 @@ class CredentialStore { } /// Returns credentials for server that [url] matches if any exists, otherwise - /// returns null. - Credential? getCredential(String url) { + /// returns null. If [alsoMatches] argument is provided, the store will check + /// for every item of [alsoMatches] matches the credential key. + Credential? getCredential(String url, {List? alsoMatches}) { for (final key in serverCredentials.keys) { if (_serverKeyMatches(key, url)) { + if (alsoMatches != null && alsoMatches.isNotEmpty) { + for (final item in alsoMatches) { + if (!_serverKeyMatches(key, item)) { + continue; + } + } + } + return serverCredentials[key]; } } diff --git a/lib/src/http.dart b/lib/src/http.dart index b350deeeb..541931fd0 100644 --- a/lib/src/http.dart +++ b/lib/src/http.dart @@ -16,15 +16,12 @@ import 'package:pedantic/pedantic.dart'; import 'package:pool/pool.dart'; import 'package:stack_trace/stack_trace.dart'; -import 'authentication/authentication_client.dart'; -import 'authentication/credential_store.dart'; import 'command.dart'; import 'io.dart'; import 'log.dart' as log; import 'oauth2.dart' as oauth2; import 'package.dart'; import 'sdk.dart'; -import 'system_cache.dart'; import 'utils.dart'; /// Headers and field names that should be censored in the log output. @@ -244,13 +241,7 @@ class _ThrowingClient extends http.BaseClient { /// The HTTP client to use for all HTTP requests. final httpClient = _ThrottleClient( 16, - _ThrowingClient(RetryClient( - AuthenticationClient( - _pubClient, - // TODO(themisir): Figure out a way to resolve system cache from - // somewhere else - credentialStore: CredentialStore(SystemCache()), - ), + _ThrowingClient(RetryClient(_pubClient, retries: math.max( 1, // Having less than 1 retry is **always** wrong. int.tryParse(Platform.environment['PUB_MAX_HTTP_RETRIES'] ?? '') ?? 7, diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index a3ee1e156..26f7cd4a2 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -15,6 +15,7 @@ import 'package:pedantic/pedantic.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:stack_trace/stack_trace.dart'; +import '../authentication/client.dart'; import '../exceptions.dart'; import '../http.dart'; import '../io.dart'; @@ -220,7 +221,13 @@ class BoundHostedSource extends CachedSource { try { // TODO(sigurdm): Implement cancellation of requests. This probably // requires resolution of: https://github.com/dart-lang/sdk/issues/22265. - bodyText = await httpClient.read(url, headers: pubApiHeaders); + bodyText = await withAuthenticatedClient( + systemCache, + url.toString(), + (client) async { + return client.read(url, headers: pubApiHeaders); + }, + ); body = jsonDecode(bodyText); result = _versionInfoFromPackageListing(body, ref, url); } catch (error, stackTrace) { @@ -557,14 +564,13 @@ class BoundHostedSource extends CachedSource { throw PackageNotFoundException( 'Package $packageName has no version $version'); } + final parsedDescription = source._parseDescription(id.description); + final server = parsedDescription.last; + var url = versionInfo.archiveUrl; - if (url == null) { - // To support old servers that has no archive_url we fall back to the - // hard-coded path. - final parsedDescription = source._parseDescription(id.description); - final server = parsedDescription.last; - url = Uri.parse('$server/packages/$packageName/versions/$version.tar.gz'); - } + // To support old servers that has no archive_url we fall back to the + // hard-coded path. + url ??= Uri.parse('$server/packages/$packageName/versions/$version.tar.gz'); log.io('Get package from $url.'); log.message('Downloading ${log.bold(id.name)} ${id.version}...'); @@ -572,7 +578,14 @@ class BoundHostedSource extends CachedSource { await withTempDir((tempDirForArchive) async { var archivePath = p.join(tempDirForArchive, '$packageName-$version.tar.gz'); - var response = await httpClient.send(http.Request('GET', url)); + var response = await withAuthenticatedClient( + systemCache, + server, + (client) { + return client.send(http.Request('GET', url)); + }, + alsoMatches: [archivePath], + ); // We download the archive to disk instead of streaming it directly into // the tar unpacking. This simplifies stream handling. From 84a6c95b32948e822bd25ee830fe3c1609266c98 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 01:52:26 +0400 Subject: [PATCH 18/77] Update list command to use authenticated client --- lib/src/authentication/client.dart | 23 ++++++++++++---- lib/src/authentication/credential_store.dart | 28 +++++++++++--------- lib/src/command/lish.dart | 5 +++- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index cc2c079eb..d34ea0a1c 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -11,14 +11,23 @@ import 'credential_store.dart'; /// in request. For example some credentials might add `Authentication` header /// to request. class _AuthenticationClient extends http.BaseClient { - _AuthenticationClient(this._inner, {required this.credential}); + _AuthenticationClient( + this._inner, { + required this.credential, + required this.serverKey, + }); final http.BaseClient _inner; final Credential credential; + final String serverKey; @override Future send(http.BaseRequest request) async { - await credential.beforeRequest(request); + // Let's last time make sure that, we're allowed to use credential for this + // request. + if (serverKeyMatches(serverKey, request.url.toString())) { + await credential.beforeRequest(request); + } return _inner.send(request); } @@ -33,10 +42,14 @@ Future withAuthenticatedClient( List? alsoMatches, }) { final store = CredentialStore(systemCache); - final credential = store.getCredential(server, alsoMatches: alsoMatches); - final http.Client client = credential == null + final match = store.getCredential(server, alsoMatches: alsoMatches); + final http.Client client = match == null ? httpClient - : _AuthenticationClient(httpClient, credential: credential); + : _AuthenticationClient( + httpClient, + credential: match.last, + serverKey: match.first, + ); return fn(client).catchError((error) { if (error is PubHttpException) { diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 8b034aa68..dd851b31b 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -6,6 +6,7 @@ import 'package:path/path.dart' as path; import '../io.dart'; import '../system_cache.dart'; +import '../utils.dart'; import 'credential.dart'; class CredentialStore { @@ -19,7 +20,7 @@ class CredentialStore { // Make sure server name ends with a backslash. It's here to deny possible // credential thief attach vectors where victim can add credential for // server 'https://safesite.com' and attacker could steal credentials by - // requesting credentials for 'https://safesite.com.attacher.com', because + // requesting credentials for 'https://safesite.com.attacker.com', because // URL matcher (_serverMatches method) matches credential keys with the // beginning of the URL. if (!key.endsWith('/')) key += '/'; @@ -29,25 +30,25 @@ class CredentialStore { /// Removes credentials for servers that [url] matches with. void removeServer(String url) { - serverCredentials.removeWhere((key, value) => _serverKeyMatches(key, url)); + serverCredentials.removeWhere((key, value) => serverKeyMatches(key, url)); _save(); } - /// Returns credentials for server that [url] matches if any exists, otherwise - /// returns null. If [alsoMatches] argument is provided, the store will check - /// for every item of [alsoMatches] matches the credential key. - Credential? getCredential(String url, {List? alsoMatches}) { + /// Returns pair of credential and server key for server that [url] and + /// [alsoMatches] matches to server key. + Pair? getCredential(String url, + {List? alsoMatches}) { for (final key in serverCredentials.keys) { - if (_serverKeyMatches(key, url)) { + if (serverKeyMatches(key, url)) { if (alsoMatches != null && alsoMatches.isNotEmpty) { for (final item in alsoMatches) { - if (!_serverKeyMatches(key, item)) { + if (!serverKeyMatches(key, item)) { continue; } } } - return serverCredentials[key]; + return Pair(key, serverCredentials[key]); } } } @@ -56,7 +57,7 @@ class CredentialStore { /// matches to. bool hasCredential(String url) { for (final key in serverCredentials.keys) { - if (_serverKeyMatches(key, url)) { + if (serverKeyMatches(key, url)) { return true; } } @@ -92,8 +93,9 @@ class CredentialStore { jsonEncode( credentials.map((key, value) => MapEntry(key, value.toJson())))); } +} - bool _serverKeyMatches(String serverKey, String url) { - return serverKey.startsWith(url.toLowerCase()); - } +bool serverKeyMatches(String serverKey, String url) { + if (!serverKey.endsWith('/')) serverKey += '/'; + return serverKey.startsWith(url.toLowerCase()); } diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index eec25fc8c..51cede03d 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -9,6 +9,7 @@ import 'dart:async'; import 'package:http/http.dart' as http; import '../ascii_tree.dart' as tree; +import '../authentication/client.dart'; import '../command.dart'; import '../exit_codes.dart' as exit_codes; import '../http.dart'; @@ -128,7 +129,9 @@ class LishCommand extends PubCommand { // If there's a saved credential for the server, publish using // httpClient which should authenticate with the server using // AuthenticationClient. - await _publishUsingClient(packageBytes, httpClient); + await withAuthenticatedClient(cache, server.toString(), (client) { + return _publishUsingClient(packageBytes, client); + }); } else { // If user had not yet logged in into the server, use OAuth2 credentials // for authentication. From 8547a32500b74e521bb0d86e147f4a52a568394a Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 02:04:22 +0400 Subject: [PATCH 19/77] Change access modifier for authenticationType to @protected --- lib/src/authentication/credential.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 7980bb6f7..6a722ceb3 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -32,6 +32,7 @@ abstract class Credential { } /// Authentication type of this credential. + @protected String get authenticationType; /// Add required details for authentication to [request]. From 7896061ed2b8b5720932b33696073df150576868 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 02:09:37 +0400 Subject: [PATCH 20/77] Add docs --- lib/src/authentication/bearer.dart | 2 ++ lib/src/authentication/credential.dart | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index d89b70a45..197e80cea 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -4,6 +4,8 @@ import 'package:http/http.dart' as http; import 'credential.dart'; +/// Bearer credential type that simply puts authorization header formatted as +/// `Bearer $token` to request.header. class BearerCredential extends Credential { BearerCredential(this.token); diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 6a722ceb3..7d9ef6e5e 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -9,7 +9,12 @@ final Map _supportedMethods = { 'Bearer': BearerCredential.fromMap, }; +/// Credentials used to authenticate requests sent to auth - protected hosted +/// pub repositories. This class is base class for different credential type +/// implementations like [BearerCredential]. abstract class Credential { + /// Parse Credential details from given [map]. If parsing fails this method + /// will return null. static Credential? fromJson(Map map) { final authMethod = map['method'] as String?; final credentials = map['credentials'] as Map?; From b2194dee6ecb9fc9873ad28271ed6ef79b6a0943 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 22 May 2021 02:38:23 +0400 Subject: [PATCH 21/77] Added file headers --- lib/src/authentication/bearer.dart | 4 ++++ lib/src/authentication/client.dart | 4 ++++ lib/src/authentication/credential.dart | 4 ++++ lib/src/authentication/credential_store.dart | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index 197e80cea..5939627e4 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2021, 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. + import 'dart:io'; import 'package:http/http.dart' as http; diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index d34ea0a1c..a8c436dfd 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2021, 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. + // ignore_for_file: import_of_legacy_library_into_null_safe import 'package:http/http.dart' as http; diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 7d9ef6e5e..75f8d8d6c 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2021, 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. + import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index dd851b31b..7f40a7c04 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2021, 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. + // ignore_for_file: import_of_legacy_library_into_null_safe import 'dart:convert'; From 6aeaec1c6d20240468450fda47fe21b3239762d4 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 26 May 2021 00:06:04 +0400 Subject: [PATCH 22/77] Update lib/src/source/hosted.dart Co-authored-by: Jonas Finnemann Jensen --- lib/src/source/hosted.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 26f7cd4a2..06d169892 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -581,9 +581,7 @@ class BoundHostedSource extends CachedSource { var response = await withAuthenticatedClient( systemCache, server, - (client) { - return client.send(http.Request('GET', url)); - }, + (client) => client.send(http.Request('GET', url)), alsoMatches: [archivePath], ); From c917ec3bed3b4502c586a5f3e406883495e8f985 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 26 May 2021 00:25:55 +0400 Subject: [PATCH 23/77] Rename server key to serverBaseUrl --- lib/src/authentication/client.dart | 26 +++++++++++--------- lib/src/authentication/credential_store.dart | 24 ++++++------------ 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index a8c436dfd..2f54f57e7 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -18,18 +18,23 @@ class _AuthenticationClient extends http.BaseClient { _AuthenticationClient( this._inner, { required this.credential, - required this.serverKey, + required this.serverBaseUrl, }); final http.BaseClient _inner; final Credential credential; - final String serverKey; + final String serverBaseUrl; @override Future send(http.BaseRequest request) async { // Let's last time make sure that, we're allowed to use credential for this // request. - if (serverKeyMatches(serverKey, request.url.toString())) { + // + // This check ensures that this client will only authenticate requests sent + // to given serverBaseUrl. Otherwise credential leaks might ocurr when + // archive_url hosted on 3rd party server that should not receive + // credentials of the first party. + if (serverBaseUrlMatches(serverBaseUrl, request.url.toString())) { await credential.beforeRequest(request); } return _inner.send(request); @@ -41,18 +46,17 @@ class _AuthenticationClient extends http.BaseClient { Future withAuthenticatedClient( SystemCache systemCache, - String server, - Future Function(http.Client) fn, { - List? alsoMatches, -}) { + String serverBaseUrl, + Future Function(http.Client) fn, +) { final store = CredentialStore(systemCache); - final match = store.getCredential(server, alsoMatches: alsoMatches); - final http.Client client = match == null + final credential = store.getCredential(serverBaseUrl); + final http.Client client = credential == null ? httpClient : _AuthenticationClient( httpClient, - credential: match.last, - serverKey: match.first, + serverBaseUrl: credential.first, + credential: credential.last, ); return fn(client).catchError((error) { diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 7f40a7c04..6bd0a021a 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -34,24 +34,16 @@ class CredentialStore { /// Removes credentials for servers that [url] matches with. void removeServer(String url) { - serverCredentials.removeWhere((key, value) => serverKeyMatches(key, url)); + serverCredentials + .removeWhere((key, value) => serverBaseUrlMatches(key, url)); _save(); } /// Returns pair of credential and server key for server that [url] and /// [alsoMatches] matches to server key. - Pair? getCredential(String url, - {List? alsoMatches}) { + Pair? getCredential(String url) { for (final key in serverCredentials.keys) { - if (serverKeyMatches(key, url)) { - if (alsoMatches != null && alsoMatches.isNotEmpty) { - for (final item in alsoMatches) { - if (!serverKeyMatches(key, item)) { - continue; - } - } - } - + if (serverBaseUrlMatches(key, url)) { return Pair(key, serverCredentials[key]); } } @@ -61,7 +53,7 @@ class CredentialStore { /// matches to. bool hasCredential(String url) { for (final key in serverCredentials.keys) { - if (serverKeyMatches(key, url)) { + if (serverBaseUrlMatches(key, url)) { return true; } } @@ -99,7 +91,7 @@ class CredentialStore { } } -bool serverKeyMatches(String serverKey, String url) { - if (!serverKey.endsWith('/')) serverKey += '/'; - return serverKey.startsWith(url.toLowerCase()); +bool serverBaseUrlMatches(String serverBaseUrl, String url) { + if (!serverBaseUrl.endsWith('/')) serverBaseUrl += '/'; + return serverBaseUrl.startsWith(url.toLowerCase()); } From 8d9100772c6caa6c5b0f1ffd027756980a56a1e4 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 26 May 2021 00:27:05 +0400 Subject: [PATCH 24/77] Removed alsoMatches argument Because: 1. It's not needed anymore because authenticated client already checks request.url with serverBaseUrl 2. The value is wrong anyways (has to be url instead of archivePath) --- lib/src/source/hosted.dart | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 06d169892..dfba37ac3 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -578,12 +578,8 @@ class BoundHostedSource extends CachedSource { await withTempDir((tempDirForArchive) async { var archivePath = p.join(tempDirForArchive, '$packageName-$version.tar.gz'); - var response = await withAuthenticatedClient( - systemCache, - server, - (client) => client.send(http.Request('GET', url)), - alsoMatches: [archivePath], - ); + var response = await withAuthenticatedClient(systemCache, server, + (client) => client.send(http.Request('GET', url))); // We download the archive to disk instead of streaming it directly into // the tar unpacking. This simplifies stream handling. From d6e02544c9c5866d92ba8f98ea317b00d111925d Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 27 May 2021 01:43:53 +0400 Subject: [PATCH 25/77] Rename _AuthenticationClient to _AuthenticatedClient --- lib/src/authentication/client.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 2f54f57e7..17018fc24 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -14,8 +14,8 @@ import 'credential_store.dart'; /// This client automatically modifies request to contain required credentials /// in request. For example some credentials might add `Authentication` header /// to request. -class _AuthenticationClient extends http.BaseClient { - _AuthenticationClient( +class _AuthenticatedClient extends http.BaseClient { + _AuthenticatedClient( this._inner, { required this.credential, required this.serverBaseUrl, @@ -53,7 +53,7 @@ Future withAuthenticatedClient( final credential = store.getCredential(serverBaseUrl); final http.Client client = credential == null ? httpClient - : _AuthenticationClient( + : _AuthenticatedClient( httpClient, serverBaseUrl: credential.first, credential: credential.last, From 094ad56e000b7599b747c6ae8a73f1e6560f5337 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 27 May 2021 01:55:34 +0400 Subject: [PATCH 26/77] Update credential Json serialization --- lib/src/authentication/bearer.dart | 22 +++++--------- lib/src/authentication/client.dart | 5 +++- lib/src/authentication/credential.dart | 41 +++++++------------------- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index 5939627e4..eaae40bea 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -2,10 +2,6 @@ // 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. -import 'dart:io'; - -import 'package:http/http.dart' as http; - import 'credential.dart'; /// Bearer credential type that simply puts authorization header formatted as @@ -13,21 +9,19 @@ import 'credential.dart'; class BearerCredential extends Credential { BearerCredential(this.token); - static BearerCredential fromMap(Map map) => + static const String kind = 'Bearer'; + + /// Deserializes [map] into [BearerCredential]. + static BearerCredential fromJson(Map map) => BearerCredential(map['token'] as String); + /// Bearer token final String token; @override - String get authenticationType => 'Bearer'; - - @override - Future beforeRequest(http.BaseRequest request) async { - request.headers[HttpHeaders.authorizationHeader] = 'Bearer $token'; - } + Map toJson() => + {'kind': kind, 'token': token}; @override - Map toMapInternal() { - return {'token': token}; - } + Future getAuthorizationHeaderValue() => Future.value('Bearer $token'); } diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 17018fc24..2876262b1 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -4,6 +4,8 @@ // ignore_for_file: import_of_legacy_library_into_null_safe +import 'dart:io'; + import 'package:http/http.dart' as http; import '../http.dart'; @@ -35,7 +37,8 @@ class _AuthenticatedClient extends http.BaseClient { // archive_url hosted on 3rd party server that should not receive // credentials of the first party. if (serverBaseUrlMatches(serverBaseUrl, request.url.toString())) { - await credential.beforeRequest(request); + request.headers[HttpHeaders.authorizationHeader] = + await credential.getAuthorizationHeaderValue(); } return _inner.send(request); } diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 75f8d8d6c..4411abdd9 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -2,15 +2,12 @@ // 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. -import 'package:http/http.dart' as http; -import 'package:meta/meta.dart'; - import 'bearer.dart'; typedef CredentialDeserializer = Credential Function(Map); -final Map _supportedMethods = { - 'Bearer': BearerCredential.fromMap, +final Map _credentialKinds = { + BearerCredential.kind: BearerCredential.fromJson, }; /// Credentials used to authenticate requests sent to auth - protected hosted @@ -20,34 +17,18 @@ abstract class Credential { /// Parse Credential details from given [map]. If parsing fails this method /// will return null. static Credential? fromJson(Map map) { - final authMethod = map['method'] as String?; - final credentials = map['credentials'] as Map?; - - if (credentials != null && - authMethod?.isNotEmpty == true && - _supportedMethods.containsKey(authMethod)) { - return _supportedMethods[authMethod]!(credentials); - } + final credentialKind = map['kind'] as String?; - return null; + return credentialKind?.isNotEmpty == true && + _credentialKinds.containsKey(credentialKind) + ? _credentialKinds[credentialKind]!(map) + : null; } /// Converts this instance into Json map. - Map toJson() { - return { - 'method': authenticationType, - 'credentials': toMapInternal(), - }; - } - - /// Authentication type of this credential. - @protected - String get authenticationType; - - /// Add required details for authentication to [request]. - Future beforeRequest(http.BaseRequest request); + Map toJson(); - /// Converts credential data into [Map]. - @protected - Map toMapInternal(); + /// Returns future that resolves "Authorization" header value used for + /// authenticating. + Future getAuthorizationHeaderValue(); } From 4f652ffcb26f4f7c9ce4b388da9f0baf452b2fe1 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 27 May 2021 03:42:22 +0400 Subject: [PATCH 27/77] Fixed credential server base url matching --- lib/src/authentication/credential_store.dart | 3 ++- lib/src/source/hosted.dart | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 6bd0a021a..d2a228d19 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -93,5 +93,6 @@ class CredentialStore { bool serverBaseUrlMatches(String serverBaseUrl, String url) { if (!serverBaseUrl.endsWith('/')) serverBaseUrl += '/'; - return serverBaseUrl.startsWith(url.toLowerCase()); + if (!url.endsWith('/')) url += '/'; + return url.startsWith(serverBaseUrl.toLowerCase()); } diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index dfba37ac3..519bbbcfa 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -224,9 +224,7 @@ class BoundHostedSource extends CachedSource { bodyText = await withAuthenticatedClient( systemCache, url.toString(), - (client) async { - return client.read(url, headers: pubApiHeaders); - }, + (client) => client.read(url, headers: pubApiHeaders), ); body = jsonDecode(bodyText); result = _versionInfoFromPackageListing(body, ref, url); From e937a378f384820b4e532cbd70263c1c8435f816 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 03:19:46 +0400 Subject: [PATCH 28/77] Rename some "key"s to "serverBaseUrl" --- lib/src/authentication/credential_store.dart | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index d2a228d19..292a7682a 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -18,42 +18,42 @@ class CredentialStore { final SystemCache cache; - /// Adds [credentials] for [server] into store. - void addServer(String server, Credential credentials) { - var key = server.toLowerCase(); + /// Adds [credentials] for [serverBaseUrl] into store. + void addServer(String serverBaseUrl, Credential credentials) { + serverBaseUrl = serverBaseUrl.toLowerCase(); // Make sure server name ends with a backslash. It's here to deny possible // credential thief attach vectors where victim can add credential for // server 'https://safesite.com' and attacker could steal credentials by // requesting credentials for 'https://safesite.com.attacker.com', because // URL matcher (_serverMatches method) matches credential keys with the // beginning of the URL. - if (!key.endsWith('/')) key += '/'; - serverCredentials[key] = credentials; + if (!serverBaseUrl.endsWith('/')) serverBaseUrl += '/'; + serverCredentials[serverBaseUrl] = credentials; _save(); } /// Removes credentials for servers that [url] matches with. void removeServer(String url) { - serverCredentials - .removeWhere((key, value) => serverBaseUrlMatches(key, url)); + serverCredentials.removeWhere( + (serverBaseUrl, _) => serverBaseUrlMatches(serverBaseUrl, url)); _save(); } - /// Returns pair of credential and server key for server that [url] and - /// [alsoMatches] matches to server key. + /// Returns pair of credential and server base url for server for + /// authenticating [url]. Pair? getCredential(String url) { - for (final key in serverCredentials.keys) { - if (serverBaseUrlMatches(key, url)) { - return Pair(key, serverCredentials[key]); + for (final serverBaseUrl in serverCredentials.keys) { + if (serverBaseUrlMatches(serverBaseUrl, url)) { + return Pair(serverBaseUrl, serverCredentials[serverBaseUrl]); } } } /// Returns whether or not store has a credential for server that [url] - /// matches to. + /// could be authenticated with. bool hasCredential(String url) { - for (final key in serverCredentials.keys) { - if (serverBaseUrlMatches(key, url)) { + for (final serverBaseUrl in serverCredentials.keys) { + if (serverBaseUrlMatches(serverBaseUrl, url)) { return true; } } From f800eef426216979a5090f5c6018d37bdb52305f Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 03:34:30 +0400 Subject: [PATCH 29/77] Updated proposal document --- doc/authentication-proposal.md | 81 +++++++++------------------------- 1 file changed, 22 insertions(+), 59 deletions(-) diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md index 1c9a2d73d..6ff703b0b 100644 --- a/doc/authentication-proposal.md +++ b/doc/authentication-proposal.md @@ -23,89 +23,52 @@ WWW-Authenticate header syntax: WWW-Authenticate: [realm=][, charset="UTF-8"] ``` -> `realm` parameter is completely optional, and is not used in this proposal. +> `realm` parameter is completely optional, and will not be used by pub. > You can read more about this parameter here: > https://datatracker.ietf.org/doc/html/rfc7235#section-2.2 -Pub CLI will only support **Basic** and **Bearer** authentication methods by -default. +Pub CLI currently only support **Bearer** authentication methods. -## Authentication flow +### Login / logout -After receiving `WWW-Authenticate` header, pub CLI will prompt user for -the credentials like this: +Users can login to 3rd party hosted pub server using **pub login** command like +that: ```plain -user@machine$ pub get -Please enter required credentials to authenticate with "https://myserver.com" -hosted repository. - -Username: bob -Password: password - -Please enter required credentials to authenticate with "https://pub.example.com" -hosted repository. - -Bearer token: 8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r -``` - -After providing credentials, the client will send those credentials to the -server in `Authorization` header. - - -```dart -// Bearer authentication -{ 'Autorization': 'Bearer $token' } - -// Basic authentication -{ 'Autorization': 'Basic ' + base64('$username:$password') } -``` - -### Explicit authentication - -Users can also explicitly define authentication credentials even before server -asks for it using a modified **pub login** command. By default if no argument is -presented, **pub login** will behave as previously: authenticate with Google. -But if you you specify server to login, it will try to authenticate with the -given server instead. - -```plain -pub login https://myspuberver.dev +pub login --server https://myspuberver.dev --token xxxxxxxxxx ``` -To discover authentication method, client will send **GET** request to `/login` -endpoint. This endpoint should be authenticated by the server as well as other -endpoints. The client might use this endpoint to: +`--server` is base url of the pub server and `--token` is bearer token for the +authentication. If the server option is not provided, **pub login** will behave +like previous versions - will try authenticating with Google account. -1. Validate cached credentials when needed -2. Discover authentication method by sending unauthenticated requests +Just like this, **pub logout** will also support 3rd party hosted pub server +de-authentication. If you provide `--server` option to the command it will +simply remove saved credentials for the server. If not, it will remove Google +account credentials. ## Storing credentials Hosted Pub Repository authentication credentials will be stored on json file -named `hosted.credentials.json` located in cache root directory. Authentication -details will be stored at this file as json values while their keys will be URLs -of the server (`PUB_HOSTED_URL`). +named `tokens.json` located in cache root directory. Authentication details will +be stored at this file as json values while their keys will be URLs of the +server (`PUB_HOSTED_URL`). ```json { "https://myserver.com": { - "method": "Basic", - "credentials": { - "username": "bob", - "password": "password" - } + "kind": "Basic", + "username": "bob", + "password": "password" }, "https://pub.example.com": { - "method": "Bearer", - "credentials": { - "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" - } + "kind": "Bearer", + "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" } } ``` -This model of storing credentials allows us to extend support to new +This model of storing credentials allows us to extend support for new authentication methods in future. ## References From cac0fee7991ea549324e74946727bb79819d9fad Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 03:38:13 +0400 Subject: [PATCH 30/77] Modified http.dart --- lib/src/http.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/http.dart b/lib/src/http.dart index 541931fd0..5b7a8722a 100644 --- a/lib/src/http.dart +++ b/lib/src/http.dart @@ -372,7 +372,6 @@ class _ThrottleClient extends http.BaseClient { /// requests. It defaults to `new http.Client()`. _ThrottleClient(int maxActiveRequests, this._inner) : _pool = Pool(maxActiveRequests); - @override Future send(http.BaseRequest request) async { var resource = await _pool.request(); From be1fee10748b720edf62b4c50dd3fbb66f04ae12 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 03:39:26 +0400 Subject: [PATCH 31/77] Revert "Modified http.dart" This reverts commit cac0fee7991ea549324e74946727bb79819d9fad. --- lib/src/http.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/http.dart b/lib/src/http.dart index 5b7a8722a..541931fd0 100644 --- a/lib/src/http.dart +++ b/lib/src/http.dart @@ -372,6 +372,7 @@ class _ThrottleClient extends http.BaseClient { /// requests. It defaults to `new http.Client()`. _ThrottleClient(int maxActiveRequests, this._inner) : _pool = Pool(maxActiveRequests); + @override Future send(http.BaseRequest request) async { var resource = await _pool.request(); From 7286e4b37b09673f820dc4439d7afa82d0b56ebb Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 04:10:12 +0400 Subject: [PATCH 32/77] Modified error handling of withAuthenticatedClient --- lib/src/authentication/client.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 2876262b1..7e7fe14a4 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -51,7 +51,7 @@ Future withAuthenticatedClient( SystemCache systemCache, String serverBaseUrl, Future Function(http.Client) fn, -) { +) async { final store = CredentialStore(systemCache); final credential = store.getCredential(serverBaseUrl); final http.Client client = credential == null @@ -62,12 +62,15 @@ Future withAuthenticatedClient( credential: credential.last, ); - return fn(client).catchError((error) { + try { + return await fn(client); + } catch (error) { if (error is PubHttpException) { if (error.response.statusCode == 401) { // TODO(themisir): authentication is required for the server or // credential might be invalid. } } - }); + rethrow; + } } From 3e5a3e2a04f2cd545d7807b0704b1a09aeaf4bed Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 15:51:05 +0400 Subject: [PATCH 33/77] Integration tests for login with server command --- lib/src/command/login.dart | 5 +- test/authentication/login_to_server_test.dart | 65 +++++++++++++++++++ test/descriptor.dart | 7 ++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 test/authentication/login_to_server_test.dart diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index d076ca740..392603631 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -53,13 +53,14 @@ class LoginCommand extends PubCommand { } Future _loginToServer(String server) async { - if (Uri.tryParse(server) == null) { + if (Uri.tryParse(server) == null || + !server.startsWith(RegExp(r'https?:\/\/'))) { usageException('Invalid or malformed server URL provided.'); } final _token = tokenStdin ? await readLine() : token; credentialStore.addServer(server, BearerCredential(_token)); - log.message('You are now logged in to $server using bearer token'); + log.message('You are now logged in to $server using bearer token.'); } Future _loginToPubDev() async { diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart new file mode 100644 index 000000000..092c41fac --- /dev/null +++ b/test/authentication/login_to_server_test.dart @@ -0,0 +1,65 @@ +// Copyright (c) 2021, 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. + +// @dart=2.10 + +import 'package:test/test.dart'; +import 'package:pub/src/exit_codes.dart' as exit_codes; + +import '../descriptor.dart' as d; +import '../test_pub.dart'; + +void main() { + test('with correct server url creates tokens.json that contains token', + () async { + await d.dir(cachePath).create(); + await runPub( + args: [ + 'login', + '--server', + 'http://server.demo', + '--token', + 'auth-token', + ], + output: contains( + 'You are now logged in to http://server.demo using bearer token.'), + ); + + await d.tokensFile({ + 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + }).validate(); + }); + + test('with invalid server url returns error', () async { + await d.dir(cachePath).create(); + await runPub( + args: [ + 'login', + '--server', + 'http:;://invalid-url,.com', + '--token', + 'auth-token', + ], + error: contains('Invalid or malformed server URL provided.'), + exitCode: exit_codes.USAGE, + ); + + await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); + }); + + test('without token returns error', () async { + await d.dir(cachePath).create(); + await runPub( + args: [ + 'login', + '--server', + 'http://server.demo', + ], + error: contains('Must specify a token.'), + exitCode: exit_codes.USAGE, + ); + + await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); + }); +} diff --git a/test/descriptor.dart b/test/descriptor.dart index b4a30de56..566135aed 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -205,6 +205,13 @@ Descriptor credentialsFile(PackageServer server, String accessToken, ]); } +/// Describes the file in the system cache that contains credentials for +/// third party hosted pub servers. +Descriptor tokensFile([Map contents = const {}]) { + return dir(cachePath, + [file('tokens.json', contents != null ? jsonEncode(contents) : null)]); +} + /// Describes the application directory, containing only a pubspec specifying /// the given [dependencies]. DirectoryDescriptor appDir([Map dependencies]) => From 1308b4a5262fea995a71013a5fc1e5a419a1bef1 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 29 May 2021 16:04:55 +0400 Subject: [PATCH 34/77] Integration tests for logout server command --- lib/src/authentication/credential_store.dart | 19 +++++++-- .../logout_from_server_test.dart | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 test/authentication/logout_from_server_test.dart diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 292a7682a..a482d8451 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -9,6 +9,7 @@ import 'dart:convert'; import 'package:path/path.dart' as path; import '../io.dart'; +import '../log.dart' as log; import '../system_cache.dart'; import '../utils.dart'; import 'credential.dart'; @@ -34,9 +35,21 @@ class CredentialStore { /// Removes credentials for servers that [url] matches with. void removeServer(String url) { - serverCredentials.removeWhere( - (serverBaseUrl, _) => serverBaseUrlMatches(serverBaseUrl, url)); - _save(); + var modified = false; + // Iterating serverCredentials.keys.toList() because otherwise we'll get + // concurrent modification during iteration error. + for (final serverBaseUrl in serverCredentials.keys.toList()) { + if (serverBaseUrlMatches(serverBaseUrl, url)) { + log.message('Logging out of $serverBaseUrl.'); + serverCredentials.remove(serverBaseUrl); + modified = true; + } + } + if (modified) { + _save(); + } else { + log.message('No matching credential found for $url. Cannot log out.'); + } } /// Returns pair of credential and server base url for server for diff --git a/test/authentication/logout_from_server_test.dart b/test/authentication/logout_from_server_test.dart new file mode 100644 index 000000000..1f0d5e165 --- /dev/null +++ b/test/authentication/logout_from_server_test.dart @@ -0,0 +1,41 @@ +// Copyright (c) 2021, 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. + +// @dart=2.10 + +import 'package:test/test.dart'; + +import '../descriptor.dart' as d; +import '../test_pub.dart'; + +void main() { + test('with an matching server url, removes the entry.', () async { + await d.tokensFile({ + 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + }).create(); + + await runPub( + args: ['logout', '--server', 'http://server.demo'], + output: contains('Logging out of http://server.demo/.'), + ); + + await d.tokensFile({}).validate(); + }); + + test('without an matching server url, does nothing.', () async { + await d.tokensFile({ + 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + }).create(); + + await runPub( + args: ['logout', '--server', 'http://another-server.demo'], + output: 'No matching credential found for http://another-server.demo. ' + 'Cannot log out.', + ); + + await d.tokensFile({ + 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + }).validate(); + }); +} From 4eea29889bfcad41bffa9bf74e22a144cc6e7dbb Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 11:38:28 +0400 Subject: [PATCH 35/77] Modified JSON (de)serialization --- lib/src/authentication/bearer.dart | 16 ++++++++++++---- lib/src/authentication/credential.dart | 17 +++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index eaae40bea..4b5efabe5 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -12,15 +12,23 @@ class BearerCredential extends Credential { static const String kind = 'Bearer'; /// Deserializes [map] into [BearerCredential]. - static BearerCredential fromJson(Map map) => - BearerCredential(map['token'] as String); + static BearerCredential fromJson(Map json) { + if (json['kind'] != kind) { + throw FormatException( + 'Token kind is not compatible with BearerCredential.'); + } + if (json['token'] is! String) { + throw FormatException('Failed to parse bearer token from json.'); + } + return BearerCredential(json['token'] as String); + } /// Bearer token final String token; @override - Map toJson() => - {'kind': kind, 'token': token}; + Map toJson() => + {'kind': kind, 'token': token}; @override Future getAuthorizationHeaderValue() => Future.value('Bearer $token'); diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 4411abdd9..38629a448 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -16,17 +16,22 @@ final Map _credentialKinds = { abstract class Credential { /// Parse Credential details from given [map]. If parsing fails this method /// will return null. - static Credential? fromJson(Map map) { + static Credential fromJson(Map map) { final credentialKind = map['kind'] as String?; + if (credentialKind?.isNotEmpty != true) { + throw FormatException('Credential kind is not provided.'); + } - return credentialKind?.isNotEmpty == true && - _credentialKinds.containsKey(credentialKind) - ? _credentialKinds[credentialKind]!(map) - : null; + if (_credentialKinds.containsKey(credentialKind)) { + return _credentialKinds[credentialKind]!(map); + } else { + throw FormatException( + 'Credential kind "$credentialKind" is not supported.'); + } } /// Converts this instance into Json map. - Map toJson(); + Map toJson(); /// Returns future that resolves "Authorization" header value used for /// authenticating. From 4ee285a92e6d52968d8327ff0016e17f5955e9b0 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 11:39:12 +0400 Subject: [PATCH 36/77] Annotate BearerCredential with @sealed --- lib/src/authentication/bearer.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index 4b5efabe5..a6832d76a 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -2,10 +2,13 @@ // 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. +import 'package:meta/meta.dart'; + import 'credential.dart'; /// Bearer credential type that simply puts authorization header formatted as /// `Bearer $token` to request.header. +@sealed class BearerCredential extends Credential { BearerCredential(this.token); From 5d57abb449f113603b242e13f64c24851f966728 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 12:00:37 +0400 Subject: [PATCH 37/77] doc, refac: Minor changes --- lib/src/authentication/client.dart | 17 ++++++++++++++--- lib/src/authentication/credential.dart | 4 ++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 7e7fe14a4..516d03624 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -13,9 +13,10 @@ import '../system_cache.dart'; import 'credential.dart'; import 'credential_store.dart'; -/// This client automatically modifies request to contain required credentials -/// in request. For example some credentials might add `Authentication` header -/// to request. +/// This client authenticates requests by injecting `Authentication` header to +/// requests. +/// +/// Requests to URLs not under [serverBaseUrl] will not be authenticated. class _AuthenticatedClient extends http.BaseClient { _AuthenticatedClient( this._inner, { @@ -24,7 +25,12 @@ class _AuthenticatedClient extends http.BaseClient { }); final http.BaseClient _inner; + + /// Authentication credentials used to generate `Authorization` header value. final Credential credential; + + /// Base URL of the pub repository server. Used to check whether or not the + /// request should be authenticated. final String serverBaseUrl; @override @@ -47,6 +53,11 @@ class _AuthenticatedClient extends http.BaseClient { void close() => _inner.close(); } +/// Invoke [fn] with a [http.Client] capable of authenticating against +/// [serverBaseUrl]. +/// +/// Importantly, requests to URLs not under [serverBaseUrl] will not be +/// authenticated. Future withAuthenticatedClient( SystemCache systemCache, String serverBaseUrl, diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index 38629a448..a58f37551 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -17,11 +17,11 @@ abstract class Credential { /// Parse Credential details from given [map]. If parsing fails this method /// will return null. static Credential fromJson(Map map) { - final credentialKind = map['kind'] as String?; - if (credentialKind?.isNotEmpty != true) { + if (map['kind'] is! String) { throw FormatException('Credential kind is not provided.'); } + final credentialKind = map['kind'] as String; if (_credentialKinds.containsKey(credentialKind)) { return _credentialKinds[credentialKind]!(map); } else { From 1cc1bd85b7b746e9d07ce32627124a6ab21fed40 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 14:12:45 +0400 Subject: [PATCH 38/77] Modified tokens.json format on auth spec doc --- doc/authentication-proposal.md | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md index 6ff703b0b..55e0fff2c 100644 --- a/doc/authentication-proposal.md +++ b/doc/authentication-proposal.md @@ -56,15 +56,24 @@ server (`PUB_HOSTED_URL`). ```json { - "https://myserver.com": { - "kind": "Basic", - "username": "bob", - "password": "password" - }, - "https://pub.example.com": { - "kind": "Bearer", - "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" - } + "version": "1.0", + "hosted": [ + { + "url": "https://myserver.com", + "credential": { + "kind": "Basic", + "username": "bob", + "password": "password" + } + }, + { + "url": "https://pub.example.com", + "credential": { + "kind": "Bearer", + "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" + } + } + ] } ``` From dbedbaab8f2951bd3149df9fa5e6870d9e0457e6 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 14:14:34 +0400 Subject: [PATCH 39/77] Modified authentication modelling scheme AuthenticationScheme will be used for storing credential and server details. --- lib/src/authentication/bearer.dart | 8 +- lib/src/authentication/client.dart | 33 +--- lib/src/authentication/credential.dart | 18 +- lib/src/authentication/credential_store.dart | 173 +++++++++++------- lib/src/authentication/scheme.dart | 78 ++++++++ lib/src/command.dart | 5 +- lib/src/command/lish.dart | 2 +- lib/src/command/login.dart | 2 +- lib/src/command/logout.dart | 2 +- lib/src/system_cache.dart | 7 +- test/authentication/login_to_server_test.dart | 8 +- .../logout_from_server_test.dart | 69 ++++++- 12 files changed, 280 insertions(+), 125 deletions(-) create mode 100644 lib/src/authentication/scheme.dart diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart index a6832d76a..f71a0d52a 100644 --- a/lib/src/authentication/bearer.dart +++ b/lib/src/authentication/bearer.dart @@ -9,13 +9,11 @@ import 'credential.dart'; /// Bearer credential type that simply puts authorization header formatted as /// `Bearer $token` to request.header. @sealed -class BearerCredential extends Credential { +class BearerCredential implements Credential { BearerCredential(this.token); - static const String kind = 'Bearer'; - /// Deserializes [map] into [BearerCredential]. - static BearerCredential fromJson(Map json) { + factory BearerCredential.fromJson(Map json) { if (json['kind'] != kind) { throw FormatException( 'Token kind is not compatible with BearerCredential.'); @@ -26,6 +24,8 @@ class BearerCredential extends Credential { return BearerCredential(json['token'] as String); } + static const String kind = 'Bearer'; + /// Bearer token final String token; diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 516d03624..1a1380645 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -10,28 +10,19 @@ import 'package:http/http.dart' as http; import '../http.dart'; import '../system_cache.dart'; -import 'credential.dart'; -import 'credential_store.dart'; +import 'scheme.dart'; /// This client authenticates requests by injecting `Authentication` header to /// requests. /// /// Requests to URLs not under [serverBaseUrl] will not be authenticated. class _AuthenticatedClient extends http.BaseClient { - _AuthenticatedClient( - this._inner, { - required this.credential, - required this.serverBaseUrl, - }); + _AuthenticatedClient(this._inner, this.scheme); final http.BaseClient _inner; - /// Authentication credentials used to generate `Authorization` header value. - final Credential credential; - - /// Base URL of the pub repository server. Used to check whether or not the - /// request should be authenticated. - final String serverBaseUrl; + /// Authentication scheme that could be used for authenticating requests. + final AuthenticationScheme scheme; @override Future send(http.BaseRequest request) async { @@ -42,9 +33,9 @@ class _AuthenticatedClient extends http.BaseClient { // to given serverBaseUrl. Otherwise credential leaks might ocurr when // archive_url hosted on 3rd party server that should not receive // credentials of the first party. - if (serverBaseUrlMatches(serverBaseUrl, request.url.toString())) { + if (scheme.canAuthenticate(request.url.toString())) { request.headers[HttpHeaders.authorizationHeader] = - await credential.getAuthorizationHeaderValue(); + await scheme.credential.getAuthorizationHeaderValue(); } return _inner.send(request); } @@ -63,15 +54,9 @@ Future withAuthenticatedClient( String serverBaseUrl, Future Function(http.Client) fn, ) async { - final store = CredentialStore(systemCache); - final credential = store.getCredential(serverBaseUrl); - final http.Client client = credential == null - ? httpClient - : _AuthenticatedClient( - httpClient, - serverBaseUrl: credential.first, - credential: credential.last, - ); + final scheme = systemCache.credentialStore.findScheme(serverBaseUrl); + final http.Client client = + scheme == null ? httpClient : _AuthenticatedClient(httpClient, scheme); try { return await fn(client); diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index a58f37551..977948002 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -6,27 +6,23 @@ import 'bearer.dart'; typedef CredentialDeserializer = Credential Function(Map); -final Map _credentialKinds = { - BearerCredential.kind: BearerCredential.fromJson, -}; - /// Credentials used to authenticate requests sent to auth - protected hosted /// pub repositories. This class is base class for different credential type /// implementations like [BearerCredential]. abstract class Credential { /// Parse Credential details from given [map]. If parsing fails this method /// will return null. - static Credential fromJson(Map map) { + factory Credential.fromJson(Map map) { if (map['kind'] is! String) { throw FormatException('Credential kind is not provided.'); } - final credentialKind = map['kind'] as String; - if (_credentialKinds.containsKey(credentialKind)) { - return _credentialKinds[credentialKind]!(map); - } else { - throw FormatException( - 'Credential kind "$credentialKind" is not supported.'); + final kind = map['kind'] as String; + switch (kind) { + case BearerCredential.kind: + return BearerCredential.fromJson(map); + default: + throw FormatException('Credential kind "$kind" is not supported.'); } } diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index a482d8451..5c58ecdd6 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -10,98 +10,129 @@ import 'package:path/path.dart' as path; import '../io.dart'; import '../log.dart' as log; -import '../system_cache.dart'; -import '../utils.dart'; import 'credential.dart'; +import 'scheme.dart'; class CredentialStore { - CredentialStore(this.cache); - - final SystemCache cache; - - /// Adds [credentials] for [serverBaseUrl] into store. - void addServer(String serverBaseUrl, Credential credentials) { - serverBaseUrl = serverBaseUrl.toLowerCase(); - // Make sure server name ends with a backslash. It's here to deny possible - // credential thief attach vectors where victim can add credential for - // server 'https://safesite.com' and attacker could steal credentials by - // requesting credentials for 'https://safesite.com.attacker.com', because - // URL matcher (_serverMatches method) matches credential keys with the - // beginning of the URL. - if (!serverBaseUrl.endsWith('/')) serverBaseUrl += '/'; - serverCredentials[serverBaseUrl] = credentials; - _save(); - } + CredentialStore(this.cacheRootDir); - /// Removes credentials for servers that [url] matches with. - void removeServer(String url) { - var modified = false; - // Iterating serverCredentials.keys.toList() because otherwise we'll get - // concurrent modification during iteration error. - for (final serverBaseUrl in serverCredentials.keys.toList()) { - if (serverBaseUrlMatches(serverBaseUrl, url)) { - log.message('Logging out of $serverBaseUrl.'); - serverCredentials.remove(serverBaseUrl); - modified = true; - } - } - if (modified) { - _save(); - } else { - log.message('No matching credential found for $url. Cannot log out.'); + /// Cache directory. + final String cacheRootDir; + + List? _schemes; + List get schemes => _schemes ??= _loadSchemes(); + + List _loadSchemes() { + final result = List.empty(growable: true); + final path = _tokensFile; + if (!fileExists(path)) { + return result; } - } - /// Returns pair of credential and server base url for server for - /// authenticating [url]. - Pair? getCredential(String url) { - for (final serverBaseUrl in serverCredentials.keys) { - if (serverBaseUrlMatches(serverBaseUrl, url)) { - return Pair(serverBaseUrl, serverCredentials[serverBaseUrl]); + try { + final json = jsonDecode(readTextFile(path)); + + if (json is! Map) { + throw FormatException('JSON contents is corrupted or not supported.'); } - } - } + if (json['version'] != '1.0') { + throw FormatException('Version is not supported.'); + } + + if (json.containsKey('hosted')) { + if (json['hosted'] is! List) { + throw FormatException( + 'tokens.json format is invalid or not supported.'); + } - /// Returns whether or not store has a credential for server that [url] - /// could be authenticated with. - bool hasCredential(String url) { - for (final serverBaseUrl in serverCredentials.keys) { - if (serverBaseUrlMatches(serverBaseUrl, url)) { - return true; + result.addAll((json['hosted'] as List) + .cast>() + .map(HostedAuthenticationScheme.fromJson)); } + } on FormatException catch (error, stackTrace) { + log.error('Failed to load tokens.json.', error, stackTrace); } - return false; + + return result; + } + + void _saveSchemes(List schemes) { + writeTextFile( + _tokensFile, + jsonEncode({ + 'version': '1.0', + 'hosted': schemes + .whereType() + .map((it) => it.toJson()) + .toList(), + })); } - void _save() { - _saveCredentials(serverCredentials); + /// Writes latest state of the store to disk. + void flush() { + if (_schemes == null) { + throw Exception('Schemes should be loaded before saving.'); + } + _saveSchemes(_schemes!); } - Map? _serverCredentials; - Map get serverCredentials => - _serverCredentials ??= _loadCredentials(); + /// Adds [scheme] into store. + void addScheme(AuthenticationScheme scheme) { + schemes.add(scheme); + flush(); + } - String get _tokensFile => path.join(cache.rootDir, 'tokens.json'); + /// Creates [HostedAuthenticationScheme] for [baseUrl] with [credential], then + /// adds it to store. + void addHostedScheme(String baseUrl, Credential credential) { + schemes.add(HostedAuthenticationScheme( + baseUrl: baseUrl, + credential: credential, + )); + flush(); + } - Map _loadCredentials() { - final path = _tokensFile; - if (!fileExists(path)) return {}; + /// Removes [HostedAuthenticationScheme] matching to [url] from store. + void removeMatchingHostedSchemes(String url) { + final schemesToRemove = + schemes.where((it) => it.canAuthenticate(url)).toList(); + if (schemesToRemove.isNotEmpty) { + for (final scheme in schemesToRemove) { + schemes.remove(scheme); + log.message('Logging out of ${scheme.baseUrl}.'); + } - final parsed = jsonDecode(readTextFile(path)) as Map; - final result = parsed - .map((key, value) => MapEntry(key, Credential.fromJson(value))) - ..removeWhere((key, value) => value == null); + flush(); + } else { + log.message('No matching credential found for $url. Cannot log out.'); + } + } - return result.cast(); + /// Returns matching authentication scheme to given [url] or returns `null` if + /// no matches found. + AuthenticationScheme? findScheme(String url) { + AuthenticationScheme? matchedScheme; + for (final scheme in schemes) { + if (scheme.canAuthenticate(url)) { + if (matchedScheme == null) { + matchedScheme = scheme; + } else { + log.warning( + 'Found multiple matching authentication schemes for url "$url". ' + 'First matching scheme will be used for authentication.', + ); + } + } + } } - void _saveCredentials(Map credentials) { - final path = _tokensFile; - writeTextFile( - path, - jsonEncode( - credentials.map((key, value) => MapEntry(key, value.toJson())))); + /// Returns whether or not store contains a scheme that could be used for + /// authenticating give [url]. + bool hasScheme(String url) { + return schemes.any((it) => it.canAuthenticate(url)); } + + String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); } bool serverBaseUrlMatches(String serverBaseUrl, String url) { diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart new file mode 100644 index 000000000..18dafa3a6 --- /dev/null +++ b/lib/src/authentication/scheme.dart @@ -0,0 +1,78 @@ +import 'credential.dart'; + +// TODO(themisir): Add authentication scopes. It'll be used to check whether or +// not the request should be authenticated. For example official pub.dev server +// scheme could define scopes as [AuthenticationScope.publish] which means it +// only requires authentication for publish command. + +/// Abstract interface for authentication scheme. It's used for validating +/// request URLs and contains [credential] which will be used for +/// authentication. +abstract class AuthenticationScheme { + /// Authentication credential which used to resolve `Authorization` header + /// value. + Credential get credential; + + /// This url used for identifying this authentication scheme in logs or + /// error messages. + String get baseUrl; + + /// Returns whether or not given [url] could be authenticated using + /// [credential]. + bool canAuthenticate(String url); + + /// Serializes this authentication scheme into json format. + Map toJson(); + + // TODO(themisir): This method will be used for prompting for user input + // to get credentials in future. + // + // For example OAuth2AuthenticationScheme could implement this method to + // launch http server and open authentication URL in browser. + // Future prompt(); +} + +/// Authentication scheme that used by +class HostedAuthenticationScheme implements AuthenticationScheme { + HostedAuthenticationScheme({ + required String baseUrl, + required this.credential, + }) : baseUrl = _normalizeUrl(baseUrl).toLowerCase(); + + /// Deserializes [HostedAuthenticationScheme] from given json [map]. + static HostedAuthenticationScheme fromJson(Map map) { + if (map['url'] is! String) { + throw FormatException( + 'Server base URL for authentication scheme is not provided.'); + } + if (map['credential'] is! Map) { + throw FormatException( + 'Authentication scheme does not contains a valid credential.'); + } + return HostedAuthenticationScheme( + baseUrl: map['url'] as String, + credential: + Credential.fromJson(map['credential'] as Map), + ); + } + + static String _normalizeUrl(String url) { + return url.endsWith('/') ? url : '$url/'; + } + + @override + final Credential credential; + + /// Hosted pub server base url. + @override + final String baseUrl; + + @override + Map toJson() => + {'url': baseUrl, 'credential': credential.toJson()}; + + @override + bool canAuthenticate(String url) { + return _normalizeUrl(url).startsWith(baseUrl.toLowerCase()); + } +} diff --git a/lib/src/command.dart b/lib/src/command.dart index a3488de77..8e874e45a 100644 --- a/lib/src/command.dart +++ b/lib/src/command.dart @@ -56,10 +56,7 @@ abstract class PubCommand extends Command { GlobalPackages _globals; - CredentialStore get credentialStore => - _credentialStore ?? CredentialStore(cache); - - CredentialStore _credentialStore; + CredentialStore get credentialStore => cache.credentialStore; /// Gets the [Entrypoint] package for the current working directory. /// diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 51cede03d..dd9a37884 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -125,7 +125,7 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (credentialStore.hasCredential(server.toString())) { + if (credentialStore.hasScheme(server.toString())) { // If there's a saved credential for the server, publish using // httpClient which should authenticate with the server using // AuthenticationClient. diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 392603631..8f597907c 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -59,7 +59,7 @@ class LoginCommand extends PubCommand { } final _token = tokenStdin ? await readLine() : token; - credentialStore.addServer(server, BearerCredential(_token)); + credentialStore.addHostedScheme(server, BearerCredential(_token)); log.message('You are now logged in to $server using bearer token.'); } diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 8ede07edb..acc279c99 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -30,7 +30,7 @@ class LogoutCommand extends PubCommand { if (server == null) { oauth2.logout(cache); } else { - credentialStore.removeServer(server); + credentialStore.removeMatchingHostedSchemes(server); } } } diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart index d9b5a748a..9cbcb4a72 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; +import 'authentication/credential_store.dart'; import 'io.dart'; import 'io.dart' as io show createTempDir; import 'log.dart' as log; @@ -79,12 +80,16 @@ class SystemCache { /// The default source bound to this cache. BoundSource get defaultSource => source(sources[null]); + /// The default credential store. + final CredentialStore credentialStore; + /// Creates a system cache and registers all sources in [sources]. /// /// If [isOffline] is `true`, then the offline hosted source will be used. /// Defaults to `false`. SystemCache({String rootDir, bool isOffline = false}) - : rootDir = rootDir ?? SystemCache.defaultDir { + : rootDir = rootDir ?? SystemCache.defaultDir, + credentialStore = CredentialStore(rootDir ?? SystemCache.defaultDir) { for (var source in sources.all) { if (source is HostedSource) { _boundSources[source] = source.bind(this, isOffline: isOffline); diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart index 092c41fac..bba05c4a9 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/authentication/login_to_server_test.dart @@ -27,7 +27,13 @@ void main() { ); await d.tokensFile({ - 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://server.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] }).validate(); }); diff --git a/test/authentication/logout_from_server_test.dart b/test/authentication/logout_from_server_test.dart index 1f0d5e165..204cad4ac 100644 --- a/test/authentication/logout_from_server_test.dart +++ b/test/authentication/logout_from_server_test.dart @@ -10,9 +10,15 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; void main() { - test('with an matching server url, removes the entry.', () async { + test('with one matching scheme, removes the entry.', () async { await d.tokensFile({ - 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://server.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] }).create(); await runPub( @@ -20,12 +26,57 @@ void main() { output: contains('Logging out of http://server.demo/.'), ); - await d.tokensFile({}).validate(); + await d.tokensFile({'version': '1.0', 'hosted': []}).validate(); }); - test('without an matching server url, does nothing.', () async { + test('with multiple matching schemes, removes all matching entries.', + () async { await d.tokensFile({ - 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://server.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + }, + { + 'url': 'http://server.demo/sub', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + }, + { + 'url': 'http://another-.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).create(); + + await runPub( + args: ['logout', '--server', 'http://server.demo/sub'], + output: allOf( + contains('Logging out of http://server.demo/.'), + contains('Logging out of http://server.demo/sub/.'), + ), + ); + + await d.tokensFile({ + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://another-.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).validate(); + }); + + test('without an matching schemes, does nothing.', () async { + await d.tokensFile({ + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://server.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] }).create(); await runPub( @@ -35,7 +86,13 @@ void main() { ); await d.tokensFile({ - 'http://server.demo/': {'kind': 'Bearer', 'token': 'auth-token'} + 'version': '1.0', + 'hosted': [ + { + 'url': 'http://server.demo/', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] }).validate(); }); } From ec703f6df170487973847882158ccf3aac95a378 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 1 Jun 2021 14:28:31 +0400 Subject: [PATCH 40/77] Refactoring credential_store.dart --- lib/src/authentication/credential_store.dart | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 5c58ecdd6..3a1ca7a9f 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -13,6 +13,7 @@ import '../log.dart' as log; import 'credential.dart'; import 'scheme.dart'; +/// Stores and manages authentication credentials. class CredentialStore { CredentialStore(this.cacheRootDir); @@ -20,8 +21,15 @@ class CredentialStore { final String cacheRootDir; List? _schemes; + + /// List of saved authentication schemes. + /// + /// Modifying this field will not write changes to the disk. You have to call + /// [flush] to save changes. List get schemes => _schemes ??= _loadSchemes(); + /// Reads "tokens.json" and parses / deserializes it into list of + /// [AuthenticationScheme]. List _loadSchemes() { final result = List.empty(growable: true); final path = _tokensFile; @@ -56,6 +64,7 @@ class CredentialStore { return result; } + /// Saves [schemes] into "tokens.json". void _saveSchemes(List schemes) { writeTextFile( _tokensFile, @@ -132,11 +141,6 @@ class CredentialStore { return schemes.any((it) => it.canAuthenticate(url)); } + /// Full path to the "tokens.json" file. String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); } - -bool serverBaseUrlMatches(String serverBaseUrl, String url) { - if (!serverBaseUrl.endsWith('/')) serverBaseUrl += '/'; - if (!url.endsWith('/')) url += '/'; - return url.startsWith(serverBaseUrl.toLowerCase()); -} From f16fbbef6ecdf938acfa40de1249e0298b1109de Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 5 Jun 2021 12:05:46 +0400 Subject: [PATCH 41/77] Request authenticated client using server base url instead of full request url --- lib/src/source/hosted.dart | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 519bbbcfa..193b23970 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -211,8 +211,9 @@ class BoundHostedSource extends CachedSource { Future> _fetchVersionsNoPrefetching( PackageRef ref) async { - var url = _makeUrl( - ref.description, (server, package) => '$server/api/packages/$package'); + final parsedDescription = source._parseDescription(ref.description); + final url = _makeUrl(parsedDescription, + (server, package) => '$server/api/packages/$package'); log.io('Get versions from $url.'); String bodyText; @@ -223,7 +224,7 @@ class BoundHostedSource extends CachedSource { // requires resolution of: https://github.com/dart-lang/sdk/issues/22265. bodyText = await withAuthenticatedClient( systemCache, - url.toString(), + parsedDescription.last, (client) => client.read(url, headers: pubApiHeaders), ); body = jsonDecode(bodyText); @@ -401,11 +402,10 @@ class BoundHostedSource extends CachedSource { /// converts that to a Uri given [pattern]. /// /// Ensures the package name is properly URL encoded. - Uri _makeUrl( - description, String Function(String server, String package) pattern) { - var parsed = source._parseDescription(description); - var server = parsed.last; - var package = Uri.encodeComponent(parsed.first); + Uri _makeUrl(Pair parsedDescription, + String Function(String server, String package) pattern) { + var server = parsedDescription.last; + var package = Uri.encodeComponent(parsedDescription.first); return Uri.parse(pattern(server, package)); } @@ -414,8 +414,9 @@ class BoundHostedSource extends CachedSource { @override Future describeUncached(PackageId id) async { final versions = await _scheduler.schedule(id.toRef()); - final url = _makeUrl( - id.description, (server, package) => '$server/api/packages/$package'); + final parsedDescription = source._parseDescription(id.description); + final url = _makeUrl(parsedDescription, + (server, package) => '$server/api/packages/$package'); return versions[id]?.pubspec ?? (throw PackageNotFoundException('Could not find package $id at $url')); } From 7dcfd9776ee7e02dd6da76a3ca45b7e0c9626bad Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 5 Jun 2021 12:19:48 +0400 Subject: [PATCH 42/77] Exact URL matching for finding schemas --- lib/src/authentication/credential_store.dart | 12 ++++++------ lib/src/authentication/scheme.dart | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 3a1ca7a9f..c8f5507e8 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -117,17 +117,17 @@ class CredentialStore { } } - /// Returns matching authentication scheme to given [url] or returns `null` if - /// no matches found. + /// Returns [AuthenticationScheme] for given server [url], or null if no + /// scheme were found. AuthenticationScheme? findScheme(String url) { AuthenticationScheme? matchedScheme; for (final scheme in schemes) { - if (scheme.canAuthenticate(url)) { + if (scheme.baseUrl == url) { if (matchedScheme == null) { matchedScheme = scheme; } else { log.warning( - 'Found multiple matching authentication schemes for url "$url". ' + 'Found multiple matching authentication schemes for "$url". ' 'First matching scheme will be used for authentication.', ); } @@ -136,9 +136,9 @@ class CredentialStore { } /// Returns whether or not store contains a scheme that could be used for - /// authenticating give [url]. + /// authenticating given [url]. bool hasScheme(String url) { - return schemes.any((it) => it.canAuthenticate(url)); + return schemes.any((it) => it.baseUrl == url); } /// Full path to the "tokens.json" file. diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart index 18dafa3a6..89e596a46 100644 --- a/lib/src/authentication/scheme.dart +++ b/lib/src/authentication/scheme.dart @@ -13,8 +13,7 @@ abstract class AuthenticationScheme { /// value. Credential get credential; - /// This url used for identifying this authentication scheme in logs or - /// error messages. + /// Server base URL which this authentication scheme could authenticate. String get baseUrl; /// Returns whether or not given [url] could be authenticated using From d6399de2339d1af8738bcf7dec0938b717eda837 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sat, 5 Jun 2021 12:21:30 +0400 Subject: [PATCH 43/77] Modify _normalizeUrl method --- lib/src/authentication/scheme.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart index 89e596a46..56acb7118 100644 --- a/lib/src/authentication/scheme.dart +++ b/lib/src/authentication/scheme.dart @@ -36,7 +36,7 @@ class HostedAuthenticationScheme implements AuthenticationScheme { HostedAuthenticationScheme({ required String baseUrl, required this.credential, - }) : baseUrl = _normalizeUrl(baseUrl).toLowerCase(); + }) : baseUrl = _normalizeUrl(baseUrl); /// Deserializes [HostedAuthenticationScheme] from given json [map]. static HostedAuthenticationScheme fromJson(Map map) { @@ -56,7 +56,7 @@ class HostedAuthenticationScheme implements AuthenticationScheme { } static String _normalizeUrl(String url) { - return url.endsWith('/') ? url : '$url/'; + return (url.endsWith('/') ? url : '$url/').toLowerCase(); } @override @@ -72,6 +72,6 @@ class HostedAuthenticationScheme implements AuthenticationScheme { @override bool canAuthenticate(String url) { - return _normalizeUrl(url).startsWith(baseUrl.toLowerCase()); + return _normalizeUrl(url).startsWith(baseUrl); } } From 72973fc2cfed4b38f56595d9926b1a3b1210b5d4 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sun, 20 Jun 2021 21:08:18 +0400 Subject: [PATCH 44/77] URL normalization, login and logout commands are modified --- lib/src/authentication/credential_store.dart | 14 +++++- lib/src/authentication/scheme.dart | 6 +-- lib/src/command/login.dart | 45 ++++++++++++++------ lib/src/command/logout.dart | 13 +++++- lib/src/io.dart | 7 ++- 5 files changed, 66 insertions(+), 19 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index c8f5507e8..9437b4e08 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -122,7 +122,7 @@ class CredentialStore { AuthenticationScheme? findScheme(String url) { AuthenticationScheme? matchedScheme; for (final scheme in schemes) { - if (scheme.baseUrl == url) { + if (_urlMatches(scheme.baseUrl, url)) { if (matchedScheme == null) { matchedScheme = scheme; } else { @@ -141,6 +141,18 @@ class CredentialStore { return schemes.any((it) => it.baseUrl == url); } + /// Deletes tokens.json file from the disk. + void deleteTokensFile() { + deleteEntry(_tokensFile); + log.message('tokens.json is deleted.'); + } + /// Full path to the "tokens.json" file. String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); } + +bool _urlMatches(String u1, String u2) { + if (!u1.endsWith('/')) u1 += '/'; + if (!u2.endsWith('/')) u2 += '/'; + return u1 == u2; +} diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart index 56acb7118..7ab409c00 100644 --- a/lib/src/authentication/scheme.dart +++ b/lib/src/authentication/scheme.dart @@ -34,9 +34,9 @@ abstract class AuthenticationScheme { /// Authentication scheme that used by class HostedAuthenticationScheme implements AuthenticationScheme { HostedAuthenticationScheme({ - required String baseUrl, + required this.baseUrl, required this.credential, - }) : baseUrl = _normalizeUrl(baseUrl); + }); /// Deserializes [HostedAuthenticationScheme] from given json [map]. static HostedAuthenticationScheme fromJson(Map map) { @@ -72,6 +72,6 @@ class HostedAuthenticationScheme implements AuthenticationScheme { @override bool canAuthenticate(String url) { - return _normalizeUrl(url).startsWith(baseUrl); + return _normalizeUrl(url).startsWith(_normalizeUrl(baseUrl)); } } diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 8f597907c..426a75083 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -23,44 +23,63 @@ class LoginCommand extends PubCommand { @override String get invocation => 'pub login'; - String get token => argResults['token']; - bool get tokenStdin => argResults['token-stdin']; String get server => argResults['server']; + bool get list => argResults['list']; LoginCommand() { argParser.addOption('server', help: 'The package server to which needs to be authenticated.'); - argParser.addOption('token', help: 'Authorization token for the server'); - - argParser.addFlag('token-stdin', - help: 'Read authorization token from stdin stream'); + argParser.addFlag('list', + help: 'Displays list of currently logged in hosted pub servers', + defaultsTo: false); } @override Future runProtected() async { - if (server == null) { + if (list) { + await _listCredentials(); + } else if (server == null) { await _loginToPubDev(); } else { if (Uri.tryParse(server) == null) { usageException('Invalid or malformed server URL provided.'); } - if (token?.isNotEmpty != true && !tokenStdin) { - usageException('Must specify a token.'); - } await _loginToServer(server); } } + Future _listCredentials() async { + log.message('Found ${cache.credentialStore.schemes.length} entries.'); + for (final scheme in cache.credentialStore.schemes) { + log.message(scheme.baseUrl); + } + } + Future _loginToServer(String server) async { + // TODO(themisir): Replace this line with validateAndNormalizeHostedUrl from + // source/hosted.dart when dart-lang/pub#3030 is merged. if (Uri.tryParse(server) == null || !server.startsWith(RegExp(r'https?:\/\/'))) { usageException('Invalid or malformed server URL provided.'); } - final _token = tokenStdin ? await readLine() : token; - credentialStore.addHostedScheme(server, BearerCredential(_token)); - log.message('You are now logged in to $server using bearer token.'); + try { + final token = await readLine('Please enter bearer token') + .timeout(const Duration(minutes: 5)); + if (token.isEmpty) { + usageException('Token is not provided.'); + } + + credentialStore.addHostedScheme(server, BearerCredential(token)); + log.message('You are now logged in to $server using bearer token.'); + } on TimeoutException catch (error, stackTrace) { + log.error( + 'Timeout error. Token is not provided within ' + '${error.duration.inSeconds} seconds.', + error, + stackTrace); + } } Future _loginToPubDev() async { diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index acc279c99..a122821e6 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -7,6 +7,7 @@ import 'dart:async'; import '../command.dart'; +import '../io.dart'; import '../oauth2.dart' as oauth2; /// Handles the `logout` pub command. @@ -19,15 +20,25 @@ class LogoutCommand extends PubCommand { bool get takesArguments => true; String get server => argResults['server']; + bool get clear => argResults['clear']; LogoutCommand() { argParser.addOption('server', help: 'The package server to which needs to be authenticated.'); + + argParser.addFlag('clear', + help: 'Removes all of previously saved credentials for hosted pub ' + 'servers', + defaultsTo: false); } @override Future runProtected() async { - if (server == null) { + if (clear) { + if (await confirm('Are you sure you want to remove all credentials')) { + credentialStore.deleteTokensFile(); + } + } else if (server == null) { oauth2.logout(cache); } else { credentialStore.removeMatchingHostedSchemes(server); diff --git a/lib/src/io.dart b/lib/src/io.dart index f8a670689..3b2fc70a7 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -569,7 +569,12 @@ Future confirm(String message) { } /// Reads a line from stdin stream. -Future readLine() { +Future readLine(String message) { + if (runningFromTest) { + log.message('$message:'); + } else { + stdout.write('$message: '); + } return _stdinLines.first; } From 99bccd88772c4e8bea5472b2c830c8e3d6faa418 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Sun, 20 Jun 2021 21:33:41 +0400 Subject: [PATCH 45/77] Modified scheme URL matching --- lib/src/authentication/credential_store.dart | 8 +------ lib/src/authentication/scheme.dart | 4 ++-- test/authentication/login_to_server_test.dart | 21 ++----------------- test/test_pub.dart | 11 +++++++++- 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart index 9437b4e08..3fcfabff3 100644 --- a/lib/src/authentication/credential_store.dart +++ b/lib/src/authentication/credential_store.dart @@ -122,7 +122,7 @@ class CredentialStore { AuthenticationScheme? findScheme(String url) { AuthenticationScheme? matchedScheme; for (final scheme in schemes) { - if (_urlMatches(scheme.baseUrl, url)) { + if (scheme.canAuthenticate(url)) { if (matchedScheme == null) { matchedScheme = scheme; } else { @@ -150,9 +150,3 @@ class CredentialStore { /// Full path to the "tokens.json" file. String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); } - -bool _urlMatches(String u1, String u2) { - if (!u1.endsWith('/')) u1 += '/'; - if (!u2.endsWith('/')) u2 += '/'; - return u1 == u2; -} diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart index 7ab409c00..912cb770f 100644 --- a/lib/src/authentication/scheme.dart +++ b/lib/src/authentication/scheme.dart @@ -34,9 +34,9 @@ abstract class AuthenticationScheme { /// Authentication scheme that used by class HostedAuthenticationScheme implements AuthenticationScheme { HostedAuthenticationScheme({ - required this.baseUrl, + required String baseUrl, required this.credential, - }); + }) : baseUrl = _normalizeUrl(baseUrl); /// Deserializes [HostedAuthenticationScheme] from given json [map]. static HostedAuthenticationScheme fromJson(Map map) { diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart index bba05c4a9..a808028d2 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/authentication/login_to_server_test.dart @@ -19,9 +19,8 @@ void main() { 'login', '--server', 'http://server.demo', - '--token', - 'auth-token', ], + input: ['auth-token'], output: contains( 'You are now logged in to http://server.demo using bearer token.'), ); @@ -44,28 +43,12 @@ void main() { 'login', '--server', 'http:;://invalid-url,.com', - '--token', - 'auth-token', ], + input: ['auth-token'], error: contains('Invalid or malformed server URL provided.'), exitCode: exit_codes.USAGE, ); await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); }); - - test('without token returns error', () async { - await d.dir(cachePath).create(); - await runPub( - args: [ - 'login', - '--server', - 'http://server.demo', - ], - error: contains('Must specify a token.'), - exitCode: exit_codes.USAGE, - ); - - await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); - }); } diff --git a/test/test_pub.dart b/test/test_pub.dart index c89f86594..eb0c697a9 100644 --- a/test/test_pub.dart +++ b/test/test_pub.dart @@ -318,6 +318,8 @@ void symlinkInSandbox(String target, String symlink) { /// /// [output], [error], and [silent] can be [String]s, [RegExp]s, or [Matcher]s. /// +/// If [input] is given, writes given lines into process stdin stream. +/// /// If [outputJson] is given, validates that pub outputs stringified JSON /// matching that object, which can be a literal JSON object or any other /// [Matcher]. @@ -332,12 +334,19 @@ Future runPub( silent, int exitCode = exit_codes.SUCCESS, String workingDirectory, - Map environment}) async { + Map environment, + List input}) async { // Cannot pass both output and outputJson. assert(output == null || outputJson == null); var pub = await startPub( args: args, workingDirectory: workingDirectory, environment: environment); + + if (input != null) { + input.forEach(pub.stdin.writeln); + await pub.stdin.flush(); + } + await pub.shouldExit(exitCode); var actualOutput = (await pub.stdoutStream().toList()).join('\n'); From 951e5c3a357619ea8f9c424b1e8335c8c57af73c Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 19 Aug 2021 20:16:49 +0400 Subject: [PATCH 46/77] Remove credential and auth schema abstractions and replace them with Token --- lib/src/authentication/bearer.dart | 38 ----- lib/src/authentication/client.dart | 12 +- lib/src/authentication/credential.dart | 35 ----- lib/src/authentication/credential_store.dart | 152 ------------------- lib/src/authentication/scheme.dart | 77 ---------- lib/src/authentication/token.dart | 75 +++++++++ lib/src/authentication/token_store.dart | 140 +++++++++++++++++ lib/src/command.dart | 4 +- lib/src/command/lish.dart | 2 +- lib/src/command/login.dart | 10 +- lib/src/command/logout.dart | 4 +- lib/src/system_cache.dart | 6 +- 12 files changed, 234 insertions(+), 321 deletions(-) delete mode 100644 lib/src/authentication/bearer.dart delete mode 100644 lib/src/authentication/credential.dart delete mode 100644 lib/src/authentication/credential_store.dart delete mode 100644 lib/src/authentication/scheme.dart create mode 100644 lib/src/authentication/token.dart create mode 100644 lib/src/authentication/token_store.dart diff --git a/lib/src/authentication/bearer.dart b/lib/src/authentication/bearer.dart deleted file mode 100644 index f71a0d52a..000000000 --- a/lib/src/authentication/bearer.dart +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2021, 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. - -import 'package:meta/meta.dart'; - -import 'credential.dart'; - -/// Bearer credential type that simply puts authorization header formatted as -/// `Bearer $token` to request.header. -@sealed -class BearerCredential implements Credential { - BearerCredential(this.token); - - /// Deserializes [map] into [BearerCredential]. - factory BearerCredential.fromJson(Map json) { - if (json['kind'] != kind) { - throw FormatException( - 'Token kind is not compatible with BearerCredential.'); - } - if (json['token'] is! String) { - throw FormatException('Failed to parse bearer token from json.'); - } - return BearerCredential(json['token'] as String); - } - - static const String kind = 'Bearer'; - - /// Bearer token - final String token; - - @override - Map toJson() => - {'kind': kind, 'token': token}; - - @override - Future getAuthorizationHeaderValue() => Future.value('Bearer $token'); -} diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 1a1380645..bb240a729 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -10,19 +10,19 @@ import 'package:http/http.dart' as http; import '../http.dart'; import '../system_cache.dart'; -import 'scheme.dart'; +import 'token.dart'; /// This client authenticates requests by injecting `Authentication` header to /// requests. /// /// Requests to URLs not under [serverBaseUrl] will not be authenticated. class _AuthenticatedClient extends http.BaseClient { - _AuthenticatedClient(this._inner, this.scheme); + _AuthenticatedClient(this._inner, this.token); final http.BaseClient _inner; /// Authentication scheme that could be used for authenticating requests. - final AuthenticationScheme scheme; + final Token token; @override Future send(http.BaseRequest request) async { @@ -33,9 +33,9 @@ class _AuthenticatedClient extends http.BaseClient { // to given serverBaseUrl. Otherwise credential leaks might ocurr when // archive_url hosted on 3rd party server that should not receive // credentials of the first party. - if (scheme.canAuthenticate(request.url.toString())) { + if (token.canAuthenticate(request.url.toString())) { request.headers[HttpHeaders.authorizationHeader] = - await scheme.credential.getAuthorizationHeaderValue(); + await token.getAuthorizationHeaderValue(); } return _inner.send(request); } @@ -54,7 +54,7 @@ Future withAuthenticatedClient( String serverBaseUrl, Future Function(http.Client) fn, ) async { - final scheme = systemCache.credentialStore.findScheme(serverBaseUrl); + final scheme = systemCache.tokenStore.findToken(serverBaseUrl); final http.Client client = scheme == null ? httpClient : _AuthenticatedClient(httpClient, scheme); diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart deleted file mode 100644 index 977948002..000000000 --- a/lib/src/authentication/credential.dart +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) 2021, 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. - -import 'bearer.dart'; - -typedef CredentialDeserializer = Credential Function(Map); - -/// Credentials used to authenticate requests sent to auth - protected hosted -/// pub repositories. This class is base class for different credential type -/// implementations like [BearerCredential]. -abstract class Credential { - /// Parse Credential details from given [map]. If parsing fails this method - /// will return null. - factory Credential.fromJson(Map map) { - if (map['kind'] is! String) { - throw FormatException('Credential kind is not provided.'); - } - - final kind = map['kind'] as String; - switch (kind) { - case BearerCredential.kind: - return BearerCredential.fromJson(map); - default: - throw FormatException('Credential kind "$kind" is not supported.'); - } - } - - /// Converts this instance into Json map. - Map toJson(); - - /// Returns future that resolves "Authorization" header value used for - /// authenticating. - Future getAuthorizationHeaderValue(); -} diff --git a/lib/src/authentication/credential_store.dart b/lib/src/authentication/credential_store.dart deleted file mode 100644 index 3fcfabff3..000000000 --- a/lib/src/authentication/credential_store.dart +++ /dev/null @@ -1,152 +0,0 @@ -// Copyright (c) 2021, 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. - -// ignore_for_file: import_of_legacy_library_into_null_safe - -import 'dart:convert'; - -import 'package:path/path.dart' as path; - -import '../io.dart'; -import '../log.dart' as log; -import 'credential.dart'; -import 'scheme.dart'; - -/// Stores and manages authentication credentials. -class CredentialStore { - CredentialStore(this.cacheRootDir); - - /// Cache directory. - final String cacheRootDir; - - List? _schemes; - - /// List of saved authentication schemes. - /// - /// Modifying this field will not write changes to the disk. You have to call - /// [flush] to save changes. - List get schemes => _schemes ??= _loadSchemes(); - - /// Reads "tokens.json" and parses / deserializes it into list of - /// [AuthenticationScheme]. - List _loadSchemes() { - final result = List.empty(growable: true); - final path = _tokensFile; - if (!fileExists(path)) { - return result; - } - - try { - final json = jsonDecode(readTextFile(path)); - - if (json is! Map) { - throw FormatException('JSON contents is corrupted or not supported.'); - } - if (json['version'] != '1.0') { - throw FormatException('Version is not supported.'); - } - - if (json.containsKey('hosted')) { - if (json['hosted'] is! List) { - throw FormatException( - 'tokens.json format is invalid or not supported.'); - } - - result.addAll((json['hosted'] as List) - .cast>() - .map(HostedAuthenticationScheme.fromJson)); - } - } on FormatException catch (error, stackTrace) { - log.error('Failed to load tokens.json.', error, stackTrace); - } - - return result; - } - - /// Saves [schemes] into "tokens.json". - void _saveSchemes(List schemes) { - writeTextFile( - _tokensFile, - jsonEncode({ - 'version': '1.0', - 'hosted': schemes - .whereType() - .map((it) => it.toJson()) - .toList(), - })); - } - - /// Writes latest state of the store to disk. - void flush() { - if (_schemes == null) { - throw Exception('Schemes should be loaded before saving.'); - } - _saveSchemes(_schemes!); - } - - /// Adds [scheme] into store. - void addScheme(AuthenticationScheme scheme) { - schemes.add(scheme); - flush(); - } - - /// Creates [HostedAuthenticationScheme] for [baseUrl] with [credential], then - /// adds it to store. - void addHostedScheme(String baseUrl, Credential credential) { - schemes.add(HostedAuthenticationScheme( - baseUrl: baseUrl, - credential: credential, - )); - flush(); - } - - /// Removes [HostedAuthenticationScheme] matching to [url] from store. - void removeMatchingHostedSchemes(String url) { - final schemesToRemove = - schemes.where((it) => it.canAuthenticate(url)).toList(); - if (schemesToRemove.isNotEmpty) { - for (final scheme in schemesToRemove) { - schemes.remove(scheme); - log.message('Logging out of ${scheme.baseUrl}.'); - } - - flush(); - } else { - log.message('No matching credential found for $url. Cannot log out.'); - } - } - - /// Returns [AuthenticationScheme] for given server [url], or null if no - /// scheme were found. - AuthenticationScheme? findScheme(String url) { - AuthenticationScheme? matchedScheme; - for (final scheme in schemes) { - if (scheme.canAuthenticate(url)) { - if (matchedScheme == null) { - matchedScheme = scheme; - } else { - log.warning( - 'Found multiple matching authentication schemes for "$url". ' - 'First matching scheme will be used for authentication.', - ); - } - } - } - } - - /// Returns whether or not store contains a scheme that could be used for - /// authenticating given [url]. - bool hasScheme(String url) { - return schemes.any((it) => it.baseUrl == url); - } - - /// Deletes tokens.json file from the disk. - void deleteTokensFile() { - deleteEntry(_tokensFile); - log.message('tokens.json is deleted.'); - } - - /// Full path to the "tokens.json" file. - String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); -} diff --git a/lib/src/authentication/scheme.dart b/lib/src/authentication/scheme.dart deleted file mode 100644 index 912cb770f..000000000 --- a/lib/src/authentication/scheme.dart +++ /dev/null @@ -1,77 +0,0 @@ -import 'credential.dart'; - -// TODO(themisir): Add authentication scopes. It'll be used to check whether or -// not the request should be authenticated. For example official pub.dev server -// scheme could define scopes as [AuthenticationScope.publish] which means it -// only requires authentication for publish command. - -/// Abstract interface for authentication scheme. It's used for validating -/// request URLs and contains [credential] which will be used for -/// authentication. -abstract class AuthenticationScheme { - /// Authentication credential which used to resolve `Authorization` header - /// value. - Credential get credential; - - /// Server base URL which this authentication scheme could authenticate. - String get baseUrl; - - /// Returns whether or not given [url] could be authenticated using - /// [credential]. - bool canAuthenticate(String url); - - /// Serializes this authentication scheme into json format. - Map toJson(); - - // TODO(themisir): This method will be used for prompting for user input - // to get credentials in future. - // - // For example OAuth2AuthenticationScheme could implement this method to - // launch http server and open authentication URL in browser. - // Future prompt(); -} - -/// Authentication scheme that used by -class HostedAuthenticationScheme implements AuthenticationScheme { - HostedAuthenticationScheme({ - required String baseUrl, - required this.credential, - }) : baseUrl = _normalizeUrl(baseUrl); - - /// Deserializes [HostedAuthenticationScheme] from given json [map]. - static HostedAuthenticationScheme fromJson(Map map) { - if (map['url'] is! String) { - throw FormatException( - 'Server base URL for authentication scheme is not provided.'); - } - if (map['credential'] is! Map) { - throw FormatException( - 'Authentication scheme does not contains a valid credential.'); - } - return HostedAuthenticationScheme( - baseUrl: map['url'] as String, - credential: - Credential.fromJson(map['credential'] as Map), - ); - } - - static String _normalizeUrl(String url) { - return (url.endsWith('/') ? url : '$url/').toLowerCase(); - } - - @override - final Credential credential; - - /// Hosted pub server base url. - @override - final String baseUrl; - - @override - Map toJson() => - {'url': baseUrl, 'credential': credential.toJson()}; - - @override - bool canAuthenticate(String url) { - return _normalizeUrl(url).startsWith(_normalizeUrl(baseUrl)); - } -} diff --git a/lib/src/authentication/token.dart b/lib/src/authentication/token.dart new file mode 100644 index 000000000..ffe332ef0 --- /dev/null +++ b/lib/src/authentication/token.dart @@ -0,0 +1,75 @@ +class Token { + const Token({required this.url, required this.kind, required this.token}); + + const Token.bearer(this.url, this.token) : kind = 'Bearer'; + + /// Deserialize [json] into [Token] type. + factory Token.fromJson(Map json) { + if (json['url'] is! String) { + throw FormatException('Url is not provided for the token'); + } + + if (json['credential'] is! Map) { + throw FormatException('Credential is not provided for the token'); + } + + final kindValue = json['credential']['kind']; + + if (kindValue is! String) { + throw FormatException('Credential kind is not provided for the token'); + } + + if (!const ['Bearer'].contains(kindValue)) { + throw FormatException('$kindValue is unsupported credential kind value'); + } + + if (json['credential']['token'] is! String) { + throw FormatException('Credential token is not provided for the token'); + } + + return Token( + url: json['url'] as String, + kind: kindValue, + token: json['credential']['token'] as String, + ); + } + + /// Server url which this token authenticates. + final String url; + + /// Specifies authentication token kind. + /// + /// The supported values are: `Bearer` + final String kind; + + /// Authentication token value + final String token; + + /// Serializes [Token] into json format. + Map toJson() { + return { + 'url': url, + 'credential': {'kind': kind, 'token': token}, + }; + } + + /// Returns future that resolves "Authorization" header value used for + /// authenticating. + /// + /// This method returns future to make sure in future we could use the [Token] + /// interface for OAuth2.0 authentication too - which requires token rotation + /// (refresh) that's async job. + Future getAuthorizationHeaderValue() { + return Future.value('$kind $token'); + } + + /// Returns whether or not given [url] could be authenticated using this + /// token. + bool canAuthenticate(String url) { + return _normalizeUrl(url).startsWith(_normalizeUrl(this.url)); + } + + static String _normalizeUrl(String url) { + return (url.endsWith('/') ? url : '$url/').toLowerCase(); + } +} diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart new file mode 100644 index 000000000..fe8a1fa8c --- /dev/null +++ b/lib/src/authentication/token_store.dart @@ -0,0 +1,140 @@ +// Copyright (c) 2021, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import 'dart:convert'; + +import 'package:path/path.dart' as path; + +import '../io.dart'; +import '../log.dart' as log; +import 'token.dart'; + +/// Stores and manages authentication credentials. +class TokenStore { + TokenStore(this.cacheRootDir); + + /// Cache directory. + final String cacheRootDir; + + List? _tokens; + + /// List of saved authentication tokens. + /// + /// Modifying this field will not write changes to the disk. You have to call + /// [flush] to save changes. + List get tokens => _tokens ??= _loadTokens(); + + /// Reads "tokens.json" and parses / deserializes it into list of + /// [Token]. + List _loadTokens() { + final result = List.empty(growable: true); + final path = _tokensFile; + if (!fileExists(path)) { + return result; + } + + try { + final json = jsonDecode(readTextFile(path)); + + if (json is! Map) { + throw FormatException('JSON contents is corrupted or not supported.'); + } + if (json['version'] != 1) { + throw FormatException('Version is not supported.'); + } + + if (json.containsKey('hosted')) { + if (json['hosted'] is! List) { + throw FormatException( + 'tokens.json format is invalid or not supported.'); + } + + result.addAll((json['hosted'] as List) + .cast>() + .map((it) => Token.fromJson(it))); + } + } on FormatException catch (error, stackTrace) { + log.error('Failed to load tokens.json.', error, stackTrace); + } + + return result; + } + + /// Writes [tokens] into "tokens.json". + void _saveTokens(List tokens) { + writeTextFile( + _tokensFile, + jsonEncode({ + 'version': 1, + 'hosted': tokens.map((it) => it.toJson()).toList(), + })); + } + + /// Writes latest state of the store to disk. + void flush() { + if (_tokens == null) { + throw Exception('Schemes should be loaded before saving.'); + } + _saveTokens(_tokens!); + } + + /// Adds [token] into store and writes into disk. + void addToken(Token token) { + tokens.add(token); + flush(); + } + + /// Removes tokens with matching [url] from store. + void removeMatchingTokens(String url) { + final tokensToRemove = + tokens.where((it) => it.canAuthenticate(url)).toList(); + if (tokensToRemove.isNotEmpty) { + for (final token in tokensToRemove) { + tokens.remove(token); + log.message('Logging out of ${token.url}.'); + } + + flush(); + } else { + log.message('No matching credential found for $url. Cannot log out.'); + } + } + + /// Returns [Token] for authenticating given url or null if no matching token + /// is found. + Token? findToken(String url) { + Token? matchedToken; + for (final token in tokens) { + if (token.url == url) { + if (matchedToken == null) { + matchedToken = token; + } else { + log.warning( + 'Found multiple matching authentication tokens for "$url". ' + 'First matching token will be used for authentication.', + ); + } + } + } + + return matchedToken; + } + + /// Returns whether or not store contains a token that could be used for + /// authenticating given [url]. + bool hasToken(String url) { + return tokens.any((it) => it.url == url); + } + + /// Deletes tokens.json file from the disk. + void deleteTokensFile() { + deleteEntry(_tokensFile); + log.message('tokens.json is deleted.'); + } + + /// Full path to the "tokens.json" file. + String get _tokensFile => path.join(cacheRootDir, 'tokens.json'); +} diff --git a/lib/src/command.dart b/lib/src/command.dart index 8e874e45a..cd4a524e0 100644 --- a/lib/src/command.dart +++ b/lib/src/command.dart @@ -12,7 +12,7 @@ import 'package:args/command_runner.dart'; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; -import 'authentication/credential_store.dart'; +import 'authentication/token_store.dart'; import 'command_runner.dart'; import 'entrypoint.dart'; import 'exceptions.dart'; @@ -56,7 +56,7 @@ abstract class PubCommand extends Command { GlobalPackages _globals; - CredentialStore get credentialStore => cache.credentialStore; + TokenStore get tokenStore => cache.tokenStore; /// Gets the [Entrypoint] package for the current working directory. /// diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index dd9a37884..c8d26b225 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -125,7 +125,7 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (credentialStore.hasScheme(server.toString())) { + if (tokenStore.hasToken(server.toString())) { // If there's a saved credential for the server, publish using // httpClient which should authenticate with the server using // AuthenticationClient. diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 426a75083..51f60fb71 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -7,7 +7,7 @@ import 'dart:async'; import 'dart:convert'; -import '../authentication/bearer.dart'; +import '../authentication/token.dart'; import '../command.dart'; import '../http.dart'; import '../io.dart'; @@ -50,9 +50,9 @@ class LoginCommand extends PubCommand { } Future _listCredentials() async { - log.message('Found ${cache.credentialStore.schemes.length} entries.'); - for (final scheme in cache.credentialStore.schemes) { - log.message(scheme.baseUrl); + log.message('Found ${cache.tokenStore.tokens.length} entries.'); + for (final scheme in cache.tokenStore.tokens) { + log.message(scheme.url); } } @@ -71,7 +71,7 @@ class LoginCommand extends PubCommand { usageException('Token is not provided.'); } - credentialStore.addHostedScheme(server, BearerCredential(token)); + tokenStore.addToken(Token.bearer(server, token)); log.message('You are now logged in to $server using bearer token.'); } on TimeoutException catch (error, stackTrace) { log.error( diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index a122821e6..95120d6ad 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -36,12 +36,12 @@ class LogoutCommand extends PubCommand { Future runProtected() async { if (clear) { if (await confirm('Are you sure you want to remove all credentials')) { - credentialStore.deleteTokensFile(); + tokenStore.deleteTokensFile(); } } else if (server == null) { oauth2.logout(cache); } else { - credentialStore.removeMatchingHostedSchemes(server); + tokenStore.removeMatchingTokens(server); } } } diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart index 9cbcb4a72..bde93768a 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart @@ -8,7 +8,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; -import 'authentication/credential_store.dart'; +import 'authentication/token_store.dart'; import 'io.dart'; import 'io.dart' as io show createTempDir; import 'log.dart' as log; @@ -81,7 +81,7 @@ class SystemCache { BoundSource get defaultSource => source(sources[null]); /// The default credential store. - final CredentialStore credentialStore; + final TokenStore tokenStore; /// Creates a system cache and registers all sources in [sources]. /// @@ -89,7 +89,7 @@ class SystemCache { /// Defaults to `false`. SystemCache({String rootDir, bool isOffline = false}) : rootDir = rootDir ?? SystemCache.defaultDir, - credentialStore = CredentialStore(rootDir ?? SystemCache.defaultDir) { + tokenStore = TokenStore(rootDir ?? SystemCache.defaultDir) { for (var source in sources.all) { if (source is HostedSource) { _boundSources[source] = source.bind(this, isOffline: isOffline); From 42fcf2bf12568d55ee4d4b623d234460fa308c76 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 19 Aug 2021 20:17:36 +0400 Subject: [PATCH 47/77] Update tests for abstraction changes --- test/authentication/login_to_server_test.dart | 2 +- test/authentication/logout_from_server_test.dart | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart index a808028d2..c626cf38b 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/authentication/login_to_server_test.dart @@ -26,7 +26,7 @@ void main() { ); await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://server.demo/', diff --git a/test/authentication/logout_from_server_test.dart b/test/authentication/logout_from_server_test.dart index 204cad4ac..63ba43bac 100644 --- a/test/authentication/logout_from_server_test.dart +++ b/test/authentication/logout_from_server_test.dart @@ -12,7 +12,7 @@ import '../test_pub.dart'; void main() { test('with one matching scheme, removes the entry.', () async { await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://server.demo/', @@ -32,7 +32,7 @@ void main() { test('with multiple matching schemes, removes all matching entries.', () async { await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://server.demo/', @@ -58,7 +58,7 @@ void main() { ); await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://another-.demo/', @@ -70,7 +70,7 @@ void main() { test('without an matching schemes, does nothing.', () async { await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://server.demo/', @@ -86,7 +86,7 @@ void main() { ); await d.tokensFile({ - 'version': '1.0', + 'version': 1, 'hosted': [ { 'url': 'http://server.demo/', From 030c6f99c078581b55277180b6637bacc27d1e01 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 19 Aug 2021 20:40:57 +0400 Subject: [PATCH 48/77] Sorted directives --- test/authentication/login_to_server_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart index c626cf38b..2b803854a 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/authentication/login_to_server_test.dart @@ -4,8 +4,8 @@ // @dart=2.10 -import 'package:test/test.dart'; import 'package:pub/src/exit_codes.dart' as exit_codes; +import 'package:test/test.dart'; import '../descriptor.dart' as d; import '../test_pub.dart'; From 946b5472be2f96bcbc06a319f0ff25e9e19282d0 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 19 Aug 2021 21:53:27 +0400 Subject: [PATCH 49/77] Normalize token.url using validateAndNormalizeHostedUrl --- lib/src/authentication/client.dart | 2 +- lib/src/authentication/token.dart | 17 ++++++++---- lib/src/authentication/token_store.dart | 8 +++--- lib/src/command/lish.dart | 4 +-- lib/src/source/hosted.dart | 6 ++--- test/authentication/login_to_server_test.dart | 6 ++--- .../logout_from_server_test.dart | 27 ++++++++++--------- 7 files changed, 40 insertions(+), 30 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index bb240a729..e1d08b5c6 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -51,7 +51,7 @@ class _AuthenticatedClient extends http.BaseClient { /// authenticated. Future withAuthenticatedClient( SystemCache systemCache, - String serverBaseUrl, + Uri serverBaseUrl, Future Function(http.Client) fn, ) async { final scheme = systemCache.tokenStore.findToken(serverBaseUrl); diff --git a/lib/src/authentication/token.dart b/lib/src/authentication/token.dart index ffe332ef0..50cb9befa 100644 --- a/lib/src/authentication/token.dart +++ b/lib/src/authentication/token.dart @@ -1,7 +1,14 @@ +// ignore_for_file: import_of_legacy_library_into_null_safe + +import '../source/hosted.dart'; + class Token { - const Token({required this.url, required this.kind, required this.token}); + Token({required String url, required this.kind, required this.token}) + : url = validateAndNormalizeHostedUrl(url); - const Token.bearer(this.url, this.token) : kind = 'Bearer'; + Token.bearer(String url, this.token) + : kind = 'Bearer', + url = validateAndNormalizeHostedUrl(url); /// Deserialize [json] into [Token] type. factory Token.fromJson(Map json) { @@ -35,7 +42,7 @@ class Token { } /// Server url which this token authenticates. - final String url; + final Uri url; /// Specifies authentication token kind. /// @@ -48,7 +55,7 @@ class Token { /// Serializes [Token] into json format. Map toJson() { return { - 'url': url, + 'url': url.toString(), 'credential': {'kind': kind, 'token': token}, }; } @@ -66,7 +73,7 @@ class Token { /// Returns whether or not given [url] could be authenticated using this /// token. bool canAuthenticate(String url) { - return _normalizeUrl(url).startsWith(_normalizeUrl(this.url)); + return _normalizeUrl(url).startsWith(_normalizeUrl(this.url.toString())); } static String _normalizeUrl(String url) { diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index fe8a1fa8c..c6d2c925c 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -10,6 +10,7 @@ import 'package:path/path.dart' as path; import '../io.dart'; import '../log.dart' as log; +import '../source/hosted.dart'; import 'token.dart'; /// Stores and manages authentication credentials. @@ -89,8 +90,9 @@ class TokenStore { /// Removes tokens with matching [url] from store. void removeMatchingTokens(String url) { + final normalizedUrl = validateAndNormalizeHostedUrl(url); final tokensToRemove = - tokens.where((it) => it.canAuthenticate(url)).toList(); + tokens.where((it) => it.url == normalizedUrl).toList(); if (tokensToRemove.isNotEmpty) { for (final token in tokensToRemove) { tokens.remove(token); @@ -105,7 +107,7 @@ class TokenStore { /// Returns [Token] for authenticating given url or null if no matching token /// is found. - Token? findToken(String url) { + Token? findToken(Uri url) { Token? matchedToken; for (final token in tokens) { if (token.url == url) { @@ -125,7 +127,7 @@ class TokenStore { /// Returns whether or not store contains a token that could be used for /// authenticating given [url]. - bool hasToken(String url) { + bool hasToken(Uri url) { return tokens.any((it) => it.url == url); } diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 21c65ec12..68d83fdd4 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -143,11 +143,11 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (tokenStore.hasToken(server.toString())) { + if (tokenStore.hasToken(server)) { // If there's a saved credential for the server, publish using // httpClient which should authenticate with the server using // AuthenticationClient. - await withAuthenticatedClient(cache, server.toString(), (client) { + await withAuthenticatedClient(cache, server, (client) { return _publishUsingClient(packageBytes, client); }); } else { diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 298b9a14d..f3ed4600c 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -282,7 +282,7 @@ class BoundHostedSource extends CachedSource { // requires resolution of: https://github.com/dart-lang/sdk/issues/22265. bodyText = await withAuthenticatedClient( systemCache, - serverUrl.toString(), + serverUrl, (client) => client.read(url, headers: pubApiHeaders), ); body = jsonDecode(bodyText); @@ -645,8 +645,8 @@ class BoundHostedSource extends CachedSource { await withTempDir((tempDirForArchive) async { var archivePath = p.join(tempDirForArchive, '$packageName-$version.tar.gz'); - var response = await withAuthenticatedClient(systemCache, - server.toString(), (client) => client.send(http.Request('GET', url))); + var response = await withAuthenticatedClient(systemCache, server, + (client) => client.send(http.Request('GET', url))); // We download the archive to disk instead of streaming it directly into // the tar unpacking. This simplifies stream handling. diff --git a/test/authentication/login_to_server_test.dart b/test/authentication/login_to_server_test.dart index 2b803854a..f689c699f 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/authentication/login_to_server_test.dart @@ -18,18 +18,18 @@ void main() { args: [ 'login', '--server', - 'http://server.demo', + 'http://server.demo/', ], input: ['auth-token'], output: contains( - 'You are now logged in to http://server.demo using bearer token.'), + 'You are now logged in to http://server.demo/ using bearer token.'), ); await d.tokensFile({ 'version': 1, 'hosted': [ { - 'url': 'http://server.demo/', + 'url': 'http://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] diff --git a/test/authentication/logout_from_server_test.dart b/test/authentication/logout_from_server_test.dart index 63ba43bac..4358a1200 100644 --- a/test/authentication/logout_from_server_test.dart +++ b/test/authentication/logout_from_server_test.dart @@ -15,7 +15,7 @@ void main() { 'version': 1, 'hosted': [ { - 'url': 'http://server.demo/', + 'url': 'http://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] @@ -23,10 +23,10 @@ void main() { await runPub( args: ['logout', '--server', 'http://server.demo'], - output: contains('Logging out of http://server.demo/.'), + output: contains('Logging out of http://server.demo.'), ); - await d.tokensFile({'version': '1.0', 'hosted': []}).validate(); + await d.tokensFile({'version': 1, 'hosted': []}).validate(); }); test('with multiple matching schemes, removes all matching entries.', @@ -35,15 +35,15 @@ void main() { 'version': 1, 'hosted': [ { - 'url': 'http://server.demo/', + 'url': 'http://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, }, { - 'url': 'http://server.demo/sub', + 'url': 'http://server.demo/sub/', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, }, { - 'url': 'http://another-.demo/', + 'url': 'http://another-.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] @@ -51,17 +51,18 @@ void main() { await runPub( args: ['logout', '--server', 'http://server.demo/sub'], - output: allOf( - contains('Logging out of http://server.demo/.'), - contains('Logging out of http://server.demo/sub/.'), - ), + output: contains('Logging out of http://server.demo/sub/.'), ); await d.tokensFile({ 'version': 1, 'hosted': [ { - 'url': 'http://another-.demo/', + 'url': 'http://server.demo', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + }, + { + 'url': 'http://another-.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] @@ -73,7 +74,7 @@ void main() { 'version': 1, 'hosted': [ { - 'url': 'http://server.demo/', + 'url': 'http://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] @@ -89,7 +90,7 @@ void main() { 'version': 1, 'hosted': [ { - 'url': 'http://server.demo/', + 'url': 'http://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] From e602167c40c60b10e6623e23f5ae2c680fc13418 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 01:43:21 +0400 Subject: [PATCH 50/77] Modified removeMatchingTokens implementation --- lib/src/authentication/token_store.dart | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index c6d2c925c..7fe1c8c3c 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -91,17 +91,24 @@ class TokenStore { /// Removes tokens with matching [url] from store. void removeMatchingTokens(String url) { final normalizedUrl = validateAndNormalizeHostedUrl(url); - final tokensToRemove = - tokens.where((it) => it.url == normalizedUrl).toList(); - if (tokensToRemove.isNotEmpty) { - for (final token in tokensToRemove) { - tokens.remove(token); - log.message('Logging out of ${token.url}.'); + + var i = 0; + var found = false; + while (i < tokens.length) { + if (tokens[i].url == normalizedUrl) { + tokens.removeAt(i); + found = true; + } else { + i++; } + } + + flush(); - flush(); + if (found) { + log.message('Token removed for server $normalizedUrl.'); } else { - log.message('No matching credential found for $url. Cannot log out.'); + log.message('No saved token found for $normalizedUrl.'); } } From 0a37f1977961d1bbd99455958d794b46b988d206 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 01:51:27 +0400 Subject: [PATCH 51/77] pub login and logout commands reverted back --- lib/src/command/login.dart | 60 +------------------------------------ lib/src/command/logout.dart | 26 ++-------------- 2 files changed, 4 insertions(+), 82 deletions(-) diff --git a/lib/src/command/login.dart b/lib/src/command/login.dart index 51f60fb71..747bb5542 100644 --- a/lib/src/command/login.dart +++ b/lib/src/command/login.dart @@ -7,10 +7,8 @@ import 'dart:async'; import 'dart:convert'; -import '../authentication/token.dart'; import '../command.dart'; import '../http.dart'; -import '../io.dart'; import '../log.dart' as log; import '../oauth2.dart' as oauth2; @@ -23,66 +21,10 @@ class LoginCommand extends PubCommand { @override String get invocation => 'pub login'; - String get server => argResults['server']; - bool get list => argResults['list']; - - LoginCommand() { - argParser.addOption('server', - help: 'The package server to which needs to be authenticated.'); - - argParser.addFlag('list', - help: 'Displays list of currently logged in hosted pub servers', - defaultsTo: false); - } + LoginCommand(); @override Future runProtected() async { - if (list) { - await _listCredentials(); - } else if (server == null) { - await _loginToPubDev(); - } else { - if (Uri.tryParse(server) == null) { - usageException('Invalid or malformed server URL provided.'); - } - await _loginToServer(server); - } - } - - Future _listCredentials() async { - log.message('Found ${cache.tokenStore.tokens.length} entries.'); - for (final scheme in cache.tokenStore.tokens) { - log.message(scheme.url); - } - } - - Future _loginToServer(String server) async { - // TODO(themisir): Replace this line with validateAndNormalizeHostedUrl from - // source/hosted.dart when dart-lang/pub#3030 is merged. - if (Uri.tryParse(server) == null || - !server.startsWith(RegExp(r'https?:\/\/'))) { - usageException('Invalid or malformed server URL provided.'); - } - - try { - final token = await readLine('Please enter bearer token') - .timeout(const Duration(minutes: 5)); - if (token.isEmpty) { - usageException('Token is not provided.'); - } - - tokenStore.addToken(Token.bearer(server, token)); - log.message('You are now logged in to $server using bearer token.'); - } on TimeoutException catch (error, stackTrace) { - log.error( - 'Timeout error. Token is not provided within ' - '${error.duration.inSeconds} seconds.', - error, - stackTrace); - } - } - - Future _loginToPubDev() async { final credentials = oauth2.loadCredentials(cache); if (credentials == null) { final userInfo = await _retrieveUserInfo(); diff --git a/lib/src/command/logout.dart b/lib/src/command/logout.dart index 95120d6ad..071e8c06c 100644 --- a/lib/src/command/logout.dart +++ b/lib/src/command/logout.dart @@ -7,7 +7,6 @@ import 'dart:async'; import '../command.dart'; -import '../io.dart'; import '../oauth2.dart' as oauth2; /// Handles the `logout` pub command. @@ -17,31 +16,12 @@ class LogoutCommand extends PubCommand { @override String get description => 'Log out of pub.dev.'; @override - bool get takesArguments => true; + bool get takesArguments => false; - String get server => argResults['server']; - bool get clear => argResults['clear']; - - LogoutCommand() { - argParser.addOption('server', - help: 'The package server to which needs to be authenticated.'); - - argParser.addFlag('clear', - help: 'Removes all of previously saved credentials for hosted pub ' - 'servers', - defaultsTo: false); - } + LogoutCommand(); @override Future runProtected() async { - if (clear) { - if (await confirm('Are you sure you want to remove all credentials')) { - tokenStore.deleteTokensFile(); - } - } else if (server == null) { - oauth2.logout(cache); - } else { - tokenStore.removeMatchingTokens(server); - } + oauth2.logout(cache); } } From 73b921e9cde488a6cbfe3268ceac778ea9090a11 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 01:53:23 +0400 Subject: [PATCH 52/77] Updated constructor signature for Token class And added some comments --- lib/src/authentication/token.dart | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/src/authentication/token.dart b/lib/src/authentication/token.dart index 50cb9befa..102d8ad10 100644 --- a/lib/src/authentication/token.dart +++ b/lib/src/authentication/token.dart @@ -2,13 +2,27 @@ import '../source/hosted.dart'; +/// Token is a structure for storing authentication credentials for third-party +/// pub registries. A token holds registry [url], credential [kind] and [token] +/// itself. +/// +/// Token could be serialized into and from JSON format structured like +/// this: +/// +/// ```json +/// { +/// "url": "https://example.com/", +/// "credential": { +/// "kind": "Bearer", +/// "token": "gjrjo7Tm2F0u64cTsECDq4jBNZYhco" +/// } +/// } +/// ``` class Token { - Token({required String url, required this.kind, required this.token}) - : url = validateAndNormalizeHostedUrl(url); + Token({required this.url, required this.kind, required this.token}); - Token.bearer(String url, this.token) - : kind = 'Bearer', - url = validateAndNormalizeHostedUrl(url); + /// Create [Token] instance with the `'Bearer'` kind. + Token.bearer(this.url, this.token) : kind = 'Bearer'; /// Deserialize [json] into [Token] type. factory Token.fromJson(Map json) { @@ -16,6 +30,8 @@ class Token { throw FormatException('Url is not provided for the token'); } + var hostedUrl = validateAndNormalizeHostedUrl(json['url'] as String); + if (json['credential'] is! Map) { throw FormatException('Credential is not provided for the token'); } @@ -35,7 +51,7 @@ class Token { } return Token( - url: json['url'] as String, + url: hostedUrl, kind: kindValue, token: json['credential']['token'] as String, ); From d431c08b8bb0539dafa5714fd471ceb960f10fc8 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 01:54:58 +0400 Subject: [PATCH 53/77] Implemented token .. commands --- lib/src/command/token.dart | 24 ++++++++++++ lib/src/command/token_add.dart | 59 +++++++++++++++++++++++++++++ lib/src/command/token_list.dart | 29 ++++++++++++++ lib/src/command/token_remove.dart | 40 +++++++++++++++++++ lib/src/pub_embeddable_command.dart | 2 + 5 files changed, 154 insertions(+) create mode 100644 lib/src/command/token.dart create mode 100644 lib/src/command/token_add.dart create mode 100644 lib/src/command/token_list.dart create mode 100644 lib/src/command/token_remove.dart diff --git a/lib/src/command/token.dart b/lib/src/command/token.dart new file mode 100644 index 000000000..63247edfb --- /dev/null +++ b/lib/src/command/token.dart @@ -0,0 +1,24 @@ +// Copyright (c) 2020, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import '../command.dart'; +import 'token_add.dart'; +import 'token_list.dart'; +import 'token_remove.dart'; + +/// Handles the `token` command. +class TokenCommand extends PubCommand { + @override + String get name => 'token'; + @override + String get description => 'Work with pub registry authentication.'; + + TokenCommand() { + addSubcommand(TokenListCommand()); + addSubcommand(TokenAddCommand()); + addSubcommand(TokenRemoveCommand()); + } +} diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart new file mode 100644 index 000000000..8629e64b0 --- /dev/null +++ b/lib/src/command/token_add.dart @@ -0,0 +1,59 @@ +// Copyright (c) 2020, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import 'dart:async'; + +import '../authentication/token.dart'; +import '../command.dart'; +import '../io.dart'; +import '../log.dart' as log; +import '../source/hosted.dart'; + +/// Handles the `token add` pub command. +class TokenAddCommand extends PubCommand { + @override + String get name => 'add'; + @override + String get description => 'Add token for a server.'; + @override + String get invocation => 'pub token add'; + + @override + Future runProtected() async { + if (argResults.rest.isEmpty) { + usageException('Must specify a hosted server URL to be added.'); + } else if (argResults.rest.length > 1) { + usageException('Takes only a single argument.'); + } + + try { + var hostedUrl = validateAndNormalizeHostedUrl(argResults.rest.first); + + if (hostedUrl.isScheme('HTTP')) { + // TODO(themisir): Improve the following message. + usageException('Unsecure pub server could not be added.'); + } + + final token = await readLine('Please enter bearer token') + .timeout(const Duration(minutes: 5)); + + if (token.isEmpty) { + usageException('Token is not provided.'); + } + + tokenStore.addToken(Token.bearer(hostedUrl, token)); + log.message('You are now logged in to $hostedUrl using bearer token.'); + } on FormatException catch (error, stackTrace) { + log.error('Invalid or malformed server URL provided.', error, stackTrace); + } on TimeoutException catch (error, stackTrace) { + // Timeout is added to readLine call to make sure automated jobs doesn't + // get stuck on noop state if user forget to pipe token to the 'token add' + // command. This behavior might be removed.. + log.error('Timeout error. Token is not provided within 5 minutes.', error, + stackTrace); + } + } +} diff --git a/lib/src/command/token_list.dart b/lib/src/command/token_list.dart new file mode 100644 index 000000000..15d0599fb --- /dev/null +++ b/lib/src/command/token_list.dart @@ -0,0 +1,29 @@ +// Copyright (c) 2020, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import '../command.dart'; +import '../log.dart' as log; + +/// Handles the `token list` pub command. +class TokenListCommand extends PubCommand { + @override + String get name => 'list'; + @override + String get description => 'List servers for which a token exists.'; + @override + String get invocation => 'pub token list'; + + @override + void runProtected() { + // TODO(themisir): The output interface could be improved even more with + // additional details line, token preview, token kind, and instructions + // to remove a token. + log.message('Found ${cache.tokenStore.tokens.length} entries.'); + for (final scheme in cache.tokenStore.tokens) { + log.message(scheme.url); + } + } +} diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart new file mode 100644 index 000000000..c44b32571 --- /dev/null +++ b/lib/src/command/token_remove.dart @@ -0,0 +1,40 @@ +// Copyright (c) 2020, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import '../command.dart'; + +/// Handles the `token remove` pub command. +class TokenRemoveCommand extends PubCommand { + @override + String get name => 'remove'; + @override + String get description => 'Remove token for server.'; + @override + String get invocation => 'pub token remove'; + + bool get isAll => argResults['all']; + + TokenRemoveCommand() { + argParser.addFlag('all', + help: 'Removes all saved tokens from token store.'); + } + + @override + void runProtected() { + if (isAll) { + return tokenStore.deleteTokensFile(); + } + + if (argResults.rest.isEmpty) { + usageException('Must specify a package to be added.'); + } else if (argResults.rest.length > 1) { + usageException('Takes only a single argument.'); + } + + final hostedUrl = argResults.rest.first; + tokenStore.removeMatchingTokens(hostedUrl); + } +} diff --git a/lib/src/pub_embeddable_command.dart b/lib/src/pub_embeddable_command.dart index da859642e..7417f527a 100644 --- a/lib/src/pub_embeddable_command.dart +++ b/lib/src/pub_embeddable_command.dart @@ -19,6 +19,7 @@ import 'command/logout.dart'; import 'command/outdated.dart'; import 'command/remove.dart'; import 'command/run.dart'; +import 'command/token.dart'; import 'command/upgrade.dart'; import 'command/uploader.dart'; import 'log.dart' as log; @@ -71,6 +72,7 @@ class PubEmbeddableCommand extends PubCommand implements PubTopLevel { addSubcommand(UploaderCommand()); addSubcommand(LoginCommand()); addSubcommand(LogoutCommand()); + addSubcommand(TokenCommand()); } @override From d2f41155b12a69678ee99d6d459a7aa1ffa1dbe5 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 01:56:26 +0400 Subject: [PATCH 54/77] Removed authentication-proposal.md --- doc/authentication-proposal.md | 86 ---------------------------------- 1 file changed, 86 deletions(-) delete mode 100644 doc/authentication-proposal.md diff --git a/doc/authentication-proposal.md b/doc/authentication-proposal.md deleted file mode 100644 index 55e0fff2c..000000000 --- a/doc/authentication-proposal.md +++ /dev/null @@ -1,86 +0,0 @@ -Authenticating with Hosted Pub Repositories Proposal -==================================================== - -This document specifies how the REST API could be authenticated with the pub CLI -tool. - -This proposal is mostly based on -[RFC7235](https://datatracker.ietf.org/doc/html/rfc7235) - HTTP Authentication -specifications. This ensures exists un-protected endpoints could be protected -just by setting up reverse proxies like NGINX, Apache, Traefik, which already -supports this specifications. - -## Requesting authentication - -If the repository requires special authentication to access resources or upload -new packages, it has to respond with `401 Unauthorized` status code with -`WWW-Authenticate` header. This header should specify authentication method that -should be used to gain access to the resource. - -WWW-Authenticate header syntax: - -```plain -WWW-Authenticate: [realm=][, charset="UTF-8"] -``` - -> `realm` parameter is completely optional, and will not be used by pub. -> You can read more about this parameter here: -> https://datatracker.ietf.org/doc/html/rfc7235#section-2.2 - -Pub CLI currently only support **Bearer** authentication methods. - -### Login / logout - -Users can login to 3rd party hosted pub server using **pub login** command like -that: - -```plain -pub login --server https://myspuberver.dev --token xxxxxxxxxx -``` - -`--server` is base url of the pub server and `--token` is bearer token for the -authentication. If the server option is not provided, **pub login** will behave -like previous versions - will try authenticating with Google account. - -Just like this, **pub logout** will also support 3rd party hosted pub server -de-authentication. If you provide `--server` option to the command it will -simply remove saved credentials for the server. If not, it will remove Google -account credentials. - -## Storing credentials - -Hosted Pub Repository authentication credentials will be stored on json file -named `tokens.json` located in cache root directory. Authentication details will -be stored at this file as json values while their keys will be URLs of the -server (`PUB_HOSTED_URL`). - -```json -{ - "version": "1.0", - "hosted": [ - { - "url": "https://myserver.com", - "credential": { - "kind": "Basic", - "username": "bob", - "password": "password" - } - }, - { - "url": "https://pub.example.com", - "credential": { - "kind": "Bearer", - "token": "8O868XsPJm-F5nyEzXfa9-YWFvrd3O8r" - } - } - ] -} -``` - -This model of storing credentials allows us to extend support for new -authentication methods in future. - -## References - -- [RFC7235](https://datatracker.ietf.org/doc/html/rfc7235) -- [MDN - WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) From f70e766e33d934004aa211cca6b11488d304a697 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 02:02:10 +0400 Subject: [PATCH 55/77] Remove tokens.json on load error --- lib/src/authentication/token_store.dart | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 7fe1c8c3c..233168243 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -58,7 +58,14 @@ class TokenStore { .map((it) => Token.fromJson(it))); } } on FormatException catch (error, stackTrace) { - log.error('Failed to load tokens.json.', error, stackTrace); + log.error('Failed to load tokens.json', error, stackTrace); + + // When an invalid, damaged or not compatible version of token.json is + // found, we remove it after showing error message. Otherwise the error + // message will be displayed on each pub command. + // Or instead we could write instructrions to execute + //`pub token remove --all` if user couldn't solve the issue. + deleteEntry(_tokensFile); } return result; From 17400b0c4bc04da4ba58e31b49f9204fb677d04b Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 02:21:28 +0400 Subject: [PATCH 56/77] Updated tests and fixed minor bugs --- lib/src/command/token_add.dart | 4 +- lib/src/command/token_list.dart | 2 +- lib/src/command/token_remove.dart | 2 +- lib/src/command_runner.dart | 2 + .../logout_from_server_test.dart | 99 ------------------- .../add_token_test.dart} | 28 +++--- test/token/remove_token_test.dart | 75 ++++++++++++++ 7 files changed, 95 insertions(+), 117 deletions(-) delete mode 100644 test/authentication/logout_from_server_test.dart rename test/{authentication/login_to_server_test.dart => token/add_token_test.dart} (67%) create mode 100644 test/token/remove_token_test.dart diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index 8629e64b0..1d53007f3 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -46,8 +46,8 @@ class TokenAddCommand extends PubCommand { tokenStore.addToken(Token.bearer(hostedUrl, token)); log.message('You are now logged in to $hostedUrl using bearer token.'); - } on FormatException catch (error, stackTrace) { - log.error('Invalid or malformed server URL provided.', error, stackTrace); + } on FormatException catch (_) { + usageException('Invalid or malformed server URL provided.'); } on TimeoutException catch (error, stackTrace) { // Timeout is added to readLine call to make sure automated jobs doesn't // get stuck on noop state if user forget to pipe token to the 'token add' diff --git a/lib/src/command/token_list.dart b/lib/src/command/token_list.dart index 15d0599fb..984c5315d 100644 --- a/lib/src/command/token_list.dart +++ b/lib/src/command/token_list.dart @@ -17,7 +17,7 @@ class TokenListCommand extends PubCommand { String get invocation => 'pub token list'; @override - void runProtected() { + Future runProtected() async { // TODO(themisir): The output interface could be improved even more with // additional details line, token preview, token kind, and instructions // to remove a token. diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index c44b32571..2cdbb30b5 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -23,7 +23,7 @@ class TokenRemoveCommand extends PubCommand { } @override - void runProtected() { + Future runProtected() async { if (isAll) { return tokenStore.deleteTokensFile(); } diff --git a/lib/src/command_runner.dart b/lib/src/command_runner.dart index 90b8a1003..8a80aaf24 100644 --- a/lib/src/command_runner.dart +++ b/lib/src/command_runner.dart @@ -27,6 +27,7 @@ import 'command/outdated.dart'; import 'command/remove.dart'; import 'command/run.dart'; import 'command/serve.dart'; +import 'command/token.dart'; import 'command/upgrade.dart'; import 'command/uploader.dart'; import 'command/version.dart'; @@ -142,6 +143,7 @@ class PubCommandRunner extends CommandRunner implements PubTopLevel { addCommand(LoginCommand()); addCommand(LogoutCommand()); addCommand(VersionCommand()); + addCommand(TokenCommand()); } @override diff --git a/test/authentication/logout_from_server_test.dart b/test/authentication/logout_from_server_test.dart deleted file mode 100644 index 4358a1200..000000000 --- a/test/authentication/logout_from_server_test.dart +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) 2021, 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. - -// @dart=2.10 - -import 'package:test/test.dart'; - -import '../descriptor.dart' as d; -import '../test_pub.dart'; - -void main() { - test('with one matching scheme, removes the entry.', () async { - await d.tokensFile({ - 'version': 1, - 'hosted': [ - { - 'url': 'http://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } - ] - }).create(); - - await runPub( - args: ['logout', '--server', 'http://server.demo'], - output: contains('Logging out of http://server.demo.'), - ); - - await d.tokensFile({'version': 1, 'hosted': []}).validate(); - }); - - test('with multiple matching schemes, removes all matching entries.', - () async { - await d.tokensFile({ - 'version': 1, - 'hosted': [ - { - 'url': 'http://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - }, - { - 'url': 'http://server.demo/sub/', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - }, - { - 'url': 'http://another-.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } - ] - }).create(); - - await runPub( - args: ['logout', '--server', 'http://server.demo/sub'], - output: contains('Logging out of http://server.demo/sub/.'), - ); - - await d.tokensFile({ - 'version': 1, - 'hosted': [ - { - 'url': 'http://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - }, - { - 'url': 'http://another-.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } - ] - }).validate(); - }); - - test('without an matching schemes, does nothing.', () async { - await d.tokensFile({ - 'version': 1, - 'hosted': [ - { - 'url': 'http://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } - ] - }).create(); - - await runPub( - args: ['logout', '--server', 'http://another-server.demo'], - output: 'No matching credential found for http://another-server.demo. ' - 'Cannot log out.', - ); - - await d.tokensFile({ - 'version': 1, - 'hosted': [ - { - 'url': 'http://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } - ] - }).validate(); - }); -} diff --git a/test/authentication/login_to_server_test.dart b/test/token/add_token_test.dart similarity index 67% rename from test/authentication/login_to_server_test.dart rename to test/token/add_token_test.dart index f689c699f..434b537e0 100644 --- a/test/authentication/login_to_server_test.dart +++ b/test/token/add_token_test.dart @@ -15,21 +15,15 @@ void main() { () async { await d.dir(cachePath).create(); await runPub( - args: [ - 'login', - '--server', - 'http://server.demo/', - ], + args: ['token', 'add', 'https://server.demo/'], input: ['auth-token'], - output: contains( - 'You are now logged in to http://server.demo/ using bearer token.'), ); await d.tokensFile({ 'version': 1, 'hosted': [ { - 'url': 'http://server.demo', + 'url': 'https://server.demo', 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, } ] @@ -39,16 +33,22 @@ void main() { test('with invalid server url returns error', () async { await d.dir(cachePath).create(); await runPub( - args: [ - 'login', - '--server', - 'http:;://invalid-url,.com', - ], - input: ['auth-token'], + args: ['token', 'add', 'http:;://invalid-url,.com'], error: contains('Invalid or malformed server URL provided.'), exitCode: exit_codes.USAGE, ); await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); }); + + test('with non-secure server url returns error', () async { + await d.dir(cachePath).create(); + await runPub( + args: ['token', 'add', 'http://mypub.com'], + error: contains('Unsecure pub server could not be added.'), + exitCode: exit_codes.USAGE, + ); + + await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); + }); } diff --git a/test/token/remove_token_test.dart b/test/token/remove_token_test.dart new file mode 100644 index 000000000..f0595ef5d --- /dev/null +++ b/test/token/remove_token_test.dart @@ -0,0 +1,75 @@ +// Copyright (c) 2021, 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. + +// @dart=2.10 + +import 'package:test/test.dart'; + +import '../descriptor.dart' as d; +import '../test_pub.dart'; + +void main() { + test('with one matching token, removes it', () async { + await d.tokensFile({ + 'version': 1, + 'hosted': [ + { + 'url': 'https://server.demo', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).create(); + + await runPub(args: ['token', 'remove', 'https://server.demo']); + + await d.tokensFile({'version': 1, 'hosted': []}).validate(); + }); + + test('without any matching schemes, does nothing', () async { + await d.tokensFile({ + 'version': 1, + 'hosted': [ + { + 'url': 'https://server.demo', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).create(); + + await runPub( + args: ['token', 'remove', 'https://another-server.demo'], + output: 'No saved token found for https://another-server.demo.', + ); + + await d.tokensFile({ + 'version': 1, + 'hosted': [ + { + 'url': 'https://server.demo', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).validate(); + }); + + test('removes all tokens', () async { + await d.tokensFile({ + 'version': 1, + 'hosted': [ + { + 'url': 'https://server.dev', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + }, + { + 'url': 'https://server2.com', + 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, + } + ] + }).create(); + + await runPub(args: ['token', 'remove', '--all']); + + await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); + }); +} From 38ca11cacfebddbda407bf1f62e0a377e35c2bd4 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 02:48:05 +0400 Subject: [PATCH 57/77] Update helptext.txt golden --- test/embedding/goldens/helptext.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/embedding/goldens/helptext.txt b/test/embedding/goldens/helptext.txt index 0026418f0..05877dc9a 100644 --- a/test/embedding/goldens/helptext.txt +++ b/test/embedding/goldens/helptext.txt @@ -46,6 +46,7 @@ $ tool/test-bin/pub_command_runner.dart pub [E] outdated Analyze your dependencies to find which ones can be upgraded. [E] publish Publish the current package to pub.dartlang.org. [E] remove Removes a dependency from the current package. +[E] token Work with pub registry authentication. [E] upgrade Upgrade the current package's dependencies to latest versions. [E] uploader Manage uploaders for a package on pub.dartlang.org. [E] @@ -74,6 +75,7 @@ Available subcommands: outdated Analyze your dependencies to find which ones can be upgraded. publish Publish the current package to pub.dartlang.org. remove Removes a dependency from the current package. + token Work with pub registry authentication. upgrade Upgrade the current package's dependencies to latest versions. uploader Manage uploaders for a package on pub.dartlang.org. From ded22ce5c2ed40a3b0a30b0a97418962551d4a48 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 04:23:18 +0400 Subject: [PATCH 58/77] Log token removal message in command body --- lib/src/authentication/token_store.dart | 11 ++++------- lib/src/command/token_remove.dart | 9 ++++++++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 233168243..0eb06913e 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -95,8 +95,9 @@ class TokenStore { flush(); } - /// Removes tokens with matching [url] from store. - void removeMatchingTokens(String url) { + /// Removes tokens with matching [url] from store. Returns whether or not + /// there's a stored token with matching url. + bool removeMatchingTokens(String url) { final normalizedUrl = validateAndNormalizeHostedUrl(url); var i = 0; @@ -112,11 +113,7 @@ class TokenStore { flush(); - if (found) { - log.message('Token removed for server $normalizedUrl.'); - } else { - log.message('No saved token found for $normalizedUrl.'); - } + return found; } /// Returns [Token] for authenticating given url or null if no matching token diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index 2cdbb30b5..d0a4d037a 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -5,6 +5,7 @@ // ignore_for_file: import_of_legacy_library_into_null_safe import '../command.dart'; +import '../log.dart' as log; /// Handles the `token remove` pub command. class TokenRemoveCommand extends PubCommand { @@ -35,6 +36,12 @@ class TokenRemoveCommand extends PubCommand { } final hostedUrl = argResults.rest.first; - tokenStore.removeMatchingTokens(hostedUrl); + final found = tokenStore.removeMatchingTokens(hostedUrl); + + if (found) { + log.message('Token removed for server $hostedUrl.'); + } else { + log.message('No saved token found for $hostedUrl.'); + } } } From 19ec461312369e076141b8868a2c5093ed263abe Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 04:39:14 +0400 Subject: [PATCH 59/77] Remove duplicate tokens when adding a new one --- lib/src/authentication/token_store.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 0eb06913e..42476ea29 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -91,6 +91,8 @@ class TokenStore { /// Adds [token] into store and writes into disk. void addToken(Token token) { + // Remove duplicate tokens + tokens.removeWhere((it) => it.url == token.url); tokens.add(token); flush(); } From fc0af7e65676cacf13dfaaab2026eba71cb2633c Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 31 Aug 2021 04:46:27 +0400 Subject: [PATCH 60/77] Print information when 401 and 403 received --- lib/src/authentication/client.dart | 58 +++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index e1d08b5c6..6a0ff3118 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'package:http/http.dart' as http; import '../http.dart'; +import '../log.dart' as log; import '../system_cache.dart'; import 'token.dart'; @@ -45,26 +46,65 @@ class _AuthenticatedClient extends http.BaseClient { } /// Invoke [fn] with a [http.Client] capable of authenticating against -/// [serverBaseUrl]. +/// [hostedUrl]. /// -/// Importantly, requests to URLs not under [serverBaseUrl] will not be +/// Importantly, requests to URLs not under [hostedUrl] will not be /// authenticated. Future withAuthenticatedClient( SystemCache systemCache, - Uri serverBaseUrl, + Uri hostedUrl, Future Function(http.Client) fn, ) async { - final scheme = systemCache.tokenStore.findToken(serverBaseUrl); + final token = systemCache.tokenStore.findToken(hostedUrl); final http.Client client = - scheme == null ? httpClient : _AuthenticatedClient(httpClient, scheme); + token == null ? httpClient : _AuthenticatedClient(httpClient, token); try { return await fn(client); - } catch (error) { - if (error is PubHttpException) { + } on PubHttpException catch (error) { + if (error.response?.statusCode == 401 || + error.response?.statusCode == 403) { + // TODO(themisir): Do we need to match error.response.request.url with + // the hostedUrl? Or at least we might need to log request.url to give + // user additional insights on what's happening. + + String? serverMessage; + + try { + final wwwAuthenticateHeaderValue = + error.response.headers[HttpHeaders.wwwAuthenticateHeader]; + if (wwwAuthenticateHeaderValue != null) { + final parsedValue = HeaderValue.parse(wwwAuthenticateHeaderValue, + parameterSeparator: ','); + if (parsedValue.parameters['realm'] == 'pub') { + serverMessage = parsedValue.parameters['message']; + } + } + } catch (_) { + // Ignore errors might be caused when parsing invalid header values + } + if (error.response.statusCode == 401) { - // TODO(themisir): authentication is required for the server or - // credential might be invalid. + systemCache.tokenStore.removeMatchingTokens(hostedUrl.toString()); + + log.error( + 'Authentication requested by hosted server at: $hostedUrl\n' + 'You can use the following command to add token for the server:\n' + '\n pub token add $hostedUrl\n', + ); + } + if (error.response.statusCode == 403) { + log.error( + 'Insufficient permissions to the resource in hosted server at: ' + '$hostedUrl\n' + 'You can use the following command to update token for the server:\n' + '\n pub token add $hostedUrl\n', + ); + } + + if (serverMessage?.isNotEmpty == true) { + // TODO(themisir): Sanitize and truncate serverMessage when needed. + log.error(serverMessage); } } rethrow; From 92352ed41a204728a82d1b0c9abbe372abd68a6d Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 1 Sep 2021 00:03:39 +0400 Subject: [PATCH 61/77] Break out of loop after finding first duplicate --- lib/src/authentication/token_store.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 42476ea29..d2036d4e9 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -131,6 +131,7 @@ class TokenStore { 'Found multiple matching authentication tokens for "$url". ' 'First matching token will be used for authentication.', ); + break; } } } From 0bfc2d05b2be7407323b211957368d098568a567 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Wed, 1 Sep 2021 00:03:58 +0400 Subject: [PATCH 62/77] Added or updated license headers --- lib/src/authentication/token.dart | 4 ++++ lib/src/command/token.dart | 2 +- lib/src/command/token_add.dart | 2 +- lib/src/command/token_list.dart | 2 +- lib/src/command/token_remove.dart | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/src/authentication/token.dart b/lib/src/authentication/token.dart index 102d8ad10..543c47e11 100644 --- a/lib/src/authentication/token.dart +++ b/lib/src/authentication/token.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2021, 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. + // ignore_for_file: import_of_legacy_library_into_null_safe import '../source/hosted.dart'; diff --git a/lib/src/command/token.dart b/lib/src/command/token.dart index 63247edfb..d1793e4df 100644 --- a/lib/src/command/token.dart +++ b/lib/src/command/token.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2021, 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. diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index 1d53007f3..c789d522e 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2021, 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. diff --git a/lib/src/command/token_list.dart b/lib/src/command/token_list.dart index 984c5315d..90062900e 100644 --- a/lib/src/command/token_list.dart +++ b/lib/src/command/token_list.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2021, 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. diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index d0a4d037a..aaa1030ea 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2021, 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. From b34dacc5aa7995230fb7dffdad61a3e5d8cd9625 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 20:20:22 +0400 Subject: [PATCH 63/77] Update lib/src/io.dart Co-authored-by: Sigurd Meldgaard --- lib/src/io.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index 3b2fc70a7..80f64da28 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -568,8 +568,8 @@ Future confirm(String message) { return _stdinLines.first.then(RegExp(r'^[yY]').hasMatch); } -/// Reads a line from stdin stream. -Future readLine(String message) { +/// Writes [prompt] and reads a line from stdin. +Future stdinPrompt(String prompt) { if (runningFromTest) { log.message('$message:'); } else { From a27e89fa65dc55d78a20810a2b5fefef0615aad3 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 20:22:33 +0400 Subject: [PATCH 64/77] Change argument type of removeMatchingTokens from String to Uri --- lib/src/authentication/client.dart | 2 +- lib/src/authentication/token_store.dart | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 6a0ff3118..3b50c013d 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -85,7 +85,7 @@ Future withAuthenticatedClient( } if (error.response.statusCode == 401) { - systemCache.tokenStore.removeMatchingTokens(hostedUrl.toString()); + systemCache.tokenStore.removeMatchingTokens(hostedUrl); log.error( 'Authentication requested by hosted server at: $hostedUrl\n' diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index d2036d4e9..edddb4d94 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -10,7 +10,6 @@ import 'package:path/path.dart' as path; import '../io.dart'; import '../log.dart' as log; -import '../source/hosted.dart'; import 'token.dart'; /// Stores and manages authentication credentials. @@ -97,15 +96,13 @@ class TokenStore { flush(); } - /// Removes tokens with matching [url] from store. Returns whether or not - /// there's a stored token with matching url. - bool removeMatchingTokens(String url) { - final normalizedUrl = validateAndNormalizeHostedUrl(url); - + /// Removes tokens with matching [hostedUrl] from store. Returns whether or + /// not there's a stored token with matching url. + bool removeMatchingTokens(Uri hostedUrl) { var i = 0; var found = false; while (i < tokens.length) { - if (tokens[i].url == normalizedUrl) { + if (tokens[i].url == hostedUrl) { tokens.removeAt(i); found = true; } else { From d4ca1516b27e2e82460689916c72937ee4d0d64f Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 20:37:41 +0400 Subject: [PATCH 65/77] Implement confirm using stdinPrompt --- lib/src/io.dart | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index 80f64da28..c9a1f4826 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -560,20 +560,15 @@ final Stream _stdinLines = /// is false. Future confirm(String message) { log.fine('Showing confirm message: $message'); - if (runningFromTest) { - log.message('$message (y/N)?'); - } else { - stdout.write('$message (y/N)? '); - } - return _stdinLines.first.then(RegExp(r'^[yY]').hasMatch); + return stdinPrompt('$message (y/N)?').then(RegExp(r'^[yY]').hasMatch); } /// Writes [prompt] and reads a line from stdin. Future stdinPrompt(String prompt) { if (runningFromTest) { - log.message('$message:'); + log.message(prompt); } else { - stdout.write('$message: '); + stdout.write('$prompt '); } return _stdinLines.first; } From b8a779e50b02ea0e65d307bc62ae0ef6fef53f46 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 20:37:52 +0400 Subject: [PATCH 66/77] Update messages for token commands --- lib/src/command/token.dart | 3 ++- lib/src/command/token_add.dart | 35 +++++++++++++++++------------- lib/src/command/token_list.dart | 20 +++++++++++------ lib/src/command/token_remove.dart | 36 ++++++++++++++++++++----------- 4 files changed, 60 insertions(+), 34 deletions(-) diff --git a/lib/src/command/token.dart b/lib/src/command/token.dart index d1793e4df..bbe35a66e 100644 --- a/lib/src/command/token.dart +++ b/lib/src/command/token.dart @@ -14,7 +14,8 @@ class TokenCommand extends PubCommand { @override String get name => 'token'; @override - String get description => 'Work with pub registry authentication.'; + String get description => + 'Manage authentication tokens for hosted pub repositories.'; TokenCommand() { addSubcommand(TokenListCommand()); diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index c789d522e..d17e52cbd 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -8,6 +8,7 @@ import 'dart:async'; import '../authentication/token.dart'; import '../command.dart'; +import '../exceptions.dart'; import '../io.dart'; import '../log.dart' as log; import '../source/hosted.dart'; @@ -17,43 +18,47 @@ class TokenAddCommand extends PubCommand { @override String get name => 'add'; @override - String get description => 'Add token for a server.'; + String get description => + 'Add authentication tokens for a package repository.'; @override String get invocation => 'pub token add'; + @override + String get argumentsDescription => '[hosted-url]'; @override Future runProtected() async { if (argResults.rest.isEmpty) { - usageException('Must specify a hosted server URL to be added.'); + usageException( + 'The [hosted-url] for a package repository must be given.'); } else if (argResults.rest.length > 1) { usageException('Takes only a single argument.'); } try { var hostedUrl = validateAndNormalizeHostedUrl(argResults.rest.first); - if (hostedUrl.isScheme('HTTP')) { - // TODO(themisir): Improve the following message. - usageException('Unsecure pub server could not be added.'); + throw DataException('Unsecure package repository could not be added.'); } - final token = await readLine('Please enter bearer token') - .timeout(const Duration(minutes: 5)); - + final token = await stdinPrompt('Enter secret token:') + .timeout(const Duration(minutes: 15)); if (token.isEmpty) { usageException('Token is not provided.'); } tokenStore.addToken(Token.bearer(hostedUrl, token)); - log.message('You are now logged in to $hostedUrl using bearer token.'); - } on FormatException catch (_) { - usageException('Invalid or malformed server URL provided.'); - } on TimeoutException catch (error, stackTrace) { + log.message( + 'Requests to $hostedUrl will now be authenticated using the secret ' + 'token.', + ); + } on FormatException catch (e) { + usageException('Invalid [hosted-url]: "${argResults.rest.first}"\n' + '${e.message}'); + } on TimeoutException catch (_) { // Timeout is added to readLine call to make sure automated jobs doesn't // get stuck on noop state if user forget to pipe token to the 'token add' - // command. This behavior might be removed.. - log.error('Timeout error. Token is not provided within 5 minutes.', error, - stackTrace); + // command. This behavior might be removed. + throw ApplicationException('Token is not provided within 5 minutes.'); } } } diff --git a/lib/src/command/token_list.dart b/lib/src/command/token_list.dart index 90062900e..264a207ca 100644 --- a/lib/src/command/token_list.dart +++ b/lib/src/command/token_list.dart @@ -18,12 +18,20 @@ class TokenListCommand extends PubCommand { @override Future runProtected() async { - // TODO(themisir): The output interface could be improved even more with - // additional details line, token preview, token kind, and instructions - // to remove a token. - log.message('Found ${cache.tokenStore.tokens.length} entries.'); - for (final scheme in cache.tokenStore.tokens) { - log.message(scheme.url); + if (cache.tokenStore.tokens.isNotEmpty) { + log.message( + 'You have secret tokens for ${cache.tokenStore.tokens.length} package ' + 'repositories:', + ); + for (final token in cache.tokenStore.tokens) { + log.message(token.url); + } + } else { + log.message( + 'You do not have any secret tokens for package repositories.\n' + 'However you can add new tokens using the command below:\n' + ' pub token add [hosted-url]', + ); } } } diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index aaa1030ea..71cd1f811 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -6,42 +6,54 @@ import '../command.dart'; import '../log.dart' as log; +import '../source/hosted.dart'; /// Handles the `token remove` pub command. class TokenRemoveCommand extends PubCommand { @override String get name => 'remove'; @override - String get description => 'Remove token for server.'; + String get description => 'Remove secret token for package repository.'; @override String get invocation => 'pub token remove'; + @override + String get argumentsDescription => '[hosted-url]'; bool get isAll => argResults['all']; TokenRemoveCommand() { - argParser.addFlag('all', - help: 'Removes all saved tokens from token store.'); + argParser.addFlag('all', help: 'Remove all secret tokens.'); } @override Future runProtected() async { if (isAll) { - return tokenStore.deleteTokensFile(); + final count = tokenStore.tokens.length; + tokenStore.deleteTokensFile(); + log.message('Removed $count secret tokens.'); + return; } if (argResults.rest.isEmpty) { - usageException('Must specify a package to be added.'); + usageException( + 'The [hosted-url] for a package repository must be specified.'); } else if (argResults.rest.length > 1) { usageException('Takes only a single argument.'); } - final hostedUrl = argResults.rest.first; - final found = tokenStore.removeMatchingTokens(hostedUrl); - - if (found) { - log.message('Token removed for server $hostedUrl.'); - } else { - log.message('No saved token found for $hostedUrl.'); + try { + final hostedUrl = validateAndNormalizeHostedUrl(argResults.rest.first); + final found = tokenStore.removeMatchingTokens(hostedUrl); + + if (found) { + log.message('Removed secret token for package repository: $hostedUrl'); + } else { + log.message( + 'No secret token for package repository "$hostedUrl" was found.'); + } + } on FormatException catch (e) { + usageException('Invalid [hosted-url]: "${argResults.rest.first}"\n' + '${e.message}'); } } } From 2e9105c19eb2b8b3f91674f8daae00e74b9508ce Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 21:08:35 +0400 Subject: [PATCH 67/77] Remaining requested changes implemented - Modified messages - Token renamed to Credential --- lib/src/authentication/client.dart | 19 ++--- lib/src/authentication/credential.dart | 74 +++++++++++++++++ lib/src/authentication/token.dart | 102 ------------------------ lib/src/authentication/token_store.dart | 100 +++++++++++++---------- lib/src/command/lish.dart | 2 +- lib/src/command/token_add.dart | 4 +- lib/src/command/token_list.dart | 8 +- lib/src/command/token_remove.dart | 4 +- 8 files changed, 152 insertions(+), 161 deletions(-) create mode 100644 lib/src/authentication/credential.dart delete mode 100644 lib/src/authentication/token.dart diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 3b50c013d..146c162cf 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -11,19 +11,19 @@ import 'package:http/http.dart' as http; import '../http.dart'; import '../log.dart' as log; import '../system_cache.dart'; -import 'token.dart'; +import 'credential.dart'; /// This client authenticates requests by injecting `Authentication` header to /// requests. /// /// Requests to URLs not under [serverBaseUrl] will not be authenticated. class _AuthenticatedClient extends http.BaseClient { - _AuthenticatedClient(this._inner, this.token); + _AuthenticatedClient(this._inner, this.credential); final http.BaseClient _inner; /// Authentication scheme that could be used for authenticating requests. - final Token token; + final Credential credential; @override Future send(http.BaseRequest request) async { @@ -34,9 +34,9 @@ class _AuthenticatedClient extends http.BaseClient { // to given serverBaseUrl. Otherwise credential leaks might ocurr when // archive_url hosted on 3rd party server that should not receive // credentials of the first party. - if (token.canAuthenticate(request.url.toString())) { + if (credential.canAuthenticate(request.url.toString())) { request.headers[HttpHeaders.authorizationHeader] = - await token.getAuthorizationHeaderValue(); + await credential.getAuthorizationHeaderValue(); } return _inner.send(request); } @@ -55,9 +55,10 @@ Future withAuthenticatedClient( Uri hostedUrl, Future Function(http.Client) fn, ) async { - final token = systemCache.tokenStore.findToken(hostedUrl); - final http.Client client = - token == null ? httpClient : _AuthenticatedClient(httpClient, token); + final credential = systemCache.tokenStore.findCredential(hostedUrl); + final http.Client client = credential == null + ? httpClient + : _AuthenticatedClient(httpClient, credential); try { return await fn(client); @@ -85,7 +86,7 @@ Future withAuthenticatedClient( } if (error.response.statusCode == 401) { - systemCache.tokenStore.removeMatchingTokens(hostedUrl); + systemCache.tokenStore.removeCredential(hostedUrl); log.error( 'Authentication requested by hosted server at: $hostedUrl\n' diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart new file mode 100644 index 000000000..d9daab5ef --- /dev/null +++ b/lib/src/authentication/credential.dart @@ -0,0 +1,74 @@ +// Copyright (c) 2021, 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. + +// ignore_for_file: import_of_legacy_library_into_null_safe + +import '../source/hosted.dart'; + +/// Token is a structure for storing authentication credentials for third-party +/// pub registries. A token holds registry [url], credential [kind] and [token] +/// itself. +/// +/// Token could be serialized into and from JSON format structured like +/// this: +/// +/// ```json +/// { +/// "url": "https://example.com/", +/// "token": "gjrjo7Tm2F0u64cTsECDq4jBNZYhco" +/// } +/// ``` +class Credential { + Credential({required this.url, required this.token}); + + /// Create [Credential] instance for bearer tokens. + Credential.bearer(this.url, this.token); + + /// Deserialize [json] into [Credential] type. + /// + /// Throws [FormatException] if [json] is not a valid [Credential]. + factory Credential.fromJson(Map json) { + if (json['url'] is! String) { + throw FormatException('Url is not provided for the token'); + } + + final hostedUrl = validateAndNormalizeHostedUrl(json['url'] as String); + + if (json['token'] is! String) { + throw FormatException('Secret token is not provided for the token'); + } + + return Credential(url: hostedUrl, token: json['token'] as String); + } + + /// Server url which this token authenticates. + final Uri url; + + /// Authentication token value + final String token; + + /// Serializes [Credential] into json format. + Map toJson() { + return {'url': url.toString(), 'token': token}; + } + + /// Returns future that resolves "Authorization" header value used for + /// authenticating. + // This method returns future to make sure in future we could use the + // [Credential] interface for OAuth2.0 authentication too - which requires + // token rotation (refresh) that's async job. + Future getAuthorizationHeaderValue() { + return Future.value('Bearer $token'); + } + + /// Returns whether or not given [url] could be authenticated using this + /// credential. + bool canAuthenticate(String url) { + return _normalizeUrl(url).startsWith(_normalizeUrl(this.url.toString())); + } + + static String _normalizeUrl(String url) { + return (url.endsWith('/') ? url : '$url/').toLowerCase(); + } +} diff --git a/lib/src/authentication/token.dart b/lib/src/authentication/token.dart deleted file mode 100644 index 543c47e11..000000000 --- a/lib/src/authentication/token.dart +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) 2021, 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. - -// ignore_for_file: import_of_legacy_library_into_null_safe - -import '../source/hosted.dart'; - -/// Token is a structure for storing authentication credentials for third-party -/// pub registries. A token holds registry [url], credential [kind] and [token] -/// itself. -/// -/// Token could be serialized into and from JSON format structured like -/// this: -/// -/// ```json -/// { -/// "url": "https://example.com/", -/// "credential": { -/// "kind": "Bearer", -/// "token": "gjrjo7Tm2F0u64cTsECDq4jBNZYhco" -/// } -/// } -/// ``` -class Token { - Token({required this.url, required this.kind, required this.token}); - - /// Create [Token] instance with the `'Bearer'` kind. - Token.bearer(this.url, this.token) : kind = 'Bearer'; - - /// Deserialize [json] into [Token] type. - factory Token.fromJson(Map json) { - if (json['url'] is! String) { - throw FormatException('Url is not provided for the token'); - } - - var hostedUrl = validateAndNormalizeHostedUrl(json['url'] as String); - - if (json['credential'] is! Map) { - throw FormatException('Credential is not provided for the token'); - } - - final kindValue = json['credential']['kind']; - - if (kindValue is! String) { - throw FormatException('Credential kind is not provided for the token'); - } - - if (!const ['Bearer'].contains(kindValue)) { - throw FormatException('$kindValue is unsupported credential kind value'); - } - - if (json['credential']['token'] is! String) { - throw FormatException('Credential token is not provided for the token'); - } - - return Token( - url: hostedUrl, - kind: kindValue, - token: json['credential']['token'] as String, - ); - } - - /// Server url which this token authenticates. - final Uri url; - - /// Specifies authentication token kind. - /// - /// The supported values are: `Bearer` - final String kind; - - /// Authentication token value - final String token; - - /// Serializes [Token] into json format. - Map toJson() { - return { - 'url': url.toString(), - 'credential': {'kind': kind, 'token': token}, - }; - } - - /// Returns future that resolves "Authorization" header value used for - /// authenticating. - /// - /// This method returns future to make sure in future we could use the [Token] - /// interface for OAuth2.0 authentication too - which requires token rotation - /// (refresh) that's async job. - Future getAuthorizationHeaderValue() { - return Future.value('$kind $token'); - } - - /// Returns whether or not given [url] could be authenticated using this - /// token. - bool canAuthenticate(String url) { - return _normalizeUrl(url).startsWith(_normalizeUrl(this.url.toString())); - } - - static String _normalizeUrl(String url) { - return (url.endsWith('/') ? url : '$url/').toLowerCase(); - } -} diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index edddb4d94..6e905b710 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -10,7 +10,7 @@ import 'package:path/path.dart' as path; import '../io.dart'; import '../log.dart' as log; -import 'token.dart'; +import 'credential.dart'; /// Stores and manages authentication credentials. class TokenStore { @@ -19,59 +19,77 @@ class TokenStore { /// Cache directory. final String cacheRootDir; - List? _tokens; + /// Cached list of [Credential]s. + List? _credentials; /// List of saved authentication tokens. /// /// Modifying this field will not write changes to the disk. You have to call /// [flush] to save changes. - List get tokens => _tokens ??= _loadTokens(); + List get credentials => _credentials ??= _loadCredentials(); /// Reads "tokens.json" and parses / deserializes it into list of - /// [Token]. - List _loadTokens() { - final result = List.empty(growable: true); + /// [Credential]. + List _loadCredentials() { + final result = List.empty(growable: true); final path = _tokensFile; if (!fileExists(path)) { return result; } try { - final json = jsonDecode(readTextFile(path)); + dynamic json; + try { + json = jsonDecode(readTextFile(path)); + } on FormatException { + throw FormatException('$path is not valid JSON'); + } if (json is! Map) { - throw FormatException('JSON contents is corrupted or not supported.'); + throw FormatException('JSON contents is corrupted or not supported'); } if (json['version'] != 1) { - throw FormatException('Version is not supported.'); + throw FormatException('Version is not supported'); } if (json.containsKey('hosted')) { - if (json['hosted'] is! List) { - throw FormatException( - 'tokens.json format is invalid or not supported.'); + final hosted = json['hosted']; + + if (hosted is! List) { + throw FormatException('Invalid or not supported format'); } - result.addAll((json['hosted'] as List) - .cast>() - .map((it) => Token.fromJson(it))); + for (final element in hosted) { + try { + if (element is! Map) { + throw FormatException('Invalid or not supported format'); + } + + result.add(Credential.fromJson(element)); + } on FormatException catch (e) { + if (element['url'] is String) { + log.warning( + 'Failed to load credentials for ${element['url']}: ' + '${e.message}', + ); + } else { + log.warning( + 'Failed to load credentials for unknown hosted repository: ' + '${e.message}', + ); + } + } + } } - } on FormatException catch (error, stackTrace) { - log.error('Failed to load tokens.json', error, stackTrace); - - // When an invalid, damaged or not compatible version of token.json is - // found, we remove it after showing error message. Otherwise the error - // message will be displayed on each pub command. - // Or instead we could write instructrions to execute - //`pub token remove --all` if user couldn't solve the issue. - deleteEntry(_tokensFile); + } on FormatException catch (e) { + log.warning('Failed to load tokens.json: ${e.message}'); } return result; } /// Writes [tokens] into "tokens.json". - void _saveTokens(List tokens) { + void _saveTokens(List tokens) { writeTextFile( _tokensFile, jsonEncode({ @@ -82,28 +100,28 @@ class TokenStore { /// Writes latest state of the store to disk. void flush() { - if (_tokens == null) { - throw Exception('Schemes should be loaded before saving.'); + if (_credentials == null) { + throw Exception('Credentials should be loaded before saving.'); } - _saveTokens(_tokens!); + _saveTokens(_credentials!); } /// Adds [token] into store and writes into disk. - void addToken(Token token) { + void addCredential(Credential token) { // Remove duplicate tokens - tokens.removeWhere((it) => it.url == token.url); - tokens.add(token); + credentials.removeWhere((it) => it.url == token.url); + credentials.add(token); flush(); } /// Removes tokens with matching [hostedUrl] from store. Returns whether or /// not there's a stored token with matching url. - bool removeMatchingTokens(Uri hostedUrl) { + bool removeCredential(Uri hostedUrl) { var i = 0; var found = false; - while (i < tokens.length) { - if (tokens[i].url == hostedUrl) { - tokens.removeAt(i); + while (i < credentials.length) { + if (credentials[i].url == hostedUrl) { + credentials.removeAt(i); found = true; } else { i++; @@ -115,11 +133,11 @@ class TokenStore { return found; } - /// Returns [Token] for authenticating given url or null if no matching token + /// Returns [Credential] for authenticating given url or null if no matching token /// is found. - Token? findToken(Uri url) { - Token? matchedToken; - for (final token in tokens) { + Credential? findCredential(Uri url) { + Credential? matchedToken; + for (final token in credentials) { if (token.url == url) { if (matchedToken == null) { matchedToken = token; @@ -138,8 +156,8 @@ class TokenStore { /// Returns whether or not store contains a token that could be used for /// authenticating given [url]. - bool hasToken(Uri url) { - return tokens.any((it) => it.url == url); + bool hasCredential(Uri url) { + return credentials.any((it) => it.url == url); } /// Deletes tokens.json file from the disk. diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 68d83fdd4..da8248626 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -143,7 +143,7 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (tokenStore.hasToken(server)) { + if (tokenStore.hasCredential(server)) { // If there's a saved credential for the server, publish using // httpClient which should authenticate with the server using // AuthenticationClient. diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index d17e52cbd..1928435e2 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -6,7 +6,7 @@ import 'dart:async'; -import '../authentication/token.dart'; +import '../authentication/credential.dart'; import '../command.dart'; import '../exceptions.dart'; import '../io.dart'; @@ -46,7 +46,7 @@ class TokenAddCommand extends PubCommand { usageException('Token is not provided.'); } - tokenStore.addToken(Token.bearer(hostedUrl, token)); + tokenStore.addCredential(Credential.bearer(hostedUrl, token)); log.message( 'Requests to $hostedUrl will now be authenticated using the secret ' 'token.', diff --git a/lib/src/command/token_list.dart b/lib/src/command/token_list.dart index 264a207ca..56d1cb439 100644 --- a/lib/src/command/token_list.dart +++ b/lib/src/command/token_list.dart @@ -18,19 +18,19 @@ class TokenListCommand extends PubCommand { @override Future runProtected() async { - if (cache.tokenStore.tokens.isNotEmpty) { + if (cache.tokenStore.credentials.isNotEmpty) { log.message( - 'You have secret tokens for ${cache.tokenStore.tokens.length} package ' + 'You have secret tokens for ${cache.tokenStore.credentials.length} package ' 'repositories:', ); - for (final token in cache.tokenStore.tokens) { + for (final token in cache.tokenStore.credentials) { log.message(token.url); } } else { log.message( 'You do not have any secret tokens for package repositories.\n' 'However you can add new tokens using the command below:\n' - ' pub token add [hosted-url]', + '\n pub token add [hosted-url]', ); } } diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index 71cd1f811..eeb3ff7d4 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -28,7 +28,7 @@ class TokenRemoveCommand extends PubCommand { @override Future runProtected() async { if (isAll) { - final count = tokenStore.tokens.length; + final count = tokenStore.credentials.length; tokenStore.deleteTokensFile(); log.message('Removed $count secret tokens.'); return; @@ -43,7 +43,7 @@ class TokenRemoveCommand extends PubCommand { try { final hostedUrl = validateAndNormalizeHostedUrl(argResults.rest.first); - final found = tokenStore.removeMatchingTokens(hostedUrl); + final found = tokenStore.removeCredential(hostedUrl); if (found) { log.message('Removed secret token for package repository: $hostedUrl'); From e92b69ea73aa0fc6eae3a9b8788e730e5d957342 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 21:21:29 +0400 Subject: [PATCH 68/77] Updated token tests according to recent changes --- test/token/add_token_test.dart | 11 ++++------- test/token/remove_token_test.dart | 29 ++++++++--------------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/test/token/add_token_test.dart b/test/token/add_token_test.dart index 434b537e0..b6f72b24b 100644 --- a/test/token/add_token_test.dart +++ b/test/token/add_token_test.dart @@ -22,10 +22,7 @@ void main() { await d.tokensFile({ 'version': 1, 'hosted': [ - { - 'url': 'https://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } + {'url': 'https://server.demo', 'token': 'auth-token'} ] }).validate(); }); @@ -34,7 +31,7 @@ void main() { await d.dir(cachePath).create(); await runPub( args: ['token', 'add', 'http:;://invalid-url,.com'], - error: contains('Invalid or malformed server URL provided.'), + error: contains('Invalid [hosted-url]'), exitCode: exit_codes.USAGE, ); @@ -45,8 +42,8 @@ void main() { await d.dir(cachePath).create(); await runPub( args: ['token', 'add', 'http://mypub.com'], - error: contains('Unsecure pub server could not be added.'), - exitCode: exit_codes.USAGE, + error: contains('Unsecure package repository could not be added.'), + exitCode: exit_codes.DATA, ); await d.dir(cachePath, [d.nothing('tokens.json')]).validate(); diff --git a/test/token/remove_token_test.dart b/test/token/remove_token_test.dart index f0595ef5d..cf8d62c45 100644 --- a/test/token/remove_token_test.dart +++ b/test/token/remove_token_test.dart @@ -14,10 +14,7 @@ void main() { await d.tokensFile({ 'version': 1, 'hosted': [ - { - 'url': 'https://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } + {'url': 'https://server.demo', 'token': 'auth-token'} ] }).create(); @@ -30,25 +27,21 @@ void main() { await d.tokensFile({ 'version': 1, 'hosted': [ - { - 'url': 'https://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } + {'url': 'https://server.demo', 'token': 'auth-token'} ] }).create(); await runPub( args: ['token', 'remove', 'https://another-server.demo'], - output: 'No saved token found for https://another-server.demo.', + output: + 'No secret token for package repository "https://another-server.demo"' + ' was found.', ); await d.tokensFile({ 'version': 1, 'hosted': [ - { - 'url': 'https://server.demo', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } + {'url': 'https://server.demo', 'token': 'auth-token'} ] }).validate(); }); @@ -57,14 +50,8 @@ void main() { await d.tokensFile({ 'version': 1, 'hosted': [ - { - 'url': 'https://server.dev', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - }, - { - 'url': 'https://server2.com', - 'credential': {'kind': 'Bearer', 'token': 'auth-token'}, - } + {'url': 'https://server.dev', 'token': 'auth-token'}, + {'url': 'https://server2.com', 'token': 'auth-token'} ] }).create(); From 98549b2fb0b0b9e568b28acd0dc5b72609c2e455 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Thu, 2 Sep 2021 21:45:35 +0400 Subject: [PATCH 69/77] Update helptext.txt golden --- test/embedding/goldens/helptext.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/embedding/goldens/helptext.txt b/test/embedding/goldens/helptext.txt index 05877dc9a..b5c206f4e 100644 --- a/test/embedding/goldens/helptext.txt +++ b/test/embedding/goldens/helptext.txt @@ -46,7 +46,7 @@ $ tool/test-bin/pub_command_runner.dart pub [E] outdated Analyze your dependencies to find which ones can be upgraded. [E] publish Publish the current package to pub.dartlang.org. [E] remove Removes a dependency from the current package. -[E] token Work with pub registry authentication. +[E] token Manage authentication tokens for hosted pub repositories. [E] upgrade Upgrade the current package's dependencies to latest versions. [E] uploader Manage uploaders for a package on pub.dartlang.org. [E] @@ -75,7 +75,7 @@ Available subcommands: outdated Analyze your dependencies to find which ones can be upgraded. publish Publish the current package to pub.dartlang.org. remove Removes a dependency from the current package. - token Work with pub registry authentication. + token Manage authentication tokens for hosted pub repositories. upgrade Upgrade the current package's dependencies to latest versions. uploader Manage uploaders for a package on pub.dartlang.org. From 5ed7e03c8dee13ec3ce1abbda615e6ec8047f5b2 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 10 Sep 2021 20:42:34 +0400 Subject: [PATCH 70/77] Do not expose oauth2 creds to 3rd party servers --- lib/src/command/lish.dart | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index da8248626..170596e84 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -143,17 +143,15 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (tokenStore.hasCredential(server)) { - // If there's a saved credential for the server, publish using - // httpClient which should authenticate with the server using - // AuthenticationClient. - await withAuthenticatedClient(cache, server, (client) { + if (const {'https://pub.dartlang.org', 'https://pub.dev'} + .contains(server.toString())) { + // Using OAuth2 authentication client for the official pub servers + await oauth2.withClient(cache, (client) { return _publishUsingClient(packageBytes, client); }); } else { - // If user had not yet logged in into the server, use OAuth2 credentials - // for authentication. - await oauth2.withClient(cache, (client) { + // For third party servers using bearer authentication client + await withAuthenticatedClient(cache, server, (client) { return _publishUsingClient(packageBytes, client); }); } From bebc6d2390ff02cd709349b8d0df07220197be5a Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 10 Sep 2021 20:48:58 +0400 Subject: [PATCH 71/77] Throw DataException when no token found for token remove --- lib/src/command/token_remove.dart | 3 ++- test/token/remove_token_test.dart | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/src/command/token_remove.dart b/lib/src/command/token_remove.dart index eeb3ff7d4..df038c287 100644 --- a/lib/src/command/token_remove.dart +++ b/lib/src/command/token_remove.dart @@ -5,6 +5,7 @@ // ignore_for_file: import_of_legacy_library_into_null_safe import '../command.dart'; +import '../exceptions.dart'; import '../log.dart' as log; import '../source/hosted.dart'; @@ -48,7 +49,7 @@ class TokenRemoveCommand extends PubCommand { if (found) { log.message('Removed secret token for package repository: $hostedUrl'); } else { - log.message( + throw DataException( 'No secret token for package repository "$hostedUrl" was found.'); } } on FormatException catch (e) { diff --git a/test/token/remove_token_test.dart b/test/token/remove_token_test.dart index cf8d62c45..a793a631a 100644 --- a/test/token/remove_token_test.dart +++ b/test/token/remove_token_test.dart @@ -4,6 +4,7 @@ // @dart=2.10 +import 'package:pub/src/exit_codes.dart' as exit_codes; import 'package:test/test.dart'; import '../descriptor.dart' as d; @@ -33,9 +34,10 @@ void main() { await runPub( args: ['token', 'remove', 'https://another-server.demo'], - output: + error: 'No secret token for package repository "https://another-server.demo"' ' was found.', + exitCode: exit_codes.DATA, ); await d.tokensFile({ From 82e5038811f1938d495a9a09aa1150d388161e72 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Fri, 10 Sep 2021 21:37:48 +0400 Subject: [PATCH 72/77] Persist unknown fields on unmodified token entries --- lib/src/authentication/credential.dart | 37 +++++++++++++++---- lib/src/authentication/token_store.dart | 37 ++++++++----------- test/token/add_token_test.dart | 49 ++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 31 deletions(-) diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index d9daab5ef..d1856ff6f 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -4,6 +4,7 @@ // ignore_for_file: import_of_legacy_library_into_null_safe +import '../exceptions.dart'; import '../source/hosted.dart'; /// Token is a structure for storing authentication credentials for third-party @@ -20,10 +21,10 @@ import '../source/hosted.dart'; /// } /// ``` class Credential { - Credential({required this.url, required this.token}); + Credential({required this.url, required this.token, this.unknownFields}); /// Create [Credential] instance for bearer tokens. - Credential.bearer(this.url, this.token); + Credential.bearer(this.url, this.token) : unknownFields = null; /// Deserialize [json] into [Credential] type. /// @@ -34,23 +35,37 @@ class Credential { } final hostedUrl = validateAndNormalizeHostedUrl(json['url'] as String); + final token = json['token'] is String ? json['token'] as String : null; - if (json['token'] is! String) { - throw FormatException('Secret token is not provided for the token'); - } + const knownKeys = {'url', 'token'}; + final unknownFields = Map.fromEntries( + json.entries.where((kv) => !knownKeys.contains(kv.key))); - return Credential(url: hostedUrl, token: json['token'] as String); + return Credential( + url: hostedUrl, + token: token, + unknownFields: unknownFields, + ); } /// Server url which this token authenticates. final Uri url; /// Authentication token value - final String token; + final String? token; + + /// Unknown fields found in 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; /// Serializes [Credential] into json format. Map toJson() { - return {'url': url.toString(), 'token': token}; + return { + 'url': url.toString(), + if (token != null) 'token': token, + if (unknownFields != null) ...unknownFields!, + }; } /// Returns future that resolves "Authorization" header value used for @@ -59,6 +74,12 @@ class Credential { // [Credential] interface for OAuth2.0 authentication too - which requires // token rotation (refresh) that's async job. Future getAuthorizationHeaderValue() { + if (token == null) { + throw DataException( + 'Saved credential for $url pub repository is not supported by current ' + 'version of Dart SDK.', + ); + } return Future.value('Bearer $token'); } diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 6e905b710..30b5ecf09 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -19,14 +19,11 @@ class TokenStore { /// Cache directory. final String cacheRootDir; - /// Cached list of [Credential]s. - List? _credentials; - /// List of saved authentication tokens. /// /// Modifying this field will not write changes to the disk. You have to call /// [flush] to save changes. - List get credentials => _credentials ??= _loadCredentials(); + Iterable get credentials => _loadCredentials(); /// Reads "tokens.json" and parses / deserializes it into list of /// [Credential]. @@ -88,47 +85,43 @@ class TokenStore { return result; } - /// Writes [tokens] into "tokens.json". - void _saveTokens(List tokens) { + /// Writes [credentials] into "tokens.json". + void _saveCredentials(List credentials) { writeTextFile( _tokensFile, jsonEncode({ 'version': 1, - 'hosted': tokens.map((it) => it.toJson()).toList(), + 'hosted': credentials.map((it) => it.toJson()).toList(), })); } - /// Writes latest state of the store to disk. - void flush() { - if (_credentials == null) { - throw Exception('Credentials should be loaded before saving.'); - } - _saveTokens(_credentials!); - } - /// Adds [token] into store and writes into disk. void addCredential(Credential token) { + final _credentials = _loadCredentials(); + // Remove duplicate tokens - credentials.removeWhere((it) => it.url == token.url); - credentials.add(token); - flush(); + _credentials.removeWhere((it) => it.url == token.url); + _credentials.add(token); + _saveCredentials(_credentials); } /// Removes tokens with matching [hostedUrl] from store. Returns whether or /// not there's a stored token with matching url. bool removeCredential(Uri hostedUrl) { + final _credentials = _loadCredentials(); + var i = 0; var found = false; - while (i < credentials.length) { - if (credentials[i].url == hostedUrl) { - credentials.removeAt(i); + while (i < _credentials.length) { + if (_credentials[i].url == hostedUrl) { + _credentials.removeAt(i); found = true; } else { i++; } } - flush(); + _saveCredentials(_credentials); return found; } diff --git a/test/token/add_token_test.dart b/test/token/add_token_test.dart index b6f72b24b..8f646ab6b 100644 --- a/test/token/add_token_test.dart +++ b/test/token/add_token_test.dart @@ -13,7 +13,44 @@ import '../test_pub.dart'; void main() { test('with correct server url creates tokens.json that contains token', () async { - await d.dir(cachePath).create(); + await d.tokensFile({ + 'version': 1, + 'hosted': [ + {'url': 'https://example.com', 'token': 'abc'}, + ] + }).create(); + + await runPub( + args: ['token', 'add', 'https://server.demo/'], + input: ['auth-token'], + ); + + await d.tokensFile({ + 'version': 1, + 'hosted': [ + {'url': 'https://example.com', 'token': 'abc'}, + {'url': 'https://server.demo', 'token': 'auth-token'} + ] + }).validate(); + }); + + test('persists unknown fields on unmodified entries', () async { + await d.tokensFile({ + 'version': 1, + 'hosted': [ + { + 'url': 'https://example.com', + 'unknownField': '123', + 'nestedField': [ + { + 'username': 'user', + 'password': 'pass', + }, + ], + } + ] + }).create(); + await runPub( args: ['token', 'add', 'https://server.demo/'], input: ['auth-token'], @@ -22,6 +59,16 @@ void main() { await d.tokensFile({ 'version': 1, 'hosted': [ + { + 'url': 'https://example.com', + 'unknownField': '123', + 'nestedField': [ + { + 'username': 'user', + 'password': 'pass', + }, + ], + }, {'url': 'https://server.demo', 'token': 'auth-token'} ] }).validate(); From 69f8f5bb4afce8972a0e5520bd8fc43af397468d Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Mon, 13 Sep 2021 17:05:26 +0400 Subject: [PATCH 73/77] Allow mock servers to use oauth2 client --- lib/src/command/lish.dart | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 170596e84..151ee79b0 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart @@ -5,6 +5,7 @@ // @dart=2.10 import 'dart:async'; +import 'dart:io'; import 'package:http/http.dart' as http; @@ -143,8 +144,23 @@ class LishCommand extends PubCommand { Future _publish(List packageBytes) async { try { - if (const {'https://pub.dartlang.org', 'https://pub.dev'} - .contains(server.toString())) { + final officialPubServers = { + 'https://pub.dartlang.org', + 'https://pub.dev', + + // Pub uses oauth2 credentials only for authenticating official pub + // servers for security purposes (to not expose pub.dev access token to + // 3rd party servers). + // For testing publish command we're using mock servers hosted on + // localhost address which is not a known pub server address. So we + // explicitly have to define mock servers as official server to test + // publish command with oauth2 credentials. + if (runningFromTest && + Platform.environment.containsKey('PUB_HOSTED_URL')) + Platform.environment['PUB_HOSTED_URL'], + }; + + if (officialPubServers.contains(server.toString())) { // Using OAuth2 authentication client for the official pub servers await oauth2.withClient(cache, (client) { return _publishUsingClient(packageBytes, client); From 7f3fa899997f378b3c952c5eab82f7189ba44c44 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Mon, 13 Sep 2021 17:09:08 +0400 Subject: [PATCH 74/77] Truncate server message --- lib/src/authentication/client.dart | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart index 146c162cf..8360e8a8e 100644 --- a/lib/src/authentication/client.dart +++ b/lib/src/authentication/client.dart @@ -86,7 +86,9 @@ Future withAuthenticatedClient( } if (error.response.statusCode == 401) { - systemCache.tokenStore.removeCredential(hostedUrl); + if (systemCache.tokenStore.removeCredential(hostedUrl)) { + log.warning('Invalid token for $hostedUrl deleted.'); + } log.error( 'Authentication requested by hosted server at: $hostedUrl\n' @@ -104,8 +106,13 @@ Future withAuthenticatedClient( } if (serverMessage?.isNotEmpty == true) { - // TODO(themisir): Sanitize and truncate serverMessage when needed. - log.error(serverMessage); + // Only allow printable ASCII, map anything else to whitespace, take + // at-most 1024 characters. + final truncatedMessage = String.fromCharCodes(serverMessage!.runes + .map((r) => 32 >= r && r <= 127 ? r : 32) + .take(1024)); + + log.error(truncatedMessage); } } rethrow; From 299a4f32e6b2c8c1a2870060b5fc145507cb0542 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Mon, 13 Sep 2021 17:35:32 +0400 Subject: [PATCH 75/77] Minor refactoring for Credential and TokenStore --- lib/src/authentication/credential.dart | 34 +++++++++++++++++-------- lib/src/authentication/token_store.dart | 29 ++++++++++++--------- lib/src/command/token_add.dart | 2 +- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/lib/src/authentication/credential.dart b/lib/src/authentication/credential.dart index d1856ff6f..1798eb66a 100644 --- a/lib/src/authentication/credential.dart +++ b/lib/src/authentication/credential.dart @@ -21,29 +21,34 @@ import '../source/hosted.dart'; /// } /// ``` class Credential { - Credential({required this.url, required this.token, this.unknownFields}); + /// Internal constructor that's only used by [fromJson]. + Credential._internal({ + required this.url, + required this.token, + required this.unknownFields, + }); - /// Create [Credential] instance for bearer tokens. - Credential.bearer(this.url, this.token) : unknownFields = null; + /// Create a new [Credential]. + Credential.token(this.url, this.token) + : unknownFields = const {}; /// Deserialize [json] into [Credential] type. /// /// Throws [FormatException] if [json] is not a valid [Credential]. factory Credential.fromJson(Map json) { if (json['url'] is! String) { - throw FormatException('Url is not provided for the token'); + throw FormatException('Url is not provided for the credential'); } final hostedUrl = validateAndNormalizeHostedUrl(json['url'] as String); - final token = json['token'] is String ? json['token'] as String : null; const knownKeys = {'url', 'token'}; final unknownFields = Map.fromEntries( json.entries.where((kv) => !knownKeys.contains(kv.key))); - return Credential( + return Credential._internal( url: hostedUrl, - token: token, + token: json['token'] is String ? json['token'] as String : null, unknownFields: unknownFields, ); } @@ -57,29 +62,32 @@ class Credential { /// Unknown fields found in 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; + final Map unknownFields; /// Serializes [Credential] into json format. Map toJson() { return { 'url': url.toString(), if (token != null) 'token': token, - if (unknownFields != null) ...unknownFields!, + ...unknownFields, }; } /// Returns future that resolves "Authorization" header value used for /// authenticating. + /// + /// Throws [DataException] if credential is not valid. // This method returns future to make sure in future we could use the // [Credential] interface for OAuth2.0 authentication too - which requires // token rotation (refresh) that's async job. Future getAuthorizationHeaderValue() { - if (token == null) { + if (!isValid()) { throw DataException( 'Saved credential for $url pub repository is not supported by current ' 'version of Dart SDK.', ); } + return Future.value('Bearer $token'); } @@ -89,6 +97,12 @@ class Credential { return _normalizeUrl(url).startsWith(_normalizeUrl(this.url.toString())); } + /// Returns boolean indicates whether or not the credentials is valid. + /// + /// This method might return `false` when a `tokens.json` file created by + /// future SDK used by pub tool from old SDK. + bool isValid() => token != null; + static String _normalizeUrl(String url) { return (url.endsWith('/') ? url : '$url/').toLowerCase(); } diff --git a/lib/src/authentication/token_store.dart b/lib/src/authentication/token_store.dart index 30b5ecf09..daf7cf20d 100644 --- a/lib/src/authentication/token_store.dart +++ b/lib/src/authentication/token_store.dart @@ -62,7 +62,12 @@ class TokenStore { throw FormatException('Invalid or not supported format'); } - result.add(Credential.fromJson(element)); + final credential = Credential.fromJson(element); + result.add(credential); + + if (!credential.isValid()) { + throw FormatException('Invalid or not supported credential'); + } } on FormatException catch (e) { if (element['url'] is String) { log.warning( @@ -126,17 +131,17 @@ class TokenStore { return found; } - /// Returns [Credential] for authenticating given url or null if no matching token - /// is found. - Credential? findCredential(Uri url) { - Credential? matchedToken; - for (final token in credentials) { - if (token.url == url) { - if (matchedToken == null) { - matchedToken = token; + /// Returns [Credential] for authenticating given [hostedUrl] or null if no + /// matching credential is found. + Credential? findCredential(Uri hostedUrl) { + Credential? matchedCredential; + for (final credential in credentials) { + if (credential.url == hostedUrl && credential.isValid()) { + if (matchedCredential == null) { + matchedCredential = credential; } else { log.warning( - 'Found multiple matching authentication tokens for "$url". ' + 'Found multiple matching authentication tokens for "$hostedUrl". ' 'First matching token will be used for authentication.', ); break; @@ -144,13 +149,13 @@ class TokenStore { } } - return matchedToken; + return matchedCredential; } /// Returns whether or not store contains a token that could be used for /// authenticating given [url]. bool hasCredential(Uri url) { - return credentials.any((it) => it.url == url); + return credentials.any((it) => it.url == url && it.isValid()); } /// Deletes tokens.json file from the disk. diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index 1928435e2..1d7a40f44 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -46,7 +46,7 @@ class TokenAddCommand extends PubCommand { usageException('Token is not provided.'); } - tokenStore.addCredential(Credential.bearer(hostedUrl, token)); + tokenStore.addCredential(Credential.token(hostedUrl, token)); log.message( 'Requests to $hostedUrl will now be authenticated using the secret ' 'token.', From 8981fe5b809833d44a6936eead53e1cc4fc31d84 Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 14 Sep 2021 15:50:00 +0400 Subject: [PATCH 76/77] Fixed messages --- lib/src/command/token_add.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/command/token_add.dart b/lib/src/command/token_add.dart index 1d7a40f44..aa2a2f529 100644 --- a/lib/src/command/token_add.dart +++ b/lib/src/command/token_add.dart @@ -37,7 +37,7 @@ class TokenAddCommand extends PubCommand { try { var hostedUrl = validateAndNormalizeHostedUrl(argResults.rest.first); if (hostedUrl.isScheme('HTTP')) { - throw DataException('Unsecure package repository could not be added.'); + throw DataException('Insecure package repository could not be added.'); } final token = await stdinPrompt('Enter secret token:') @@ -58,7 +58,7 @@ class TokenAddCommand extends PubCommand { // Timeout is added to readLine call to make sure automated jobs doesn't // get stuck on noop state if user forget to pipe token to the 'token add' // command. This behavior might be removed. - throw ApplicationException('Token is not provided within 5 minutes.'); + throw ApplicationException('Token is not provided within 15 minutes.'); } } } From 9ac7ff64ec98e46b07ca3a508936edd1dca9971f Mon Sep 17 00:00:00 2001 From: Misir Jafarov Date: Tue, 14 Sep 2021 16:01:59 +0400 Subject: [PATCH 77/77] Fix message on tests --- test/token/add_token_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/add_token_test.dart b/test/token/add_token_test.dart index 8f646ab6b..b10e042fd 100644 --- a/test/token/add_token_test.dart +++ b/test/token/add_token_test.dart @@ -89,7 +89,7 @@ void main() { await d.dir(cachePath).create(); await runPub( args: ['token', 'add', 'http://mypub.com'], - error: contains('Unsecure package repository could not be added.'), + error: contains('Insecure package repository could not be added.'), exitCode: exit_codes.DATA, );