Merge pull request #24 from supabase/fix/api-route-clash

Fix/api route clash
supabase · May 26, 2021 · ddfbcda · ddfbcda
2 parents 414096a + 0abd6fe
commit ddfbcda
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 48 deletions.
diff --git a/package.json b/package.json
@@ -17,7 +17,7 @@
     "prettier:check": "prettier -c src/**",
     "eslint:check": "eslint 'src/**'",
     "stop:db": "docker-compose --project-dir . -f src/test/db/docker-compose.yml down  --remove-orphans",
-    "restart:db": "docker-compose --project-dir . -f src/test/db/docker-compose.yml down && docker-compose --project-dir . -f src/test/db/docker-compose.yml up -d && sleep 1 && npm run migration:run"
+    "restart:db": "docker-compose --project-dir . -f src/test/db/docker-compose.yml down && docker-compose --project-dir . -f src/test/db/docker-compose.yml up -d && sleep 5 && npm run migration:run"
   },
   "author": "Supabase",
   "license": "ISC",

diff --git a/src/routes/object/getObject.ts b/src/routes/object/getObject.ts
@@ -1,5 +1,6 @@
-import { FastifyInstance } from 'fastify'
+import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'
 import { FromSchema } from 'json-schema-to-ts'
+import { IncomingMessage, Server, ServerResponse } from 'http'
 import { AuthenticatedRequest, Obj } from '../../types/types'
 import { getPostgrestClient, isValidKey, transformPostgrestError } from '../../utils'
 import { getConfig } from '../../utils/config'
@@ -21,11 +22,64 @@ interface getObjectRequestInterface extends AuthenticatedRequest {
   Params: FromSchema<typeof getObjectParamsSchema>
 }
 
+async function requestHandler(
+  request: FastifyRequest<getObjectRequestInterface, Server, IncomingMessage>,
+  response: FastifyReply<
+    Server,
+    IncomingMessage,
+    ServerResponse,
+    getObjectRequestInterface,
+    unknown
+  >
+) {
+  const authHeader = request.headers.authorization
+  const jwt = authHeader.substring('Bearer '.length)
+
+  const postgrest = getPostgrestClient(jwt)
+
+  const { bucketName } = request.params
+  const objectName = request.params['*']
+
+  if (!isValidKey(objectName) || !isValidKey(bucketName)) {
+    return response
+      .status(400)
+      .send(createResponse('The key contains invalid characters', '400', 'Invalid key'))
+  }
+
+  const objectResponse = await postgrest
+    .from<Obj>('objects')
+    .select('id')
+    .match({
+      name: objectName,
+      bucket_id: bucketName,
+    })
+    .single()
+
+  if (objectResponse.error) {
+    const { status, error } = objectResponse
+    request.log.error({ error }, 'error object')
+    return response.status(400).send(transformPostgrestError(error, status))
+  }
+
+  // send the object from s3
+  const s3Key = `${projectRef}/${bucketName}/${objectName}`
+  request.log.info(s3Key)
+  const data = await getObject(client, globalS3Bucket, s3Key)
+
+  return response
+    .status(data.$metadata.httpStatusCode ?? 200)
+    .header('Content-Type', data.ContentType)
+    .header('Cache-Control', data.CacheControl)
+    .header('ETag', data.ETag)
+    .header('Last-Modified', data.LastModified)
+    .send(data.Body)
+}
+
 // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
 export default async function routes(fastify: FastifyInstance) {
   const summary = 'Retrieve an object'
   fastify.get<getObjectRequestInterface>(
-    '/:bucketName/*',
+    '/authenticated/:bucketName/*',
     {
       // @todo add success response schema here
       schema: {
@@ -37,47 +91,25 @@ export default async function routes(fastify: FastifyInstance) {
       },
     },
     async (request, response) => {
-      const authHeader = request.headers.authorization
-      const jwt = authHeader.substring('Bearer '.length)
-
-      const postgrest = getPostgrestClient(jwt)
-
-      const { bucketName } = request.params
-      const objectName = request.params['*']
-
-      if (!isValidKey(objectName) || !isValidKey(bucketName)) {
-        return response
-          .status(400)
-          .send(createResponse('The key contains invalid characters', '400', 'Invalid key'))
-      }
-
-      const objectResponse = await postgrest
-        .from<Obj>('objects')
-        .select('id')
-        .match({
-          name: objectName,
-          bucket_id: bucketName,
-        })
-        .single()
-
-      if (objectResponse.error) {
-        const { status, error } = objectResponse
-        request.log.error({ error }, 'error object')
-        return response.status(400).send(transformPostgrestError(error, status))
-      }
-
-      // send the object from s3
-      const s3Key = `${projectRef}/${bucketName}/${objectName}`
-      request.log.info(s3Key)
-      const data = await getObject(client, globalS3Bucket, s3Key)
+      return requestHandler(request, response)
+    }
+  )
 
-      return response
-        .status(data.$metadata.httpStatusCode ?? 200)
-        .header('Content-Type', data.ContentType)
-        .header('Cache-Control', data.CacheControl)
-        .header('ETag', data.ETag)
-        .header('Last-Modified', data.LastModified)
-        .send(data.Body)
+  // to be deprecated
+  fastify.get<getObjectRequestInterface>(
+    '/:bucketName/*',
+    {
+      // @todo add success response schema here
+      schema: {
+        params: getObjectParamsSchema,
+        headers: { $ref: 'authSchema#' },
+        summary: 'Deprecated (use /authenticated/bucketName/object instead): Retrieve an object',
+        response: { '4xx': { $ref: 'errorSchema#' } },
+        tags: ['object'],
+      },
+    },
+    async (request, response) => {
+      return requestHandler(request, response)
     }
   )
 }
diff --git a/src/test/object.test.ts b/src/test/object.test.ts
@@ -94,7 +94,7 @@ describe('testing GET object', () => {
   test('check if RLS policies are respected: authenticated user is able to read authenticated resource', async () => {
     const response = await app().inject({
       method: 'GET',
-      url: '/object/bucket2/authenticated/casestudy.png',
+      url: '/object/authenticated/bucket2/authenticated/casestudy.png',
       headers: {
         authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
       },
@@ -106,7 +106,7 @@ describe('testing GET object', () => {
   test('check if RLS policies are respected: anon user is not able to read authenticated resource', async () => {
     const response = await app().inject({
       method: 'GET',
-      url: '/object/bucket2/authenticated/casestudy.png',
+      url: '/object/authenticated/bucket2/authenticated/casestudy.png',
       headers: {
         authorization: `Bearer ${anonKey}`,
       },
@@ -118,7 +118,7 @@ describe('testing GET object', () => {
   test('user is not able to read a resource without Auth header', async () => {
     const response = await app().inject({
       method: 'GET',
-      url: '/object/bucket2/authenticated/casestudy.png',
+      url: '/object/authenticated/bucket2/authenticated/casestudy.png',
     })
     expect(response.statusCode).toBe(400)
     expect(mockGetObject).not.toHaveBeenCalled()
@@ -127,7 +127,7 @@ describe('testing GET object', () => {
   test('return 400 when reading a non existent object', async () => {
     const response = await app().inject({
       method: 'GET',
-      url: '/object/bucket2/authenticated/notfound',
+      url: '/object/authenticated/bucket2/authenticated/notfound',
       headers: {
         authorization: `Bearer ${anonKey}`,
       },
@@ -139,7 +139,7 @@ describe('testing GET object', () => {
   test('return 400 when reading a non existent bucket', async () => {
     const response = await app().inject({
       method: 'GET',
-      url: '/object/notfound/authenticated/casestudy.png',
+      url: '/object/authenticated/notfound/authenticated/casestudy.png',
       headers: {
         authorization: `Bearer ${anonKey}`,
       },