Skip to content

Bug with Authorized() (only 0.7.1) #240

New issue

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

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

Already on GitHub? # to your account

Closed
maxsh8x opened this issue Aug 5, 2017 · 16 comments · Fixed by #283
Closed

Bug with Authorized() (only 0.7.1) #240

maxsh8x opened this issue Aug 5, 2017 · 16 comments · Fixed by #283
Assignees
Labels
type: fix Issues describing a broken feature.

Comments

@maxsh8x
Copy link

maxsh8x commented Aug 5, 2017

  @Authorized()
  @Post('/v1/user')
  async createUser() {
    await this.userRepository.findOne() // only if any async operation inside controller
ERROR:  { AssertionError [ERR_ASSERTION]: headers have already been sent
    at Object.set status [as status] (/home/max/attn-backend/node_modules/koa/lib/response.js:85:5)
    at Object.status (/home/max/attn-backend/node_modules/delegates/index.js:92:31)
    at KoaDriver.handleError (/home/max/attn-backend/src/driver/koa/KoaDriver.ts:298:39)
    at /home/max/attn-backend/src/RoutingControllers.ts:143:40
    at tryCatcher (/home/max/attn-backend/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/max/attn-backend/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/home/max/attn-backend/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/home/max/attn-backend/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/home/max/attn-backend/node_modules/bluebird/js/release/promise.js:689:18)
    at Async._drainQueue (/home/max/attn-backend/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/home/max/attn-backend/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues (/home/max/attn-backend/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)
  generatedMessage: false,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '==' }

0.7.0-alpha.15 works fine

@MichalLytek
Copy link
Contributor

Could you describe the problem with more details and code snippets? I don't know what happens, what is the shape of request, etc. etc.
The best would be the gist with minimal bug reproducing codebase so I could debug what happens inside.

BTW I see that you use async/await with bluebird, I don't know if it's a good idea 😕

@maxsh8x
Copy link
Author

maxsh8x commented Aug 6, 2017

i tried to reproduce from zero and

POST http://localhost:3001/questions/
routing-controllers@0.7.1 - 404 Not found
routing-controllers@0.7.0-alpha.15 - 200 { "test": "ok" }

app.js

import "reflect-metadata";
import { Action } from 'routing-controllers'
import { QuestionController } from "./QuestionController";
import { createKoaServer } from 'routing-controllers'


createKoaServer({
  controllers: [QuestionController],
  authorizationChecker: async (action: Action, roles?: string[]) => {
    return true;
  }
}).listen(3001);

console.log("Express server is running on port 3001. Open http://localhost:3001/questions/");

QuestionController.ts

import { Service } from 'typedi'
import { Authorized, JsonController, Post, Body } from 'routing-controllers';

@Service()
@JsonController()
export class QuestionController {
  @Authorized()
  @Post("/questions")
  all() {
    return { "test": "ok" }
  }
}

or with class validator

import { Service } from 'typedi'
import { Authorized, JsonController, Post, Body } from 'routing-controllers';
import { IsString } from 'class-validator'

export class LoginParams {
  @IsString()
  username: string

  @IsString()
  password: string
}

@Service()
@JsonController()
export class QuestionController {
  @Authorized()
  @Post("/questions")
  all( @Body() params: LoginParams) {
    return { "test": "ok" }
  }
}
(node:17480) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): AssertionError [ERR_ASSERTION]: headers have already been sent
(node:17480) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@MichalLytek
Copy link
Contributor

Earlier you said that:

// only if any async operation inside controller

So it also happens with sync ones like all() { return { "test": "ok" } }, right?

@maxsh8x
Copy link
Author

maxsh8x commented Aug 7, 2017

Apparently yes. In my project slightly different behavior. But in both cases i got 404 and downgrade to alpha.15 solve this problem.

@NoNameProvided NoNameProvided added type: fix Issues describing a broken feature. and removed needs more info labels Aug 7, 2017
@NoNameProvided
Copy link
Member

NoNameProvided commented Aug 7, 2017

@maxsh8x thanks for the report, I confirm the issue. It was introduced after 0.7.0 so only 0.7.1 is affected.

A minimal reproducible use case:

import 'reflect-metadata';
import { createKoaServer, JsonController, Authorized, Post, Action } from 'routing-controllers';


@JsonController()
export class SomeController {

  @Authorized()
  @Post("/questions")
  public save() {
    return {};
  }

}

createKoaServer({
  authorizationChecker: async (action: Action, roles: string[]) => {
    const token = action.request.headers["authorization"];
    return token ? true : false;
  }
}).listen(3000)

The above snippet have the following behaviour:

  • When @Authorized() decorator is used and the authorizationChecker returns true the driver will return with a 404 error.
  • When @Authorized() decorator is used and the authorizationChecker returns false, then the proper AuthorizationRequiredError error is returned.
  • When the @Authorized() decorator is removed the empty object is returned

PS: You can write the type of the code after the backticks to have highlighting like: ```ts

@NoNameProvided
Copy link
Member

NoNameProvided commented Aug 7, 2017

The problem is most likely here KoaDriver.ts#L106 it was introduced in 177143a we should return the promise as before.

@NoNameProvided
Copy link
Member

Created a PR for this at #242.

@MichalLytek
Copy link
Contributor

Ok, so I'm closing this issue. @maxsh8x feel free to reopen if the issue will still exist in 0.7.2.

@maxsh8x
Copy link
Author

maxsh8x commented Sep 11, 2017

@NoNameProvided @19majkel94 not fixed yet. Still 404 for decorated routes after 0.7.0-alpha.15

@NoNameProvided
Copy link
Member

It was fixed in 0.7.2.

@maxsh8x
Copy link
Author

maxsh8x commented Sep 14, 2017

@NoNameProvided
Yeah, i know, but code below with Authorized
0.7.2 - 404 not found
0.7.0-alpha.15 - {}

app.ts

import 'reflect-metadata';
import { Container } from 'typedi';
import { createKoaServer, useContainer, Action } from 'routing-controllers';
import { SomeController } from './controller'

useContainer(Container);

createKoaServer({
  controllers: [SomeController],
  authorizationChecker: async (action: Action, roles: string[]) => {
    const token = action.request.headers["authorization"];
    return token ? true : false;
  }
}).listen(3000)

controller.ts

import { Service } from 'typedi'
import { JsonController, Authorized, Get } from 'routing-controllers';
import { SomeRepository } from './repository'

@Service()
@JsonController()
export class SomeController {
  constructor(
    private someRepository: SomeRepository
  ) { }

  @Authorized()
  @Get("/questions")
  async save() {
    const data = await this.someRepository.save()
    return data
  }
}

repository.ts

import { Service } from 'typedi';

@Service()
export class SomeRepository {
  save() {
    return Promise.resolve({})
  }
}

@MichalLytek
Copy link
Contributor

Works ok on 0.7.2:

import "reflect-metadata";
import { Service, Container } from "typedi";
import { JsonController, Authorized, Get, createKoaServer, useContainer, Action } from "routing-controllers";

@Service()
export class SomeRepository {
    save() {
        return Promise.resolve({});
    }
}

@Service()
@JsonController()
export class SomeController {
    constructor(private someRepository: SomeRepository) {}

    @Authorized()
    @Get("/questions")
    async save() {
        const data = await this.someRepository.save();
        return data;
    }
}

useContainer(Container);

createKoaServer({
    controllers: [SomeController],
    authorizationChecker: async (action: Action, roles: string[]) => {
        return true;
    },
}).listen(3000, () => console.log("listening on 3000"));

@maxsh8x
Copy link
Author

maxsh8x commented Sep 14, 2017

@19majkel94
i tried your code on another machine with clean installation

➜ example curl localhost:3000/questions                    
Not Found% // with false - raised auth error                                                                                                                                                                                                  
➜  example cat package.json 
{
  "name": "example",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "dependencies": {
    "koa": "^2.0.0-alpha.8",
    "koa-bodyparser": "^4.2.0",
    "koa-router": "^7.1.1",
    "routing-controllers": "^0.7.2",
    "stable": "^0.1.6",
    "ts-node": "^3.3.0",
    "typedi": "^0.5.2",
    "typescript": "^2.5.2"
  },
  "devDependencies": {},
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}
➜  example node -v
v8.2.1
➜  example cat tsconfig.json 
{
  "compilerOptions": {
    "module": "commonjs",
    "moduleResolution": "node",
    "target": "es6",
    "noImplicitAny": true,
    "sourceMap": true,
    "outDir": "./build",
    "rootDir": "./src",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true
  },
  "compileOnSave": false,
  "include": [
    "./src/**/*.ts"
  ],
  "exclude": [
    "node_modules",
    "build"
  ],
  "formatCodeOptions": {
    "indentSize": 2,
    "tabSize": 2,
    "newLineCharacter": "\r\n",
    "convertTabsToSpaces": true,
    "insertSpaceAfterCommaDelimiter": true,
    "insertSpaceAfterSemicolonInForStatements": true,
    "insertSpaceBeforeAndAfterBinaryOperators": true,
    "insertSpaceAfterKeywordsInControlFlowStatements": true,
    "insertSpaceAfterFunctionKeywordForAnonymousFunctions": false,
    "insertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis": false,
    "placeOpenBraceOnNewLineForFunctions": false,
    "placeOpenBraceOnNewLineForControlBlocks": false
  }
}%          

@MichalLytek
Copy link
Contributor

"koa": "^2.0.0-alpha.8",
"koa-router": "^7.1.1",

I don't know, try this?

"koa": "^2.3.0",
"koa-router": "^7.2.1",

@maxsh8x
Copy link
Author

maxsh8x commented Sep 14, 2017

unfortunately same result.
if I find the reason, I'll write here

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
type: fix Issues describing a broken feature.
Development

Successfully merging a pull request may close this issue.

3 participants