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

Unable to set more than one cookie #637

Closed
1 task
rustam-crunch opened this issue Mar 31, 2020 · 5 comments · Fixed by #864
Closed
1 task

Unable to set more than one cookie #637

rustam-crunch opened this issue Mar 31, 2020 · 5 comments · Fixed by #864
Labels
good first issue This issue could be an easy PR for those looking to help contribute help wanted

Comments

@rustam-crunch
Copy link

Unable to set more than one cookie

Sorting

  • I'm submitting a ...

    • bug report
    • [*] feature request
    • [*] support request
  • I confirm that I

    • [*] used the search to make sure that a similar issue hasn't already been submitted

Expected Behavior

Multiple Set-Cookie headers with different names should be allowed to set with Controller.setHeader() method

Current Behavior

Controller.setHeader() method now adds new header to the object with signature:
{ [name: string]: string | undefined }
Which makes impossible to add multiple headers of the same type. But it should be possible, at least for Set-Cookies header RFC6265

Possible Solution

Change the signature of headers property in Controller to { [name: string]: string | undefined }[]
Change templates for routes with support for set Multiple headers

@WoH
Copy link
Collaborator

WoH commented Apr 1, 2020

Sorry about my lack of knowledge here. Does this only apply to Set-Cookie or are there other headers the server should provide multiple times without folding them?

@rustam-crunch
Copy link
Author

rustam-crunch commented Apr 2, 2020

I've made some investigations about the subject.
This applies at least Set-Cookie header, but definitely there could be few more, especially taking into account mess in the related standards and definitions

Another thing I discovered: Issue can(and probably should) be solved just by allowing Controller.setHeader() method to accept an array of strings but not only strings.
For Express and Koa middleware templates, this wouldn't need any changes. But looks like Hapi.js middleware template should be changed to accept header value as an array of string but not a single string.

@WoH WoH added the help wanted label Apr 9, 2020
@lassemon
Copy link

Bump, I have this exact same issue. I want to use passport jwt with tsoa which requires me to set both the token and the refreshToken from the login endpoint. i.e.:,

this.setHeader('Set-Cookie', `token=${authToken}; HttpOnly`)
this.setHeader('Set-Cookie', `refreshToken=${refreshToken}; HttpOnly`)

Currently this results in only the refreshToken being set, because there can be only one Set-Cookie header.

I tried to checkout the tsoa code and make relevant changes for a PR, but unfortunately I don't undestrand the code of tsoa well enough to make that change.

@tsimbalar
Copy link
Contributor

@lassemon At least with express, you can actually pass an array of strings to setHeader.

We used it in our project.

You should be able to do :

this.setHeader('Set-Cookie', [`token=${authToken}; HttpOnly`, `refreshToken=${refreshToken}; HttpOnly`]);

Typescript will complain, as theoretically the method accepts string | undefined , but you can add a "typescript ignore comment" for now, such as // @ts-expect-error.

So you would end up with

// @ts-expect-error
this.setHeader('Set-Cookie', [`token=${authToken}; HttpOnly`, `refreshToken=${refreshToken}; HttpOnly`]);

@lassemon
Copy link

@tsimbalar Yes thank you!!
I was/am indeed using express and the above solution works! This solved my problem for now.

@WoH WoH added the good first issue This issue could be an easy PR for those looking to help contribute label Oct 16, 2020
aldenquimby added a commit to aldenquimby/tsoa that referenced this issue Dec 9, 2020
@WoH WoH closed this as completed in #864 Dec 12, 2020
WoH added a commit that referenced this issue Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This issue could be an easy PR for those looking to help contribute help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants