From 0ffd76a704ce477971bf873c0b21096fdedc3162 Mon Sep 17 00:00:00 2001
From: Joern Schneeweisz <jschneeweisz@gitlab.com>
Date: Mon, 5 Jun 2023 13:51:53 +0200
Subject: [PATCH] Implement path-dependent feed token
Change the feed_token to be generated per-path,
this change is backwards compatible with previously
generated feed_token.
Changelog: changed
---
app/helpers/calendar_helper.rb | 2 +-
app/helpers/feed_token_helper.rb | 12 ++++++
app/helpers/rss_helper.rb | 2 +-
doc/security/token_overview.md | 5 ++-
lib/gitlab/auth/auth_finders.rb | 27 +++++++++++-
spec/features/dashboard/issues_filter_spec.rb | 9 +++-
spec/helpers/calendar_helper_spec.rb | 5 ++-
spec/helpers/feed_token_helper_spec.rb | 28 +++++++++++++
spec/helpers/rss_helper_spec.rb | 5 ++-
spec/lib/gitlab/auth/auth_finders_spec.rb | 41 ++++++++++++++++++-
.../features/rss_shared_examples.rb | 14 +++++--
11 files changed, 136 insertions(+), 14 deletions(-)
create mode 100644 app/helpers/feed_token_helper.rb
create mode 100644 spec/helpers/feed_token_helper_spec.rb
diff --git a/app/helpers/calendar_helper.rb b/app/helpers/calendar_helper.rb
index ad4116fc3da82..d70a860d468be 100644
--- a/app/helpers/calendar_helper.rb
+++ b/app/helpers/calendar_helper.rb
@@ -3,7 +3,7 @@
module CalendarHelper
def calendar_url_options
{ format: :ics,
- feed_token: current_user.try(:feed_token),
+ feed_token: generate_feed_token(:ics),
due_date: Issue::DueNextMonthAndPreviousTwoWeeks.name,
sort: 'closest_future_date' }
end
diff --git a/app/helpers/feed_token_helper.rb b/app/helpers/feed_token_helper.rb
new file mode 100644
index 0000000000000..751a8df4782ac
--- /dev/null
+++ b/app/helpers/feed_token_helper.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module FeedTokenHelper
+ def generate_feed_token(type)
+ feed_token = current_user&.feed_token
+ return unless feed_token
+
+ final_path = "#{current_request.path}.#{type}"
+ digest = OpenSSL::HMAC.hexdigest("SHA256", feed_token, final_path)
+ "#{User::FEED_TOKEN_PREFIX}#{digest}-#{current_user.id}"
+ end
+end
diff --git a/app/helpers/rss_helper.rb b/app/helpers/rss_helper.rb
index 67c7d244f11b5..90dd4e8fedb0b 100644
--- a/app/helpers/rss_helper.rb
+++ b/app/helpers/rss_helper.rb
@@ -2,6 +2,6 @@
module RssHelper
def rss_url_options
- { format: :atom, feed_token: current_user.try(:feed_token) }
+ { format: :atom, feed_token: generate_feed_token(:atom) }
end
end
diff --git a/doc/security/token_overview.md b/doc/security/token_overview.md
index 5a772106562fc..80dbe8c03ccff 100644
--- a/doc/security/token_overview.md
+++ b/doc/security/token_overview.md
@@ -139,7 +139,10 @@ Each user has a long-lived feed token that does not expire. This token allows au
- RSS readers to load a personalized RSS feed.
- Calendar applications to load a personalized calendar.
-This token is visible in those feed URLs. You cannot use this token to access any other data.
+You cannot use this token to access any other data.
+
+The user-scoped feed token can be used for all feeds, however feed and calendar URLs are generated
+with a different token that is only valid for one feed.
Anyone who has your token can read activity and issue RSS feeds or your calendar feed as if they were you, including confidential issues. If that happens, [reset the token](../user/profile/contributions_calendar.md#reset-the-user-activity-feed-token).
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index 4a610b26290a0..111fac6f8a57f 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -30,6 +30,7 @@ module AuthFinders
DEPLOY_TOKEN_HEADER = 'HTTP_DEPLOY_TOKEN'
RUNNER_TOKEN_PARAM = :token
RUNNER_JOB_TOKEN_PARAM = :token
+ PATH_DEPENDENT_FEED_TOKEN_REGEX = /\A#{User::FEED_TOKEN_PREFIX}(\h{64})-(\d+)\z/
# Check the Rails session for valid authentication details
def find_user_from_warden
@@ -54,7 +55,7 @@ def find_user_from_feed_token(request_format)
token = current_request.params[:feed_token].presence || current_request.params[:rss_token].presence
return unless token
- User.find_by_feed_token(token) || raise(UnauthorizedError)
+ find_feed_token_user(token) || raise(UnauthorizedError)
end
def find_user_from_bearer_token
@@ -277,6 +278,30 @@ def find_personal_access_token_from_http_basic_auth
PersonalAccessToken.find_by_token(password)
end
+ def find_feed_token_user(token)
+ find_user_from_path_feed_token(token) || User.find_by_feed_token(token)
+ end
+
+ def find_user_from_path_feed_token(token)
+ glft = token.match(PATH_DEPENDENT_FEED_TOKEN_REGEX)
+
+ return unless glft
+
+ # make sure that user id uses decimal notation
+ user_id = glft[2].to_i(10)
+ digest = glft[1]
+
+ user = User.find_by_id(user_id)
+ return unless user
+
+ feed_token = user.feed_token
+ our_digest = OpenSSL::HMAC.hexdigest("SHA256", feed_token, current_request.path)
+
+ return unless ActiveSupport::SecurityUtils.secure_compare(digest, our_digest)
+
+ user
+ end
+
def parsed_oauth_token
Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods)
end
diff --git a/spec/features/dashboard/issues_filter_spec.rb b/spec/features/dashboard/issues_filter_spec.rb
index 964ac2f714dfd..ab3aa29a3aa9a 100644
--- a/spec/features/dashboard/issues_filter_spec.rb
+++ b/spec/features/dashboard/issues_filter_spec.rb
@@ -61,10 +61,15 @@
auto_discovery_link = find('link[type="application/atom+xml"]', visible: false)
auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query)
- expect(params).to include('feed_token' => [user.feed_token])
+ feed_token_param = params['feed_token']
+ expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX])
+ expect(feed_token_param.first).to end_with(user.id.to_s)
expect(params).to include('milestone_title' => [''])
expect(params).to include('assignee_username' => [user.username.to_s])
- expect(auto_discovery_params).to include('feed_token' => [user.feed_token])
+
+ feed_token_param = auto_discovery_params['feed_token']
+ expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX])
+ expect(feed_token_param.first).to end_with(user.id.to_s)
expect(auto_discovery_params).to include('milestone_title' => [''])
expect(auto_discovery_params).to include('assignee_username' => [user.username.to_s])
end
diff --git a/spec/helpers/calendar_helper_spec.rb b/spec/helpers/calendar_helper_spec.rb
index 08993dd1dd084..a18ed47946547 100644
--- a/spec/helpers/calendar_helper_spec.rb
+++ b/spec/helpers/calendar_helper_spec.rb
@@ -8,7 +8,10 @@
it "includes the current_user's feed_token" do
current_user = create(:user)
allow(helper).to receive(:current_user).and_return(current_user)
- expect(helper.calendar_url_options).to include feed_token: current_user.feed_token
+
+ feed_token = helper.calendar_url_options[:feed_token]
+ expect(feed_token).to match(Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX)
+ expect(feed_token).to end_with(current_user.id.to_s)
end
end
diff --git a/spec/helpers/feed_token_helper_spec.rb b/spec/helpers/feed_token_helper_spec.rb
new file mode 100644
index 0000000000000..4382758965cb9
--- /dev/null
+++ b/spec/helpers/feed_token_helper_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeedTokenHelper, feature_category: :system_access do
+ describe '#generate_feed_token' do
+ context 'with type :atom' do
+ let(:current_user) { build(:user, feed_token: 'KNOWN VALUE') }
+
+ it "returns the current_user's atom feed_token" do
+ allow(helper).to receive(:current_user).and_return(current_user)
+ allow(helper).to receive(:current_request).and_return(instance_double(ActionDispatch::Request, path: 'url'))
+
+ expect(helper.generate_feed_token(:atom))
+ # The middle part is the output of OpenSSL::HMAC.hexdigest("SHA256", 'KNOWN VALUE', 'url.atom')
+ .to eq("glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{current_user.id}")
+ end
+ end
+
+ context 'when signed out' do
+ it "returns nil" do
+ allow(helper).to receive(:current_user).and_return(nil)
+
+ expect(helper.generate_feed_token(:atom)).to be_nil
+ end
+ end
+ end
+end
diff --git a/spec/helpers/rss_helper_spec.rb b/spec/helpers/rss_helper_spec.rb
index 05f6ebb6c1bb2..f99a1f6d54704 100644
--- a/spec/helpers/rss_helper_spec.rb
+++ b/spec/helpers/rss_helper_spec.rb
@@ -8,7 +8,10 @@
it "includes the current_user's feed_token" do
current_user = create(:user)
allow(helper).to receive(:current_user).and_return(current_user)
- expect(helper.rss_url_options).to include feed_token: current_user.feed_token
+
+ feed_token = helper.rss_url_options[:feed_token]
+ expect(feed_token).to match(Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX)
+ expect(feed_token).to end_with(current_user.id.to_s)
end
end
diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb
index 4498e369695db..1a8a2ec2980c0 100644
--- a/spec/lib/gitlab/auth/auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/auth_finders_spec.rb
@@ -181,7 +181,7 @@ def set_bearer_token(token)
set_header('HTTP_ACCEPT', 'application/atom+xml')
end
- context 'when feed_token param is provided' do
+ context 'when old format feed_token param is provided' do
it 'returns user if valid feed_token' do
set_param(:feed_token, user.feed_token)
@@ -206,7 +206,44 @@ def set_bearer_token(token)
end
end
- context 'when rss_token param is provided' do
+ context 'when path-dependent format feed_token param is provided' do
+ let_it_be(:feed_user, freeze: true) { create(:user, feed_token: 'KNOWN VALUE').tap(&:feed_token) }
+ # The middle part is the output of OpenSSL::HMAC.hexdigest("SHA256", 'KNOWN VALUE', 'url.atom')
+ let(:feed_token) { "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{feed_user.id}" }
+
+ it 'returns user if valid feed_token' do
+ set_param(:feed_token, feed_token)
+
+ expect(find_user_from_feed_token(:rss)).to eq feed_user
+ end
+
+ it 'returns nil if valid feed_token and disabled' do
+ allow(Gitlab::CurrentSettings).to receive_messages(disable_feed_token: true)
+ set_param(:feed_token, feed_token)
+
+ expect(find_user_from_feed_token(:rss)).to be_nil
+ end
+
+ it 'returns exception if token has same HMAC but different user ID' do
+ set_param(:feed_token, "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{user.id}")
+
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+
+ it 'returns exception if token has wrong HMAC but same user ID' do
+ set_param(:feed_token, "glft-aaaaaaaaaade004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{feed_user.id}")
+
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+
+ it 'returns exception if user does not exist' do
+ set_param(:feed_token, "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{non_existing_record_id}")
+
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+ end
+
+ context 'when old format rss_token param is provided' do
it 'returns user if valid rss_token' do
set_param(:rss_token, user.feed_token)
diff --git a/spec/support/shared_examples/features/rss_shared_examples.rb b/spec/support/shared_examples/features/rss_shared_examples.rb
index f6566214e32d8..a6b9c98923a79 100644
--- a/spec/support/shared_examples/features/rss_shared_examples.rb
+++ b/spec/support/shared_examples/features/rss_shared_examples.rb
@@ -2,20 +2,20 @@
RSpec.shared_examples "an autodiscoverable RSS feed with current_user's feed token" do
it "has an RSS autodiscovery link tag with current_user's feed token" do
- expect(page).to have_css("link[type*='atom+xml'][href*='feed_token=#{user.feed_token}']", visible: false)
+ expect(page).to have_css("link[type*='atom+xml'][href*='feed_token=glft-'][href*='-#{user.id}']", visible: false)
end
end
RSpec.shared_examples "it has an RSS button with current_user's feed token" do
it "shows the RSS button with current_user's feed token" do
expect(page)
- .to have_css("a:has([data-testid='rss-icon'])[href*='feed_token=#{user.feed_token}']")
+ .to have_css("a:has([data-testid='rss-icon'])[href*='feed_token=glft-'][href*='-#{user.id}']")
end
end
RSpec.shared_examples "it has an RSS link with current_user's feed token" do
it "shows the RSS link with current_user's feed token" do
- expect(page).to have_link 'Subscribe to RSS feed', href: /feed_token=#{user.feed_token}/
+ expect(page).to have_link 'Subscribe to RSS feed', href: /feed_token=glft-.*-#{user.id}/
end
end
@@ -51,11 +51,17 @@
auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query)
expected = {
- 'feed_token' => [user.feed_token],
'assignee_id' => [user.id.to_s]
}
expect(params).to include(expected)
+ feed_token_param = params['feed_token']
+ expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX])
+ expect(feed_token_param.first).to end_with(user.id.to_s)
+
expect(auto_discovery_params).to include(expected)
+ feed_token_param = auto_discovery_params['feed_token']
+ expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX])
+ expect(feed_token_param.first).to end_with(user.id.to_s)
end
end
--
GitLab