diff --git a/app/lib/frontend/handlers/pubapi.client.dart b/app/lib/frontend/handlers/pubapi.client.dart index 1275375348..16148f2d24 100644 --- a/app/lib/frontend/handlers/pubapi.client.dart +++ b/app/lib/frontend/handlers/pubapi.client.dart @@ -74,11 +74,13 @@ class PubApiClient { ); } - Future> createPublisher(String publisherId) async { - return await _client.requestBytes( + Future<_i3.PublisherInfo> createPublisher( + String publisherId, _i3.CreatePublisherRequest payload) async { + return _i3.PublisherInfo.fromJson(await _client.requestJson( verb: 'post', path: '/api/publishers/$publisherId', - ); + body: payload.toJson(), + )); } Future<_i3.PublisherInfo> publisherInfo(String publisherId) async { diff --git a/app/lib/frontend/handlers/pubapi.dart b/app/lib/frontend/handlers/pubapi.dart index 80c20aa6b1..fe5d20235d 100644 --- a/app/lib/frontend/handlers/pubapi.dart +++ b/app/lib/frontend/handlers/pubapi.dart @@ -11,7 +11,6 @@ import 'account.dart'; import 'custom_api.dart'; import 'listing.dart'; import 'package.dart'; -import 'publisher.dart'; part 'pubapi.g.dart'; @@ -101,8 +100,12 @@ class PubApi { /// Starts publisher creation flow. @EndPoint.post('/api/publishers/') - Future createPublisher(Request request, String publisherId) => - createPublisherApiHandler(request, publisherId); + Future createPublisher( + Request request, + String publisherId, + CreatePublisherRequest body, + ) => + publisherBackend.createPublisher(publisherId, body); /// Returns publisher data in a JSON form. @EndPoint.get('/api/publishers/') diff --git a/app/lib/frontend/handlers/pubapi.g.dart b/app/lib/frontend/handlers/pubapi.g.dart index 27c086aa1b..7a0021a863 100644 --- a/app/lib/frontend/handlers/pubapi.g.dart +++ b/app/lib/frontend/handlers/pubapi.g.dart @@ -99,8 +99,13 @@ Router _$PubApiRouter(PubApi service) { router.add('POST', r'/api/publishers/', (Request request, String publisherId) async { try { - final _$result = await service.createPublisher(request, publisherId); - return _$result; + final _$result = await service.createPublisher( + request, + publisherId, + await $utilities.decodeJson(request, (o) { + return CreatePublisherRequest.fromJson(o); + })); + return $utilities.jsonResponse(_$result.toJson()); } on ApiResponseException catch (e) { return e.asApiResponse(); } catch (e, st) { diff --git a/app/lib/frontend/handlers/publisher.dart b/app/lib/frontend/handlers/publisher.dart index 0419a38a9e..a56170af2a 100644 --- a/app/lib/frontend/handlers/publisher.dart +++ b/app/lib/frontend/handlers/publisher.dart @@ -13,10 +13,3 @@ Future createPublisherPageHandler(shelf.Request request) async { // TODO: implement return notFoundHandler(request); } - -/// Handles requests for POST /api/publisher/ -Future createPublisherApiHandler( - shelf.Request request, String publisherId) async { - // TODO: implement - return notFoundHandler(request); -} diff --git a/app/lib/publisher/backend.dart b/app/lib/publisher/backend.dart index fe16c84671..95b33f515f 100644 --- a/app/lib/publisher/backend.dart +++ b/app/lib/publisher/backend.dart @@ -12,6 +12,7 @@ import '../account/backend.dart'; import '../account/consent_backend.dart'; import '../shared/email.dart'; import '../shared/exceptions.dart'; +import 'domain_verifier.dart' show domainVerifier; import 'models.dart'; @@ -60,6 +61,87 @@ class PublisherBackend { }); } + /// Create publisher. + Future createPublisher( + String publisherId, + api.CreatePublisherRequest body, + ) async { + // Sanity check that domains are: + // - lowercase (because we want that in pub.dev) + // - consist of a-z, 0-9 and dashes + // We do not care if they end in dash, as such domains can't be verified. + InvalidInputException.checkMatchPattern( + publisherId, + 'publisherId', + RegExp(r'^[a-z0-9-]{1,63}\.[a-z0-9-]{1,63}$'), + ); + InvalidInputException.checkStringLength( + publisherId, + 'publisherId', + maximum: 255, // Some upper limit for sanity. + ); + InvalidInputException.checkNotNull(body.accessToken, 'accessToken'); + InvalidInputException.checkStringLength( + body.accessToken, + 'accessToken', + minimum: 1, + maximum: 4096, + ); + + // Verify ownership of domain. + final isOwner = await domainVerifier.verifyDomainOwnership( + publisherId, + body.accessToken, + ); + if (!isOwner) { + throw AuthorizationException.userIsNotDomainOwner(publisherId); + } + + // Create the publisher + final now = DateTime.now().toUtc(); + await _db.withTransaction((tx) async { + final key = _db.emptyKey.append(Publisher, id: publisherId); + final p = (await tx.lookup([key])).single; + if (p != null) { + // Check that publisher is the same as what we would create. + if (p.created.isBefore(now.subtract(Duration(minutes: 10))) || + p.updated.isBefore(now.subtract(Duration(minutes: 10))) || + p.contactEmail != authenticatedUser.email || + p.description != '' || + p.websiteUrl != 'https://$publisherId') { + throw ConflictException.publisherAlreadyExists(publisherId); + } + // Avoid creating the same publisher again, this end-point is idempotent + // if we just do nothing here. + return; + } + + // Create publisher + tx.queueMutations(inserts: [ + Publisher() + ..parentKey = _db.emptyKey + ..id = publisherId + ..created = now + ..description = '' + ..contactEmail = authenticatedUser.email + ..updated = now + ..websiteUrl = 'https://$publisherId', + PublisherMember() + ..parentKey = _db.emptyKey.append(Publisher, id: publisherId) + ..id = authenticatedUser.userId + ..created = now + ..updated = now + ..role = PublisherMemberRole.admin + ]); + await tx.commit(); + }); + + // Return publisher as it was created + final key = _db.emptyKey.append(Publisher, id: publisherId); + final p = (await _db.lookup([key])).single; + return _asPublisherInfo(p); + } + /// Gets the publisher data Future getPublisher(String publisherId) async { final p = await _getPublisher(publisherId); diff --git a/app/lib/publisher/domain_verifier.dart b/app/lib/publisher/domain_verifier.dart new file mode 100644 index 0000000000..2471d4c8d2 --- /dev/null +++ b/app/lib/publisher/domain_verifier.dart @@ -0,0 +1,62 @@ +import 'package:gcloud/service_scope.dart' as ss; +import 'package:googleapis_auth/auth.dart' as auth; +import 'package:googleapis/webmasters/v3.dart' as wmx; +import 'package:http/http.dart' as http; +import 'package:retry/retry.dart' show retry; + +import '../shared/exceptions.dart' show AuthorizationException; + +/// Sets the [DomainVerifier] service. +void registerDomainVerifier(DomainVerifier domainVerifier) => + ss.register(#_domainVerifier, domainVerifier); + +/// The active [DomainVerifier] service. +DomainVerifier get domainVerifier => + ss.lookup(#_domainVerifier) as DomainVerifier; + +/// Service that can verify ownership of a domain by asking Search Console +/// through Webmaster API v3. +/// +/// Please obtain instances of this from [domainVerifier], as this allows for +/// dependency injection during testing. +class DomainVerifier { + /// Verify ownership of [domain] using [accessToken] which has the read-only + /// scope for Search Console API. + Future verifyDomainOwnership(String domain, String accessToken) async { + // Create client for talking to Webmasters API: + // https://developers.google.com/webmaster-tools/search-console-api-original/v3/parameters + final client = auth.authenticatedClient( + http.Client(), + auth.AccessCredentials( + auth.AccessToken( + 'Bearer', + accessToken, + DateTime.now().toUtc().add(Duration(minutes: 20)), // avoid refresh + ), + null, + [wmx.WebmastersApi.WebmastersReadonlyScope], + ), + ); + try { + // Request list of sites/domains from the Search Console API. + final sites = await retry( + () => wmx.WebmastersApi(client).sites.list(), + maxAttempts: 3, + maxDelay: Duration(milliseconds: 500), + retryIf: (e) => e is! auth.AccessDeniedException, + ); + // Determine if the user is in fact owner of the domain in question. + // Note. domains are prefixed 'sc-domain:' and 'siteOwner' is the only + // permission that ensures the user actually did DNS verification. + return sites.siteEntry.any( + (s) => + s.siteUrl.toLowerCase() == 'sc-domain:$domain' && + s.permissionLevel == 'siteOwner', // must be 'siteOwner'! + ); + } on auth.AccessDeniedException { + throw AuthorizationException.missingSearchConsoleReadAccess(); + } finally { + client.close(); + } + } +} diff --git a/app/lib/shared/exceptions.dart b/app/lib/shared/exceptions.dart index 3183ebf329..8c8a14f836 100644 --- a/app/lib/shared/exceptions.dart +++ b/app/lib/shared/exceptions.dart @@ -165,7 +165,7 @@ class AuthenticationException extends ResponseException /// /// Example: /// * Modifying a package for which the user doesn't have permissions, -/// * Creating a package without domain validation. +/// * Creating a publisher without domain validation. class AuthorizationException extends ResponseException implements UnauthorizedAccessException { AuthorizationException._(String message) @@ -201,6 +201,39 @@ class AuthorizationException extends ResponseException 'package `$publisher`.', ); + static final _domainVerificationUrl = + Uri.parse('https://www.google.com/webmasters/verification/verification'); + + /// Signaling that the user is not a verified owner of the [domain] for which + /// the user is trying to create a publisher. + factory AuthorizationException.userIsNotDomainOwner(String domain) => + AuthorizationException._([ + 'Insufficient permissions to create publisher `$domain`, to create ', + 'this publisher the domain `$domain` must be _verified_ in the ', + '[search console](https://search.google.com/search-console/welcome).', + '', + 'It is not sufficient to be granted access to the domain, the domain ', + 'must be verified with the Google account used to created the ', + 'publisher. It is also insufficient to verify a URL or URL prefix,', + 'the domain must be verified with a **DNS record**.', + '', + 'Open domain verification flow.', + '', + 'Note, once the publisher is created the domain verification need not', + 'remain in place. This is only required for publisher creation.', + ].join('\n')); + + /// Signaling that the user did not grant read-only access to the + /// search console, making it impossible for the server to verify the users + /// domain ownership. + factory AuthorizationException.missingSearchConsoleReadAccess() => + AuthorizationException._([ + 'Read-only access to Search Console data was not granted, preventing', + '`pub.dev` from verifying that you own the domain.', + ].join('\n')); + @override String toString() => '$code: $message'; // used by package:pub_server } @@ -225,6 +258,11 @@ class ConflictException extends ResponseException { /// The active user can't update their own role. factory ConflictException.cantUpdateOwnRole() => ConflictException._('User can\'t update their own role.'); + + /// The user is trying to create a publisher that already exists. + factory ConflictException.publisherAlreadyExists(String domain) => + ConflictException._( + 'A publisher with the domain `$domain` already exists'); } /// Thrown when the analysis for a package is not done yet. diff --git a/app/lib/shared/services.dart b/app/lib/shared/services.dart index 7ec3a40232..cf48c6faf1 100644 --- a/app/lib/shared/services.dart +++ b/app/lib/shared/services.dart @@ -14,6 +14,7 @@ import '../frontend/search_service.dart'; import '../history/backend.dart'; import '../job/backend.dart'; import '../publisher/backend.dart'; +import '../publisher/domain_verifier.dart'; import '../scorecard/backend.dart'; import '../search/backend.dart'; import '../search/index_simple.dart'; @@ -71,6 +72,7 @@ Future withPubServices(FutureOr Function() fn) async { PopularityStorage(await getOrCreateBucket( storageService, activeConfiguration.popularityDumpBucketName)), ); + registerDomainVerifier(DomainVerifier()); registerPublisherBackend(PublisherBackend(dbService)); registerScoreCardBackend(ScoreCardBackend(dbService)); registerSearchBackend(SearchBackend(dbService)); diff --git a/app/test/publisher/api_test.dart b/app/test/publisher/api_test.dart index 692f788884..249201b96b 100644 --- a/app/test/publisher/api_test.dart +++ b/app/test/publisher/api_test.dart @@ -33,6 +33,55 @@ void main() { }); }); + group('Create publisher', () { + testWithServices('verified.com', () async { + final api = createPubApiClient(authToken: hansAuthenticated.userId); + + // Check that we can create the publisher + final r1 = await api.createPublisher( + 'verified.com', + CreatePublisherRequest(accessToken: 'dummy-token-for-testing'), + ); + expect(r1.contactEmail, hansAuthenticated.email); + + // Check that creating again idempotently works too + final r2 = await api.createPublisher( + 'verified.com', + CreatePublisherRequest(accessToken: 'dummy-token-for-testing'), + ); + expect(r2.contactEmail, hansAuthenticated.email); + + // Check that we can update the description + final r3 = await api.updatePublisher( + 'verified.com', + UpdatePublisherRequest(description: 'hello-world'), + ); + expect(r3.description, 'hello-world'); + + // Check that we get a sane result from publisherInfo + final r4 = await api.publisherInfo('verified.com'); + expect(r4.toJson(), { + 'description': 'hello-world', + 'contactEmail': hansAuthenticated.email, + }); + }); + + testWithServices('notverified.com', () async { + final api = createPubApiClient(authToken: hansAuthenticated.userId); + + // Check that we can create the publisher + final rs = api.createPublisher( + 'notverified.com', + CreatePublisherRequest(accessToken: 'dummy-token-for-testing'), + ); + await expectApiException( + rs, + status: 403, + code: 'InsufficientPermissions', + ); + }); + }); + group('Update description', () { _testAdminAuthIssues( (client) => client.updatePublisher( diff --git a/app/test/shared/test_services.dart b/app/test/shared/test_services.dart index d87a12fcd1..c789aa0b38 100644 --- a/app/test/shared/test_services.dart +++ b/app/test/shared/test_services.dart @@ -2,6 +2,8 @@ // 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:fake_gcloud/mem_datastore.dart'; import 'package:fake_gcloud/mem_storage.dart'; import 'package:gcloud/db.dart'; @@ -16,11 +18,14 @@ import 'package:pub_dartlang_org/account/backend.dart'; import 'package:pub_dartlang_org/account/testing/fake_auth_provider.dart'; import 'package:pub_dartlang_org/frontend/handlers.dart'; import 'package:pub_dartlang_org/frontend/handlers/pubapi.client.dart'; +import 'package:pub_dartlang_org/publisher/domain_verifier.dart'; import 'package:pub_dartlang_org/scorecard/backend.dart'; import 'package:pub_dartlang_org/search/backend.dart'; import 'package:pub_dartlang_org/search/handlers.dart'; import 'package:pub_dartlang_org/search/index_simple.dart'; import 'package:pub_dartlang_org/shared/configuration.dart'; +import 'package:pub_dartlang_org/shared/exceptions.dart' + show AuthorizationException; import 'package:pub_dartlang_org/shared/handler_helpers.dart'; import 'package:pub_dartlang_org/shared/popularity_storage.dart'; import 'package:pub_dartlang_org/shared/redis_cache.dart'; @@ -34,6 +39,8 @@ import 'test_models.dart'; /// and fake storage) for tests. void testWithServices(String name, Future fn()) { scopedTest(name, () async { + _setupLogging(); + // registering config with bad ports, as we won't access them via network registerActiveConfiguration(Configuration.fakePubServer( port: 0, @@ -78,6 +85,7 @@ void testWithServices(String name, Future fn()) { await fork(() async { registerAccountBackend( AccountBackend(db, authProvider: FakeAuthProvider())); + registerDomainVerifier(_FakeDomainVerifier()); await dartSdkIndex.merge(); @@ -153,3 +161,37 @@ http_testing.MockClientHandler _wrapShelfHandler( ); }; } + +/// Fake implementation of [DomainVerifier] for testing. +class _FakeDomainVerifier implements DomainVerifier { + @override + Future verifyDomainOwnership(String domain, String accessToken) async { + if (domain == 'verified.com') { + return true; + } + if (domain == 'notverified.net') { + return false; + } + throw AuthorizationException.missingSearchConsoleReadAccess(); + } +} + +bool _loggingDone = false; + +/// Setup logging if environment variable `DEBUG` is defined. +void _setupLogging() { + if (_loggingDone) { + return; + } + _loggingDone = true; + if ((Platform.environment['DEBUG'] ?? '') == '') { + return; + } + Logger.root.level = Level.ALL; + Logger.root.onRecord.listen((LogRecord rec) { + print('${rec.level.name}: ${rec.time}: ${rec.message}'); + if (rec.error != null) { + print('ERROR: ${rec.error}, ${rec.stackTrace}'); + } + }); +} diff --git a/pkg/web_app/lib/src/google_auth_js.dart b/pkg/web_app/lib/src/google_auth_js.dart index 5abe3ae30c..623eb23123 100644 --- a/pkg/web_app/lib/src/google_auth_js.dart +++ b/pkg/web_app/lib/src/google_auth_js.dart @@ -58,6 +58,11 @@ abstract class GoogleAuth { /// sign-out is finalized. external dynamic signOut(); + /// Revokes all of the scopes that the user granted. + /// + /// (Useful when debugging oauth2 scope grant flow). + external void disconnect(); + /// Calls the onInit function when the GoogleAuth object is fully initialized. /// If an error is raised while initializing, the onError function will be /// called instead.