From 6dab27e2bc5751b44f3216f9fc4ce07d3aa34187 Mon Sep 17 00:00:00 2001
From: Paul Kraft <pauljohanneskraft@users.noreply.github.com>
Date: Thu, 7 Nov 2024 10:59:38 -0800
Subject: [PATCH] Only use session claims

---
 functions/src/functions/blocking.ts           |  4 +-
 functions/src/functions/onUserWritten.ts      | 34 ++++++---------
 .../src/services/user/databaseUserService.ts  | 42 ++++---------------
 .../src/services/user/userService.mock.ts     |  4 +-
 functions/src/services/user/userService.ts    |  3 +-
 5 files changed, 29 insertions(+), 58 deletions(-)

diff --git a/functions/src/functions/blocking.ts b/functions/src/functions/blocking.ts
index 3cc367a0..8af9fe21 100644
--- a/functions/src/functions/blocking.ts
+++ b/functions/src/functions/blocking.ts
@@ -72,10 +72,12 @@ export const beforeUserSignedInFunction = beforeUserSignedIn(
   async (event) => {
     try {
       const userService = getServiceFactory().user()
-      await userService.updateClaims(event.data.uid)
+      const claims = await userService.getClaims(event.data.uid)
       logger.info(`beforeUserSignedIn finished successfully.`)
+      return { sessionClaims: claims }
     } catch (error) {
       logger.error(`beforeUserSignedIn finished with error: ${String(error)}`)
+      return { sessionClaims: {} }
     }
   },
 )
diff --git a/functions/src/functions/onUserWritten.ts b/functions/src/functions/onUserWritten.ts
index bb819670..084e7fda 100644
--- a/functions/src/functions/onUserWritten.ts
+++ b/functions/src/functions/onUserWritten.ts
@@ -8,41 +8,33 @@
 
 import { type User, userConverter } from '@stanfordbdhg/engagehf-models'
 import { logger } from 'firebase-functions'
-import { onDocumentWritten } from 'firebase-functions/v2/firestore'
+import {
+  onDocumentCreated,
+  onDocumentWritten,
+} from 'firebase-functions/v2/firestore'
 import { DatabaseConverter } from '../services/database/databaseConverter.js'
 import { type Document } from '../services/database/databaseService.js'
 import { getServiceFactory } from '../services/factory/getServiceFactory.js'
 
-export const onUserWritten = onDocumentWritten(
+export const onUserWritten = onDocumentCreated(
   'users/{userId}',
   async (event) => {
+    if (event.data === undefined) return
     const factory = getServiceFactory()
     const userService = factory.user()
     try {
-      if (
-        event.data?.before.exists !== true &&
-        event.data?.after.exists === true
-      ) {
-        const converter = new DatabaseConverter(userConverter.value)
-        const userDoc: Document<User> = {
-          id: event.params.userId,
-          path: event.document,
-          content: converter.fromFirestore(event.data.after),
-          lastUpdate: new Date(event.time),
-        }
-        await userService.finishUserEnrollment(userDoc)
+      const converter = new DatabaseConverter(userConverter.value)
+      const userDoc: Document<User> = {
+        id: event.params.userId,
+        path: event.document,
+        content: converter.fromFirestore(event.data),
+        lastUpdate: new Date(event.time),
       }
+      await userService.finishUserEnrollment(userDoc)
     } catch (error) {
       logger.error(
         `Error finishing enrollment for user with id '${event.params.userId}' on change of user: ${String(error)}`,
       )
     }
-    try {
-      await userService.updateClaims(event.params.userId)
-    } catch (error) {
-      logger.error(
-        `Error processing claims update for userId '${event.params.userId}' on change of user: ${String(error)}`,
-      )
-    }
   },
 )
diff --git a/functions/src/services/user/databaseUserService.ts b/functions/src/services/user/databaseUserService.ts
index 4c6bad16..fcaf3c74 100644
--- a/functions/src/services/user/databaseUserService.ts
+++ b/functions/src/services/user/databaseUserService.ts
@@ -63,36 +63,17 @@ export class DatabaseUserService implements UserService {
     })
   }
 
-  async updateClaims(userId: string): Promise<void> {
-    try {
-      const user = await this.getUser(userId)
-      if (user !== undefined) {
-        const claims: UserClaims = {
-          type: user.content.type,
-        }
-        if (user.content.organization !== undefined)
-          claims.organization = user.content.organization
-        logger.info(
-          `Will set claims for user ${userId}: ${JSON.stringify(claims)}`,
-        )
-        await this.auth.setCustomUserClaims(userId, claims)
-        logger.info(`Successfully set claims for user ${userId}.`)
-      } else {
-        await this.auth.setCustomUserClaims(userId, {})
-        logger.info(
-          `Successfully set claims for not-yet-enrolled user ${userId}.`,
-        )
+  async getClaims(userId: string): Promise<object> {
+    const user = await this.getUser(userId)
+    if (user !== undefined) {
+      const claims: UserClaims = {
+        type: user.content.type,
       }
-    } catch (error) {
-      logger.error(
-        `Failed to update claims for user ${userId}: ${String(error)}`,
-      )
-      await this.auth.setCustomUserClaims(userId, {})
-      logger.debug(
-        `Successfully reset claims for user ${userId} to empty object.`,
-      )
-      throw error
+      if (user.content.organization !== undefined)
+        claims.organization = user.content.organization
+      return claims
     }
+    return {}
   }
 
   // Invitations
@@ -157,7 +138,6 @@ export class DatabaseUserService implements UserService {
         if (!options.isSingleSignOn) {
           await this.auth.updateUser(userId, {
             displayName: invitation.content.auth?.displayName ?? undefined,
-            email: invitation.content.auth?.email ?? undefined,
             phoneNumber: invitation.content.auth?.phoneNumber ?? undefined,
             photoURL: invitation.content.auth?.photoURL ?? undefined,
           })
@@ -176,10 +156,6 @@ export class DatabaseUserService implements UserService {
         })
         transaction.set(userRef, userData)
 
-        if (!options.isSingleSignOn) {
-          await this.updateClaims(userId)
-        }
-
         return {
           id: userId,
           path: userRef.path,
diff --git a/functions/src/services/user/userService.mock.ts b/functions/src/services/user/userService.mock.ts
index 5e9a9c2f..f3b49942 100644
--- a/functions/src/services/user/userService.mock.ts
+++ b/functions/src/services/user/userService.mock.ts
@@ -44,8 +44,8 @@ export class MockUserService implements UserService {
     return
   }
 
-  async updateClaims(userId: string): Promise<void> {
-    return
+  async getClaims(userId: string): Promise<object> {
+    return {}
   }
 
   // Methods - Invitations
diff --git a/functions/src/services/user/userService.ts b/functions/src/services/user/userService.ts
index 2ab2c592..4d2dee55 100644
--- a/functions/src/services/user/userService.ts
+++ b/functions/src/services/user/userService.ts
@@ -7,6 +7,7 @@
 //
 
 import {
+  UserType,
   type Invitation,
   type Organization,
   type User,
@@ -24,7 +25,7 @@ export interface UserService {
 
   getAuth(userId: string): Promise<UserAuth>
   updateAuth(userId: string, auth: UserAuth): Promise<void>
-  updateClaims(userId: string): Promise<void>
+  getClaims(userId: string): Promise<object>
 
   // Invitations