harness / gitness

Gitness is an Open Source developer platform with Source Control management, Continuous Integration and Continuous Delivery.
https://gitness.com
Apache License 2.0
32.09k stars 2.8k forks source link

fix: Set 'Secure' attribute to true for sensitive cookies #3407

Closed DharunKumar04 closed 11 months ago

DharunKumar04 commented 11 months ago

This pull request addresses the security issue raised by Snyk regarding sensitive cookies in the account package not having the 'Secure' attribute set. The 'Secure' attribute is essential for ensuring that cookies are transmitted securely over HTTPS, preventing potential man-in-the-middle attacks.

Changes Made:

DharunKumar04 commented 11 months ago

Hi @enver-bisevac

Thank you for your feedback and the reference to the MDN documentation on cookies.As you mentioned, a cookie with the 'Secure' attribute is sent to the server only with an encrypted request over the HTTPS protocol.I understand the concern and would like to clarify my approach The reason for enforcing the 'Secure' attribute as true in the includeTokenCookie and deleteTokenCookieIfPresent functions is to provide an explicit security measure. Since the authentication token cookie lacks the 'Secure' attribute, it maybe sent in plaintext over an unencrypted HTTP connection.And attackers can use this vulnerability as an attack surface.

By setting the secure attribute to true it serves as a clear indication that these cookies should only be transmitted over HTTPS, aligning with the best practices outlined in the MDN documentation.And also explicitly setting 'Secure' within these functions, we adopt a defensive coding approach. This ensures that even if the isSecure variable in the newEmptyTokenCookie function were to change in the future due to code evolution, our security measure remains intact.

If you believe it's unnecessary for our codebase or if there are better ways to handle this, please let me know, or we can close this pull request.Thanks :)

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

CLAassistant commented 11 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: DharunKumar04
:x: ATBBt26NFnTCjw8nhPR8HVcqYZxeBF349DC8


ATBBt26NFnTCjw8nhPR8HVcqYZxeBF349DC8 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.