From 0383ae171cc2fd7a5bc91b9d34b61573d2b297d3 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 27 Mar 2026 14:23:16 +0100 Subject: [PATCH] Consider Sec-Fetch-Site header for session auth This warden strategy is primarily used to allow APIv3 requests from the browser, which only authenticates using its session cookie. Since this is susceptible to cross-site-request-forgery, prevention of CSRF must take place. This was so far only ensured through the usage of the X-Requested-With header. When a client sent along this header, the server could know that a CORS-preflight request must have been made and thus the browser most certainly has validated that the request is valid according to CORS rules. However, the header itself is a non-standard header and while some JavaScript frameworks add it to requests, not all of them do. For us this was practically visible on the API docs hosted under `/api/docs`. The solution is to expect the browser to send the Sec-Fetch-Site header with a value of same-origin. This header can't be set through JavaScript, but only by the browser and the value "same-origin" ensures that scheme, host and port are the same for requester and requested endpoint, thus eliminating CSRF concerns. This feature is widely supported by all major browsers, the last of which was Safari which added support 3 years ago. We might want to consider dropping the check for X-Requested-With entirely, since it should be superfluous. For now it was left in place for greater compatibility. --- .../strategies/warden/session.rb | 9 ++- spec/requests/api/v3/authentication_spec.rb | 61 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/lib_static/open_project/authentication/strategies/warden/session.rb b/lib_static/open_project/authentication/strategies/warden/session.rb index b5a89522cbd..1a7242fb191 100644 --- a/lib_static/open_project/authentication/strategies/warden/session.rb +++ b/lib_static/open_project/authentication/strategies/warden/session.rb @@ -50,8 +50,9 @@ module OpenProject return true if request.get? # For all other requests, to mitigate CSRF vectors, - # require the frontend header to be present. - xml_request_header_set? + # require frontend headers to be present or browser indication + # that this was a same-origin request + xml_request_header_set? || same_origin? end def authenticate! @@ -64,6 +65,10 @@ module OpenProject request.env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" end + def same_origin? + request.env["HTTP_SEC_FETCH_SITE"] == "same-origin" + end + def user_id Hash(session)["user_id"] end diff --git a/spec/requests/api/v3/authentication_spec.rb b/spec/requests/api/v3/authentication_spec.rb index 36ab7eb7830..49413a5fd97 100644 --- a/spec/requests/api/v3/authentication_spec.rb +++ b/spec/requests/api/v3/authentication_spec.rb @@ -42,6 +42,67 @@ RSpec.describe "API V3 Authentication" do end let(:resource_metadata) { 'resource_metadata="http://test.host/.well-known/oauth-protected-resource"' } + describe "session auth" do + let(:user) { create(:admin) } + let(:session_data) { ActiveSupport::HashWithIndifferentAccess.new(user_id: user.id, updated_at: Time.current) } + + before do + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(OpenProject::Authentication::Strategies::Warden::Session) + .to receive(:session) + .and_return(session_data) + # rubocop:enable RSpec/AnyInstance + end + + context "when making a GET request" do + before do + get resource + end + + it "authenticates successfully" do + expect(last_response).to have_http_status :ok + end + end + + context "when making a POST request" do + let(:add_additional_headers) { nil } + + before do + header "Content-Type", "application/json" + add_additional_headers + post resource, { name: "Test" }.to_json + end + + it "is not authenticated" do + expect(last_response).to have_http_status :unauthorized + end + + context "and when the Sec-Fetch-Site header indicates a same-origin request" do + let(:add_additional_headers) { header "Sec-Fetch-Site", "same-origin" } + + it "authenticates successfully" do + expect(last_response).to have_http_status :created + end + end + + context "and when the Sec-Fetch-Site header indicates a cross-site request" do + let(:add_additional_headers) { header "Sec-Fetch-Site", "cross-site" } + + it "is not authenticated" do + expect(last_response).to have_http_status :unauthorized + end + end + + context "and when the Sec-Fetch-Site header indicates a same-site request" do + let(:add_additional_headers) { header "Sec-Fetch-Site", "same-site" } + + it "is not authenticated" do + expect(last_response).to have_http_status :unauthorized + end + end + end + end + describe "oauth" do let(:oauth_access_token) { "" } let(:expected_message) { "You did not provide the correct credentials." }