Skip to content

Commit e257d5a

Browse files
authored
Replace passlist regexp with @Domain doc-id entries (#2523)
1 parent e4cd77f commit e257d5a

4 files changed

Lines changed: 37 additions & 37 deletions

File tree

firestore/firestore.rules

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,15 @@ rules_version = '2';
5555
}
5656

5757
/**
58-
* Returns the regular expression matching emails granted access.
59-
*/
60-
function getPassRegexp() {
61-
let doc = get(/databases/$(database)/documents/passlist/regexp);
62-
return doc == null ? null : doc.data.regexp;
63-
}
64-
65-
/**
66-
* Returns true iff the user's email is listed in the passlist or allowed via regex.
58+
* Returns true iff the user's email is granted access by the passlist:
59+
* either by an exact-email entry (`passlist/<email>`) or by a domain-wide
60+
* entry (`passlist/@<domain>`).
6761
*/
6862
function isPasslisted() {
69-
let regexp = getPassRegexp();
70-
return (regexp != null && request.auth.token.email.matches(regexp))
71-
|| exists(/databases/$(database)/documents/passlist/$(request.auth.token.email));
63+
let email = request.auth.token.email;
64+
let domain = email.split('@')[1];
65+
return exists(/databases/$(database)/documents/passlist/$(email))
66+
|| exists(/databases/$(database)/documents/passlist/@$(domain));
7267
}
7368
7469
/**
@@ -172,10 +167,11 @@ rules_version = '2';
172167
allow write: if userId == request.auth.uid;
173168
}
174169

175-
// All users need to be able to read the passlist for rules to work.
170+
// Users may only read their own passlist entry. Domain-wide entries
171+
// (`@<domain>`) are evaluated via `exists()` in rules and the client and
172+
// must not be readable by signed-in users (would leak allowed domains).
176173
match /passlist/{entry} {
177-
// TODO(#1602): Replace "regexp" with simple pattern matching.
178-
allow read: if isSignedIn() && entry in ['regexp', request.auth.token.email];
174+
allow read: if isSignedIn() && entry == request.auth.token.email;
179175
}
180176

181177
// Apply passlist and survey-level ACLs to survey documents.

functions/src/on-create-passlist-entry.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,14 @@ describe('onCreatePasslistEntry()', () => {
8787
expect(mailServiceMock.sendMail).toHaveBeenCalled();
8888
expect(mailServiceMock.sendMail).toHaveBeenCalledWith(mail);
8989
});
90+
91+
it('skips email for domain-wide passlist entry', async () => {
92+
mockFirestore.doc('config/mail').set({ server: serverConfig });
93+
mockFirestore.doc('config/mail/templates/passlisted').set(mail);
94+
await onCreatePasslistEntryHandler({
95+
data: newDocumentSnapshot({}) as QueryDocumentSnapshot,
96+
params: { entryId: '@example.com' },
97+
} as any);
98+
expect(getMailServiceMock).not.toHaveBeenCalled();
99+
});
90100
});

functions/src/on-create-passlist-entry.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ export async function onCreatePasslistEntryHandler(
3434
) {
3535
const entryId = event.params.entryId;
3636

37+
// Domain-wide entries (`@<domain>`) are not user mailboxes, so skip mail.
38+
if (typeof entryId === 'string' && entryId.startsWith('@')) {
39+
return;
40+
}
41+
3742
const db = getDatastore();
3843

3944
const template = await db.fetchMailTemplate('passlisted');

web/src/app/services/data-store/data-store.service.ts

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -479,38 +479,27 @@ export class DataStoreService {
479479
}
480480

481481
/**
482-
* Returns a promise resolving to true if the user's email is either
483-
* explicitly listed as a document ID in the 'passlist' collection or
484-
* matches the regular expression stored in the 'passlist/regexp' document.
482+
* Returns a promise resolving to true if the user's email is granted access
483+
* by the passlist: either an exact-email entry (`passlist/<email>`) or a
484+
* domain-wide entry (`passlist/@<domain>`).
485485
*
486486
* @param userEmail The email of the user to check against the passlist.
487487
*/
488488
async isPasslisted(userEmail: string): Promise<boolean> {
489-
const docSnapshot = await runInInjectionContext(this.injector, () =>
489+
const emailSnapshot = await runInInjectionContext(this.injector, () =>
490490
getDoc(doc(this.db, 'passlist', userEmail))
491491
);
492492

493-
if (docSnapshot.exists()) return true;
493+
if (emailSnapshot.exists()) return true;
494494

495-
const regexpSnapshot = await runInInjectionContext(this.injector, () =>
496-
getDoc(doc(this.db, 'passlist', 'regexp'))
497-
);
498-
499-
if (regexpSnapshot.exists()) {
500-
const regexpData = regexpSnapshot.data() as
501-
| { regexp?: string }
502-
| undefined;
503-
504-
const regexpString = regexpData?.regexp;
505-
506-
if (regexpString) {
507-
const regex = new RegExp(regexpString);
495+
const domain = userEmail.split('@')[1];
496+
if (!domain) return false;
508497

509-
return regex.test(userEmail);
510-
}
511-
}
498+
const domainSnapshot = await runInInjectionContext(this.injector, () =>
499+
getDoc(doc(this.db, 'passlist', `@${domain}`))
500+
);
512501

513-
return false;
502+
return domainSnapshot.exists();
514503
}
515504

516505
private toLocationsOfInterest(

0 commit comments

Comments
 (0)