Skip to content

Commit 7d89fe9

Browse files
authored
Merge pull request #917 from aldenhallak/allow-admins-to-save
Allow admins to make updates to any experiment.
2 parents 9de2fd4 + a6b0f94 commit 7d89fe9

File tree

5 files changed

+85
-11
lines changed

5 files changed

+85
-11
lines changed

frontend/src/components/experiment_dashboard/cohort_settings_dialog.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export class CohortSettingsDialog extends MobxLitElement {
6161
Delete cohort
6262
</pr-button>
6363
<pr-button
64+
?disabled=${!this.experimentManager.isCreator}
6465
@click=${() => {
6566
this.analyticsService.trackButtonClick(
6667
ButtonClick.COHORT_SAVE_EXISTING,

functions/src/cohort.endpoints.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,12 @@ export const updateCohortMetadata = onCall(async (request) => {
9292
await app.firestore().runTransaction(async (transaction) => {
9393
const cohortConfig = (await document.get()).data() as CohortConfig;
9494

95-
// Verify that the experimenter is the creator
96-
// before updating.
97-
if (cohortConfig.metadata.creator !== request.auth?.token.email) {
95+
const canUpdate = await AuthGuard.isCreatorOrAdmin(
96+
request,
97+
cohortConfig.metadata.creator,
98+
);
99+
100+
if (!canUpdate) {
98101
success = false;
99102
return;
100103
}
@@ -128,7 +131,6 @@ export const deleteCohort = onCall(async (request) => {
128131
return {success: false};
129132
}
130133

131-
// Verify that experimenter is the creator before enabling delete
132134
const experiment = (
133135
await app.firestore().collection('experiments').doc(data.experimentId).get()
134136
).data();
@@ -139,8 +141,13 @@ export const deleteCohort = onCall(async (request) => {
139141
);
140142
}
141143

142-
if (request.auth?.token.email?.toLowerCase() !== experiment.metadata.creator)
143-
return;
144+
const canDelete = await AuthGuard.isCreatorOrAdmin(
145+
request,
146+
experiment.metadata.creator,
147+
);
148+
if (!canDelete) {
149+
return {success: false};
150+
}
144151

145152
// Delete document
146153
const doc = app

functions/src/experiment.endpoints.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ export const updateExperiment = onCall(async (request) => {
6868

6969
// Use shared utility to update experiment
7070
// TODO: Enable admins to update experiment?
71+
72+
// Define document reference
73+
const document = app
74+
.firestore()
75+
.collection(data.collectionName)
76+
.doc(data.experimentTemplate.id);
77+
78+
// If experiment does not exist, return false
79+
const oldExperiment = await document.get();
80+
if (!oldExperiment.exists) {
81+
return {success: false};
82+
}
83+
84+
// Use shared utility to update experiment
7185
const result = await updateExperimentFromTemplate(
7286
app.firestore(),
7387
data.experimentTemplate,
@@ -97,8 +111,21 @@ export const deleteExperiment = onCall(async (request) => {
97111

98112
const experimenterId = request.auth?.token.email?.toLowerCase() || '';
99113

114+
const experiment = (
115+
await app
116+
.firestore()
117+
.collection(data.collectionName)
118+
.doc(data.experimentId)
119+
.get()
120+
).data();
121+
if (!experiment) {
122+
throw new HttpsError(
123+
'not-found',
124+
`Experiment ${data.experimentId} not found in collection ${data.collectionName}`,
125+
);
126+
}
127+
100128
// Use shared utility to delete experiment
101-
// TODO: Enable admins to delete?
102129
const result = await deleteExperimentById(
103130
app.firestore(),
104131
data.experimentId,

functions/src/experiment.utils.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@deliberation-lab/utils';
1616
import {getExperimentDownload} from './data';
1717
import {generateVariablesForScope} from './variables.utils';
18+
import {AuthGuard} from './utils/auth-guard';
1819

1920
/**
2021
* Options for writing an experiment from a template
@@ -284,9 +285,13 @@ export async function updateExperimentFromTemplate(
284285
return {success: false, error: 'not-found'};
285286
}
286287

287-
// Verify that the experimenter is the creator
288+
// Verify that the experimenter is the creator or an admin
288289
if (experimenterId !== oldExperiment.data()?.metadata.creator) {
289-
return {success: false, error: 'not-owner'};
290+
const isAdmin = await AuthGuard.isAdminEmail(firestore, experimenterId);
291+
292+
if (!isAdmin) {
293+
return {success: false, error: 'not-owner'};
294+
}
290295
}
291296

292297
// Regenerate variable values based on current variable configs
@@ -384,10 +389,14 @@ export async function deleteExperimentById(
384389
return {success: false, error: 'not-found'};
385390
}
386391

387-
// Verify ownership
392+
// Verify ownership or admin status
388393
const experiment = experimentDoc.data();
389394
if (experimenterId !== experiment?.metadata?.creator) {
390-
return {success: false, error: 'not-owner'};
395+
const isAdmin = await AuthGuard.isAdminEmail(firestore, experimenterId);
396+
397+
if (!isAdmin) {
398+
return {success: false, error: 'not-owner'};
399+
}
391400
}
392401

393402
// Delete experiment and all subcollections

functions/src/utils/auth-guard.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,34 @@ export class AuthGuard {
4848

4949
throwUnauthIfNot(isAdmin);
5050
}
51+
52+
public static async isCreatorOrAdmin(
53+
request: CallableRequest,
54+
creatorEmail: string,
55+
) {
56+
const email = request.auth?.token.email?.toLowerCase();
57+
if (!email) return false;
58+
if (email === creatorEmail) return true;
59+
60+
const allowlistDoc = await app
61+
.firestore()
62+
.collection('allowlist')
63+
.doc(email)
64+
.get();
65+
66+
return allowlistDoc.exists && allowlistDoc.data()?.isAdmin === true;
67+
}
68+
69+
public static async isAdminEmail(
70+
firestore: FirebaseFirestore.Firestore,
71+
email: string,
72+
) {
73+
if (!email) return false;
74+
const allowlistDoc = await firestore
75+
.collection('allowlist')
76+
.doc(email)
77+
.get();
78+
79+
return allowlistDoc.exists && allowlistDoc.data()?.isAdmin === true;
80+
}
5181
}

0 commit comments

Comments
 (0)