fix(cors): avoid setting Access-Control-Allow-Origin if there is no…

… matching origin (#3510)
honojs · Oct 14, 2024 · cebf4e8 · cebf4e8
1 parent 3311664
commit cebf4e8
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 6 deletions.
diff --git a/src/middleware/cors/index.test.ts b/src/middleware/cors/index.test.ts
@@ -34,6 +34,12 @@ describe('CORS by Middleware', () => {
 
   app.use('/api5/*', cors())
 
+  app.use(
+    '/api6/*',
+    cors({
+      origin: 'http://example.com',
+    })
+  )
   app.use(
     '/api6/*',
     cors({
@@ -83,7 +89,10 @@ describe('CORS by Middleware', () => {
   })
 
   it('Preflight with options', async () => {
-    const req = new Request('https://localhost/api2/abc', { method: 'OPTIONS' })
+    const req = new Request('https://localhost/api2/abc', {
+      method: 'OPTIONS',
+      headers: { origin: 'http://example.com' },
+    })
     const res = await app.request(req)
 
     expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://example.com')
@@ -105,6 +114,15 @@ describe('CORS by Middleware', () => {
     expect(res.headers.get('Access-Control-Allow-Credentials')).toBe('true')
   })
 
+  it('Disallow an unmatched origin', async () => {
+    const req = new Request('https://localhost/api2/abc', {
+      method: 'OPTIONS',
+      headers: { origin: 'http://example.net' },
+    })
+    const res = await app.request(req)
+    expect(res.headers.has('Access-Control-Allow-Origin')).toBeFalsy()
+  })
+
   it('Allow multiple origins', async () => {
     let req = new Request('http://localhost/api3/abc', {
       headers: {
@@ -116,21 +134,28 @@ describe('CORS by Middleware', () => {
 
     req = new Request('http://localhost/api3/abc')
     res = await app.request(req)
-    expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://example.com')
+    expect(
+      res.headers.has('Access-Control-Allow-Origin'),
+      'An unmatched origin should be disallowed'
+    ).toBeFalsy()
 
     req = new Request('http://localhost/api3/abc', {
       headers: {
         Referer: 'http://example.net/',
       },
     })
     res = await app.request(req)
-    expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://example.com')
+    expect(
+      res.headers.has('Access-Control-Allow-Origin'),
+      'An unmatched origin should be disallowed'
+    ).toBeFalsy()
   })
 
   it('Allow different Vary header value', async () => {
     const res = await app.request('http://localhost/api3/abc', {
       headers: {
         Vary: 'accept-encoding',
+        Origin: 'http://example.com',
       },
     })
 
@@ -169,7 +194,11 @@ describe('CORS by Middleware', () => {
   })
 
   it('Should not return duplicate header values', async () => {
-    const res = await app.request('http://localhost/api6/abc')
+    const res = await app.request('http://localhost/api6/abc', {
+      headers: {
+        origin: 'http://example.com',
+      },
+    })
 
     expect(res.headers.get('Access-Control-Allow-Origin')).toBe('http://example.com')
   })

diff --git a/src/middleware/cors/index.ts b/src/middleware/cors/index.ts
@@ -68,11 +68,15 @@ export const cors = (options?: CORSOptions): MiddlewareHandler => {
 
   const findAllowOrigin = ((optsOrigin) => {
     if (typeof optsOrigin === 'string') {
-      return () => optsOrigin
+      if (optsOrigin === '*') {
+        return () => optsOrigin
+      } else {
+        return (origin: string) => (optsOrigin === origin ? origin : null)
+      }
     } else if (typeof optsOrigin === 'function') {
       return optsOrigin
     } else {
-      return (origin: string) => (optsOrigin.includes(origin) ? origin : optsOrigin[0])
+      return (origin: string) => (optsOrigin.includes(origin) ? origin : null)
     }
   })(opts.origin)