From 6bd343ec9eaf3518fc4e23b424c8961e22a266c5 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Fri, 4 Oct 2024 17:11:48 +0200 Subject: [PATCH 1/4] ReservedPackage + admin action. --- app/lib/admin/actions/actions.dart | 2 + app/lib/admin/actions/package_reserve.dart | 61 ++++++++++++ app/lib/package/backend.dart | 37 ++++++-- app/lib/package/models.dart | 22 +++++ app/test/admin/package_reserve_test.dart | 102 +++++++++++++++++++++ 5 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 app/lib/admin/actions/package_reserve.dart create mode 100644 app/test/admin/package_reserve_test.dart diff --git a/app/lib/admin/actions/actions.dart b/app/lib/admin/actions/actions.dart index 833ea85fa1..01e81f6ef2 100644 --- a/app/lib/admin/actions/actions.dart +++ b/app/lib/admin/actions/actions.dart @@ -18,6 +18,7 @@ import 'moderation_case_list.dart'; import 'moderation_case_resolve.dart'; import 'moderation_case_update.dart'; import 'package_info.dart'; +import 'package_reserve.dart'; import 'package_version_info.dart'; import 'package_version_retraction.dart'; import 'publisher_block.dart'; @@ -98,6 +99,7 @@ final class AdminAction { moderationCaseResolve, moderationCaseUpdate, packageInfo, + packageReserve, packageVersionInfo, packageVersionRetraction, publisherBlock, diff --git a/app/lib/admin/actions/package_reserve.dart b/app/lib/admin/actions/package_reserve.dart new file mode 100644 index 0000000000..c4a53fc0d5 --- /dev/null +++ b/app/lib/admin/actions/package_reserve.dart @@ -0,0 +1,61 @@ +// Copyright (c) 2024, 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:pub_dev/package/backend.dart'; +import 'package:pub_dev/package/models.dart'; +import 'package:pub_dev/shared/datastore.dart'; + +import 'actions.dart'; + +final packageReserve = AdminAction( + name: 'package-reserve', + summary: 'Creates a ReservedPackage entity.', + description: ''' +Reserves a package name that can be claimed by @google.com accounts or +the allowed list of email addresses. + +The action can be re-run with the same package name. In such cases the provided +email list will be added to the existing ReservedPackage entity. +''', + options: { + 'package': 'The package name to be reserved.', + 'emails': 'The list of email addresses, separated by comma.' + }, + invoke: (options) async { + final package = options['package']; + InvalidInputException.check( + package != null && package.isNotEmpty, + '`package` must be given', + ); + final emails = options['emails']?.split(','); + + final p = await packageBackend.lookupPackage(package!); + if (p != null) { + throw InvalidInputException('Package `$package` exists.'); + } + final mp = await packageBackend.lookupModeratedPackage(package); + if (mp != null) { + throw InvalidInputException('ModeratedPackage `$package` exists.'); + } + + final entry = await withRetryTransaction(dbService, (tx) async { + final existing = await tx.lookupOrNull( + dbService.emptyKey.append(ReservedPackage, id: package)); + final entry = existing ?? ReservedPackage.init(package); + if (emails != null) { + entry.emails = {...entry.emails, ...emails}.toList(); + } + tx.insert(entry); + return entry; + }); + + return { + 'ReservedPackage': { + 'name': entry.name, + 'created': entry.created.toIso8601String(), + 'emails': entry.emails, + }, + }; + }, +); diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index 3f8f4a2007..b87d1771b8 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -183,6 +183,14 @@ class PackageBackend { return await db.lookupOrNull(packageKey); } + /// Looks up a reserved package by name. + /// + /// Returns `null` if the package doesn't exist. + Future lookupReservedPackage(String packageName) async { + final packageKey = db.emptyKey.append(ReservedPackage, id: packageName); + return await db.lookupOrNull(packageKey); + } + /// Looks up a package by name. Future> lookupPackages(Iterable packageNames) async { return (await db.lookup(packageNames @@ -1014,10 +1022,19 @@ class PackageBackend { required String name, required AuthenticatedAgent agent, }) async { - final isGoogleComUser = - agent is AuthenticatedUser && agent.user.email!.endsWith('@google.com'); - final isReservedName = matchesReservedPackageName(name); - final isExempted = isGoogleComUser && isReservedName; + final reservedPackage = await lookupReservedPackage(name); + final reservedEmails = reservedPackage?.emails ?? const []; + + bool isAllowedUser = false; + if (agent is AuthenticatedUser) { + final email = agent.user.email; + isAllowedUser = email != null && + (email.endsWith('@google.com') || reservedEmails.contains(email)); + } + + final isReservedName = + reservedPackage != null || matchesReservedPackageName(name); + final isExempted = isReservedName && isAllowedUser; final conflictingName = await nameTracker.accept(name); if (conflictingName != null && !isExempted) { @@ -1039,8 +1056,8 @@ class PackageBackend { throw PackageRejectedException(newNameIssues.first.message); } - // reserved package names for the Dart team - if (isReservedName && !isGoogleComUser) { + // reserved package names for the Dart team or allowlisted users + if (isReservedName && !isAllowedUser) { throw PackageRejectedException.nameReserved(name); } } @@ -1125,6 +1142,14 @@ class PackageBackend { throw PackageRejectedException.nameReserved(newVersion.package); } + if (isNew) { + final reservedPackage = await tx.lookupOrNull( + db.emptyKey.append(ReservedPackage, id: newVersion.package)); + if (reservedPackage != null) { + tx.delete(reservedPackage.key); + } + } + // If the version already exists, we fail. if (version != null) { _logger.info('Version ${version.version} of package ' diff --git a/app/lib/package/models.dart b/app/lib/package/models.dart index 9cf1165d35..89c10e598c 100644 --- a/app/lib/package/models.dart +++ b/app/lib/package/models.dart @@ -875,6 +875,28 @@ class ModeratedPackage extends db.ExpandoModel { List? versions; } +/// Entity representing a reserved package: the name is available only +/// for a subset of the users (`@google.com` + list of [emails]). +@db.Kind(name: 'ReservedPackage', idType: db.IdType.String) +class ReservedPackage extends db.ExpandoModel { + @db.StringProperty(required: true) + String? name; + + @db.DateTimeProperty() + late DateTime created; + + /// List of email addresses that are allowed to claim this package name. + /// This is on top of the `@google.com` email addresses. + @db.StringListProperty() + List emails = []; + + ReservedPackage(); + ReservedPackage.init(this.name) { + id = name; + created = clock.now().toUtc(); + } +} + /// An identifier to point to a specific [package] and [version]. class QualifiedVersionKey { final String? package; diff --git a/app/test/admin/package_reserve_test.dart b/app/test/admin/package_reserve_test.dart new file mode 100644 index 0000000000..13ce53dcc4 --- /dev/null +++ b/app/test/admin/package_reserve_test.dart @@ -0,0 +1,102 @@ +// Copyright (c) 2024, 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:_pub_shared/data/admin_api.dart'; +import 'package:clock/clock.dart'; +import 'package:pub_dev/fake/backend/fake_auth_provider.dart'; +import 'package:pub_dev/package/backend.dart'; +import 'package:pub_dev/package/models.dart'; +import 'package:pub_dev/shared/datastore.dart'; +import 'package:test/test.dart'; + +import '../package/backend_test_utils.dart'; +import '../shared/handlers_test_utils.dart'; +import '../shared/test_models.dart'; +import '../shared/test_services.dart'; + +void main() { + group('Reserve package', () { + Future _reserve( + String package, { + List? emails, + }) async { + final api = createPubApiClient(authToken: siteAdminToken); + await api.adminInvokeAction( + 'package-reserve', + AdminInvokeActionArguments(arguments: { + 'package': package, + if (emails != null) 'emails': emails.join(','), + }), + ); + } + + testWithProfile('cannot reserve existing package', fn: () async { + await expectApiException( + _reserve('oxygen'), + code: 'InvalidInput', + status: 400, + message: 'Package `oxygen` exists.', + ); + }); + + testWithProfile('cannot reserve ModeratedPackage', fn: () async { + await dbService.commit(inserts: [ + ModeratedPackage() + ..id = 'pkg' + ..name = 'pkg' + ..moderated = clock.now() + ..uploaders = [] + ..versions = ['1.0.0'] + ]); + await expectApiException( + _reserve('pkg'), + code: 'InvalidInput', + status: 400, + message: 'ModeratedPackage `pkg` exists.', + ); + }); + + testWithProfile('prevents non-whitelisted publishing', fn: () async { + await _reserve('pkg'); + + final pubspecContent = generatePubspecYaml('pkg', '1.0.0'); + final bytes = await packageArchiveBytes(pubspecContent: pubspecContent); + await expectApiException( + createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes(bytes), + code: 'PackageRejected', + status: 400, + message: 'Package name pkg is reserved.', + ); + }); + + testWithProfile('allows whitelisted publishing', fn: () async { + await _reserve('pkg'); + // update email addresses in a second request + await _reserve('pkg', emails: ['admin@pub.dev']); + + final pubspecContent = generatePubspecYaml('pkg', '1.0.0'); + final bytes = await packageArchiveBytes(pubspecContent: pubspecContent); + await createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes(bytes); + + final rp = await packageBackend.lookupReservedPackage('pkg'); + expect(rp, isNull); + }); + + testWithProfile('allows Dart-team publishing', fn: () async { + await _reserve('pkg'); + + final pubspecContent = generatePubspecYaml('pkg', '1.0.0'); + final bytes = await packageArchiveBytes(pubspecContent: pubspecContent); + await createPubApiClient( + authToken: createFakeAuthTokenForEmail('developer@google.com', + audience: 'fake-client-audience')) + .uploadPackageBytes(bytes); + + final rp = await packageBackend.lookupReservedPackage('pkg'); + expect(rp, isNull); + }); + }); +} From 855fd172313ced1afb286a62aa5d134566bbeb0c Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Mon, 7 Oct 2024 15:48:21 +0200 Subject: [PATCH 2/4] Only update email list, no google.com exemption --- app/lib/admin/actions/package_reserve.dart | 15 ++++++++------- app/lib/package/backend.dart | 9 ++++++--- app/test/admin/package_reserve_test.dart | 16 ++++++++-------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/lib/admin/actions/package_reserve.dart b/app/lib/admin/actions/package_reserve.dart index c4a53fc0d5..3371e362d5 100644 --- a/app/lib/admin/actions/package_reserve.dart +++ b/app/lib/admin/actions/package_reserve.dart @@ -12,11 +12,14 @@ final packageReserve = AdminAction( name: 'package-reserve', summary: 'Creates a ReservedPackage entity.', description: ''' -Reserves a package name that can be claimed by @google.com accounts or -the allowed list of email addresses. +Reserves a package name that can be claimed by the specified list of email addresses. -The action can be re-run with the same package name. In such cases the provided -email list will be added to the existing ReservedPackage entity. +The action can be re-run with the same package name. In such cases the previous +email list will be discarded, and the specified email list will be updated to the +existing ReservedPackage entity. + +When no emails are specified, the package will be reserved, but no user may be +able to claim it. ''', options: { 'package': 'The package name to be reserved.', @@ -43,9 +46,7 @@ email list will be added to the existing ReservedPackage entity. final existing = await tx.lookupOrNull( dbService.emptyKey.append(ReservedPackage, id: package)); final entry = existing ?? ReservedPackage.init(package); - if (emails != null) { - entry.emails = {...entry.emails, ...emails}.toList(); - } + entry.emails = {...?emails}.toList(); tx.insert(entry); return entry; }); diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index b87d1771b8..8eb5cf9f2c 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -1023,13 +1023,16 @@ class PackageBackend { required AuthenticatedAgent agent, }) async { final reservedPackage = await lookupReservedPackage(name); - final reservedEmails = reservedPackage?.emails ?? const []; bool isAllowedUser = false; if (agent is AuthenticatedUser) { final email = agent.user.email; - isAllowedUser = email != null && - (email.endsWith('@google.com') || reservedEmails.contains(email)); + if (reservedPackage != null) { + final reservedEmails = reservedPackage.emails; + isAllowedUser = email != null && reservedEmails.contains(email); + } else { + isAllowedUser = email != null && email.endsWith('@google.com'); + } } final isReservedName = diff --git a/app/test/admin/package_reserve_test.dart b/app/test/admin/package_reserve_test.dart index 13ce53dcc4..28b78490bd 100644 --- a/app/test/admin/package_reserve_test.dart +++ b/app/test/admin/package_reserve_test.dart @@ -85,18 +85,18 @@ void main() { expect(rp, isNull); }); - testWithProfile('allows Dart-team publishing', fn: () async { + testWithProfile('no longer allows Dart-team exemption', fn: () async { await _reserve('pkg'); final pubspecContent = generatePubspecYaml('pkg', '1.0.0'); final bytes = await packageArchiveBytes(pubspecContent: pubspecContent); - await createPubApiClient( - authToken: createFakeAuthTokenForEmail('developer@google.com', - audience: 'fake-client-audience')) - .uploadPackageBytes(bytes); - - final rp = await packageBackend.lookupReservedPackage('pkg'); - expect(rp, isNull); + await expectApiException( + createPubApiClient(authToken: adminClientToken) + .uploadPackageBytes(bytes), + code: 'PackageRejected', + status: 400, + message: 'Package name pkg is reserved.', + ); }); }); } From 037b3a388ee3ba9358995af72f329eab02be1249 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Mon, 7 Oct 2024 16:01:25 +0200 Subject: [PATCH 3/4] remove unused import --- app/test/admin/package_reserve_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/app/test/admin/package_reserve_test.dart b/app/test/admin/package_reserve_test.dart index 28b78490bd..c637f86d26 100644 --- a/app/test/admin/package_reserve_test.dart +++ b/app/test/admin/package_reserve_test.dart @@ -4,7 +4,6 @@ import 'package:_pub_shared/data/admin_api.dart'; import 'package:clock/clock.dart'; -import 'package:pub_dev/fake/backend/fake_auth_provider.dart'; import 'package:pub_dev/package/backend.dart'; import 'package:pub_dev/package/models.dart'; import 'package:pub_dev/shared/datastore.dart'; From c1895cfffc4621e4d6a283828ab1ab8e84f81265 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Mon, 7 Oct 2024 16:27:43 +0200 Subject: [PATCH 4/4] Rename action to package-reservation-create --- app/lib/admin/actions/actions.dart | 4 ++-- .../{package_reserve.dart => package_reservation_create.dart} | 4 ++-- ...ackage_reserve_test.dart => package_reservation_test.dart} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename app/lib/admin/actions/{package_reserve.dart => package_reservation_create.dart} (96%) rename app/test/admin/{package_reserve_test.dart => package_reservation_test.dart} (98%) diff --git a/app/lib/admin/actions/actions.dart b/app/lib/admin/actions/actions.dart index 01e81f6ef2..b97439f4b3 100644 --- a/app/lib/admin/actions/actions.dart +++ b/app/lib/admin/actions/actions.dart @@ -18,7 +18,7 @@ import 'moderation_case_list.dart'; import 'moderation_case_resolve.dart'; import 'moderation_case_update.dart'; import 'package_info.dart'; -import 'package_reserve.dart'; +import 'package_reservation_create.dart'; import 'package_version_info.dart'; import 'package_version_retraction.dart'; import 'publisher_block.dart'; @@ -99,7 +99,7 @@ final class AdminAction { moderationCaseResolve, moderationCaseUpdate, packageInfo, - packageReserve, + packageReservationCreate, packageVersionInfo, packageVersionRetraction, publisherBlock, diff --git a/app/lib/admin/actions/package_reserve.dart b/app/lib/admin/actions/package_reservation_create.dart similarity index 96% rename from app/lib/admin/actions/package_reserve.dart rename to app/lib/admin/actions/package_reservation_create.dart index 3371e362d5..f06cce1d75 100644 --- a/app/lib/admin/actions/package_reserve.dart +++ b/app/lib/admin/actions/package_reservation_create.dart @@ -8,8 +8,8 @@ import 'package:pub_dev/shared/datastore.dart'; import 'actions.dart'; -final packageReserve = AdminAction( - name: 'package-reserve', +final packageReservationCreate = AdminAction( + name: 'package-reservation-create', summary: 'Creates a ReservedPackage entity.', description: ''' Reserves a package name that can be claimed by the specified list of email addresses. diff --git a/app/test/admin/package_reserve_test.dart b/app/test/admin/package_reservation_test.dart similarity index 98% rename from app/test/admin/package_reserve_test.dart rename to app/test/admin/package_reservation_test.dart index c637f86d26..aa18947716 100644 --- a/app/test/admin/package_reserve_test.dart +++ b/app/test/admin/package_reservation_test.dart @@ -22,7 +22,7 @@ void main() { }) async { final api = createPubApiClient(authToken: siteAdminToken); await api.adminInvokeAction( - 'package-reserve', + 'package-reservation-create', AdminInvokeActionArguments(arguments: { 'package': package, if (emails != null) 'emails': emails.join(','),