diff --git a/src/driver/express/ExpressDriver.ts b/src/driver/express/ExpressDriver.ts index d9bf5365..e1f2fa15 100644 --- a/src/driver/express/ExpressDriver.ts +++ b/src/driver/express/ExpressDriver.ts @@ -12,6 +12,7 @@ import {AccessDeniedError} from "../../error/AccessDeniedError"; import {AuthorizationCheckerNotDefinedError} from "../../error/AuthorizationCheckerNotDefinedError"; import {isPromiseLike} from "../../util/isPromiseLike"; import {getFromContainer} from "../../container"; +import {AuthorizationRequiredError} from "../../error/AuthorizationRequiredError"; const cookie = require("cookie"); const templateUrl = require("template-url"); @@ -114,21 +115,20 @@ export class ExpressDriver extends BaseDriver implements Driver { const action: Action = {request, response, next}; const checkResult = this.authorizationChecker(action, actionMetadata.authorizedRoles); - if (isPromiseLike(checkResult)) { - checkResult.then(result => { - if (!result) { - return this.handleError(new AccessDeniedError(action), actionMetadata, action); - } else { - next(); - } - }); - } else { - if (!checkResult) { - this.handleError(new AccessDeniedError(action), actionMetadata, action); + const handleError = (result: any) => { + if (!result) { + let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action); + return this.handleError(error, actionMetadata, action); } else { next(); } + }; + + if (isPromiseLike(checkResult)) { + checkResult.then(result => handleError(result)); + } else { + handleError(checkResult); } }); } diff --git a/src/driver/koa/KoaDriver.ts b/src/driver/koa/KoaDriver.ts index e5055e95..f141433d 100644 --- a/src/driver/koa/KoaDriver.ts +++ b/src/driver/koa/KoaDriver.ts @@ -12,6 +12,7 @@ import {AccessDeniedError} from "../../error/AccessDeniedError"; import {isPromiseLike} from "../../util/isPromiseLike"; import {getFromContainer} from "../../container"; import {RoleChecker} from "../../RoleChecker"; +import {AuthorizationRequiredError} from "../../error/AuthorizationRequiredError"; const cookie = require("cookie"); const templateUrl = require("template-url"); @@ -92,21 +93,19 @@ export class KoaDriver extends BaseDriver implements Driver { getFromContainer(actionMetadata.authorizedRoles).check(action) : this.authorizationChecker(action, actionMetadata.authorizedRoles); - if (isPromiseLike(checkResult)) { - return checkResult.then(result => { - if (!result) { - return this.handleError(new AccessDeniedError(action), actionMetadata, action); - - } else { - return next(); - } - }); - } else { - if (!checkResult) { - return this.handleError(new AccessDeniedError(action), actionMetadata, action); + const handleError = (result: any) => { + if (!result) { + let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action); + return this.handleError(error, actionMetadata, action); } else { - return next(); + next(); } + }; + + if (isPromiseLike(checkResult)) { + checkResult.then(result => handleError(result)); + } else { + handleError(checkResult); } }); } diff --git a/src/error/AccessDeniedError.ts b/src/error/AccessDeniedError.ts index 6a2a0633..f81b82e3 100644 --- a/src/error/AccessDeniedError.ts +++ b/src/error/AccessDeniedError.ts @@ -1,10 +1,10 @@ import {Action} from "../Action"; -import {UnauthorizedError} from "../http-error/UnauthorizedError"; +import {ForbiddenError} from "../http-error/ForbiddenError"; /** * Thrown when route is guarded by @Authorized decorator. */ -export class AccessDeniedError extends UnauthorizedError { +export class AccessDeniedError extends ForbiddenError { name = "AccessDeniedError"; diff --git a/src/metadata/ActionMetadata.ts b/src/metadata/ActionMetadata.ts index e1474dc2..1817dfee 100644 --- a/src/metadata/ActionMetadata.ts +++ b/src/metadata/ActionMetadata.ts @@ -56,12 +56,12 @@ export class ActionMetadata { /** * Route to be registered for the action. */ - route: string|RegExp; + route: string | RegExp; /** * Full route to this action (includes controller base route). */ - fullRoute: string|RegExp; + fullRoute: string | RegExp; /** * Indicates if this action uses Body. @@ -96,12 +96,12 @@ export class ActionMetadata { /** * Http code to be used on undefined action returned content. */ - undefinedResultCode: number|Function; + undefinedResultCode: number | Function; /** * Http code to be used on null action returned content. */ - nullResultCode: number|Function; + nullResultCode: number | Function; /** * Http code to be set on successful response. @@ -141,12 +141,12 @@ export class ActionMetadata { /** * Special function that will be called instead of orignal method of the target. */ - methodOverride?: (actionMetadata: ActionMetadata, action: Action, params: any[]) => Promise|any; + methodOverride?: (actionMetadata: ActionMetadata, action: Action, params: any[]) => Promise | any; // ------------------------------------------------------------------------- // Constructor // ------------------------------------------------------------------------- - + constructor(controllerMetadata: ControllerMetadata, args: ActionMetadataArgs) { this.controllerMetadata = controllerMetadata; this.route = args.route; @@ -197,7 +197,7 @@ export class ActionMetadata { this.headers = this.buildHeaders(responseHandlers); this.isAuthorizedUsed = this.controllerMetadata.isAuthorizedUsed || !!authorizedHandler; - this.authorizedRoles = (this.controllerMetadata.authorizedRoles || []).concat(authorizedHandler ? authorizedHandler.value : []); + this.authorizedRoles = (this.controllerMetadata.authorizedRoles || []).concat(authorizedHandler && authorizedHandler.value ? authorizedHandler.value : []); } // ------------------------------------------------------------------------- @@ -207,7 +207,7 @@ export class ActionMetadata { /** * Builds full action route. */ - private buildFullRoute(): string|RegExp { + private buildFullRoute(): string | RegExp { if (this.route instanceof RegExp) { if (this.controllerMetadata.route) { return ActionMetadata.appendBaseRoute(this.controllerMetadata.route, this.route); @@ -263,7 +263,6 @@ export class ActionMetadata { */ static appendBaseRoute(baseRoute: string, route: RegExp|string) { const prefix = `${baseRoute.length > 0 && baseRoute.indexOf("/") < 0 ? "/" : ""}${baseRoute}`; - if (typeof route === "string") return `${prefix}${route}`; @@ -273,5 +272,5 @@ export class ActionMetadata { return new RegExp(fullPath, route.flags); } - + } \ No newline at end of file diff --git a/test/functional/auth-decorator.spec.ts b/test/functional/auth-decorator.spec.ts new file mode 100644 index 00000000..24a639eb --- /dev/null +++ b/test/functional/auth-decorator.spec.ts @@ -0,0 +1,69 @@ +import "reflect-metadata"; +import {Get} from "../../src/decorator/Get"; +import {createExpressServer, createKoaServer, getMetadataArgsStorage} from "../../src/index"; +import {assertRequest} from "./test-utils"; +import {JsonController} from "../../src/decorator/JsonController"; +import {Authorized} from "../../src/decorator/Authorized"; +import {Action} from "../../src/Action"; +import {RoutingControllersOptions} from "../../src/RoutingControllersOptions"; +const chakram = require("chakram"); +const expect = chakram.expect; + +describe("Authorized Decorators Http Status Code", function () { + + before(() => { + + // reset metadata args storage + getMetadataArgsStorage().reset(); + + @JsonController() + class AuthController { + + @Authorized() + @Get("/auth1") + auth1() { + return {test: "auth1"}; + } + + @Authorized(["role1"]) + @Get("/auth2") + auth2() { + return {test: "auth2"}; + } + + } + }); + + const serverOptions: RoutingControllersOptions = { + authorizationChecker: async (action: Action, roles?: string[]) => { + return false; + } + }; + + let expressApp: any; + before(done => { + const server = createExpressServer(serverOptions); + expressApp = server.listen(3001, done); + }); + after(done => expressApp.close(done)); + + let koaApp: any; + before(done => { + const server = createKoaServer(serverOptions); + koaApp = server.listen(3002, done); + }); + after(done => koaApp.close(done)); + + describe("without roles", () => { + assertRequest([3001, 3002], "get", "auth1", response => { + expect(response).to.have.status(401); + }); + }); + + describe("with roles", () => { + assertRequest([3001, 3002], "get", "auth2", response => { + expect(response).to.have.status(403); + }); + }); + +}); \ No newline at end of file