Skip to content

Commit 0d0035f

Browse files
authored
Added createPublisher end-point (#2630)
* Added createPublisher end-point * Added tests for createPublisher
1 parent 799c022 commit 0d0035f

File tree

11 files changed

+299
-16
lines changed

11 files changed

+299
-16
lines changed

app/lib/frontend/handlers/pubapi.client.dart

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/lib/frontend/handlers/pubapi.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import 'account.dart';
1111
import 'custom_api.dart';
1212
import 'listing.dart';
1313
import 'package.dart';
14-
import 'publisher.dart';
1514

1615
part 'pubapi.g.dart';
1716

@@ -101,8 +100,12 @@ class PubApi {
101100

102101
/// Starts publisher creation flow.
103102
@EndPoint.post('/api/publishers/<publisherId>')
104-
Future<Response> createPublisher(Request request, String publisherId) =>
105-
createPublisherApiHandler(request, publisherId);
103+
Future<PublisherInfo> createPublisher(
104+
Request request,
105+
String publisherId,
106+
CreatePublisherRequest body,
107+
) =>
108+
publisherBackend.createPublisher(publisherId, body);
106109

107110
/// Returns publisher data in a JSON form.
108111
@EndPoint.get('/api/publishers/<publisherId>')

app/lib/frontend/handlers/pubapi.g.dart

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/lib/frontend/handlers/publisher.dart

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,3 @@ Future<shelf.Response> createPublisherPageHandler(shelf.Request request) async {
1313
// TODO: implement
1414
return notFoundHandler(request);
1515
}
16-
17-
/// Handles requests for POST /api/publisher/<publisherId>
18-
Future<shelf.Response> createPublisherApiHandler(
19-
shelf.Request request, String publisherId) async {
20-
// TODO: implement
21-
return notFoundHandler(request);
22-
}

app/lib/publisher/backend.dart

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import '../account/backend.dart';
1212
import '../account/consent_backend.dart';
1313
import '../shared/email.dart';
1414
import '../shared/exceptions.dart';
15+
import 'domain_verifier.dart' show domainVerifier;
1516

1617
import 'models.dart';
1718

@@ -60,6 +61,87 @@ class PublisherBackend {
6061
});
6162
}
6263

64+
/// Create publisher.
65+
Future<api.PublisherInfo> createPublisher(
66+
String publisherId,
67+
api.CreatePublisherRequest body,
68+
) async {
69+
// Sanity check that domains are:
70+
// - lowercase (because we want that in pub.dev)
71+
// - consist of a-z, 0-9 and dashes
72+
// We do not care if they end in dash, as such domains can't be verified.
73+
InvalidInputException.checkMatchPattern(
74+
publisherId,
75+
'publisherId',
76+
RegExp(r'^[a-z0-9-]{1,63}\.[a-z0-9-]{1,63}$'),
77+
);
78+
InvalidInputException.checkStringLength(
79+
publisherId,
80+
'publisherId',
81+
maximum: 255, // Some upper limit for sanity.
82+
);
83+
InvalidInputException.checkNotNull(body.accessToken, 'accessToken');
84+
InvalidInputException.checkStringLength(
85+
body.accessToken,
86+
'accessToken',
87+
minimum: 1,
88+
maximum: 4096,
89+
);
90+
91+
// Verify ownership of domain.
92+
final isOwner = await domainVerifier.verifyDomainOwnership(
93+
publisherId,
94+
body.accessToken,
95+
);
96+
if (!isOwner) {
97+
throw AuthorizationException.userIsNotDomainOwner(publisherId);
98+
}
99+
100+
// Create the publisher
101+
final now = DateTime.now().toUtc();
102+
await _db.withTransaction((tx) async {
103+
final key = _db.emptyKey.append(Publisher, id: publisherId);
104+
final p = (await tx.lookup<Publisher>([key])).single;
105+
if (p != null) {
106+
// Check that publisher is the same as what we would create.
107+
if (p.created.isBefore(now.subtract(Duration(minutes: 10))) ||
108+
p.updated.isBefore(now.subtract(Duration(minutes: 10))) ||
109+
p.contactEmail != authenticatedUser.email ||
110+
p.description != '' ||
111+
p.websiteUrl != 'https://$publisherId') {
112+
throw ConflictException.publisherAlreadyExists(publisherId);
113+
}
114+
// Avoid creating the same publisher again, this end-point is idempotent
115+
// if we just do nothing here.
116+
return;
117+
}
118+
119+
// Create publisher
120+
tx.queueMutations(inserts: [
121+
Publisher()
122+
..parentKey = _db.emptyKey
123+
..id = publisherId
124+
..created = now
125+
..description = ''
126+
..contactEmail = authenticatedUser.email
127+
..updated = now
128+
..websiteUrl = 'https://$publisherId',
129+
PublisherMember()
130+
..parentKey = _db.emptyKey.append(Publisher, id: publisherId)
131+
..id = authenticatedUser.userId
132+
..created = now
133+
..updated = now
134+
..role = PublisherMemberRole.admin
135+
]);
136+
await tx.commit();
137+
});
138+
139+
// Return publisher as it was created
140+
final key = _db.emptyKey.append(Publisher, id: publisherId);
141+
final p = (await _db.lookup<Publisher>([key])).single;
142+
return _asPublisherInfo(p);
143+
}
144+
63145
/// Gets the publisher data
64146
Future<api.PublisherInfo> getPublisher(String publisherId) async {
65147
final p = await _getPublisher(publisherId);
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import 'package:gcloud/service_scope.dart' as ss;
2+
import 'package:googleapis_auth/auth.dart' as auth;
3+
import 'package:googleapis/webmasters/v3.dart' as wmx;
4+
import 'package:http/http.dart' as http;
5+
import 'package:retry/retry.dart' show retry;
6+
7+
import '../shared/exceptions.dart' show AuthorizationException;
8+
9+
/// Sets the [DomainVerifier] service.
10+
void registerDomainVerifier(DomainVerifier domainVerifier) =>
11+
ss.register(#_domainVerifier, domainVerifier);
12+
13+
/// The active [DomainVerifier] service.
14+
DomainVerifier get domainVerifier =>
15+
ss.lookup(#_domainVerifier) as DomainVerifier;
16+
17+
/// Service that can verify ownership of a domain by asking Search Console
18+
/// through Webmaster API v3.
19+
///
20+
/// Please obtain instances of this from [domainVerifier], as this allows for
21+
/// dependency injection during testing.
22+
class DomainVerifier {
23+
/// Verify ownership of [domain] using [accessToken] which has the read-only
24+
/// scope for Search Console API.
25+
Future<bool> verifyDomainOwnership(String domain, String accessToken) async {
26+
// Create client for talking to Webmasters API:
27+
// https://developers.google.com/webmaster-tools/search-console-api-original/v3/parameters
28+
final client = auth.authenticatedClient(
29+
http.Client(),
30+
auth.AccessCredentials(
31+
auth.AccessToken(
32+
'Bearer',
33+
accessToken,
34+
DateTime.now().toUtc().add(Duration(minutes: 20)), // avoid refresh
35+
),
36+
null,
37+
[wmx.WebmastersApi.WebmastersReadonlyScope],
38+
),
39+
);
40+
try {
41+
// Request list of sites/domains from the Search Console API.
42+
final sites = await retry(
43+
() => wmx.WebmastersApi(client).sites.list(),
44+
maxAttempts: 3,
45+
maxDelay: Duration(milliseconds: 500),
46+
retryIf: (e) => e is! auth.AccessDeniedException,
47+
);
48+
// Determine if the user is in fact owner of the domain in question.
49+
// Note. domains are prefixed 'sc-domain:' and 'siteOwner' is the only
50+
// permission that ensures the user actually did DNS verification.
51+
return sites.siteEntry.any(
52+
(s) =>
53+
s.siteUrl.toLowerCase() == 'sc-domain:$domain' &&
54+
s.permissionLevel == 'siteOwner', // must be 'siteOwner'!
55+
);
56+
} on auth.AccessDeniedException {
57+
throw AuthorizationException.missingSearchConsoleReadAccess();
58+
} finally {
59+
client.close();
60+
}
61+
}
62+
}

app/lib/shared/exceptions.dart

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class AuthenticationException extends ResponseException
165165
///
166166
/// Example:
167167
/// * Modifying a package for which the user doesn't have permissions,
168-
/// * Creating a package without domain validation.
168+
/// * Creating a publisher without domain validation.
169169
class AuthorizationException extends ResponseException
170170
implements UnauthorizedAccessException {
171171
AuthorizationException._(String message)
@@ -201,6 +201,39 @@ class AuthorizationException extends ResponseException
201201
'package `$publisher`.',
202202
);
203203

204+
static final _domainVerificationUrl =
205+
Uri.parse('https://www.google.com/webmasters/verification/verification');
206+
207+
/// Signaling that the user is not a verified owner of the [domain] for which
208+
/// the user is trying to create a publisher.
209+
factory AuthorizationException.userIsNotDomainOwner(String domain) =>
210+
AuthorizationException._([
211+
'Insufficient permissions to create publisher `$domain`, to create ',
212+
'this publisher the domain `$domain` must be _verified_ in the ',
213+
'[search console](https://search.google.com/search-console/welcome).',
214+
'',
215+
'It is not sufficient to be granted access to the domain, the domain ',
216+
'must be verified with the Google account used to created the ',
217+
'publisher. It is also insufficient to verify a URL or URL prefix,',
218+
'the domain must be verified with a **DNS record**.',
219+
'',
220+
'<b><a href="${_domainVerificationUrl.replace(queryParameters: {
221+
"domain": domain
222+
})}" target="_blank">Open domain verification flow.</a></b>',
223+
'',
224+
'Note, once the publisher is created the domain verification need not',
225+
'remain in place. This is only required for publisher creation.',
226+
].join('\n'));
227+
228+
/// Signaling that the user did not grant read-only access to the
229+
/// search console, making it impossible for the server to verify the users
230+
/// domain ownership.
231+
factory AuthorizationException.missingSearchConsoleReadAccess() =>
232+
AuthorizationException._([
233+
'Read-only access to Search Console data was not granted, preventing',
234+
'`pub.dev` from verifying that you own the domain.',
235+
].join('\n'));
236+
204237
@override
205238
String toString() => '$code: $message'; // used by package:pub_server
206239
}
@@ -225,6 +258,11 @@ class ConflictException extends ResponseException {
225258
/// The active user can't update their own role.
226259
factory ConflictException.cantUpdateOwnRole() =>
227260
ConflictException._('User can\'t update their own role.');
261+
262+
/// The user is trying to create a publisher that already exists.
263+
factory ConflictException.publisherAlreadyExists(String domain) =>
264+
ConflictException._(
265+
'A publisher with the domain `$domain` already exists');
228266
}
229267

230268
/// Thrown when the analysis for a package is not done yet.

app/lib/shared/services.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../frontend/search_service.dart';
1414
import '../history/backend.dart';
1515
import '../job/backend.dart';
1616
import '../publisher/backend.dart';
17+
import '../publisher/domain_verifier.dart';
1718
import '../scorecard/backend.dart';
1819
import '../search/backend.dart';
1920
import '../search/index_simple.dart';
@@ -71,6 +72,7 @@ Future<void> withPubServices(FutureOr<void> Function() fn) async {
7172
PopularityStorage(await getOrCreateBucket(
7273
storageService, activeConfiguration.popularityDumpBucketName)),
7374
);
75+
registerDomainVerifier(DomainVerifier());
7476
registerPublisherBackend(PublisherBackend(dbService));
7577
registerScoreCardBackend(ScoreCardBackend(dbService));
7678
registerSearchBackend(SearchBackend(dbService));

app/test/publisher/api_test.dart

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,55 @@ void main() {
3333
});
3434
});
3535

36+
group('Create publisher', () {
37+
testWithServices('verified.com', () async {
38+
final api = createPubApiClient(authToken: hansAuthenticated.userId);
39+
40+
// Check that we can create the publisher
41+
final r1 = await api.createPublisher(
42+
'verified.com',
43+
CreatePublisherRequest(accessToken: 'dummy-token-for-testing'),
44+
);
45+
expect(r1.contactEmail, hansAuthenticated.email);
46+
47+
// Check that creating again idempotently works too
48+
final r2 = await api.createPublisher(
49+
'verified.com',
50+
CreatePublisherRequest(accessToken: 'dummy-token-for-testing'),
51+
);
52+
expect(r2.contactEmail, hansAuthenticated.email);
53+
54+
// Check that we can update the description
55+
final r3 = await api.updatePublisher(
56+
'verified.com',
57+
UpdatePublisherRequest(description: 'hello-world'),
58+
);
59+
expect(r3.description, 'hello-world');
60+
61+
// Check that we get a sane result from publisherInfo
62+
final r4 = await api.publisherInfo('verified.com');
63+
expect(r4.toJson(), {
64+
'description': 'hello-world',
65+
'contactEmail': hansAuthenticated.email,
66+
});
67+
});
68+
69+
testWithServices('notverified.com', () async {
70+
final api = createPubApiClient(authToken: hansAuthenticated.userId);
71+
72+
// Check that we can create the publisher
73+
final rs = api.createPublisher(
74+
'notverified.com',
75+
CreatePublisherRequest(accessToken: 'dummy-token-for-testing'),
76+
);
77+
await expectApiException(
78+
rs,
79+
status: 403,
80+
code: 'InsufficientPermissions',
81+
);
82+
});
83+
});
84+
3685
group('Update description', () {
3786
_testAdminAuthIssues(
3887
(client) => client.updatePublisher(

0 commit comments

Comments
 (0)