From ff8bb69b28ddec07323d56fcfdb88593115eb756 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Wed, 24 Apr 2024 15:19:12 +0200 Subject: [PATCH] Report page with subject parameter + its form processing. --- app/lib/admin/models.dart | 90 ++++++++++++++++----- app/lib/frontend/handlers/report.dart | 47 +++++++++++ app/lib/frontend/templates/report.dart | 16 ++++ app/lib/search/search_service.dart | 7 +- app/lib/shared/utils.dart | 7 ++ app/test/admin/models_test.dart | 44 ++++++++++ app/test/frontend/handlers/report_test.dart | 45 ++++++++++- pkg/_pub_shared/lib/data/account_api.dart | 2 + pkg/_pub_shared/lib/data/account_api.g.dart | 2 + 9 files changed, 234 insertions(+), 26 deletions(-) create mode 100644 app/test/admin/models_test.dart diff --git a/app/lib/admin/models.dart b/app/lib/admin/models.dart index 014c9335f..b612d5ada 100644 --- a/app/lib/admin/models.dart +++ b/app/lib/admin/models.dart @@ -42,23 +42,12 @@ class ModerationCase extends db.ExpandoModel { @db.StringProperty(required: true) late String kind; - /// The kind of the entity this notification or appeal concerns. On of: - /// `package`, `package-version` or `publisher`. - @db.StringProperty() - String? subjectKind; - /// The fully qualified name of the entity this notification or appeal concerns. /// - `package:` /// - `package-version:/` /// - `publisher:` @db.StringProperty() - String? subjectFqn; - - /// The local name of the entity (without the type qualifier) this notification or appeal concerns. - /// - `package`: the package name - /// - `package-version`: the `/` - /// - `publisher`: the publisher ID - String? subjectLocalName; + String? subject; /// The `caseId` of the appeal (or null). @db.StringProperty() @@ -90,17 +79,10 @@ class ModerationCase extends db.ExpandoModel { required this.detectedBy, required this.kind, required this.status, - this.subjectKind, - this.subjectLocalName, + this.subject, }) { id = caseId; opened = clock.now().toUtc(); - if (subjectKind != null && - subjectKind!.isNotEmpty && - subjectLocalName != null && - subjectLocalName!.isNotEmpty) { - subjectFqn = '$subjectKind:$subjectLocalName'; - } } } @@ -115,3 +97,71 @@ abstract class ModerationKind { abstract class ModerationStatus { static const pending = 'pending'; } + +class ModerationSubject { + final String kind; + final String localName; + final String? package; + final String? version; + final String? publisherId; + + ModerationSubject._({ + required this.kind, + required this.localName, + this.package, + this.version, + this.publisherId, + }); + + static ModerationSubject? tryParse(String value) { + final parts = value.split(':'); + if (parts.length != 2) { + return null; + } + if (parts.any((p) => p.isEmpty || p.trim() != p)) { + return null; + } + final kind = parts.first; + switch (kind) { + case ModerationSubjectKind.package: + final package = parts[1]; + return ModerationSubject._( + kind: kind, + localName: package, + package: package, + ); + case ModerationSubjectKind.packageVersion: + final localName = parts[1]; + final pvParts = localName.split('/'); + if (pvParts.length != 2) { + return null; + } + final package = pvParts.first; + final version = pvParts[1]; + return ModerationSubject._( + kind: kind, + localName: localName, + package: package, + version: version, + ); + case ModerationSubjectKind.publisher: + final publisherId = parts[1]; + return ModerationSubject._( + kind: kind, + localName: publisherId, + publisherId: publisherId, + ); + default: + return null; + } + } + + late final fqn = '$kind:$localName'; +} + +class ModerationSubjectKind { + static const unspecified = 'unspecified'; + static const package = 'package'; + static const packageVersion = 'package-version'; + static const publisher = 'publisher'; +} diff --git a/app/lib/frontend/handlers/report.dart b/app/lib/frontend/handlers/report.dart index f8ca8f170..86d4cc9a3 100644 --- a/app/lib/frontend/handlers/report.dart +++ b/app/lib/frontend/handlers/report.dart @@ -12,6 +12,8 @@ import '../../account/backend.dart'; import '../../admin/models.dart'; import '../../frontend/email_sender.dart'; import '../../frontend/handlers/cache_control.dart'; +import '../../package/backend.dart'; +import '../../publisher/backend.dart'; import '../../shared/datastore.dart'; import '../../shared/email.dart'; import '../../shared/exceptions.dart'; @@ -26,14 +28,50 @@ Future reportPageHandler(shelf.Request request) async { return notFoundHandler(request); } + final subjectParam = request.requestedUri.queryParameters['subject']; + ModerationSubject? subject; + if (subjectParam != null) { + subject = ModerationSubject.tryParse(subjectParam); + if (subject == null) { + throw InvalidInputException('Invalid "subject" parameter.'); + } + await _verifySubject(subject); + } + return htmlResponse( renderReportPage( sessionData: requestContext.sessionData, + subject: subject, ), headers: CacheControl.explicitlyPrivate.headers, ); } +Future _verifySubject(ModerationSubject? subject) async { + final package = subject?.package; + final version = subject?.version; + final publisherId = subject?.publisherId; + if (package != null) { + final p = await packageBackend.lookupPackage(package); + if (p == null) { + throw NotFoundException('Package "$package" does not exist.'); + } + if (version != null) { + final pv = await packageBackend.lookupPackageVersion(package, version); + if (pv == null) { + throw NotFoundException( + 'Package version "$package/$version" does not exist.'); + } + } + } + if (publisherId != null) { + final p = await publisherBackend.getPublisher(publisherId); + if (p == null) { + throw NotFoundException('Publisher "$publisherId" does not exist.'); + } + } +} + /// Handles POST /api/report Future processReportPageHandler( shelf.Request request, ReportForm form) async { @@ -56,6 +94,13 @@ Future processReportPageHandler( InvalidInputException.checkNull(form.email, 'email'); } + ModerationSubject? subject; + if (form.subject != null) { + subject = ModerationSubject.tryParse(form.subject!); + InvalidInputException.check(subject != null, 'Invalid subject.'); + await _verifySubject(subject); + } + InvalidInputException.checkStringLength( form.message, 'message', @@ -73,12 +118,14 @@ Future processReportPageHandler( detectedBy: ModerationDetectedBy.externalNotification, kind: ModerationKind.notification, status: ModerationStatus.pending, + subject: subject?.fqn, ); tx.insert(mc); }); final bodyText = [ 'New report recieved on ${now.toIso8601String()}: $caseId', + if (subject != null) 'Subject: ${subject.fqn}', 'Message:\n${form.message}', ].join('\n\n'); diff --git a/app/lib/frontend/templates/report.dart b/app/lib/frontend/templates/report.dart index 343e867f6..e43a77a9a 100644 --- a/app/lib/frontend/templates/report.dart +++ b/app/lib/frontend/templates/report.dart @@ -3,13 +3,21 @@ // BSD-style license that can be found in the LICENSE file. import '../../account/models.dart'; +import '../../admin/models.dart'; import '../dom/dom.dart' as d; import '../dom/material.dart' as material; import 'layout.dart'; +const _subjectKindLabels = { + ModerationSubjectKind.package: 'Package: ', + ModerationSubjectKind.packageVersion: 'Package version: ', + ModerationSubjectKind.publisher: 'Publisher: ', +}; + /// Renders the create publisher page. String renderReportPage({ SessionData? sessionData, + ModerationSubject? subject, }) { return renderLayoutPage( PageType.standalone, @@ -31,6 +39,14 @@ String renderReportPage({ label: 'Email', ), ]), + if (subject != null) + d.fragment([ + d.input(type: 'hidden', name: 'subject', value: subject.fqn), + d.p(children: [ + d.text(_subjectKindLabels[subject.kind] ?? ''), + d.code(text: subject.localName), + ]), + ]), d.p(text: 'Please describe the issue you want to report:'), material.textArea( id: 'message', diff --git a/app/lib/search/search_service.dart b/app/lib/search/search_service.dart index 08db1066d..a78762f65 100644 --- a/app/lib/search/search_service.dart +++ b/app/lib/search/search_service.dart @@ -10,6 +10,7 @@ import 'package:_pub_shared/search/tags.dart'; import 'package:clock/clock.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:pub_dev/admin/actions/actions.dart'; +import 'package:pub_dev/shared/utils.dart'; part 'search_service.g.dart'; @@ -148,8 +149,6 @@ class ApiDocPage { Map toJson() => _$ApiDocPageToJson(this); } -String? _stringToNull(String? v) => (v == null || v.isEmpty) ? null : v; - class ServiceSearchQuery { final String? query; final ParsedQueryText parsedQuery; @@ -176,7 +175,7 @@ class ServiceSearchQuery { this.limit, }) : parsedQuery = ParsedQueryText.parse(query), tagsPredicate = tagsPredicate ?? TagsPredicate(), - publisherId = _stringToNull(publisherId); + publisherId = publisherId?.trimToNull(); factory ServiceSearchQuery.parse({ String? query, @@ -188,7 +187,7 @@ class ServiceSearchQuery { int offset = 0, int? limit = 10, }) { - final q = _stringToNull(query?.trim()); + final q = query?.trimToNull(); return ServiceSearchQuery._( query: q, tagsPredicate: tagsPredicate, diff --git a/app/lib/shared/utils.dart b/app/lib/shared/utils.dart index 6b8b6473b..d60012de9 100644 --- a/app/lib/shared/utils.dart +++ b/app/lib/shared/utils.dart @@ -293,3 +293,10 @@ bool fixedTimeIntListEquals(List a, List b) { } return result == 0; } + +extension StringExt on String { + String? trimToNull() { + final v = trim(); + return v.isEmpty ? null : v; + } +} diff --git a/app/test/admin/models_test.dart b/app/test/admin/models_test.dart new file mode 100644 index 000000000..faac635e4 --- /dev/null +++ b/app/test/admin/models_test.dart @@ -0,0 +1,44 @@ +// Copyright (c) 2022, 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/admin/models.dart'; +import 'package:test/test.dart'; + +void main() { + group('ModerationSubject', () { + test('Invalid values', () { + expect(ModerationSubject.tryParse(''), null); + expect(ModerationSubject.tryParse('x:x'), null); + expect(ModerationSubject.tryParse('package'), null); + expect(ModerationSubject.tryParse('package:'), null); + }); + + test('package', () { + final ms = ModerationSubject.tryParse('package:x'); + expect(ms!.kind, ModerationSubjectKind.package); + expect(ms.localName, 'x'); + expect(ms.package, 'x'); + expect(ms.version, isNull); + expect(ms.publisherId, isNull); + }); + + test('package version', () { + final ms = ModerationSubject.tryParse('package-version:x/1.0.0'); + expect(ms!.kind, ModerationSubjectKind.packageVersion); + expect(ms.localName, 'x/1.0.0'); + expect(ms.package, 'x'); + expect(ms.version, '1.0.0'); + expect(ms.publisherId, isNull); + }); + + test('publisher', () { + final ms = ModerationSubject.tryParse('publisher:example.com'); + expect(ms!.kind, ModerationSubjectKind.publisher); + expect(ms.localName, 'example.com'); + expect(ms.package, isNull); + expect(ms.version, isNull); + expect(ms.publisherId, 'example.com'); + }); + }); +} diff --git a/app/test/frontend/handlers/report_test.dart b/app/test/frontend/handlers/report_test.dart index 83f3bcb4c..fedb83477 100644 --- a/app/test/frontend/handlers/report_test.dart +++ b/app/test/frontend/handlers/report_test.dart @@ -3,10 +3,12 @@ // BSD-style license that can be found in the LICENSE file. import 'package:_pub_shared/data/account_api.dart'; +import 'package:pub_dev/admin/models.dart'; import 'package:pub_dev/fake/backend/fake_auth_provider.dart'; import 'package:pub_dev/fake/backend/fake_email_sender.dart'; import 'package:pub_dev/frontend/handlers/experimental.dart'; import 'package:pub_dev/frontend/request_context.dart'; +import 'package:pub_dev/shared/datastore.dart'; import 'package:test/test.dart'; import '../../shared/handlers_test_utils.dart'; @@ -39,6 +41,35 @@ void main() { absent: ['Contact information'], ); }); + + testWithProfile('page with bad subject', fn: () async { + await expectHtmlResponse( + await issueGet( + '/report?subject=x', + headers: {'cookie': '$experimentalCookieName=report'}, + ), + present: [ + 'Invalid "subject" parameter.', + ], + absent: [ + 'Please describe the issue you want to report:', + ], + status: 400, + ); + }); + + testWithProfile('page with package version subject', fn: () async { + await expectHtmlResponse( + await issueGet( + '/report?subject=package-version:oxygen/1.0.0', + headers: {'cookie': '$experimentalCookieName=report'}, + ), + present: [ + 'Please describe the issue you want to report:', + 'oxygen/1.0.0', + ], + ); + }); }); group('Report API test', () { @@ -111,7 +142,7 @@ void main() { experimental: {'report'}, fn: (client) async { final msg = await client.postReport(ReportForm( - email: 'user@pub.dev', + email: 'user2@pub.dev', message: 'Huston, we have a problem.', )); @@ -120,7 +151,11 @@ void main() { final email = fakeEmailSender.sentMessages.single; expect(email.from.email, 'noreply@pub.dev'); expect(email.recipients.single.email, 'support@pub.dev'); - expect(email.ccRecipients.single.email, 'user@pub.dev'); + expect(email.ccRecipients.single.email, 'user2@pub.dev'); + + final mc = await dbService.query().run().single; + expect(mc.subject, isNull); + expect(mc.reporterEmail, 'user2@pub.dev'); }, ); }); @@ -136,13 +171,19 @@ void main() { fn: (client) async { final msg = await client.postReport(ReportForm( message: 'Huston, we have a problem.', + subject: 'package:oxygen', )); expect(msg.message, 'Report submitted successfully.'); expect(fakeEmailSender.sentMessages, hasLength(1)); final email = fakeEmailSender.sentMessages.single; + expect(email.bodyText, contains('Subject: package:oxygen')); expect(email.from.email, 'noreply@pub.dev'); expect(email.recipients.single.email, 'support@pub.dev'); expect(email.ccRecipients.single.email, 'user@pub.dev'); + + final mc = await dbService.query().run().single; + expect(mc.subject, 'package:oxygen'); + expect(mc.reporterEmail, 'user@pub.dev'); }, ); }); diff --git a/pkg/_pub_shared/lib/data/account_api.dart b/pkg/_pub_shared/lib/data/account_api.dart index 173c04ea4..c7b794440 100644 --- a/pkg/_pub_shared/lib/data/account_api.dart +++ b/pkg/_pub_shared/lib/data/account_api.dart @@ -182,10 +182,12 @@ class InviteStatus { @JsonSerializable() class ReportForm { final String? email; + final String? subject; final String message; ReportForm({ this.email, + this.subject, required this.message, }); diff --git a/pkg/_pub_shared/lib/data/account_api.g.dart b/pkg/_pub_shared/lib/data/account_api.g.dart index 732d7e45e..7e7089ce9 100644 --- a/pkg/_pub_shared/lib/data/account_api.g.dart +++ b/pkg/_pub_shared/lib/data/account_api.g.dart @@ -133,11 +133,13 @@ Map _$InviteStatusToJson(InviteStatus instance) => ReportForm _$ReportFormFromJson(Map json) => ReportForm( email: json['email'] as String?, + subject: json['subject'] as String?, message: json['message'] as String, ); Map _$ReportFormToJson(ReportForm instance) => { 'email': instance.email, + 'subject': instance.subject, 'message': instance.message, };