Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support CSRF token retrieval from header "X-CSRF-Token" #422

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions lib/hanami/action/csrf_protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ class Action
# This security mechanism is enabled automatically if sessions are turned on.
#
# It stores a "challenge" token in session. For each "state changing request"
# (eg. <tt>POST</tt>, <tt>PATCH</tt> etc..), we should send a special param:
# <tt>_csrf_token</tt>.
# (eg. <tt>POST</tt>, <tt>PATCH</tt> etc..), we should send a special param
# <tt>_csrf_token</tt> or header <tt>X-CSRF-Token</tt> which contain the "challenge"
# token.
#
# If the param matches with the challenge token, the flow can continue.
# If the request token matches with the challenge token, the flow can continue.
# Otherwise the application detects an attack attempt, it reset the session
# and <tt>Hanami::Action::InvalidCSRFTokenError</tt> is raised.
#
Expand Down Expand Up @@ -107,6 +108,16 @@ def set_csrf_token(_req, res)
res.session[CSRF_TOKEN] ||= generate_csrf_token
end

# Get CSRF Token in request.
#
# Retreives the CSRF token from the request param <tt>_csrf_token</tt> or the request header <tt>X-CSRF-Token</tt>.
#
# @since 2.X.X
Copy link
Author

@masterT masterT Apr 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should the version be?

# @api private
def request_csrf_token(req)
req.params[CSRF_TOKEN] || req.get_header("HTTP_X_CSRF_TOKEN")
end

# Verify if CSRF token from params, matches the one stored in session.
# If not, it raises an error.
#
Expand All @@ -131,14 +142,14 @@ def invalid_csrf_token?(req, res)
return false unless verify_csrf_token?(req, res)

missing_csrf_token?(req, res) ||
!::Rack::Utils.secure_compare(req.session[CSRF_TOKEN], req.params[CSRF_TOKEN])
!::Rack::Utils.secure_compare(req.session[CSRF_TOKEN], request_csrf_token(req))
end

# Verify the CSRF token was passed in params.
#
# @api private
def missing_csrf_token?(req, *)
Hanami::Utils::Blank.blank?(req.params[CSRF_TOKEN])
def missing_csrf_token?(req, res)
Hanami::Utils::Blank.blank?(request_csrf_token(req))
end

# Generates a random CSRF Token
Expand Down
40 changes: 36 additions & 4 deletions spec/unit/hanami/action/csrf_protection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,22 @@
context "No existing session" do
let(:request) { super().merge("rack.session" => {}) }

context "non-matching CSRF token in request" do
context "non-matching CSRF token in request param" do
let(:request) { super().merge(_csrf_token: "non-matching") }

it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
end
end

context "non-matching CSRF token in request header" do
let(:request) { super().merge("HTTP_X_CSRF_TOKEN" => "non-matching") }

it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
end
end

context "missing CSRF token in request" do
it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
Expand All @@ -35,22 +43,38 @@
let(:session) { {_csrf_token: session_token} }
let(:session_token) { "abc123" }

context "matching CSRF token in request" do
context "matching CSRF token in request param" do
let(:request) { super().merge(_csrf_token: session_token) }

it "accepts the request" do
expect(response.status).to eq 200
end
end

context "non-matching CSRF token in request" do
context "matching CSRF token in request header" do
let(:request) { super().merge("HTTP_X_CSRF_TOKEN" => session_token) }

it "accepts the request" do
expect(response.status).to eq 200
end
end

context "non-matching CSRF token in request param" do
let(:request) { super().merge(_csrf_token: "non-matching") }

it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
end
end

context "non-matching CSRF token in request header" do
let(:request) { super().merge("HTTP_X_CSRF_TOKEN" => "non-matching") }

it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
end
end

context "missing CSRF token in request" do
it "rejects the request" do
expect { response }.to raise_error Hanami::Action::InvalidCSRFTokenError
Expand All @@ -74,13 +98,21 @@ def verify_csrf_token?(_req, _res)
end
end

context "non-matching CSRF token in request" do
context "non-matching CSRF token in request param" do
let(:request) { super().merge(_csrf_token: "non-matching") }

it "accepts the request" do
expect(response.status).to eq 200
end
end

context "non-matching CSRF token in request header" do
let(:request) { super().merge("HTTP_X_CSRF_TOKEN" => "non-matching") }

it "accepts the request" do
expect(response.status).to eq 200
end
end
end
end
end
Expand Down