Skip to content

security: remove our CSRF tokens to improve security and reduce complexity of our threat model

Warren Gifford requested to merge sg/remove-csrf-tokens into main

Created by: slimsag

This brings back the original change, #27780, which was reverted as it caused a build failure on main (it was NOT reverted for any security reason) We saw failed builds on main with:

sourcegraph-upgrade: 2021/11/19 03:31:23 Failed to create site admin: authenticate: Forbidden - CSRF token invalid

This was because I had removed the logic from internal/gqltestutil/client.go to extract and send CSRF tokens to the Sourcegraph server. This code is only used for testing against the Sourcegraph server, and given we no longer had CSRF tokens it seemed apt to remove it. However, it turned out our CI uses this code as part of the upgrade tests in order to create the initial site admin account on the previous version of Sourcegraph. This means the code must support both the old version of Sourcegraph by sending the CSRF token, and the new version of Sourcegraph where there is no CSRF token.

Reviewing

Only the third commit needs to be reviewed. The others are just vanilla reverts which bring back the original change (already approved) and reset some test code to the latest version on main.

Merge request reports

Loading