diff --git a/app/config/dartlang-pub.yaml b/app/config/dartlang-pub.yaml index 87432b6243..372de0ca23 100644 --- a/app/config/dartlang-pub.yaml +++ b/app/config/dartlang-pub.yaml @@ -68,6 +68,10 @@ tools: previewDartSdkPath: '/tool/preview/dart-sdk' previewFlutterSdkPath: '/tool/preview/flutter' rateLimits: + - operation: package-created + scope: user + burst: 4 + daily: 12 - operation: package-published scope: package burst: 3 diff --git a/app/lib/audit/models.dart b/app/lib/audit/models.dart index 01023bbe8c..ce49d383ee 100644 --- a/app/lib/audit/models.dart +++ b/app/lib/audit/models.dart @@ -218,30 +218,39 @@ class AuditLogRecord extends db.ExpandoModel { ..publishers = [if (publisherId != null) publisherId]; } - factory AuditLogRecord.packagePublished({ + static Map _dataForPublishing({ + required AuthenticatedAgent uploader, + }) { + if (uploader is AuthenticatedGithubAction) { + final runId = uploader.payload.runId; + final sha = uploader.payload.sha; + return { + 'repository': uploader.payload.repository, + if (runId != null) 'run_id': runId, + if (sha != null) 'sha': sha, + }; + } else { + return const {}; + } + } + + static String _summaryForPublishing({ required AuthenticatedAgent uploader, required String package, - required String version, - required DateTime created, + String? version, String? publisherId, }) { - final summaryParts = [ - 'Package `$package` version `$version`', + final prefix = [ + 'Package `$package`', + if (version != null) ' version `$version`', if (publisherId != null) ' owned by publisher `$publisherId`', ]; - var fields = const {}; - late String summary; if (uploader is AuthenticatedGithubAction) { final repository = uploader.payload.repository; final runId = uploader.payload.runId; final sha = uploader.payload.sha; - fields = { - 'repository': repository, - if (runId != null) 'run_id': runId, - if (sha != null) 'sha': sha, - }; - summary = [ - ...summaryParts, + return [ + ...prefix, ' was published from GitHub Actions', if (runId != null) ' (`run_id`: [`$runId`](https://github.com/$repository/actions/runs/$runId))', @@ -250,29 +259,70 @@ class AuditLogRecord extends db.ExpandoModel { ' to the `$repository` repository.', ].join(); } else if (uploader is AuthenticatedGcpServiceAccount) { - summary = [ - ...summaryParts, + return [ + ...prefix, ' was published by Google Cloud service account: `${uploader.payload.email}`.' ].join(); } else { - summary = [ - ...summaryParts, + return [ + ...prefix, ' was published by `${uploader.displayId}`.', ].join(); } + } + + factory AuditLogRecord.packageCreated({ + required AuthenticatedAgent uploader, + required String package, + required DateTime created, + String? publisherId, + }) { + return AuditLogRecord._init() + ..created = created + ..kind = AuditLogRecordKind.packageCreated + ..agent = uploader.agentId + ..summary = _summaryForPublishing( + uploader: uploader, + package: package, + publisherId: publisherId, + ) + ..data = { + 'package': package, + if (uploader.email != null) 'email': uploader.email, + if (publisherId != null) 'publisherId': publisherId, + ..._dataForPublishing(uploader: uploader), + } + ..users = [if (uploader is AuthenticatedUser) uploader.user.userId] + ..packages = [package] + ..packageVersions = [] + ..publishers = [if (publisherId != null) publisherId]; + } + + factory AuditLogRecord.packagePublished({ + required AuthenticatedAgent uploader, + required String package, + required String version, + required DateTime created, + String? publisherId, + }) { return AuditLogRecord() ..id = createUuid() ..created = created ..expires = auditLogRecordExpiresInFarFuture ..kind = AuditLogRecordKind.packagePublished ..agent = uploader.agentId - ..summary = summary + ..summary = _summaryForPublishing( + uploader: uploader, + package: package, + version: version, + publisherId: publisherId, + ) ..data = { 'package': package, 'version': version, if (uploader.email != null) 'email': uploader.email, if (publisherId != null) 'publisherId': publisherId, - ...fields, + ..._dataForPublishing(uploader: uploader), } ..users = [if (uploader is AuthenticatedUser) uploader.user.userId] ..packages = [package] @@ -730,6 +780,11 @@ abstract class AuditLogRecordKind { /// Event that a package version was updated with new options static const packageVersionOptionsUpdated = 'package-version-options-updated'; + /// Event that a new package was created. This event is created alonside the + /// [packagePublished] event, but is only kept until the default expirty period + /// (to enforce rate limits). + static const packageCreated = 'package-created'; + /// Event that a package was published. /// /// This can be an entirely new package or just a new version to an existing package. diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index 4e67e6e889..5e87b2eef8 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -1008,6 +1008,7 @@ class PackageBackend { final newVersion = entities.packageVersion; final currentDartSdk = await getDartSdkVersion(); final existingPackage = await lookupPackage(newVersion.package); + final isNew = existingPackage == null; // check authorizations before the transaction await _requireUploadAuthorization( @@ -1036,6 +1037,7 @@ class PackageBackend { await verifyPackageUploadRateLimit( agent: agent, package: newVersion.package, + isNew: isNew, ); final email = createPackageUploadedEmail( @@ -1128,6 +1130,13 @@ class PackageBackend { ...entities.assets, if (activeConfiguration.isPublishedEmailNotificationEnabled) outgoingEmail, + if (isNew) + AuditLogRecord.packageCreated( + uploader: agent, + package: newVersion.package, + created: newVersion.created!, + publisherId: package!.publisherId, + ), AuditLogRecord.packagePublished( uploader: agent, package: newVersion.package, diff --git a/app/lib/service/rate_limit/rate_limit.dart b/app/lib/service/rate_limit/rate_limit.dart index 15089a783f..ecee04b402 100644 --- a/app/lib/service/rate_limit/rate_limit.dart +++ b/app/lib/service/rate_limit/rate_limit.dart @@ -17,25 +17,36 @@ import '../../shared/redis_cache.dart'; Future verifyPackageUploadRateLimit({ required AuthenticatedAgent agent, required String package, + required bool isNew, }) async { - final operation = AuditLogRecordKind.packagePublished; + final packagePublishedOp = AuditLogRecordKind.packagePublished; if (agent is AuthenticatedUser) { await _verifyRateLimit( - rateLimit: _getRateLimit(operation, RateLimitScope.user), + rateLimit: _getRateLimit(packagePublishedOp, RateLimitScope.user), userId: agent.userId, ); + + if (isNew) { + await _verifyRateLimit( + rateLimit: _getRateLimit( + AuditLogRecordKind.packageCreated, + RateLimitScope.user, + ), + userId: agent.userId, + ); + } } else { // apply per-user rate limit on non-user agents as they were package-specific limits await _verifyRateLimit( - rateLimit: _getRateLimit(operation, RateLimitScope.user), + rateLimit: _getRateLimit(packagePublishedOp, RateLimitScope.user), package: package, ); } // regular package-specific limits await _verifyRateLimit( - rateLimit: _getRateLimit(operation, RateLimitScope.package), + rateLimit: _getRateLimit(packagePublishedOp, RateLimitScope.package), package: package, ); } diff --git a/app/test/frontend/golden/my_activity_log_page.html b/app/test/frontend/golden/my_activity_log_page.html index 88357ad922..58787aaa82 100644 --- a/app/test/frontend/golden/my_activity_log_page.html +++ b/app/test/frontend/golden/my_activity_log_page.html @@ -236,6 +236,22 @@

admin

+ + + %%x-ago%% + + +
+

+ Package + oxygen + was published by + admin@pub.dev + . +

+
+ + %%x-ago%% @@ -272,6 +288,22 @@

admin

+ + + %%x-ago%% + + +
+

+ Package + neon + was published by + admin@pub.dev + . +

+
+ + %%x-ago%% @@ -290,6 +322,22 @@

admin

+ + + %%x-ago%% + + +
+

+ Package + flutter_titanium + was published by + admin@pub.dev + . +

+
+ + %%x-ago%% diff --git a/app/test/frontend/golden/pkg_activity_log_page.html b/app/test/frontend/golden/pkg_activity_log_page.html index 85edad8d6c..5a4457be03 100644 --- a/app/test/frontend/golden/pkg_activity_log_page.html +++ b/app/test/frontend/golden/pkg_activity_log_page.html @@ -263,10 +263,6 @@

Metadata

- - - Only package publication events are retained past 2 months. - %%x-ago%% @@ -303,6 +299,26 @@

Metadata

+ + + %%x-ago%% + + +
+

+ Package + oxygen + was published by + admin@pub.dev + . +

+
+ + + + + Only package publication events are retained past 2 months. + %%x-ago%% @@ -323,7 +339,7 @@

Metadata

- %%x-ago%% + %%x-ago%%
diff --git a/app/test/package/upload_rate_limit_test.dart b/app/test/package/upload_rate_limit_test.dart index 1ef4a5ab66..affce47040 100644 --- a/app/test/package/upload_rate_limit_test.dart +++ b/app/test/package/upload_rate_limit_test.dart @@ -24,10 +24,11 @@ void main() { group('Upload rate limit', () { Future upload({ + String package = 'new_package', required String version, required Duration time, }) async { - final pubspecContent = '''name: new_package + final pubspecContent = '''name: $package version: $version author: $userAtPubDevEmail homepage: https://github.com/example/package @@ -63,7 +64,7 @@ environment: } testWithProfile( - '1 per minute', + 'new versions 1 per minute', testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail), fn: () async { await _withRateLimits( @@ -94,7 +95,7 @@ environment: ); testWithProfile( - '12 per hour', + 'new versions 12 per hour', testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail), fn: () async { await _withRateLimits( @@ -130,7 +131,7 @@ environment: ); testWithProfile( - '24 per day', + 'new versions 24 per day', testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail), fn: () async { await _withRateLimits( @@ -164,5 +165,40 @@ environment: ); }, ); + + testWithProfile( + 'new packages 1 per minute', + testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail), + fn: () async { + await _withRateLimits( + [ + RateLimit( + operation: AuditLogRecordKind.packageCreated, + scope: RateLimitScope.user, + burst: 1, + ), + ], + () async { + await upload(package: 'a', version: '1.0.0', time: Duration.zero); + await upload( + package: 'a', version: '1.1.0', time: Duration(seconds: 11)); + final rs = upload( + package: 'b', version: '0.1.0', time: Duration(seconds: 23)); + await expectLater( + rs, + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('(1 in the last few minutes)'), + ), + ), + ); + await upload( + package: 'b', version: '0.1.0', time: Duration(minutes: 3)); + }, + ); + }, + ); }); } diff --git a/app/test/package/upload_test.dart b/app/test/package/upload_test.dart index daec434442..7690acb420 100644 --- a/app/test/package/upload_test.dart +++ b/app/test/package/upload_test.dart @@ -131,7 +131,8 @@ void main() { final audits = await auditBackend.listRecordsForPackageVersion( 'new_package', '1.2.3'); - final publishedAudit = audits.records.first; + final publishedAudit = audits.records + .firstWhere((e) => e.kind == AuditLogRecordKind.packagePublished); expect(publishedAudit.kind, AuditLogRecordKind.packagePublished); expect(publishedAudit.created, isNotNull); expect(publishedAudit.expires!.year, greaterThan(9998));