Skip to content

Commit

Permalink
Merge branch 'google:master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
rfontanarosa authored Aug 14, 2024
2 parents 1bac06c + 39471eb commit e040df7
Show file tree
Hide file tree
Showing 100 changed files with 2,500 additions and 1,107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

name: deploy-to-firebase-on-merge
name: Deploy to dev
on:
push:
branches:
Expand Down
4 changes: 2 additions & 2 deletions docs/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ GEM
rb-fsevent (0.11.0)
rb-inotify (0.10.1)
ffi (~> 1.0)
rexml (3.2.8)
strscan (>= 3.0.9)
rexml (3.3.3)
strscan
rouge (3.26.0)
ruby2_keywords (0.0.5)
rubyzip (2.3.2)
Expand Down
3 changes: 2 additions & 1 deletion e2e-tests/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"rules": {
"node/no-unpublished-import": ["error", {
"allowModules": ["jasmine"]
}]
}],
"eqeqeq": ["error", "always", {"null": "ignore"}]
}
}
29 changes: 16 additions & 13 deletions firestore/firestore.rules
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,17 @@
}
/**
* Returns true iff the user with the provided email created the specified resource.
* Returns true iff the current user is the owner of the specified LOI.
*/
function isCreator(resource) {
// TODO(#1154): Make `owner` its own field and stop assuming `creator==owner`.
return resource.data.created.user.email == request.auth.token.email;
function isLoiOwner(loi) {
return loi.data['5'] == request.auth.uid;
}
/**
* Returns true iff the current user is the owner of the specified submission.
*/
function isSubmissionOwner(submission) {
return submission.data['5'] == request.auth.uid;
}
// Allow users to read, create, and write their own profiles in the db.
Expand Down Expand Up @@ -138,30 +144,27 @@
// Allow if user has has read access to the survey.
allow read: if canViewSurvey(getSurvey(surveyId));
// Allow if user is owner of the new LOI and can collect data.
// TODO(#1154): Require owner as well.
allow create: if canCollectData(getSurvey(surveyId));
allow create: if isLoiOwner(request.resource) && canCollectData(getSurvey(surveyId));
// Allow if user is owner of the existing LOI or can manage survey.
allow write: if isCreator(resource) || canManageSurvey(getSurvey(surveyId));
allow write: if isLoiOwner(resource) || canManageSurvey(getSurvey(surveyId));
}
// Apply passlist and survey-level ACLs to submission documents.
match /surveys/{surveyId}/submissions/{submissionId} {
// Allow if user has has read access to the survey.
allow read: if canViewSurvey(getSurvey(surveyId));
// Allow if user is owner of the new submission and can collect data.
allow create: if isCreator(request.resource) && canCollectData(getSurvey(surveyId));
allow create: if isSubmissionOwner(request.resource) && canCollectData(getSurvey(surveyId));
// Allow if user is owner of the existing submission or can manage survey.
allow write: if isCreator(resource) || canManageSurvey(getSurvey(surveyId));
allow write: if isSubmissionOwner(resource) || canManageSurvey(getSurvey(surveyId));
}
// Apply passlist and survey-level ACLs to job documents.
match /surveys/{surveyId}/jobs/{jobId} {
// Allow if user has has read access to the survey.
allow read: if canViewSurvey(getSurvey(surveyId));
// Allow if user is owner of the new submission and can collect data.
allow create: if canManageSurvey(getSurvey(surveyId)) || isCreator(request.resource);
// Allow if user is owner of the existing submission or can manage survey.
allow write: if canManageSurvey(getSurvey(surveyId)) || isCreator(resource);
// Allow if user can manage survey.
allow create, write: if canManageSurvey(getSurvey(surveyId));
}
}
}
5 changes: 4 additions & 1 deletion functions/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
"parserOptions": {
"sourceType": "module"
},
"root": true
"root": true,
"rules": {
"eqeqeq": ["error", "always", {"null": "ignore"}]
}
}
18 changes: 14 additions & 4 deletions functions/src/common/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ import {DecodedIdToken, getAuth} from 'firebase-admin/auth';
import {DocumentSnapshot} from 'firebase-admin/firestore';
import {https, Response} from 'firebase-functions/v1';
import {EmulatorIdToken} from '../handlers';
import {GroundProtos} from '@ground/proto';
import {registry} from '@ground/lib';

import Pb = GroundProtos.ground.v1beta1;
const s = registry.getFieldIds(Pb.Survey);

// This is the only cookie not stripped by Firebase CDN.
// https://firebase.google.com/docs/hosting/manage-cache#using_cookies
Expand Down Expand Up @@ -81,12 +86,13 @@ function isEmulatorIdToken(user: DecodedIdToken): boolean {
function getRole(
user: DecodedIdToken,
survey: DocumentSnapshot
): string | null {
): string | number | null {
if (isEmulatorIdToken(user)) {
return OWNER_ROLE;
}
const acl = survey.get('acl');
return user.email ? acl?.[user.email] : null;
// TODO(#1858): Remove reference to "acl" field.
const acl = survey.get(s.acl) || survey.get('acl');
return acl && user.email ? acl[user.email] : null;
}

export function canExport(
Expand All @@ -101,5 +107,9 @@ export function canImport(
survey: DocumentSnapshot
): boolean {
const role = getRole(user, survey);
return role === OWNER_ROLE || role === SURVEY_ORGANIZER_ROLE;
// TODO(#1858): Remove old roles.
return (
!!role &&
[OWNER_ROLE, SURVEY_ORGANIZER_ROLE, Pb.Role.SURVEY_ORGANIZER].includes(role)
);
}
24 changes: 20 additions & 4 deletions functions/src/common/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
import * as functions from 'firebase-functions';
import {firestore} from 'firebase-admin';
import {DocumentData, GeoPoint, QuerySnapshot} from 'firebase-admin/firestore';
import {registry} from '@ground/lib';
import {GroundProtos} from '@ground/proto';

import Pb = GroundProtos.ground.v1beta1;
const l = registry.getFieldIds(Pb.LocationOfInterest);
const sb = registry.getFieldIds(Pb.Submission);

/**
*
Expand Down Expand Up @@ -46,6 +52,12 @@ export const surveys = () => 'surveys';
*/
export const survey = (surveyId: string) => surveys() + '/' + surveyId;

/**
* Returns the path of job doc with the specified id.
*/
export const job = (surveyId: string, jobId: string) =>
`${survey(surveyId)}/jobs/${jobId}`;

/**
* Returns the path of the survey collection in the survey with the specified id.
*/
Expand Down Expand Up @@ -116,10 +128,14 @@ export class Datastore {
return this.db_.doc(survey(surveyId)).get();
}

fetchJob(surveyId: string, jobId: string) {
return this.db_.doc(job(surveyId, jobId)).get();
}

fetchSubmissionsByJobId(surveyId: string, jobId: string) {
return this.db_
.collection(submissions(surveyId))
.where('jobId', '==', jobId)
.where(sb.jobId, '==', jobId)
.get();
}

Expand All @@ -133,7 +149,7 @@ export class Datastore {
): Promise<QuerySnapshot<DocumentData, DocumentData>> {
return this.db_
.collection(lois(surveyId))
.where('jobId', '==', jobId)
.where(l.jobId, '==', jobId)
.get();
}

Expand All @@ -150,14 +166,14 @@ export class Datastore {
loiId: string
): Promise<number> {
const submissionsRef = this.db_.collection(submissions(surveyId));
const submissionsForLoiQuery = submissionsRef.where('loiId', '==', loiId);
const submissionsForLoiQuery = submissionsRef.where(sb.loiId, '==', loiId);
const snapshot = await submissionsForLoiQuery.count().get();
return snapshot.data().count;
}

async updateSubmissionCount(surveyId: string, loiId: string, count: number) {
const loiRef = this.db_.doc(loi(surveyId, loiId));
await loiRef.update({submissionCount: count});
await loiRef.update({[l.submissionCount]: count});
}

async updateLoiProperties(
Expand Down
Loading

0 comments on commit e040df7

Please sign in to comment.