fix(server): protecting against scope elevation in PAT creation (#1901)

* test DX improvements + tests for token:write scope

* protecting against scope elevation

packages/server/codegen.yml

@@ -47,6 +47,7 @@ generates:
     plugins:
       - 'typescript'
       - 'typescript-operations'
+      - 'typed-document-node'
     documents:
       - 'test/graphql/*.{js,ts}'
     config:

packages/server/modules/core/graph/resolvers/apitoken.js

@@ -6,8 +6,10 @@ const {
   revokeToken,
   getUserTokens
 } = require('../../services/tokens')
+const { canCreateToken } = require('@/modules/core/helpers/token')
 
-module.exports = {
+/** @type {import('@/modules/core/graph/generated/graphql').Resolvers} */
+const resolvers = {
   Query: {},
   User: {
     async apiTokens(parent, args, context) {
@@ -21,6 +23,12 @@ module.exports = {
   },
   Mutation: {
     async apiTokenCreate(parent, args, context) {
+      if (!canCreateToken(context.scopes || [], args.token.scopes)) {
+        throw new ForbiddenError(
+          "You can't create a token with scopes that you don't have"
+        )
+      }
+
       return await createPersonalAccessToken(
         context.userId,
         args.token.name,
@@ -37,3 +45,5 @@ module.exports = {
     }
   }
 }
+
+module.exports = resolvers

packages/server/modules/core/helpers/token.ts

@@ -0,0 +1,3 @@
+export const canCreateToken = (userScopes: string[], tokenScopes: string[]) => {
+  return tokenScopes.every((scope) => userScopes.includes(scope))
+}

packages/server/modules/core/tests/apitokens.spec.ts

@@ -0,0 +1,124 @@
+import { BasicTestUser, createTestUsers } from '@/test/authHelper'
+import {
+  CreateTokenDocument,
+  RevokeTokenDocument
+} from '@/test/graphql/generated/graphql'
+import {
+  TestApolloServer,
+  createTestContext,
+  testApolloServer
+} from '@/test/graphqlHelper'
+import { beforeEachContext } from '@/test/hooks'
+import { AllScopes, Roles, Scopes } from '@speckle/shared'
+import { expect } from 'chai'
+import { difference } from 'lodash'
+
+/**
+ * Older API token test cases can be found in `graph.spec.js`
+ */
+describe('API Tokens', () => {
+  const userOne: BasicTestUser = {
+    name: 'Dimitrie Stefanescu',
+    email: '[email protected]',
+    password: 'sn3aky-1337-b1m',
+    id: ''
+  }
+
+  let apollo: TestApolloServer
+
+  before(async () => {
+    await beforeEachContext()
+    await createTestUsers([userOne])
+
+    apollo = await testApolloServer({
+      context: createTestContext({
+        auth: true,
+        userId: userOne.id,
+        role: Roles.Server.Admin,
+        token: 'asd',
+        scopes: AllScopes
+      })
+    })
+  })
+
+  it("can't create PATs with scopes that the authenticated req itself doesn't have", async () => {
+    const { data, errors } = await apollo.execute(
+      CreateTokenDocument,
+      {
+        token: {
+          name: 'invalidone',
+          scopes: [Scopes.Profile.Read, Scopes.Streams.Read]
+        }
+      },
+      {
+        context: {
+          scopes: [Scopes.Profile.Read, Scopes.Tokens.Write]
+        }
+      }
+    )
+
+    expect(data?.apiTokenCreate).to.not.be.ok
+    expect(errors).to.be.ok
+    expect(
+      errors!.find((e) =>
+        e.message.includes("You can't create a token with scopes that you don't have")
+      )
+    ).to.be.ok
+  })
+
+  describe('without the tokens:write scope', () => {
+    const limitedTokenScopes = difference(AllScopes, [Scopes.Tokens.Write])
+    let limitedToken: string
+
+    before(async () => {
+      const res = await apollo.execute(CreateTokenDocument, {
+        token: { name: 'limited', scopes: limitedTokenScopes }
+      })
+
+      limitedToken = res.data?.apiTokenCreate || ''
+      if (!limitedToken.length) {
+        throw new Error("Couldn't prepare token for test")
+      }
+    })
+
+    it("can't create PAT tokens", async () => {
+      const { data, errors } = await apollo.execute(
+        CreateTokenDocument,
+        {
+          token: { name: 'invalidone', scopes: [Scopes.Profile.Read] }
+        },
+        {
+          context: {
+            scopes: limitedTokenScopes,
+            token: limitedToken
+          }
+        }
+      )
+
+      expect(data?.apiTokenCreate).to.not.be.ok
+      expect(errors).to.be.ok
+      expect(
+        errors!.find((e) => e.message.includes('do not have the required privileges'))
+      ).to.be.ok
+    })
+
+    it("can't delete PAT tokens", async () => {
+      const { data, errors } = await apollo.execute(
+        RevokeTokenDocument,
+        { token: limitedToken },
+        {
+          context: {
+            scopes: limitedTokenScopes,
+            token: limitedToken
+          }
+        }
+      )
+
+      expect(data?.apiTokenRevoke).to.not.be.ok
+      expect(errors).to.be.ok
+      expect(
+        errors!.find((e) => e.message.includes('do not have the required privileges'))
+      ).to.be.ok
+    })
+  })
+})

packages/server/package.json

@@ -109,6 +109,7 @@
     "@bull-board/express": "^4.2.2",
     "@faker-js/faker": "^7.1.0",
     "@graphql-codegen/cli": "^2.16.3",
+    "@graphql-codegen/typed-document-node": "^5.0.1",
     "@graphql-codegen/typescript": "2.7.2",
     "@graphql-codegen/typescript-operations": "^2.5.2",
     "@graphql-codegen/typescript-resolvers": "2.7.2",

packages/server/test/graphql/apiTokens.ts

@@ -0,0 +1,13 @@
+import { gql } from 'apollo-server-express'
+
+export const createTokenMutation = gql`
+  mutation CreateToken($token: ApiTokenCreateInput!) {
+    apiTokenCreate(token: $token)
+  }
+`
+
+export const revokeTokenMutation = gql`
+  mutation RevokeToken($token: String!) {
+    apiTokenRevoke(token: $token)
+  }
+`

packages/server/test/graphqlHelper.ts

@@ -2,7 +2,11 @@
 import { ApolloServer } from 'apollo-server-express'
 import { GraphQLResponse } from 'apollo-server-types'
 import { DocumentNode } from 'graphql'
-import { Nullable } from '@/modules/shared/helpers/typeHelper'
+import { GraphQLContext, Nullable } from '@/modules/shared/helpers/typeHelper'
+import { TypedDocumentNode } from '@graphql-typed-document-node/core'
+import { buildApolloServer } from '@/app'
+import { addLoadersToCtx } from '@/modules/shared/middleware'
+import { buildUnauthenticatedApolloServer } from '@/test/serverHelper'
 
 type TypedGraphqlResponse<R = Record<string, any>> = GraphQLResponse & {
   data: Nullable<R>
@@ -25,3 +29,67 @@ export async function executeOperation<
     variables
   })) as TypedGraphqlResponse<R>
 }
+
+/**
+ * Create a test context for a GraphQL request. Optionally override any of the default values.
+ * By default the context will be unauthenticated
+ */
+export const createTestContext = (
+  ctx?: Partial<Parameters<typeof addLoadersToCtx>[0]>
+): GraphQLContext =>
+  addLoadersToCtx({
+    auth: false,
+    userId: undefined,
+    role: undefined,
+    token: undefined,
+    scopes: [],
+    stream: undefined,
+    err: undefined,
+    ...(ctx || {})
+  })
+
+/**
+ * Utilities that make it easier to test against an Apollo Server instance
+ */
+export const testApolloServer = async (params?: { context?: GraphQLContext }) => {
+  const instance = params?.context
+    ? await buildApolloServer({
+        context: params.context
+      })
+    : await buildUnauthenticatedApolloServer()
+
+  /**
+   * Execute an operation against Apollo and get a properly typed response
+   */
+  const execute = async <
+    R extends Record<string, any> = Record<string, any>,
+    V extends Record<string, any> = Record<string, any>
+  >(
+    query: TypedDocumentNode<R, V>,
+    variables: V,
+    options?: Partial<{
+      /**
+       * Optionally override the instance's context
+       */
+      context: Parameters<typeof createTestContext>[0]
+    }>
+  ): Promise<TypedGraphqlResponse<R>> => {
+    const realInstance = options?.context
+      ? await buildApolloServer({
+          context: createTestContext({
+            ...(params?.context || {}),
+            ...options.context
+          })
+        })
+      : instance
+
+    return (await realInstance.executeOperation({
+      query,
+      variables
+    })) as TypedGraphqlResponse<R>
+  }
+
+  return { execute, server: instance }
+}
+
+export type TestApolloServer = Awaited<ReturnType<typeof testApolloServer>>