Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sync and async auth checker bug #283

Merged
merged 1 commit into from
Sep 9, 2017
Merged

Fix sync and async auth checker bug #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 19 additions & 15 deletions src/driver/express/ExpressDriver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,27 @@ export class ExpressDriver extends BaseDriver implements Driver {
throw new AuthorizationCheckerNotDefinedError();

const action: Action = { request, response, next };
const checkResult = this.authorizationChecker(action, actionMetadata.authorizedRoles);

const handleError = (result: any) => {
if (!result) {
let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action);
return this.handleError(error, actionMetadata, action);
try {
const checkResult = this.authorizationChecker(action, actionMetadata.authorizedRoles);

const handleError = (result: any) => {
if (!result) {
let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action);
this.handleError(error, actionMetadata, action);
} else {
next();
}
};

if (isPromiseLike(checkResult)) {
checkResult
.then(result => handleError(result))
.catch(error => this.handleError(error, actionMetadata, action));
} else {
next();
handleError(checkResult);
}
};

if (isPromiseLike(checkResult)) {
checkResult
.then(result => handleError(result))
.catch(error => this.handleError(error, actionMetadata, action));
} else {
handleError(checkResult);
} catch (error) {
this.handleError(error, actionMetadata, action);
}
});
}
Expand Down
38 changes: 21 additions & 17 deletions src/driver/koa/KoaDriver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,25 +77,29 @@ export class KoaDriver extends BaseDriver implements Driver {
throw new AuthorizationCheckerNotDefinedError();

const action: Action = { request: context.request, response: context.response, context, next };
const checkResult = actionMetadata.authorizedRoles instanceof Function ?
getFromContainer<RoleChecker>(actionMetadata.authorizedRoles).check(action) :
this.authorizationChecker(action, actionMetadata.authorizedRoles);

const handleError = (result: any) => {
if (!result) {
let error = actionMetadata.authorizedRoles.length === 0 ? new AuthorizationRequiredError(action) : new AccessDeniedError(action);
return this.handleError(error, actionMetadata, action);
try {
const checkResult = actionMetadata.authorizedRoles instanceof Function ?
getFromContainer<RoleChecker>(actionMetadata.authorizedRoles).check(action) :
this.authorizationChecker(action, actionMetadata.authorizedRoles);

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();
}
};

if (isPromiseLike(checkResult)) {
return checkResult
.then(result => handleError(result))
.catch(error => this.handleError(error, actionMetadata, action));
} else {
next();
return handleError(checkResult);
}
};

if (isPromiseLike(checkResult)) {
return checkResult
.then(result => handleError(result))
.catch(error => this.handleError(error, actionMetadata, action));
} else {
handleError(checkResult);
} catch (error) {
return this.handleError(error, actionMetadata, action);
}
});
}
Expand Down
195 changes: 149 additions & 46 deletions test/functional/auth-decorator.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import "reflect-metadata";
import {Get} from "../../src/decorator/Get";
import {createExpressServer, createKoaServer, getMetadataArgsStorage} from "../../src/index";
import { createExpressServer, createKoaServer, getMetadataArgsStorage, NotAcceptableError } from "../../src/index";
import {assertRequest} from "./test-utils";
import {JsonController} from "../../src/decorator/JsonController";
import {Authorized} from "../../src/decorator/Authorized";
Expand All @@ -9,7 +9,7 @@ import {RoutingControllersOptions} from "../../src/RoutingControllersOptions";
const chakram = require("chakram");
const expect = chakram.expect;

describe("Controller responds with value when Authorization succeeds", function () {
describe("Controller responds with value when Authorization succeeds (async)", function () {

before(() => {

Expand Down Expand Up @@ -70,6 +70,67 @@ describe("Controller responds with value when Authorization succeeds", function

});

describe("Controller responds with value when Authorization succeeds (sync)", 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: (action: Action, roles?: string[]) => {
return true;
}
};

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(200);
expect(response.body).to.eql({ test: "auth1" });
});
});

describe("with roles", () => {
assertRequest([3001, 3002], "get", "auth2", response => {
expect(response).to.have.status(200);
expect(response.body).to.eql({ test: "auth2" });
});
});

});

describe("Authorized Decorators Http Status Code", function () {

before(() => {
Expand Down Expand Up @@ -129,48 +190,90 @@ describe("Authorized Decorators Http Status Code", function () {

});

describe("Authorization checker allows to throw", function() {
before(() => {
// reset metadata args storage
getMetadataArgsStorage().reset();

@JsonController()
class AuthController {
@Authorized()
@Get("/auth1")
auth1() {
return { test: "auth1" };
}

}
});

const serverOptions: RoutingControllersOptions = {
authorizationChecker: async (action: Action, roles?: string[]) => {
throw new Error('Custom Error');
}
};

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("custom errors", () => {
assertRequest([3001, 3002], "get", "auth1", response => {
expect(response).to.have.status(500);
expect(response.body).to.have.property("name", "Error");
expect(response.body).to.have.property("message", "Custom Error");

});
});
describe("Authorization checker allows to throw (async)", function() {
before(() => {
// reset metadata args storage
getMetadataArgsStorage().reset();

@JsonController()
class AuthController {
@Authorized()
@Get("/auth1")
auth1() {
return { test: "auth1" };
}
}
});

const serverOptions: RoutingControllersOptions = {
authorizationChecker: async (action: Action, roles?: string[]) => {
throw new NotAcceptableError("Custom Error");
},
};

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("custom errors", () => {
assertRequest([3001, 3002], "get", "auth1", response => {
expect(response).to.have.status(406);
expect(response.body).to.have.property("name", "NotAcceptableError");
expect(response.body).to.have.property("message", "Custom Error");
});
});
});

describe("Authorization checker allows to throw (sync)", function() {
before(() => {
// reset metadata args storage
getMetadataArgsStorage().reset();

@JsonController()
class AuthController {
@Authorized()
@Get("/auth1")
auth1() {
return { test: "auth1" };
}
}
});

const serverOptions: RoutingControllersOptions = {
authorizationChecker: (action: Action, roles?: string[]) => {
throw new NotAcceptableError("Custom Error");
},
};

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("custom errors", () => {
assertRequest([3001, 3002], "get", "auth1", response => {
expect(response).to.have.status(406);
expect(response.body).to.have.property("name", "NotAcceptableError");
expect(response.body).to.have.property("message", "Custom Error");
});
});
});