Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/driver/express/ExpressDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, do you like this one?

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);
}
});
}
Expand Down
25 changes: 12 additions & 13 deletions src/driver/koa/KoaDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -92,21 +93,19 @@ export class KoaDriver extends BaseDriver implements Driver {
getFromContainer<RoleChecker>(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);
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/error/AccessDeniedError.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down
19 changes: 9 additions & 10 deletions src/metadata/ActionMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>|any;
methodOverride?: (actionMetadata: ActionMetadata, action: Action, params: any[]) => Promise<any> | any;

// -------------------------------------------------------------------------
// Constructor
// -------------------------------------------------------------------------

constructor(controllerMetadata: ControllerMetadata, args: ActionMetadataArgs) {
this.controllerMetadata = controllerMetadata;
this.route = args.route;
Expand Down Expand Up @@ -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 : []);
}

// -------------------------------------------------------------------------
Expand All @@ -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);
Expand Down Expand Up @@ -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}`;

Expand All @@ -273,5 +272,5 @@ export class ActionMetadata {

return new RegExp(fullPath, route.flags);
}

}
69 changes: 69 additions & 0 deletions test/functional/auth-decorator.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});

});