fix security handling of multiple auth types for hapi/koa

Closes #974
lukeautry · May 15, 2021 · bf331ae · bf331ae
1 parent 22cb926
commit bf331ae
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 33 deletions.
diff --git a/packages/cli/src/routeGeneration/templates/hapi.hbs b/packages/cli/src/routeGeneration/templates/hapi.hbs
@@ -60,8 +60,7 @@ export function RegisterRoutes(server: any) {
                 pre: [
                     {{#if security.length}}
                     {
-                      method: authenticateMiddleware({{json security}}),
-                      assign: "user"
+                      method: authenticateMiddleware({{json security}})
                     },
                     {{/if}}
                     {{#if uploadFile}}
@@ -147,7 +146,6 @@ export function RegisterRoutes(server: any) {
                     responded++;
                     request['user'] = user;
                 }
-                return user;
             };
 
             // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa
@@ -172,26 +170,27 @@ export function RegisterRoutes(server: any) {
 
             // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa
 
+            const secMethodOrPromises: Promise<any>[] = [];
             for (const secMethod of security) {
                 if (Object.keys(secMethod).length > 1) {
-                    let promises: Promise<any>[] = [];
+                    const secMethodAndPromises: Promise<any>[] = [];
 
                     for (const name in secMethod) {
-                        promises.push(hapiAuthentication(request, name, secMethod[name]));
+                        secMethodAndPromises.push(hapiAuthentication(request, name, secMethod[name]));
                     }
 
-                    return Promise.all(promises)
+                    secMethodOrPromises.push(Promise.all(secMethodAndPromises)
                         .then((users) => { succeed(users[0]); })
-                        .catch(fail);
+                        .catch(fail));
                 } else {
                     for (const name in secMethod) {
-                        return hapiAuthentication(request, name, secMethod[name])
+                        secMethodOrPromises.push(hapiAuthentication(request, name, secMethod[name])
                             .then(succeed)
-                            .catch(fail);
+                            .catch(fail));
                     }
                 }
             }
-            return null;
+            return Promise.all(secMethodOrPromises);
         }
     }
     {{/if}}

diff --git a/packages/cli/src/routeGeneration/templates/koa.hbs b/packages/cli/src/routeGeneration/templates/koa.hbs
@@ -109,12 +109,11 @@ export function RegisterRoutes(router: KoaRouter) {
           let responded = 0;
           let success = false;
 
-          const succeed = async (user: any) => {
+          const succeed = (user: any) => {
               if (!success) {
                   success = true;
                   responded++;
                   context.request['user'] = user;
-                  await next();
               }
           };
 
@@ -126,35 +125,34 @@ export function RegisterRoutes(router: KoaRouter) {
                   // this is an authentication error
                   context.status = error.status || 401;
                   context.throw(context.status, error.message, error);
-              } else if (success) {
-                  // the authentication was a success but arriving here means the controller
-                  // probably threw an error that we caught as well
-                  // so just pass it on
-                  throw error;
               }
           };
 
           // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa
 
+          const secMethodOrPromises: Promise<any>[] = [];
           for (const secMethod of security) {
               if (Object.keys(secMethod).length > 1) {
-                  let promises: Promise<any>[] = [];
+                  const secMethodAndPromises: Promise<any>[] = [];
 
                   for (const name in secMethod) {
-                      promises.push(koaAuthentication(context.request, name, secMethod[name]));
+                      secMethodAndPromises.push(koaAuthentication(context.request, name, secMethod[name]));
                   }
 
-                  return Promise.all(promises)
+                  secMethodOrPromises.push(Promise.all(secMethodAndPromises)
                       .then((users) => succeed(users[0]))
-                      .catch(fail);
+                      .catch(fail));
               } else {
                   for (const name in secMethod) {
-                      return koaAuthentication(context.request, name, secMethod[name])
+                      secMethodOrPromises.push(koaAuthentication(context.request, name, secMethod[name])
                           .then(succeed)
-                          .catch(fail);
+                          .catch(fail));
                   }
               }
           }
+
+          await Promise.all(secMethodOrPromises);
+          await next();
       }
   }
   {{/if}}

diff --git a/tests/fixtures/controllers/securityController.ts b/tests/fixtures/controllers/securityController.ts
@@ -59,4 +59,22 @@ export class SecurityTestController {
   public async GetServerError(@Request() request: RequestWithUser): Promise<UserResponseModel> {
     return Promise.reject(new Error('Unexpected'));
   }
+
+  @Response<ErrorResponseModel>('default', 'Unexpected error')
+  @Security('api_key', [])
+  @Security('tsoa_auth', ['write:pets', 'read:pets'])
+  @Get('ServerErrorOauthOrApiKey')
+  public async GetServerErrorOrAuth(@Request() request: RequestWithUser): Promise<UserResponseModel> {
+    return Promise.reject(new Error('Unexpected'));
+  }
+
+  @Response<ErrorResponseModel>('default', 'Unexpected error')
+  @Security({
+    api_key: [],
+    tsoa_auth: ['write:pets', 'read:pets'],
+  })
+  @Get('ServerErrorOauthAndApiKey')
+  public async GetServerErrorAndAuth(@Request() request: RequestWithUser): Promise<UserResponseModel> {
+    return Promise.reject(new Error('Unexpected'));
+  }
 }
diff --git a/tests/integration/hapi-server.spec.ts b/tests/integration/hapi-server.spec.ts
@@ -928,6 +928,10 @@ describe('Hapi Server', () => {
   });
 
   describe('Security', () => {
+    const emptyHandler = (err, res) => {
+      // This is an empty handler
+    };
+
     it('can handle get request with access_token user id == 1', () => {
       return verifyGetRequest(basePath + '/SecurityTest/Hapi?access_token=abc123456', (err, res) => {
         const model = res.body as Model;
@@ -941,6 +945,79 @@ describe('Hapi Server', () => {
         expect(model.id).to.equal(2);
       });
     });
+
+    describe('API key or tsoa auth', () => {
+      it('returns 200 if the API key is correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=abc123456&tsoa=invalid';
+        return verifyGetRequest(
+          basePath + path,
+          (err, res) => {
+            const model = res.body as Model;
+            expect(model.id).to.equal(1);
+          },
+          200,
+        );
+      });
+
+      it('returns 200 if tsoa auth is correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=invalid&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 200);
+      });
+
+      it('returns 200 if multiple auth handlers are correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=abc123456&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 200);
+      });
+
+      it('returns 401 if neither API key nor tsoa auth are correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=invalid&tsoa=invalid';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('should pass through error if controller method crashes', () => {
+        return verifyGetRequest(
+          basePath + `/SecurityTest/ServerErrorOauthOrApiKey?access_token=abc123456`,
+          (err, res) => {
+            expect(res.status).to.equal(500);
+          },
+          500,
+        );
+      });
+    });
+
+    describe('API key and tsoa auth', () => {
+      it('returns 200 if API and tsoa auth are correct, resolved with user from first key in object', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=abc123456&tsoa=abc123456';
+        return verifyGetRequest(
+          basePath + path,
+          (err, res) => {
+            const model = res.body as Model;
+            expect(model.id).to.equal(1);
+          },
+          200,
+        );
+      });
+
+      it('returns 401 if API key is incorrect', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=abc123456&tsoa=invalid';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('returns 401 if tsoa auth is incorrect', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=invalid&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('should pass through error if controller method crashes', () => {
+        return verifyGetRequest(
+          basePath + `/SecurityTest/ServerErrorOauthAndApiKey?access_token=abc123456&tsoa=abc123456`,
+          (err, res) => {
+            expect(res.status).to.equal(500);
+          },
+          500,
+        );
+      });
+    });
   });
 
   describe('Parameter data', () => {
@@ -1108,7 +1185,7 @@ describe('Hapi Server', () => {
           },
           statusCode,
         );
-      })
+      }),
     );
 
     it('Should not modify the response after headers sent', () => {

diff --git a/tests/integration/koa-server.spec.ts b/tests/integration/koa-server.spec.ts
@@ -907,6 +907,10 @@ describe('Koa Server', () => {
   });
 
   describe('Security', () => {
+    const emptyHandler = (err, res) => {
+      // This is an empty handler
+    };
+
     it('can handle get request with access_token user id == 1', () => {
       return verifyGetRequest(basePath + '/SecurityTest/Koa?access_token=abc123456', (err, res) => {
         const model = res.body as Model;
@@ -921,14 +925,77 @@ describe('Koa Server', () => {
       });
     });
 
-    it('should pass through error if controller method crashes', () => {
-      return verifyGetRequest(
-        basePath + `/SecurityTest/ServerError?access_token=abc123456`,
-        (err, res) => {
-          expect(res.status).to.equal(500);
-        },
-        500,
-      );
+    describe('API key or tsoa auth', () => {
+      it('returns 200 if the API key is correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=abc123456&tsoa=invalid';
+        return verifyGetRequest(
+          basePath + path,
+          (err, res) => {
+            const model = res.body as Model;
+            expect(model.id).to.equal(1);
+          },
+          200,
+        );
+      });
+
+      it('returns 200 if tsoa auth is correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=invalid&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 200);
+      });
+
+      it('returns 200 if multiple auth handlers are correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=abc123456&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 200);
+      });
+
+      it('returns 401 if neither API key nor tsoa auth are correct', () => {
+        const path = '/SecurityTest/OauthOrApiKey?access_token=invalid&tsoa=invalid';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('should pass through error if controller method crashes', () => {
+        return verifyGetRequest(
+          basePath + `/SecurityTest/ServerErrorOauthOrApiKey?access_token=abc123456`,
+          (err, res) => {
+            expect(res.status).to.equal(500);
+          },
+          500,
+        );
+      });
+    });
+
+    describe('API key and tsoa auth', () => {
+      it('returns 200 if API and tsoa auth pass validation, resolved with user from first key in object', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=abc123456&tsoa=abc123456';
+        return verifyGetRequest(
+          basePath + path,
+          (err, res) => {
+            const model = res.body as Model;
+            expect(model.id).to.equal(1);
+          },
+          200,
+        );
+      });
+
+      it('returns 401 if API key is incorrect', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=abc123456&tsoa=invalid';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('returns 401 if tsoa auth is incorrect', () => {
+        const path = '/SecurityTest/OauthAndApiKey?access_token=invalid&tsoa=abc123456';
+        return verifyGetRequest(basePath + path, emptyHandler, 401);
+      });
+
+      it('should pass through error if controller method crashes', () => {
+        return verifyGetRequest(
+          basePath + `/SecurityTest/ServerErrorOauthAndApiKey?access_token=abc123456&tsoa=abc123456`,
+          (err, res) => {
+            expect(res.status).to.equal(500);
+          },
+          500,
+        );
+      });
     });
   });
 
@@ -1097,7 +1164,7 @@ describe('Koa Server', () => {
           },
           statusCode,
         );
-      })
+      }),
     );
 
     it('Should not modify the response after headers sent', () => {