fix: safer queries

Signed-off-by: Matt Krick <matt.krick@gmail.com>
ParabolInc · Jul 10, 2024 · 8d33999 · 8d33999
1 parent 3e3634c
commit 8d33999
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 26 deletions.
diff --git a/packages/server/graphql/graphql.ts b/packages/server/graphql/graphql.ts
@@ -15,6 +15,7 @@ export interface GQLContext {
   dataLoader: DataLoaderWorker
 }
 
+export type SubscriptionContext = Omit<GQLContext, 'dataLoader'>
 export interface InternalContext {
   dataLoader: DataLoaderWorker
   authToken: AuthToken

diff --git a/packages/server/graphql/private/queries/suOrgCount.ts b/packages/server/graphql/private/queries/suOrgCount.ts
@@ -6,14 +6,19 @@ import {QueryResolvers} from '../resolverTypes'
 const suOrgCount: QueryResolvers['suOrgCount'] = async (_source, {minOrgSize, tier}) => {
   const pg = getKysely()
   const pgResults = await pg
-    .selectFrom('OrganizationUser')
-    .select(({fn}) => fn.count('id').as('orgSize'))
-    .where('tier', '=', tier)
-    .where('inactive', '=', false)
-    .where('removedAt', 'is', null)
-    .groupBy('orgId')
-    .having(({eb, fn}) => eb(fn.count('id'), '>=', minOrgSize))
-    .execute()
+    .with('BigOrgs', (qb) =>
+      qb
+        .selectFrom('OrganizationUser')
+        .select(({fn}) => fn.count('id').as('orgSize'))
+        .where('tier', '=', tier)
+        .where('inactive', '=', false)
+        .where('removedAt', 'is', null)
+        .groupBy('orgId')
+        .having(({eb, fn}) => eb(fn.count('id'), '>=', minOrgSize))
+    )
+    .selectFrom('BigOrgs')
+    .select(({fn}) => fn.count('orgSize').as('count'))
+    .executeTakeFirstOrThrow()
 
   // TEST in Phase 2!
   console.log(pgResults)

diff --git a/packages/server/graphql/private/queries/suProOrgInfo.ts b/packages/server/graphql/private/queries/suProOrgInfo.ts
@@ -1,3 +1,4 @@
+import {sql} from 'kysely'
 import getRethink from '../../../database/rethinkDriver'
 import {RDatum} from '../../../database/stricterR'
 import {selectOrganizations} from '../../../dataloader/primaryKeyLoaderMakers'
@@ -14,7 +15,8 @@ const suProOrgInfo: QueryResolvers['suProOrgInfo'] = async (_source, {includeIna
   const pgResults = await pg
     .selectFrom('OrganizationUser')
     .select(({fn}) => fn.count('id').as('orgSize'))
-    .where('orgId', 'in', proOrgIds)
+    // use ANY to support case where proOrgIds is empty array. Please use `in` after RethinkDB is gone
+    .where('orgId', '=', sql<string>`ANY(${proOrgIds})`)
     .where('inactive', '=', false)
     .where('removedAt', 'is', null)
     .groupBy('orgId')

diff --git a/packages/server/graphql/public/mutations/setOrgUserRole.ts b/packages/server/graphql/public/mutations/setOrgUserRole.ts
@@ -63,14 +63,14 @@ const setOrgUserRole: MutationResolvers['setOrgUserRole'] = async (
     }
   }
 
-  // if someone is leaving, make sure there is someone else to take their place
-  if (userId === viewerId) {
+  // if removing a role, make sure someone else has elevated permissions
+  if (!roleToSet) {
     const leaders = orgUsers.filter(
       ({role}) => role && ['BILLING_LEADER', 'ORG_ADMIN'].includes(role)
     )
     const leaderCount = leaders.length
-    if (leaderCount === 1 && !roleToSet) {
-      return standardError(new Error('You’re the last leader, you can’t give that up'), {
+    if (leaderCount === 1) {
+      return standardError(new Error('Cannot remove permissions of the last leader'), {
         userId: viewerId
       })
     }

diff --git a/packages/server/graphql/public/subscriptions/organizationSubscription.ts b/packages/server/graphql/public/subscriptions/organizationSubscription.ts
@@ -1,21 +1,28 @@
 import {SubscriptionChannel} from 'parabol-client/types/constEnums'
+import getRethink from '../../../database/rethinkDriver'
 import {getUserId} from '../../../utils/authorization'
 import getPubSub from '../../../utils/getPubSub'
+import {SubscriptionContext} from '../../graphql'
 import {SubscriptionResolvers} from '../resolverTypes'
 
-const organizationSubscription: SubscriptionResolvers['organizationSubscription'] = {
-  subscribe: async (_source, _args, {authToken, dataLoader}) => {
-    // AUTH
-    const viewerId = getUserId(authToken)
+const organizationSubscription: SubscriptionResolvers<SubscriptionContext>['organizationSubscription'] =
+  {
+    subscribe: async (_source, _args, {authToken}) => {
+      // AUTH
+      const viewerId = getUserId(authToken)
+      const r = await getRethink()
+      const organizationUsers = await r
+        .table('OrganizationUser')
+        .getAll(viewerId, {index: 'userId'})
+        .filter({removedAt: null})
+        .run()
+      const orgIds = organizationUsers.map(({orgId}) => orgId)
 
-    const organizationUsers = await dataLoader.get('organizationUsersByUserId').load(viewerId)
-    const orgIds = organizationUsers.map(({orgId}) => orgId)
-
-    // RESOLUTION
-    const channelNames = orgIds
-      .concat(viewerId)
-      .map((id) => `${SubscriptionChannel.ORGANIZATION}.${id}`)
-    return getPubSub().subscribe(channelNames)
+      // RESOLUTION
+      const channelNames = orgIds
+        .concat(viewerId)
+        .map((id) => `${SubscriptionChannel.ORGANIZATION}.${id}`)
+      return getPubSub().subscribe(channelNames)
+    }
   }
-}
 export default organizationSubscription