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.

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

Bug with Authorized() (only 0.7.1) #240

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/
[email protected] - 404 Not found
[email protected] - 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: fix Issues describing a broken feature.
Development

Successfully merging a pull request may close this issue.

3 participants