Cookies created without Secure property set

[steve-forth]

A recent pen test of an application using this code highlighted that the NONCE and CV cookies are set without specifying the Secure property. I understand these are used to connect callbacks after redirection to the authentication provider such as Okta.

Background of the secure flag
If the secure flag is set on a cookie, then browsers will not submit the cookie in any requests that use an unencrypted HTTP connection, thereby preventing the cookie from being trivially intercepted by an attacker monitoring network traffic. If the secure flag is not set, then the cookie will be transmitted in clear-text if the user visits any HTTP URLs within the cookie's scope. An attacker may be able to induce this event by feeding a user suitable links, either directly or via another web site. Even if the domain that issued the cookie does not host any content that is accessed over HTTP, an attacker may be able to use links of the form http://example.com:443/ to perform the same attack.

The secure flag should be set on all cookies that are used for transmitting sensitive data when accessing content over HTTPS. If cookies are used to transmit session tokens, then areas of the application that are accessed over HTTPS should employ their own session handling mechanism, and the session tokens used should never be transmitted over unencrypted communications.

The code in this repo does set the secure flag on the TOKEN cookie when setting its' value to a valid token.

Proposed remediation
The proposal is to set the secure flag for ALL cookies created by the code.
In doing so we minimise the risks of insecure cookies listed above.

Considerations
However this could be considered a breaking change as this solution will no longer support HTTP only applications. I would argue that all applications wanting to use an authentication module should only be using HTTPS.

[SiCoe]

Following the issue raised by @steve-forth, I've raised a PR #39 to set the NONCE and CV as secure when using pkce.
I also agree it seems like a breaking change and therefore suggest we create a v4.0.0 release containing this change and probably a similar change for github and openid.

[SiCoe]

Changes for this issue have been merged into master and released as v4.0.0