adding 'auto' to secure attribute

[zekth]

Fix #84
secureAuto will set the Secure flag regarding the protocol used for the request. I'd prefer to add another attribute to not arm the current behaviour.

[williamzhao87]

Suggestion:
Could we implement the secureAuto flag with the same logic as express-session so the two middlewares are more consistent? So the ‘secure’ flag could take ‘true’, ‘false’ or ‘auto’.

See https://github.com/expressjs/session

The cookie.secure option can also be set to the special value 'auto' to have this setting automatically match the determined security of the connection. Be careful when using this setting if the site is available both as HTTP and HTTPS, as once the cookie is set on HTTPS, it will no longer be visible over HTTP. This is useful when the Express "trust proxy" setting is properly setup to simplify development vs production configuration.

[SerayaEryn]

LGTM

[SerayaEryn]

Suggestion:
Could we implement the secureAuto flag with the same logic as express-session so the two middlewares are more consistent? So the ‘secure’ flag could take ‘true’, ‘false’ or ‘auto’.

See https://github.com/expressjs/session

The cookie.secure option can also be set to the special value 'auto' to have this setting automatically match the determined security of the connection. Be careful when using this setting if the site is available both as HTTP and HTTPS, as once the cookie is set on HTTPS, it will no longer be visible over HTTP. This is useful when the Express "trust proxy" setting is properly setup to simplify development vs production configuration.

I think this is a good idea.

@zekth What do you think?

[zekth]

Suggestion:
Could we implement the secureAuto flag with the same logic as express-session so the two middlewares are more consistent? So the ‘secure’ flag could take ‘true’, ‘false’ or ‘auto’.
See https://github.com/expressjs/session
The cookie.secure option can also be set to the special value 'auto' to have this setting automatically match the determined security of the connection. Be careful when using this setting if the site is available both as HTTP and HTTPS, as once the cookie is set on HTTPS, it will no longer be visible over HTTP. This is useful when the Express "trust proxy" setting is properly setup to simplify development vs production configuration.

I think this is a good idea.

@zekth What do you think?

I agree. I will change to it. This just add polymorphism that's why i added another attribute but as long as you're ok with it i'll do it.

[zekth]

ready for review @SerayaEryn . I'm not sure about the Documentation. Maybe @williamzhao87 can review it as you're english native speaking.

[williamzhao87]

Documentation looks fine to me. Thanks for the update!

[williamzhao87]

I think we should also do some other checks besides just the X-Forwarded-Proto header, if possible:
See the issecure function in express-session:
https://github.com/expressjs/session/blob/b22384b712fea118f1c3eb5b0d79312ebd25e97c/index.js#L612

We may also want a way to check a custom header in the case that X-Forwarded-Proto header is overwritten by some ELBs such as AWS ALB)

Can we add another option?
cookie.secureAutoHeader

e.g. If cookie.secureAutoHeader = X-Forwarded-Scheme, instead of checking X-Forwarded-Proto header, we will check the header X-Forwarded-Scheme instead to determine the value for secure when set to auto

[zekth]

Adding secureAutoHeader is too custom here. A solution for that would be to add a function for the auto behavior but it's out of scope here IMO

[williamzhao87]

Okay that's fine, thanks

[zekth]

@SerayaEryn I've fixed a case regarding secure : auto and encrypted connection without header. Now it's shippable to me.

[williamzhao87]

Should we update the possibility of string type in types/types.d.ts?
interface CookieOptions {
...
secure?: boolean | string;

[zekth]

Should we update the possibility of string type in types/types.d.ts?
interface CookieOptions {
...
secure?: boolean | string;

Nice catch tho.

[SerayaEryn]

Thanks for your contribution.