From 55e2338de00216ac7e37bb1017928fad4463231a Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Tue, 14 Jan 2025 22:11:19 +0100 Subject: [PATCH 01/11] Add disabled users --- functions/models/src/functions/disableUser.ts | 17 +++++++ functions/models/src/functions/enableUser.ts | 17 +++++++ functions/models/src/index.ts | 2 + functions/models/src/types/user.ts | 1 + .../models/src/types/userRegistration.ts | 17 +++++-- .../src/functions/deleteInvitation.test.ts | 1 + functions/src/functions/disableUser.ts | 41 +++++++++++++++++ functions/src/functions/enableUser.ts | 41 +++++++++++++++++ functions/src/functions/enrollUser.test.ts | 1 + .../src/services/credential/credential.ts | 44 ++++++++++++------- .../src/services/trigger/triggerService.ts | 1 + .../services/user/databaseUserService.test.ts | 3 ++ .../src/services/user/databaseUserService.ts | 20 +++++++++ .../src/services/user/userService.mock.ts | 10 +++++ functions/src/services/user/userService.ts | 2 + .../src/tests/functions/testEnvironment.ts | 2 + 16 files changed, 199 insertions(+), 21 deletions(-) create mode 100644 functions/models/src/functions/disableUser.ts create mode 100644 functions/models/src/functions/enableUser.ts create mode 100644 functions/src/functions/disableUser.ts create mode 100644 functions/src/functions/enableUser.ts diff --git a/functions/models/src/functions/disableUser.ts b/functions/models/src/functions/disableUser.ts new file mode 100644 index 00000000..50269220 --- /dev/null +++ b/functions/models/src/functions/disableUser.ts @@ -0,0 +1,17 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { z } from 'zod' + +export const disableUserInputSchema = z.object({ + userId: z.string(), +}) + +export type DisableUserInput = z.input + +export type DisableUserOutput = undefined diff --git a/functions/models/src/functions/enableUser.ts b/functions/models/src/functions/enableUser.ts new file mode 100644 index 00000000..4c36acef --- /dev/null +++ b/functions/models/src/functions/enableUser.ts @@ -0,0 +1,17 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { z } from 'zod' + +export const enableUserInputSchema = z.object({ + userId: z.string(), +}) + +export type EnableUserInput = z.input + +export type EnableUserOutput = undefined diff --git a/functions/models/src/index.ts b/functions/models/src/index.ts index 82b79b5d..d383f463 100644 --- a/functions/models/src/index.ts +++ b/functions/models/src/index.ts @@ -31,7 +31,9 @@ export * from './functions/customSeed.js' export * from './functions/defaultSeed.js' export * from './functions/deleteInvitation.js' export * from './functions/deleteUser.js' +export * from './functions/disableUser.js' export * from './functions/dismissMessage.js' +export * from './functions/enableUser.js' export * from './functions/enrollUser.js' export * from './functions/exportHealthSummary.js' export * from './functions/getUsersInformation.js' diff --git a/functions/models/src/types/user.ts b/functions/models/src/types/user.ts index 62568213..968aa587 100644 --- a/functions/models/src/types/user.ts +++ b/functions/models/src/types/user.ts @@ -47,6 +47,7 @@ export class User extends UserRegistration { constructor(input: { type: UserType + disabled: boolean organization?: string dateOfBirth?: Date clinician?: string diff --git a/functions/models/src/types/userRegistration.ts b/functions/models/src/types/userRegistration.ts index ae23cc55..0640d7ed 100644 --- a/functions/models/src/types/userRegistration.ts +++ b/functions/models/src/types/userRegistration.ts @@ -18,6 +18,7 @@ export const userRegistrationInputConverter = new Lazy( new SchemaConverter({ schema: z.object({ type: z.nativeEnum(UserType), + disabled: optionalishDefault(z.boolean(), false), organization: optionalish(z.string()), dateOfBirth: optionalish(dateConverter.schema), clinician: optionalish(z.string()), @@ -33,6 +34,7 @@ export const userRegistrationInputConverter = new Lazy( }), encode: (object) => ({ type: object.type, + disabled: object.disabled, organization: object.organization ?? null, dateOfBirth: object.dateOfBirth ? dateConverter.encode(object.dateOfBirth) : null, @@ -60,15 +62,19 @@ export const userRegistrationConverter = new Lazy( }), ) -export interface UserClaims { - type: UserType - organization?: string -} +export const userClaimsSchema = z.object({ + type: z.nativeEnum(UserType), + organization: optionalish(z.string()), + disabled: optionalishDefault(z.boolean(), false), +}) + +export type UserClaims = z.output export class UserRegistration { // Stored Properties readonly type: UserType + readonly disabled: boolean readonly organization?: string readonly dateOfBirth?: Date @@ -90,6 +96,7 @@ export class UserRegistration { get claims(): UserClaims { const result: UserClaims = { type: this.type, + disabled: this.disabled, } if (this.organization !== undefined) { result.organization = this.organization @@ -101,6 +108,7 @@ export class UserRegistration { constructor(input: { type: UserType + disabled: boolean organization?: string dateOfBirth?: Date clinician?: string @@ -115,6 +123,7 @@ export class UserRegistration { timeZone?: string }) { this.type = input.type + this.disabled = input.disabled this.organization = input.organization this.dateOfBirth = input.dateOfBirth this.clinician = input.clinician diff --git a/functions/src/functions/deleteInvitation.test.ts b/functions/src/functions/deleteInvitation.test.ts index fefacfd5..18dff47b 100644 --- a/functions/src/functions/deleteInvitation.test.ts +++ b/functions/src/functions/deleteInvitation.test.ts @@ -29,6 +29,7 @@ describeWithEmulators('function: deleteInvitation', (env) => { code: 'TESTCODE', user: new UserRegistration({ type: UserType.patient, + disabled: false, organization: 'stanford', receivesAppointmentReminders: false, receivesInactivityReminders: true, diff --git a/functions/src/functions/disableUser.ts b/functions/src/functions/disableUser.ts new file mode 100644 index 00000000..ac7c1d70 --- /dev/null +++ b/functions/src/functions/disableUser.ts @@ -0,0 +1,41 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { + disableUserInputSchema, + DisableUserOutput, +} from '@stanfordbdhg/engagehf-models' +import { UserRole } from '../services/credential/credential.js' +import { getServiceFactory } from '../services/factory/getServiceFactory.js' +import { validatedOnCall } from './helpers.js' + +export const disableUser = validatedOnCall( + 'disableUser', + disableUserInputSchema, + async (request): Promise => { + const factory = getServiceFactory() + const credential = factory.credential(request.auth) + const userId = request.data.userId + const userService = factory.user() + + await credential.checkAsync( + () => [UserRole.admin], + async () => { + const user = await userService.getUser(credential.userId) + return user?.content.organization !== undefined ? + [ + UserRole.owner(user.content.organization), + UserRole.clinician(user.content.organization), + ] + : [] + }, + ) + + await userService.disableUser(userId) + }, +) diff --git a/functions/src/functions/enableUser.ts b/functions/src/functions/enableUser.ts new file mode 100644 index 00000000..3b101bcb --- /dev/null +++ b/functions/src/functions/enableUser.ts @@ -0,0 +1,41 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { + enableUserInputSchema, + EnableUserOutput, +} from '@stanfordbdhg/engagehf-models' +import { UserRole } from '../services/credential/credential.js' +import { getServiceFactory } from '../services/factory/getServiceFactory.js' +import { validatedOnCall } from './helpers.js' + +export const enableUser = validatedOnCall( + 'enableUser', + enableUserInputSchema, + async (request): Promise => { + const factory = getServiceFactory() + const credential = factory.credential(request.auth) + const userId = request.data.userId + const userService = factory.user() + + await credential.checkAsync( + () => [UserRole.admin], + async () => { + const user = await userService.getUser(credential.userId) + return user?.content.organization !== undefined ? + [ + UserRole.owner(user.content.organization), + UserRole.clinician(user.content.organization), + ] + : [] + }, + ) + + await userService.enableUser(userId) + }, +) diff --git a/functions/src/functions/enrollUser.test.ts b/functions/src/functions/enrollUser.test.ts index 7d18a06e..51984647 100644 --- a/functions/src/functions/enrollUser.test.ts +++ b/functions/src/functions/enrollUser.test.ts @@ -48,6 +48,7 @@ describeWithEmulators('function: enrollUser', (env) => { code: 'TESTCODE', user: new UserRegistration({ type: UserType.patient, + disabled: false, organization: 'stanford', receivesAppointmentReminders: true, receivesInactivityReminders: true, diff --git a/functions/src/services/credential/credential.ts b/functions/src/services/credential/credential.ts index 1cce49f8..5a882187 100644 --- a/functions/src/services/credential/credential.ts +++ b/functions/src/services/credential/credential.ts @@ -6,7 +6,11 @@ // SPDX-License-Identifier: MIT // -import { UserType } from '@stanfordbdhg/engagehf-models' +import { + UserClaims, + userClaimsSchema, + UserType, +} from '@stanfordbdhg/engagehf-models' import { https } from 'firebase-functions/v2' import { type AuthData } from 'firebase-functions/v2/tasks' @@ -67,13 +71,8 @@ export class UserRole { export class Credential { // Stored Properties - private readonly authData: AuthData - - // Computed Properties - - get userId(): string { - return this.authData.uid - } + readonly userId: string + private readonly claims: UserClaims // Constructor @@ -83,7 +82,12 @@ export class Credential { 'unauthenticated', 'User is not authenticated.', ) - this.authData = authData + try { + this.claims = userClaimsSchema.parse(authData.token) + } catch { + throw this.permissionDeniedError() + } + this.userId = authData.uid } // Methods @@ -112,33 +116,39 @@ export class Credential { ) } + disabledError(): https.HttpsError { + return new https.HttpsError('permission-denied', 'User is disabled.') + } + // Helpers private checkSingle(role: UserRole): boolean { + if (this.claims.disabled) throw this.disabledError() + switch (role.type) { case UserRoleType.admin: { - return this.authData.token.type === UserType.admin + return this.claims.type === UserType.admin } case UserRoleType.owner: { return ( - this.authData.token.type === UserType.owner && - this.authData.token.organization === role.organization + this.claims.type === UserType.owner && + this.claims.organization === role.organization ) } case UserRoleType.clinician: { return ( - this.authData.token.type === UserType.clinician && - this.authData.token.organization === role.organization + this.claims.type === UserType.clinician && + this.claims.organization === role.organization ) } case UserRoleType.patient: { return ( - this.authData.token.type === UserType.patient && - this.authData.token.organization === role.organization + this.claims.type === UserType.patient && + this.claims.organization === role.organization ) } case UserRoleType.user: { - return this.authData.uid === role.userId + return this.userId === role.userId } } } diff --git a/functions/src/services/trigger/triggerService.ts b/functions/src/services/trigger/triggerService.ts index 58009c2b..6085792f 100644 --- a/functions/src/services/trigger/triggerService.ts +++ b/functions/src/services/trigger/triggerService.ts @@ -777,6 +777,7 @@ export class TriggerService { code: '', user: new UserRegistration({ type: UserType.admin, + disabled: false, receivesAppointmentReminders: false, receivesInactivityReminders: false, receivesMedicationUpdates: false, diff --git a/functions/src/services/user/databaseUserService.test.ts b/functions/src/services/user/databaseUserService.test.ts index 7923d717..a64195dd 100644 --- a/functions/src/services/user/databaseUserService.test.ts +++ b/functions/src/services/user/databaseUserService.test.ts @@ -69,6 +69,7 @@ describe('DatabaseUserService', () => { expect(userData?.dateOfEnrollment).to.exist expect(userData?.claims).to.deep.equal({ type: UserType.admin, + disabled: false, }) }) @@ -114,6 +115,7 @@ describe('DatabaseUserService', () => { expect(userData?.claims).to.deep.equal({ type: UserType.clinician, organization: 'mockOrganization', + disabled: false, }) }) @@ -161,6 +163,7 @@ describe('DatabaseUserService', () => { expect(userData?.claims).to.deep.equal({ type: UserType.patient, organization: 'mockOrganization', + disabled: false, }) }) }) diff --git a/functions/src/services/user/databaseUserService.ts b/functions/src/services/user/databaseUserService.ts index 980f08b2..866e7853 100644 --- a/functions/src/services/user/databaseUserService.ts +++ b/functions/src/services/user/databaseUserService.ts @@ -285,6 +285,26 @@ export class DatabaseUserService implements UserService { // Users + async disableUser(userId: string): Promise { + await this.databaseService.runTransaction((collections, transaction) => { + transaction.update(collections.users.doc(userId), { + disabled: true, + }) + }) + + await this.updateClaims(userId) + } + + async enableUser(userId: string): Promise { + await this.databaseService.runTransaction((collections, transaction) => { + transaction.update(collections.users.doc(userId), { + disabled: false, + }) + }) + + await this.updateClaims(userId) + } + async getAllOwners(organizationId: string): Promise>> { return this.databaseService.getQuery((collections) => collections.users diff --git a/functions/src/services/user/userService.mock.ts b/functions/src/services/user/userService.mock.ts index 5e9a9c2f..001476f1 100644 --- a/functions/src/services/user/userService.mock.ts +++ b/functions/src/services/user/userService.mock.ts @@ -64,6 +64,7 @@ export class MockUserService implements UserService { content: new Invitation({ user: new UserRegistration({ type: UserType.patient, + disabled: false, dateOfBirth: new Date('1970-01-02'), clinician: 'mockPatient', receivesAppointmentReminders: true, @@ -127,6 +128,14 @@ export class MockUserService implements UserService { // Methods - User + async disableUser(userId: string): Promise { + return + } + + async enableUser(userId: string): Promise { + return + } + async getAllOwners(organizationId: string): Promise>> { return [] } @@ -142,6 +151,7 @@ export class MockUserService implements UserService { lastUpdate: new Date(), content: new User({ type: UserType.clinician, + disabled: false, dateOfBirth: new Date('1970-01-02'), clinician: 'mockClinician', lastActiveDate: new Date('2024-04-04'), diff --git a/functions/src/services/user/userService.ts b/functions/src/services/user/userService.ts index 2ab2c592..911b3e19 100644 --- a/functions/src/services/user/userService.ts +++ b/functions/src/services/user/userService.ts @@ -52,6 +52,8 @@ export interface UserService { // Users + disableUser(userId: string): Promise + enableUser(userId: string): Promise getAllOwners(organizationId: string): Promise>> getAllPatients(): Promise>> getUser(userId: string): Promise | undefined> diff --git a/functions/src/tests/functions/testEnvironment.ts b/functions/src/tests/functions/testEnvironment.ts index 53459750..9c4842b7 100644 --- a/functions/src/tests/functions/testEnvironment.ts +++ b/functions/src/tests/functions/testEnvironment.ts @@ -127,6 +127,7 @@ export class EmulatorTestEnvironment { async createUser( options: { type: UserType + disabled?: boolean organization?: string clinician?: string dateOfEnrollment?: Date @@ -145,6 +146,7 @@ export class EmulatorTestEnvironment { await this.collections.users.doc(authUser.uid).set( new User({ type: options.type, + disabled: options.disabled ?? false, organization: options.organization, dateOfEnrollment: options.dateOfEnrollment ?? new Date(), clinician: options.clinician, From e0a1d9af6d777dc53c052cb1ef5e09d8be6169bf Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 10:13:28 +0100 Subject: [PATCH 02/11] Add README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 127a56ee..2799b6fb 100644 --- a/README.md +++ b/README.md @@ -310,6 +310,7 @@ In this section, we describe all user-related data to be stored. The security ru |Property|Type|Values|Comments| |-|-|-|-| |type|string|e.g. "admin", "owners", "clinician", "patient"|The type of the user.| +|disabled|optional boolean|If set to `true`, the users looses permission to perform any action except reading its own data stored in `users/$userId$`.| |dateOfEnrollment|Date|-|The date when the invitation code was used to create this user.| |dateOfBirth|optional Date|-|The date when the user was born.| |genderIdentity|optional string|"female","male","transgender","nonBinary","preferNotToState"|The gender identity chosen when a patient redeemed the invitation.| From 9cb3041de5ff21e3e0c9e453fc0905d1ad3888bd Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 10:20:34 +0100 Subject: [PATCH 03/11] Adapt rules --- firestore.rules | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/firestore.rules b/firestore.rules index 39f44514..493daf29 100644 --- a/firestore.rules +++ b/firestore.rules @@ -2,7 +2,12 @@ rules_version = '2'; service cloud.firestore { match /databases/{databaseId}/documents { function isAuthenticated() { - return request.auth != null && ('type' in request.auth.token); + return request.auth != null + && ('type' in request.auth.token) + && ( + !('disabled' in request.auth.token) + || request.auth.token == false + ); } function isAdmin() { @@ -42,10 +47,14 @@ service cloud.firestore { return get(/databases/$(databaseId)/documents/invitations/$(invitationId)); } + function valueExists(object, property) { + return (property in object) && object[property] == null; + } + function isEqualOptional(object0, property0, object1, property1) { - return (property0 in object0) - ? ((property1 in object1) && object0[property0] == object1[property1]) - : !(property1 in object1); + return valueExists(object0, property0) + ? (valueExists(object) && object0[property0] == object1[property1]) + : !valueExists(object1, property1); } match /invitations/{invitationId} { @@ -107,6 +116,7 @@ service cloud.firestore { match /users/{userId} { function securityRelatedFieldsDidNotChange() { return isEqualOptional(request.resource.data, 'type', resource.data, 'type') + && isEqualOptional(request.resource.data, 'disabled', resource.data, 'disabled') && isEqualOptional(request.resource.data, 'organization', resource.data, 'organization'); } From 0beb24d75510c4a918bcdc356e02f26dd63bb5c2 Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 10:21:04 +0100 Subject: [PATCH 04/11] Adapt rules --- firestore.rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firestore.rules b/firestore.rules index 493daf29..708a3154 100644 --- a/firestore.rules +++ b/firestore.rules @@ -48,7 +48,7 @@ service cloud.firestore { } function valueExists(object, property) { - return (property in object) && object[property] == null; + return (property in object) && object[property] != null; } function isEqualOptional(object0, property0, object1, property1) { From 2971fbb24be7b3c16147ca8503450c1c8901f509 Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 10:22:34 +0100 Subject: [PATCH 05/11] Adapt rules --- firestore.rules | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/firestore.rules b/firestore.rules index 708a3154..5929edcf 100644 --- a/firestore.rules +++ b/firestore.rules @@ -1,12 +1,22 @@ rules_version = '2'; service cloud.firestore { match /databases/{databaseId}/documents { + function valueExists(object, property) { + return (property in object) && object[property] != null; + } + + function isEqualOptional(object0, property0, object1, property1) { + return valueExists(object0, property0) + ? (valueExists(object) && object0[property0] == object1[property1]) + : !valueExists(object1, property1); + } + function isAuthenticated() { return request.auth != null && ('type' in request.auth.token) && ( - !('disabled' in request.auth.token) - || request.auth.token == false + !valueExists(request.auth.token, 'disabled') + || request.auth.token.disabled == false ); } @@ -47,16 +57,6 @@ service cloud.firestore { return get(/databases/$(databaseId)/documents/invitations/$(invitationId)); } - function valueExists(object, property) { - return (property in object) && object[property] != null; - } - - function isEqualOptional(object0, property0, object1, property1) { - return valueExists(object0, property0) - ? (valueExists(object) && object0[property0] == object1[property1]) - : !valueExists(object1, property1); - } - match /invitations/{invitationId} { function securityRelatedFieldsDidNotChange() { return isEqualOptional(request.resource.data.user, 'type', resource.data.user, 'type') From 94d063a6ede49cbf156228c1157d83fbcab4addc Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 11:08:16 +0100 Subject: [PATCH 06/11] lint:fix --- functions/src/functions/disableUser.ts | 4 ++-- functions/src/functions/enableUser.ts | 4 ++-- functions/src/services/credential/credential.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functions/src/functions/disableUser.ts b/functions/src/functions/disableUser.ts index ac7c1d70..b9d827f0 100644 --- a/functions/src/functions/disableUser.ts +++ b/functions/src/functions/disableUser.ts @@ -8,11 +8,11 @@ import { disableUserInputSchema, - DisableUserOutput, + type DisableUserOutput, } from '@stanfordbdhg/engagehf-models' +import { validatedOnCall } from './helpers.js' import { UserRole } from '../services/credential/credential.js' import { getServiceFactory } from '../services/factory/getServiceFactory.js' -import { validatedOnCall } from './helpers.js' export const disableUser = validatedOnCall( 'disableUser', diff --git a/functions/src/functions/enableUser.ts b/functions/src/functions/enableUser.ts index 3b101bcb..1b642d10 100644 --- a/functions/src/functions/enableUser.ts +++ b/functions/src/functions/enableUser.ts @@ -8,11 +8,11 @@ import { enableUserInputSchema, - EnableUserOutput, + type EnableUserOutput, } from '@stanfordbdhg/engagehf-models' +import { validatedOnCall } from './helpers.js' import { UserRole } from '../services/credential/credential.js' import { getServiceFactory } from '../services/factory/getServiceFactory.js' -import { validatedOnCall } from './helpers.js' export const enableUser = validatedOnCall( 'enableUser', diff --git a/functions/src/services/credential/credential.ts b/functions/src/services/credential/credential.ts index 5a882187..0c5ed73f 100644 --- a/functions/src/services/credential/credential.ts +++ b/functions/src/services/credential/credential.ts @@ -7,7 +7,7 @@ // import { - UserClaims, + type UserClaims, userClaimsSchema, UserType, } from '@stanfordbdhg/engagehf-models' From e27cdfb5b51d61047a4f46dceff309d9bb7fb929 Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 15:53:16 +0100 Subject: [PATCH 07/11] Add tests and fix rules --- firestore.rules | 4 +- functions/src/functions/disableUser.test.ts | 91 +++++++++++++++++++++ functions/src/functions/enableUser.test.ts | 91 +++++++++++++++++++++ 3 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 functions/src/functions/disableUser.test.ts create mode 100644 functions/src/functions/enableUser.test.ts diff --git a/firestore.rules b/firestore.rules index 5929edcf..7b545d96 100644 --- a/firestore.rules +++ b/firestore.rules @@ -7,7 +7,7 @@ service cloud.firestore { function isEqualOptional(object0, property0, object1, property1) { return valueExists(object0, property0) - ? (valueExists(object) && object0[property0] == object1[property1]) + ? (valueExists(object1, property1) && object0[property0] == object1[property1]) : !valueExists(object1, property1); } @@ -132,7 +132,7 @@ service cloud.firestore { || isOwnerOrClinicianOf(resource.data.organization); allow create: if isAdmin() - || (isUser(userId) && !('organization' in request.resource.data) && !('type' in request.resource.data)); + || (isUser(userId) && !valueExists(request.resource.data, 'organization') && !valueExists(request.resource.data, 'type')); allow update: if isAdmin() || (securityRelatedFieldsDidNotChange() && (isUser(userId) || isAllowedUpdateWithinOrganization())); diff --git a/functions/src/functions/disableUser.test.ts b/functions/src/functions/disableUser.test.ts new file mode 100644 index 00000000..9611b401 --- /dev/null +++ b/functions/src/functions/disableUser.test.ts @@ -0,0 +1,91 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { UserType } from '@stanfordbdhg/engagehf-models' +import { describeWithEmulators } from '../tests/functions/testEnvironment.js' +import { expect } from 'chai' +import { disableUser } from './disableUser.js' + +describeWithEmulators('function: disableUser', (env) => { + it('disables an enabled user', async () => { + const clinicianId = await env.createUser({ + type: UserType.clinician, + organization: 'stanford', + }) + + const userId = await env.createUser({ + type: UserType.patient, + organization: 'stanford', + clinician: clinicianId, + }) + + const userService = env.factory.user() + + const originalUser = await userService.getUser(userId) + expect(originalUser).to.exist + expect(originalUser?.content.claims.disabled).to.be.false + expect(originalUser?.content.disabled).to.be.false + + await env.call( + disableUser, + { userId: userId }, + { + uid: clinicianId, + token: { + type: UserType.clinician, + organization: 'stanford', + disabled: false, + }, + }, + ) + + const user = await userService.getUser(userId) + expect(user).to.exist + expect(user?.content.claims.disabled).to.be.true + expect(user?.content.disabled).to.be.true + }) + + it('keeps disabled users disabled', async () => { + const clinicianId = await env.createUser({ + type: UserType.clinician, + organization: 'stanford', + }) + + const userId = await env.createUser({ + type: UserType.patient, + organization: 'stanford', + clinician: clinicianId, + disabled: true, + }) + + const userService = env.factory.user() + + const originalUser = await userService.getUser(userId) + expect(originalUser).to.exist + expect(originalUser?.content.claims.disabled).to.be.true + expect(originalUser?.content.disabled).to.be.true + + await env.call( + disableUser, + { userId: userId }, + { + uid: clinicianId, + token: { + type: UserType.clinician, + organization: 'stanford', + disabled: false, + }, + }, + ) + + const user = await userService.getUser(userId) + expect(user).to.exist + expect(user?.content.claims.disabled).to.be.true + expect(user?.content.disabled).to.be.true + }) +}) diff --git a/functions/src/functions/enableUser.test.ts b/functions/src/functions/enableUser.test.ts new file mode 100644 index 00000000..02a11371 --- /dev/null +++ b/functions/src/functions/enableUser.test.ts @@ -0,0 +1,91 @@ +// +// This source file is part of the ENGAGE-HF project based on the Stanford Spezi Template Application project +// +// SPDX-FileCopyrightText: 2023 Stanford University +// +// SPDX-License-Identifier: MIT +// + +import { UserType } from '@stanfordbdhg/engagehf-models' +import { describeWithEmulators } from '../tests/functions/testEnvironment.js' +import { enableUser } from './enableUser.js' +import { expect } from 'chai' + +describeWithEmulators('function: enableUser', (env) => { + it('enables a disabled user', async () => { + const clinicianId = await env.createUser({ + type: UserType.clinician, + organization: 'stanford', + }) + + const userId = await env.createUser({ + type: UserType.patient, + organization: 'stanford', + clinician: clinicianId, + disabled: true, + }) + + const userService = env.factory.user() + + const originalUser = await userService.getUser(userId) + expect(originalUser).to.exist + expect(originalUser?.content.claims.disabled).to.be.true + expect(originalUser?.content.disabled).to.be.true + + await env.call( + enableUser, + { userId: userId }, + { + uid: clinicianId, + token: { + type: UserType.clinician, + organization: 'stanford', + disabled: false, + }, + }, + ) + + const user = await userService.getUser(userId) + expect(user).to.exist + expect(user?.content.claims.disabled).to.be.false + expect(user?.content.disabled).to.be.false + }) + + it('keeps enabled users enabled', async () => { + const clinicianId = await env.createUser({ + type: UserType.clinician, + organization: 'stanford', + }) + + const userId = await env.createUser({ + type: UserType.patient, + organization: 'stanford', + clinician: clinicianId, + }) + + const userService = env.factory.user() + + const originalUser = await userService.getUser(userId) + expect(originalUser).to.exist + expect(originalUser?.content.claims.disabled).to.be.false + expect(originalUser?.content.disabled).to.be.false + + await env.call( + enableUser, + { userId: userId }, + { + uid: clinicianId, + token: { + type: UserType.clinician, + organization: 'stanford', + disabled: false, + }, + }, + ) + + const user = await userService.getUser(userId) + expect(user).to.exist + expect(user?.content.claims.disabled).to.be.false + expect(user?.content.disabled).to.be.false + }) +}) From 3e2d6485bb46ff6a2624d98f7fa5d52407f2623a Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 15:54:24 +0100 Subject: [PATCH 08/11] Add tests and fix rules --- functions/src/functions/updateUserInformation.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/functions/updateUserInformation.test.ts b/functions/src/functions/updateUserInformation.test.ts index fea3056b..bef58bfc 100644 --- a/functions/src/functions/updateUserInformation.test.ts +++ b/functions/src/functions/updateUserInformation.test.ts @@ -25,7 +25,7 @@ describeWithEmulators('function: updateUserInformation', (env) => { }, }, }, - { uid: authUser.uid }, + { uid: authUser.uid, token: { disabled: false } }, ) const updatedUser = await env.auth.getUser(authUser.uid) From 5bd1d3b2f2ffa0bb30a97cc71babc8f70fecb9ad Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Wed, 15 Jan 2025 16:02:31 +0100 Subject: [PATCH 09/11] update --- functions/src/functions/updateUserInformation.test.ts | 2 +- functions/src/services/credential/credential.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/functions/src/functions/updateUserInformation.test.ts b/functions/src/functions/updateUserInformation.test.ts index bef58bfc..fea3056b 100644 --- a/functions/src/functions/updateUserInformation.test.ts +++ b/functions/src/functions/updateUserInformation.test.ts @@ -25,7 +25,7 @@ describeWithEmulators('function: updateUserInformation', (env) => { }, }, }, - { uid: authUser.uid, token: { disabled: false } }, + { uid: authUser.uid }, ) const updatedUser = await env.auth.getUser(authUser.uid) diff --git a/functions/src/services/credential/credential.ts b/functions/src/services/credential/credential.ts index 0c5ed73f..878e7cbb 100644 --- a/functions/src/services/credential/credential.ts +++ b/functions/src/services/credential/credential.ts @@ -72,7 +72,7 @@ export class Credential { // Stored Properties readonly userId: string - private readonly claims: UserClaims + private readonly claims: Partial // Constructor @@ -83,7 +83,7 @@ export class Credential { 'User is not authenticated.', ) try { - this.claims = userClaimsSchema.parse(authData.token) + this.claims = userClaimsSchema.partial().parse(authData.token) } catch { throw this.permissionDeniedError() } @@ -123,7 +123,7 @@ export class Credential { // Helpers private checkSingle(role: UserRole): boolean { - if (this.claims.disabled) throw this.disabledError() + if (this.claims.disabled === true) throw this.disabledError() switch (role.type) { case UserRoleType.admin: { From 87206b187c62f380151ca170876132e40c6f5418 Mon Sep 17 00:00:00 2001 From: Paul Kraft Date: Thu, 16 Jan 2025 17:29:27 +0100 Subject: [PATCH 10/11] Update rules --- firestore.rules | 13 ++++---- functions/src/tests/rules/users.test.ts | 40 +++++++++++++++++++++---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/firestore.rules b/firestore.rules index 7b545d96..94482bfa 100644 --- a/firestore.rules +++ b/firestore.rules @@ -121,15 +121,18 @@ service cloud.firestore { } function isAllowedUpdateWithinOrganization() { - return resource.data.type in ['patient'] - ? isOwnerOf(resource.data.organization) - : isOwnerOrClinicianOf(resource.data.organization) && resource.data.type in ['clinician']; + return valueExists(resource.data, 'type') ? + ( + resource.data.type in ['patient'] + ? isOwnerOf(resource.data.organization) + : isOwnerOrClinicianOf(resource.data.organization) && resource.data.type in ['clinician'] + ) : false; } allow read: if isAdmin() - || (resource == null && isAuthenticated()) || (request.auth != null && request.auth.uid == userId) - || isOwnerOrClinicianOf(resource.data.organization); + || (resource == null && isAuthenticated()) + || (resource != null && valueExists(resource.data, 'organization') && isOwnerOrClinicianOf(resource.data.organization)); allow create: if isAdmin() || (isUser(userId) && !valueExists(request.resource.data, 'organization') && !valueExists(request.resource.data, 'type')); diff --git a/functions/src/tests/rules/users.test.ts b/functions/src/tests/rules/users.test.ts index a297f5cd..16f7a97d 100644 --- a/functions/src/tests/rules/users.test.ts +++ b/functions/src/tests/rules/users.test.ts @@ -28,6 +28,7 @@ describe('firestore.rules: users/{userId}', () => { const patientId = 'mockPatient' const userId = 'mockUser' const unknownId = 'mockUnknown' + const disabledUserId = 'disabledMockUser' let testEnvironment: RulesTestEnvironment let adminFirestore: firebase.firestore.Firestore @@ -35,6 +36,7 @@ describe('firestore.rules: users/{userId}', () => { let clinicianFirestore: firebase.firestore.Firestore let patientFirestore: firebase.firestore.Firestore let userFirestore: firebase.firestore.Firestore + let disabledUserFirestore: firebase.firestore.Firestore before(async () => { testEnvironment = await initializeTestEnvironment({ @@ -72,6 +74,14 @@ describe('firestore.rules: users/{userId}', () => { .firestore() userFirestore = testEnvironment.authenticatedContext(userId, {}).firestore() + + disabledUserFirestore = testEnvironment + .authenticatedContext(disabledUserId, { + type: UserType.patient, + organization: organizationId, + disabled: true, + }) + .firestore() }) beforeEach(async () => { @@ -91,6 +101,11 @@ describe('firestore.rules: users/{userId}', () => { .doc(`users/${patientId}`) .set({ type: UserType.patient, organization: organizationId }) await firestore.doc(`users/${userId}`).set({}) + await firestore.doc(`users/${disabledUserId}`).set({ + type: UserType.patient, + organization: organizationId, + disabled: true, + }) }) }) @@ -99,45 +114,57 @@ describe('firestore.rules: users/{userId}', () => { }) it('gets users/{userId}', async () => { - console.log('admin') await assertSucceeds(adminFirestore.doc(`users/${adminId}`).get()) await assertSucceeds(adminFirestore.doc(`users/${ownerId}`).get()) await assertSucceeds(adminFirestore.doc(`users/${clinicianId}`).get()) await assertSucceeds(adminFirestore.doc(`users/${patientId}`).get()) await assertSucceeds(adminFirestore.doc(`users/${userId}`).get()) await assertSucceeds(adminFirestore.doc(`users/${unknownId}`).get()) + await assertSucceeds(adminFirestore.doc(`users/${disabledUserId}`).get()) - console.log('owner') await assertFails(ownerFirestore.doc(`users/${adminId}`).get()) await assertSucceeds(ownerFirestore.doc(`users/${ownerId}`).get()) await assertSucceeds(ownerFirestore.doc(`users/${clinicianId}`).get()) await assertSucceeds(ownerFirestore.doc(`users/${patientId}`).get()) await assertFails(ownerFirestore.doc(`users/${userId}`).get()) await assertSucceeds(ownerFirestore.doc(`users/${unknownId}`).get()) + await assertSucceeds(ownerFirestore.doc(`users/${disabledUserId}`).get()) - console.log('clinician') await assertFails(clinicianFirestore.doc(`users/${adminId}`).get()) await assertSucceeds(clinicianFirestore.doc(`users/${ownerId}`).get()) await assertSucceeds(clinicianFirestore.doc(`users/${clinicianId}`).get()) await assertSucceeds(clinicianFirestore.doc(`users/${patientId}`).get()) await assertFails(clinicianFirestore.doc(`users/${userId}`).get()) await assertSucceeds(clinicianFirestore.doc(`users/${unknownId}`).get()) + await assertSucceeds( + clinicianFirestore.doc(`users/${disabledUserId}`).get(), + ) - console.log('patient') await assertFails(patientFirestore.doc(`users/${adminId}`).get()) await assertFails(patientFirestore.doc(`users/${ownerId}`).get()) await assertFails(patientFirestore.doc(`users/${clinicianId}`).get()) await assertSucceeds(patientFirestore.doc(`users/${patientId}`).get()) await assertFails(patientFirestore.doc(`users/${userId}`).get()) await assertSucceeds(patientFirestore.doc(`users/${unknownId}`).get()) + await assertFails(patientFirestore.doc(`users/${disabledUserId}`).get()) - console.log('user') await assertFails(userFirestore.doc(`users/${adminId}`).get()) await assertFails(userFirestore.doc(`users/${ownerId}`).get()) await assertFails(userFirestore.doc(`users/${clinicianId}`).get()) await assertFails(userFirestore.doc(`users/${patientId}`).get()) await assertSucceeds(userFirestore.doc(`users/${userId}`).get()) await assertFails(userFirestore.doc(`users/${unknownId}`).get()) + await assertFails(userFirestore.doc(`users/${disabledUserId}`).get()) + + await assertFails(disabledUserFirestore.doc(`users/${adminId}`).get()) + await assertFails(disabledUserFirestore.doc(`users/${ownerId}`).get()) + await assertFails(disabledUserFirestore.doc(`users/${clinicianId}`).get()) + await assertFails(disabledUserFirestore.doc(`users/${patientId}`).get()) + await assertFails(disabledUserFirestore.doc(`users/${userId}`).get()) + await assertFails(disabledUserFirestore.doc(`users/${unknownId}`).get()) + await assertSucceeds( + disabledUserFirestore.doc(`users/${disabledUserId}`).get(), + ) }) it('lists users', async () => { @@ -211,6 +238,9 @@ describe('firestore.rules: users/{userId}', () => { await environment.firestore().doc(`users/${userId}`).delete() }) await assertFails(userFirestore.doc(`users/${userId}`).set({})) + await assertFails( + disabledUserFirestore.doc(`users/${disabledUserId}`).set({}), + ) }) it('updates users/{userId} as admin', async () => { From faa787fe99ce1cdce635122703623d3dd68b1a7f Mon Sep 17 00:00:00 2001 From: Arkadiusz Bachorski <60391032+arkadiuszbachorski@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:18:11 +0100 Subject: [PATCH 11/11] Export functions --- functions/src/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/functions/src/index.ts b/functions/src/index.ts index 59f1c50d..426cf3bf 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -30,3 +30,5 @@ export * from './functions/registerDevice.js' export * from './functions/unregisterDevice.js' export * from './functions/updateStaticData.js' export * from './functions/updateUserInformation.js' +export * from './functions/disableUser.js' +export * from './functions/enableUser.js'