fix: Avoid caching permissions with different arguments (#8670)

* fix: Avoid caching permissions with different arguments Permissions are cached by function name. When these depend on the arguments passed, they should have their own cache key, see https://the-guild.dev/graphql/shield/docs/rules#limitations * Make isViewerOnTeam strict The passed function might access source or args and thus we should use the strict cache.
ParabolInc · Nov 17, 2023 · a6dcd7f · a6dcd7f
1 parent 0fdef2d
commit a6dcd7f
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 20 deletions.
diff --git a/packages/server/graphql/public/rules/isEnvVarTrue.ts b/packages/server/graphql/public/rules/isEnvVarTrue.ts
@@ -1,7 +1,7 @@
 import {rule} from 'graphql-shield'
 
 const isEnvVarTrue = (varName: string) =>
-  rule({cache: 'contextual'})(() => {
+  rule(`isEnvVarTrue-${varName}`, {cache: 'contextual'})(() => {
     const isEnabled = process.env[varName] === 'true'
     return isEnabled || new Error(`${varName} is not enabled`)
   })

diff --git a/packages/server/graphql/public/rules/isViewerOnTeam.ts b/packages/server/graphql/public/rules/isViewerOnTeam.ts
@@ -6,15 +6,17 @@ import {GQLContext} from '../../graphql'
 type GetTeamId = (source: any, args: any, context: GQLContext) => Promise<string | Error>
 
 const isViewerOnTeam = (getTeamId: GetTeamId) =>
-  rule({cache: 'contextual'})(async (source, args, context: GQLContext) => {
-    const {authToken, dataLoader} = context
-    const viewerId = getUserId(authToken)
-    const teamId = await getTeamId(source, args, context)
-    if (teamId instanceof Error) return teamId
-    const teamMemberId = TeamMemberId.join(teamId, viewerId)
-    const teamMember = await dataLoader.get('teamMembers').load(teamMemberId)
-    if (!teamMember) return new Error('Viewer in not on team')
-    return true
-  })
+  rule(`isViewerOnTeam-${getTeamId.name || getTeamId}`, {cache: 'strict'})(
+    async (source, args, context: GQLContext) => {
+      const {authToken, dataLoader} = context
+      const viewerId = getUserId(authToken)
+      const teamId = await getTeamId(source, args, context)
+      if (teamId instanceof Error) return teamId
+      const teamMemberId = TeamMemberId.join(teamId, viewerId)
+      const teamMember = await dataLoader.get('teamMembers').load(teamMemberId)
+      if (!teamMember) return new Error('Viewer is not on team')
+      return true
+    }
+  )
 
 export default isViewerOnTeam
diff --git a/packages/server/graphql/public/rules/rateLimit.ts b/packages/server/graphql/public/rules/rateLimit.ts
@@ -8,15 +8,17 @@ interface RateLimitOptions {
 }
 
 const rateLimit = ({perMinute, perHour}: RateLimitOptions) =>
-  rule({cache: 'contextual'})((_source, _args, context: GQLContext, info) => {
-    const {authToken, rateLimiter, ip} = context
-    const {fieldName} = info
-    const userId = getUserId(authToken) || ip
-    const {lastMinute, lastHour} = rateLimiter.log(userId, fieldName, !!perHour)
-    if (lastMinute > perMinute || (lastHour && lastHour > perHour)) {
-      return new Error('429 Too Many Requests')
+  rule(`rateLimit-${perMinute}-${perHour}`, {cache: 'contextual'})(
+    (_source, _args, context: GQLContext, info) => {
+      const {authToken, rateLimiter, ip} = context
+      const {fieldName} = info
+      const userId = getUserId(authToken) || ip
+      const {lastMinute, lastHour} = rateLimiter.log(userId, fieldName, !!perHour)
+      if (lastMinute > perMinute || (lastHour && lastHour > perHour)) {
+        return new Error('429 Too Many Requests')
+      }
+      return true
     }
-    return true
-  })
+  )
 
 export default rateLimit