diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9b40ffb26a204496acac8d93a926fea38069f665..dbb22127e8243c0bc805116c405dd0ca2e04f54e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -12,11 +12,11 @@ class ApplicationController < ActionController::Base
include WorkhorseHelper
include EnforcesTwoFactorAuthentication
include WithPerformanceBar
+ include SessionlessAuthentication
# this can be removed after switching to rails 5
# https://gitlab.com/gitlab-org/gitlab-ce/issues/51908
include InvalidUTF8ErrorHandler unless Gitlab.rails5?
- before_action :authenticate_sessionless_user!
before_action :authenticate_user!
before_action :enforce_terms!, if: :should_enforce_terms?
before_action :validate_user_service_ticket!
@@ -153,13 +153,6 @@ def auth_user
end
end
- # This filter handles personal access tokens, and atom requests with rss tokens
- def authenticate_sessionless_user!
- user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user
-
- sessionless_sign_in(user) if user
- end
-
def log_exception(exception)
Raven.capture_exception(exception) if sentry_enabled?
@@ -426,25 +419,11 @@ def set_locale(&block)
Gitlab::I18n.with_user_locale(current_user, &block)
end
- def sessionless_sign_in(user)
- if user && can?(user, :log_in)
- # Notice we are passing store false, so the user is not
- # actually stored in the session and a token is needed
- # for every request. If you want the token to work as a
- # sign in token, you can simply remove store: false.
- sign_in(user, store: false, message: :sessionless_sign_in)
- end
- end
-
def set_page_title_header
# Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8
response.headers['Page-Title'] = URI.escape(page_title('GitLab'))
end
- def sessionless_user?
- current_user && !session.keys.include?('warden.user.user.key')
- end
-
def peek_request?
request.path.start_with?('/-/peek')
end
diff --git a/app/controllers/concerns/sessionless_authentication.rb b/app/controllers/concerns/sessionless_authentication.rb
new file mode 100644
index 0000000000000000000000000000000000000000..590eefc6dab94ecd2f0a060dd7b59305b6b3cec5
--- /dev/null
+++ b/app/controllers/concerns/sessionless_authentication.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+# == SessionlessAuthentication
+#
+# Controller concern to handle PAT and RSS token authentication methods
+#
+module SessionlessAuthentication
+ # This filter handles personal access tokens, and atom requests with rss tokens
+ def authenticate_sessionless_user!(request_format)
+ user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user(request_format)
+
+ sessionless_sign_in(user) if user
+ end
+
+ def sessionless_user?
+ current_user && !session.keys.include?('warden.user.user.key')
+ end
+
+ def sessionless_sign_in(user)
+ if user && can?(user, :log_in)
+ # Notice we are passing store false, so the user is not
+ # actually stored in the session and a token is needed
+ # for every request. If you want the token to work as a
+ # sign in token, you can simply remove store: false.
+ sign_in(user, store: false, message: :sessionless_sign_in)
+ end
+ end
+end
diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb
index e9686ed8d066aeb7cfec85ca996a5536bc6456a8..57e612d89d3b064c22dccfce18552874e8cb8abf 100644
--- a/app/controllers/dashboard/projects_controller.rb
+++ b/app/controllers/dashboard/projects_controller.rb
@@ -4,6 +4,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
include ParamsBackwardCompatibility
include RendersMemberAccess
+ prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
before_action :set_non_archived_param
before_action :default_sorting
skip_cross_project_access_check :index, :starred
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 4ce9be44403b31916c349c314871ab994bcc9617..be2d9512c01fc38882622627aaaabe2fe47b3eb2 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -4,6 +4,9 @@ class DashboardController < Dashboard::ApplicationController
include IssuesAction
include MergeRequestsAction
+ prepend_before_action(only: [:issues]) { authenticate_sessionless_user!(:rss) }
+ prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) }
+
before_action :event_filter, only: :activity
before_action :projects, only: [:issues, :merge_requests]
before_action :set_show_full_reference, only: [:issues, :merge_requests]
diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb
index a1ec144410bec466e3d9bf0c65730625deaf3ea2..6ea4758ec3258aa901ad0465da64fa77ddd942ef 100644
--- a/app/controllers/graphql_controller.rb
+++ b/app/controllers/graphql_controller.rb
@@ -3,6 +3,7 @@
class GraphqlController < ApplicationController
# Unauthenticated users have access to the API for public data
skip_before_action :authenticate_user!
+ prepend_before_action(only: [:execute]) { authenticate_sessionless_user!(:api) }
before_action :check_graphql_feature_flag!
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 062c8c4e9e106c5d45325c0679482f69b0e05775..c5d8ac2ed772e87c1860dd41252f7ef50e4ddc6d 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -9,6 +9,9 @@ class GroupsController < Groups::ApplicationController
respond_to :html
+ prepend_before_action(only: [:show, :issues]) { authenticate_sessionless_user!(:rss) }
+ prepend_before_action(only: [:issues_calendar]) { authenticate_sessionless_user!(:ics) }
+
before_action :authenticate_user!, only: [:new, :create]
before_action :group, except: [:index, :new, :create]
diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb
index 84a2a461da76671344d3a98b91b687d47195e561..8ba18aacc587405e9fb0d215b85dc45dfb792ce3 100644
--- a/app/controllers/projects/commits_controller.rb
+++ b/app/controllers/projects/commits_controller.rb
@@ -6,6 +6,7 @@ class Projects::CommitsController < Projects::ApplicationController
include ExtractsPath
include RendersCommits
+ prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
before_action :whitelist_query_limiting, except: :commits_root
before_action :require_non_empty_project
before_action :assign_ref_vars, except: :commits_root
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index d6d7110355b0068b98448afe3380456138b7592d..c6ab6b4642e892706b2c1dadc6476bed78144e35 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -9,10 +9,6 @@ class Projects::IssuesController < Projects::ApplicationController
include IssuesCalendar
include SpammableActions
- def self.authenticate_user_only_actions
- %i[new]
- end
-
def self.issue_except_actions
%i[index calendar new create bulk_update]
end
@@ -21,7 +17,10 @@ def self.set_issuables_index_only_actions
%i[index calendar]
end
- prepend_before_action :authenticate_user!, only: authenticate_user_only_actions
+ prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
+ prepend_before_action(only: [:calendar]) { authenticate_sessionless_user!(:ics) }
+ prepend_before_action :authenticate_new_issue!, only: [:new]
+ prepend_before_action :store_uri, only: [:new, :show]
before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update]
before_action :check_issues_available!
@@ -232,16 +231,18 @@ def issue_params_attributes
] + [{ label_ids: [], assignee_ids: [] }]
end
- def authenticate_user!
+ def authenticate_new_issue!
return if current_user
notice = "Please sign in to create the new issue."
+ redirect_to new_user_session_path, notice: notice
+ end
+
+ def store_uri
if request.get? && !request.xhr?
store_location_for :user, request.fullpath
end
-
- redirect_to new_user_session_path, notice: notice
end
def serializer
diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index c8442ff359273b5a095845d12c856eeda8e69372..2b28670a49b47024a0594f53c20d763fcdbb2b3b 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -3,6 +3,8 @@
class Projects::TagsController < Projects::ApplicationController
include SortingHelper
+ prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
+
# Authorize
before_action :require_non_empty_project
before_action :authorize_download_code!
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 7f4a9f5151bacbddb7ca9a1ae999edc548c80b58..8bf93bfd68dc9fe139dfa29e726499f8aecf5245 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -7,6 +7,8 @@ class ProjectsController < Projects::ApplicationController
include PreviewMarkdown
include SendFileUpload
+ prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
+
before_action :whitelist_query_limiting, only: [:create]
before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
before_action :redirect_git_extension, only: [:show]
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 5b70c69d7f451f46beec944058d854e411d6275b..8b040dc080ecf5b3b35fcaadd31a97f92c16899e 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -14,6 +14,7 @@ class UsersController < ApplicationController
calendar_activities: true
skip_before_action :authenticate_user!
+ prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
before_action :user, except: [:exists]
before_action :authorize_read_user_profile!,
only: [:calendar, :calendar_activities, :groups, :projects, :contributed_projects, :snippets]
diff --git a/changelogs/unreleased/security-fix-pat-web-access.yml b/changelogs/unreleased/security-fix-pat-web-access.yml
new file mode 100644
index 0000000000000000000000000000000000000000..62ffb908fe532a1c9d50afd896d819c5cd33e15b
--- /dev/null
+++ b/changelogs/unreleased/security-fix-pat-web-access.yml
@@ -0,0 +1,5 @@
+---
+title: Restrict Personal Access Tokens to API scope on web requests
+merge_request:
+author:
+type: security
diff --git a/config/initializers/rack_attack_global.rb b/config/initializers/rack_attack_global.rb
index 45963831c41455508adb59f950681d0e0d76bcf0..86cb930eca9efef3cb370c799e0559aebe56277f 100644
--- a/config/initializers/rack_attack_global.rb
+++ b/config/initializers/rack_attack_global.rb
@@ -33,22 +33,22 @@ class Rack::Attack
throttle('throttle_authenticated_api', Gitlab::Throttle.authenticated_api_options) do |req|
Gitlab::Throttle.settings.throttle_authenticated_api_enabled &&
req.api_request? &&
- req.authenticated_user_id
+ req.authenticated_user_id([:api])
end
throttle('throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req|
Gitlab::Throttle.settings.throttle_authenticated_web_enabled &&
req.web_request? &&
- req.authenticated_user_id
+ req.authenticated_user_id([:api, :rss, :ics])
end
class Request
def unauthenticated?
- !authenticated_user_id
+ !authenticated_user_id([:api, :rss, :ics])
end
- def authenticated_user_id
- Gitlab::Auth::RequestAuthenticator.new(self).user&.id
+ def authenticated_user_id(request_formats)
+ Gitlab::Auth::RequestAuthenticator.new(self).user(request_formats)&.id
end
def api_request?
diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb
index cb9f2582936534032f5380b6da4b02f681fa4515..176766d1a8b95bf7e01346810fceca68ee6109f1 100644
--- a/lib/gitlab/auth/request_authenticator.rb
+++ b/lib/gitlab/auth/request_authenticator.rb
@@ -13,12 +13,18 @@ def initialize(request)
@request = request
end
- def user
- find_sessionless_user || find_user_from_warden
+ def user(request_formats)
+ request_formats.each do |format|
+ user = find_sessionless_user(format)
+
+ return user if user
+ end
+
+ find_user_from_warden
end
- def find_sessionless_user
- find_user_from_access_token || find_user_from_feed_token
+ def find_sessionless_user(request_format)
+ find_user_from_web_access_token(request_format) || find_user_from_feed_token(request_format)
rescue Gitlab::Auth::AuthenticationError
nil
end
diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb
index c304adc64db05217baf55afecb30b8760cc5e884..adba90848452e232f43449bbe3cb225ed3970f72 100644
--- a/lib/gitlab/auth/user_auth_finders.rb
+++ b/lib/gitlab/auth/user_auth_finders.rb
@@ -27,8 +27,8 @@ def find_user_from_warden
current_request.env['warden']&.authenticate if verified_request?
end
- def find_user_from_feed_token
- return unless rss_request? || ics_request?
+ def find_user_from_feed_token(request_format)
+ return unless valid_rss_format?(request_format)
# NOTE: feed_token was renamed from rss_token but both needs to be supported because
# users might have already added the feed to their RSS reader before the rename
@@ -38,6 +38,17 @@ def find_user_from_feed_token
User.find_by_feed_token(token) || raise(UnauthorizedError)
end
+ # We only allow Private Access Tokens with `api` scope to be used by web
+ # requests on RSS feeds or ICS files for backwards compatibility.
+ # It is also used by GraphQL/API requests.
+ def find_user_from_web_access_token(request_format)
+ return unless access_token && valid_web_access_format?(request_format)
+
+ validate_access_token!(scopes: [:api])
+
+ access_token.user || raise(UnauthorizedError)
+ end
+
def find_user_from_access_token
return unless access_token
@@ -109,6 +120,26 @@ def current_request
@current_request ||= ensure_action_dispatch_request(request)
end
+ def valid_web_access_format?(request_format)
+ case request_format
+ when :rss
+ rss_request?
+ when :ics
+ ics_request?
+ when :api
+ api_request?
+ end
+ end
+
+ def valid_rss_format?(request_format)
+ case request_format
+ when :rss
+ rss_request?
+ when :ics
+ ics_request?
+ end
+ end
+
def rss_request?
current_request.path.ends_with?('.atom') || current_request.format.atom?
end
@@ -116,6 +147,10 @@ def rss_request?
def ics_request?
current_request.path.ends_with?('.ics') || current_request.format.ics?
end
+
+ def api_request?
+ current_request.path.starts_with?("/api/")
+ end
end
end
end
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index efc3ce74627b9040ff63565e2628d78b0f64e9ae..1b585bcd4c69bd5516735463793bf594132aa0ed 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -107,59 +107,6 @@ def index
end
end
- describe "#authenticate_user_from_personal_access_token!" do
- before do
- stub_authentication_activity_metrics(debug: false)
- end
-
- controller(described_class) do
- def index
- render text: 'authenticated'
- end
- end
-
- let(:personal_access_token) { create(:personal_access_token, user: user) }
-
- context "when the 'personal_access_token' param is populated with the personal access token" do
- it "logs the user in" do
- expect(authentication_metrics)
- .to increment(:user_authenticated_counter)
- .and increment(:user_session_override_counter)
- .and increment(:user_sessionless_authentication_counter)
-
- get :index, private_token: personal_access_token.token
-
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq('authenticated')
- end
- end
-
- context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
- it "logs the user in" do
- expect(authentication_metrics)
- .to increment(:user_authenticated_counter)
- .and increment(:user_session_override_counter)
- .and increment(:user_sessionless_authentication_counter)
-
- @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
- get :index
-
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq('authenticated')
- end
- end
-
- it "doesn't log the user in otherwise" do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
-
- get :index, private_token: "token"
-
- expect(response.status).not_to eq(200)
- expect(response.body).not_to eq('authenticated')
- end
- end
-
describe 'session expiration' do
controller(described_class) do
# The anonymous controller will report 401 and fail to run any actions.
@@ -224,74 +171,6 @@ def index
end
end
- describe '#authenticate_sessionless_user!' do
- before do
- stub_authentication_activity_metrics(debug: false)
- end
-
- describe 'authenticating a user from a feed token' do
- controller(described_class) do
- def index
- render text: 'authenticated'
- end
- end
-
- context "when the 'feed_token' param is populated with the feed token" do
- context 'when the request format is atom' do
- it "logs the user in" do
- expect(authentication_metrics)
- .to increment(:user_authenticated_counter)
- .and increment(:user_session_override_counter)
- .and increment(:user_sessionless_authentication_counter)
-
- get :index, feed_token: user.feed_token, format: :atom
-
- expect(response).to have_gitlab_http_status 200
- expect(response.body).to eq 'authenticated'
- end
- end
-
- context 'when the request format is ics' do
- it "logs the user in" do
- expect(authentication_metrics)
- .to increment(:user_authenticated_counter)
- .and increment(:user_session_override_counter)
- .and increment(:user_sessionless_authentication_counter)
-
- get :index, feed_token: user.feed_token, format: :ics
-
- expect(response).to have_gitlab_http_status 200
- expect(response.body).to eq 'authenticated'
- end
- end
-
- context 'when the request format is neither atom nor ics' do
- it "doesn't log the user in" do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
-
- get :index, feed_token: user.feed_token
-
- expect(response.status).not_to have_gitlab_http_status 200
- expect(response.body).not_to eq 'authenticated'
- end
- end
- end
-
- context "when the 'feed_token' param is populated with an invalid feed token" do
- it "doesn't log the user" do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
-
- get :index, feed_token: 'token', format: :atom
-
- expect(response.status).not_to eq 200
- expect(response.body).not_to eq 'authenticated'
- end
- end
- end
- end
-
describe '#route_not_found' do
it 'renders 404 if authenticated' do
allow(controller).to receive(:current_user).and_return(user)
@@ -557,36 +436,6 @@ def index
expect(response).to have_gitlab_http_status(200)
end
-
- context 'for sessionless users' do
- render_views
-
- before do
- sign_out user
- end
-
- it 'renders a 403 when the sessionless user did not accept the terms' do
- get :index, feed_token: user.feed_token, format: :atom
-
- expect(response).to have_gitlab_http_status(403)
- end
-
- it 'renders the error message when the format was html' do
- get :index,
- private_token: create(:personal_access_token, user: user).token,
- format: :html
-
- expect(response.body).to have_content /accept the terms of service/i
- end
-
- it 'renders a 200 when the sessionless user accepted the terms' do
- accept_terms(user)
-
- get :index, feed_token: user.feed_token, format: :atom
-
- expect(response).to have_gitlab_http_status(200)
- end
- end
end
end
diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2975205e09c3f7cf9be1532ea6f6b2b762c822bf
--- /dev/null
+++ b/spec/controllers/dashboard/projects_controller_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Dashboard::ProjectsController do
+ it_behaves_like 'authenticates sessionless user', :index, :atom
+end
diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb
index 187542ba30ceb82c4a3cd2efc6d49bb431c6f3dd..c857a78d5e8c0928b8d8c855d30250c8fb499049 100644
--- a/spec/controllers/dashboard_controller_spec.rb
+++ b/spec/controllers/dashboard_controller_spec.rb
@@ -1,21 +1,26 @@
require 'spec_helper'
describe DashboardController do
- let(:user) { create(:user) }
- let(:project) { create(:project) }
+ context 'signed in' do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
- before do
- project.add_maintainer(user)
- sign_in(user)
- end
+ before do
+ project.add_maintainer(user)
+ sign_in(user)
+ end
- describe 'GET issues' do
- it_behaves_like 'issuables list meta-data', :issue, :issues
- it_behaves_like 'issuables requiring filter', :issues
- end
+ describe 'GET issues' do
+ it_behaves_like 'issuables list meta-data', :issue, :issues
+ it_behaves_like 'issuables requiring filter', :issues
+ end
- describe 'GET merge requests' do
- it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests
- it_behaves_like 'issuables requiring filter', :merge_requests
+ describe 'GET merge requests' do
+ it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests
+ it_behaves_like 'issuables requiring filter', :merge_requests
+ end
end
+
+ it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first
+ it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics
end
diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb
index 1449036e14871ba7049be45901a3742e75bb369f..949ad532365329676204625906e737ee953b5c9d 100644
--- a/spec/controllers/graphql_controller_spec.rb
+++ b/spec/controllers/graphql_controller_spec.rb
@@ -52,15 +52,58 @@
end
end
+ context 'token authentication' do
+ before do
+ stub_authentication_activity_metrics(debug: false)
+ end
+
+ let(:user) { create(:user, username: 'Simon') }
+ let(:personal_access_token) { create(:personal_access_token, user: user) }
+
+ context "when the 'personal_access_token' param is populated with the personal access token" do
+ it 'logs the user in' do
+ expect(authentication_metrics)
+ .to increment(:user_authenticated_counter)
+ .and increment(:user_session_override_counter)
+ .and increment(:user_sessionless_authentication_counter)
+
+ run_test_query!(private_token: personal_access_token.token)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(query_response).to eq('echo' => '"Simon" says: test success')
+ end
+ end
+
+ context 'when the personal access token has no api scope' do
+ it 'does not log the user in' do
+ personal_access_token.update(scopes: [:read_user])
+
+ run_test_query!(private_token: personal_access_token.token)
+
+ expect(response).to have_gitlab_http_status(200)
+
+ expect(query_response).to eq('echo' => 'nil says: test success')
+ end
+ end
+
+ context 'without token' do
+ it 'shows public data' do
+ run_test_query!
+
+ expect(query_response).to eq('echo' => 'nil says: test success')
+ end
+ end
+ end
+
# Chosen to exercise all the moving parts in GraphqlController#execute
- def run_test_query!(variables: { 'text' => 'test success' })
+ def run_test_query!(variables: { 'text' => 'test success' }, private_token: nil)
query = <<~QUERY
query Echo($text: String) {
echo(text: $text)
}
QUERY
- post :execute, query: query, operationName: 'Echo', variables: variables
+ post :execute, query: query, operationName: 'Echo', variables: variables, private_token: private_token
end
def query_response
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 4de61b65f71d2bb9c64f0c6516ea0be889c945c5..f6c851028306c092f595e4b6d52cec10b6fe986a 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -606,4 +606,24 @@ def group_moved_message(redirect_route, group)
end
end
end
+
+ context 'token authentication' do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+ before do
+ default_params.merge!(id: group)
+ end
+ end
+
+ it_behaves_like 'authenticates sessionless user', :issues, :atom, public: true do
+ before do
+ default_params.merge!(id: group, author_id: user.id)
+ end
+ end
+
+ it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics, public: true do
+ before do
+ default_params.merge!(id: group)
+ end
+ end
+ end
end
diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb
index a43bdd3ea80f8d6b3a2b497f579cbd3b25f80c23..5c72dab698c67ccbcf388ea1f1ae048e6a659619 100644
--- a/spec/controllers/projects/commits_controller_spec.rb
+++ b/spec/controllers/projects/commits_controller_spec.rb
@@ -5,87 +5,115 @@
let(:user) { create(:user) }
before do
- sign_in(user)
project.add_maintainer(user)
end
- describe "GET commits_root" do
- context "no ref is provided" do
- it 'should redirect to the default branch of the project' do
- get(:commits_root,
- namespace_id: project.namespace,
- project_id: project)
+ context 'signed in' do
+ before do
+ sign_in(user)
+ end
+
+ describe "GET commits_root" do
+ context "no ref is provided" do
+ it 'should redirect to the default branch of the project' do
+ get(:commits_root,
+ namespace_id: project.namespace,
+ project_id: project)
- expect(response).to redirect_to project_commits_path(project)
+ expect(response).to redirect_to project_commits_path(project)
+ end
end
end
- end
- describe "GET show" do
- render_views
+ describe "GET show" do
+ render_views
- context 'with file path' do
- before do
- get(:show,
- namespace_id: project.namespace,
- project_id: project,
- id: id)
- end
+ context 'with file path' do
+ before do
+ get(:show,
+ namespace_id: project.namespace,
+ project_id: project,
+ id: id)
+ end
- context "valid branch, valid file" do
- let(:id) { 'master/README.md' }
+ context "valid branch, valid file" do
+ let(:id) { 'master/README.md' }
- it { is_expected.to respond_with(:success) }
- end
+ it { is_expected.to respond_with(:success) }
+ end
- context "valid branch, invalid file" do
- let(:id) { 'master/invalid-path.rb' }
+ context "valid branch, invalid file" do
+ let(:id) { 'master/invalid-path.rb' }
- it { is_expected.to respond_with(:not_found) }
- end
+ it { is_expected.to respond_with(:not_found) }
+ end
- context "invalid branch, valid file" do
- let(:id) { 'invalid-branch/README.md' }
+ context "invalid branch, valid file" do
+ let(:id) { 'invalid-branch/README.md' }
- it { is_expected.to respond_with(:not_found) }
+ it { is_expected.to respond_with(:not_found) }
+ end
end
- end
- context "when the ref name ends in .atom" do
- context "when the ref does not exist with the suffix" do
- before do
- get(:show,
- namespace_id: project.namespace,
- project_id: project,
- id: "master.atom")
+ context "when the ref name ends in .atom" do
+ context "when the ref does not exist with the suffix" do
+ before do
+ get(:show,
+ namespace_id: project.namespace,
+ project_id: project,
+ id: "master.atom")
+ end
+
+ it "renders as atom" do
+ expect(response).to be_success
+ expect(response.content_type).to eq('application/atom+xml')
+ end
+
+ it 'renders summary with type=html' do
+ expect(response.body).to include('<summary type="html">')
+ end
end
- it "renders as atom" do
- expect(response).to be_success
- expect(response.content_type).to eq('application/atom+xml')
- end
+ context "when the ref exists with the suffix" do
+ before do
+ commit = project.repository.commit('master')
- it 'renders summary with type=html' do
- expect(response.body).to include('<summary type="html">')
+ allow_any_instance_of(Repository).to receive(:commit).and_call_original
+ allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit)
+
+ get(:show,
+ namespace_id: project.namespace,
+ project_id: project,
+ id: "master.atom")
+ end
+
+ it "renders as HTML" do
+ expect(response).to be_success
+ expect(response.content_type).to eq('text/html')
+ end
end
end
+ end
+ end
- context "when the ref exists with the suffix" do
+ context 'token authentication' do
+ context 'public project' do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
before do
- commit = project.repository.commit('master')
+ public_project = create(:project, :repository, :public)
- allow_any_instance_of(Repository).to receive(:commit).and_call_original
- allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit)
-
- get(:show,
- namespace_id: project.namespace,
- project_id: project,
- id: "master.atom")
+ default_params.merge!(namespace_id: public_project.namespace, project_id: public_project, id: "master.atom")
end
+ end
+ end
+
+ context 'private project' do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do
+ before do
+ private_project = create(:project, :repository, :private)
+ private_project.add_maintainer(user)
- it "renders as HTML" do
- expect(response).to be_success
- expect(response.content_type).to eq('text/html')
+ default_params.merge!(namespace_id: private_project.namespace, project_id: private_project, id: "master.atom")
end
end
end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 80138183c0797a9623e835a43b9921222d219b8f..02930edbf72a1d584481ab5c91ce63be0307b986 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1068,4 +1068,40 @@ def create_merge_request
end
end
end
+
+ context 'private project with token authentication' do
+ let(:private_project) { create(:project, :private) }
+
+ it_behaves_like 'authenticates sessionless user', :index, :atom do
+ before do
+ default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+ private_project.add_maintainer(user)
+ end
+ end
+
+ it_behaves_like 'authenticates sessionless user', :calendar, :ics do
+ before do
+ default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+ private_project.add_maintainer(user)
+ end
+ end
+ end
+
+ context 'public project with token authentication' do
+ let(:public_project) { create(:project, :public) }
+
+ it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do
+ before do
+ default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+ end
+ end
+
+ it_behaves_like 'authenticates sessionless user', :calendar, :ics, public: true do
+ before do
+ default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+ end
+ end
+ end
end
diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb
index c48f41ca12e72fe04c0a9a9ccfb6c0432ceeea9c..6fbf75d025963bdf7018b43ae2659e9e17062ac6 100644
--- a/spec/controllers/projects/tags_controller_spec.rb
+++ b/spec/controllers/projects/tags_controller_spec.rb
@@ -35,4 +35,26 @@
it { is_expected.to respond_with(:not_found) }
end
end
+
+ context 'private project with token authentication' do
+ let(:private_project) { create(:project, :repository, :private) }
+
+ it_behaves_like 'authenticates sessionless user', :index, :atom do
+ before do
+ default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
+
+ private_project.add_maintainer(user)
+ end
+ end
+ end
+
+ context 'public project with token authentication' do
+ let(:public_project) { create(:project, :repository, :public) }
+
+ it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do
+ before do
+ default_params.merge!(project_id: public_project, namespace_id: public_project.namespace)
+ end
+ end
+ end
end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 3bc9cbe64c523eff75d438168fcdc06350c51006..7849bec4762b0ff045dd32a975f0392d21d768db 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -882,6 +882,28 @@ def update_project(**parameters)
end
end
+ context 'private project with token authentication' do
+ let(:private_project) { create(:project, :private) }
+
+ it_behaves_like 'authenticates sessionless user', :show, :atom do
+ before do
+ default_params.merge!(id: private_project, namespace_id: private_project.namespace)
+
+ private_project.add_maintainer(user)
+ end
+ end
+ end
+
+ context 'public project with token authentication' do
+ let(:public_project) { create(:project, :public) }
+
+ it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+ before do
+ default_params.merge!(id: public_project, namespace_id: public_project.namespace)
+ end
+ end
+ end
+
def project_moved_message(redirect_route, project)
"Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path."
end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 071f96a729efa58057556dd099132c5f6abd5cde..fe438e71e9eaaaffff3924c04f85820f8ab96541 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -395,6 +395,14 @@
end
end
+ context 'token authentication' do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+ before do
+ default_params.merge!(username: user.username)
+ end
+ end
+ end
+
def user_moved_message(redirect_route, user)
"User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path."
end
diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb
index 242ab4a91dd83c03ff6ba0e8631119112a53d98d..3d979132880eaaea308ac3bfd60b33484504827f 100644
--- a/spec/lib/gitlab/auth/request_authenticator_spec.rb
+++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb
@@ -19,17 +19,17 @@
allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user)
allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user)
- expect(subject.user).to eq sessionless_user
+ expect(subject.user([:api])).to eq sessionless_user
end
it 'returns session user if no sessionless user found' do
allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user)
- expect(subject.user).to eq session_user
+ expect(subject.user([:api])).to eq session_user
end
it 'returns nil if no user found' do
- expect(subject.user).to be_blank
+ expect(subject.user([:api])).to be_blank
end
it 'bubbles up exceptions' do
@@ -42,26 +42,26 @@
let!(:feed_token_user) { build(:user) }
it 'returns access_token user first' do
- allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_return(access_token_user)
+ allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user)
allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user)
- expect(subject.find_sessionless_user).to eq access_token_user
+ expect(subject.find_sessionless_user([:api])).to eq access_token_user
end
it 'returns feed_token user if no access_token user found' do
allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user)
- expect(subject.find_sessionless_user).to eq feed_token_user
+ expect(subject.find_sessionless_user([:api])).to eq feed_token_user
end
it 'returns nil if no user found' do
- expect(subject.find_sessionless_user).to be_blank
+ expect(subject.find_sessionless_user([:api])).to be_blank
end
it 'rescue Gitlab::Auth::AuthenticationError exceptions' do
- allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_raise(Gitlab::Auth::UnauthorizedError)
+ allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_raise(Gitlab::Auth::UnauthorizedError)
- expect(subject.find_sessionless_user).to be_blank
+ expect(subject.find_sessionless_user([:api])).to be_blank
end
end
end
diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb
index 454ad1589b9e28c8f7cc2b2f1b38344002f2dfc5..5d3fbba264f90b70e5f95c22962c0f045c0777de 100644
--- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb
@@ -9,7 +9,7 @@
'rack.input' => ''
}
end
- let(:request) { Rack::Request.new(env)}
+ let(:request) { Rack::Request.new(env) }
def set_param(key, value)
request.update_param(key, value)
@@ -49,6 +49,7 @@ def set_param(key, value)
describe '#find_user_from_feed_token' do
context 'when the request format is atom' do
before do
+ env['SCRIPT_NAME'] = 'url.atom'
env['HTTP_ACCEPT'] = 'application/atom+xml'
end
@@ -56,17 +57,17 @@ def set_param(key, value)
it 'returns user if valid feed_token' do
set_param(:feed_token, user.feed_token)
- expect(find_user_from_feed_token).to eq user
+ expect(find_user_from_feed_token(:rss)).to eq user
end
it 'returns nil if feed_token is blank' do
- expect(find_user_from_feed_token).to be_nil
+ expect(find_user_from_feed_token(:rss)).to be_nil
end
it 'returns exception if invalid feed_token' do
set_param(:feed_token, 'invalid_token')
- expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
end
@@ -74,34 +75,38 @@ def set_param(key, value)
it 'returns user if valid rssd_token' do
set_param(:rss_token, user.feed_token)
- expect(find_user_from_feed_token).to eq user
+ expect(find_user_from_feed_token(:rss)).to eq user
end
it 'returns nil if rss_token is blank' do
- expect(find_user_from_feed_token).to be_nil
+ expect(find_user_from_feed_token(:rss)).to be_nil
end
it 'returns exception if invalid rss_token' do
set_param(:rss_token, 'invalid_token')
- expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
end
end
context 'when the request format is not atom' do
it 'returns nil' do
+ env['SCRIPT_NAME'] = 'json'
+
set_param(:feed_token, user.feed_token)
- expect(find_user_from_feed_token).to be_nil
+ expect(find_user_from_feed_token(:rss)).to be_nil
end
end
context 'when the request format is empty' do
it 'the method call does not modify the original value' do
+ env['SCRIPT_NAME'] = 'url.atom'
+
env.delete('action_dispatch.request.formats')
- find_user_from_feed_token
+ find_user_from_feed_token(:rss)
expect(env['action_dispatch.request.formats']).to be_nil
end
@@ -111,8 +116,12 @@ def set_param(key, value)
describe '#find_user_from_access_token' do
let(:personal_access_token) { create(:personal_access_token, user: user) }
+ before do
+ env['SCRIPT_NAME'] = 'url.atom'
+ end
+
it 'returns nil if no access_token present' do
- expect(find_personal_access_token).to be_nil
+ expect(find_user_from_access_token).to be_nil
end
context 'when validate_access_token! returns valid' do
@@ -131,9 +140,59 @@ def set_param(key, value)
end
end
+ describe '#find_user_from_web_access_token' do
+ let(:personal_access_token) { create(:personal_access_token, user: user) }
+
+ before do
+ env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+ end
+
+ it 'returns exception if token has no user' do
+ allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil)
+
+ expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+ end
+
+ context 'no feed or API requests' do
+ it 'returns nil if the request is not RSS' do
+ expect(find_user_from_web_access_token(:rss)).to be_nil
+ end
+
+ it 'returns nil if the request is not ICS' do
+ expect(find_user_from_web_access_token(:ics)).to be_nil
+ end
+
+ it 'returns nil if the request is not API' do
+ expect(find_user_from_web_access_token(:api)).to be_nil
+ end
+ end
+
+ it 'returns the user for RSS requests' do
+ env['SCRIPT_NAME'] = 'url.atom'
+
+ expect(find_user_from_web_access_token(:rss)).to eq(user)
+ end
+
+ it 'returns the user for ICS requests' do
+ env['SCRIPT_NAME'] = 'url.ics'
+
+ expect(find_user_from_web_access_token(:ics)).to eq(user)
+ end
+
+ it 'returns the user for API requests' do
+ env['SCRIPT_NAME'] = '/api/endpoint'
+
+ expect(find_user_from_web_access_token(:api)).to eq(user)
+ end
+ end
+
describe '#find_personal_access_token' do
let(:personal_access_token) { create(:personal_access_token, user: user) }
+ before do
+ env['SCRIPT_NAME'] = 'url.atom'
+ end
+
context 'passed as header' do
it 'returns token if valid personal_access_token' do
env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
diff --git a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..7e4958f177a2629515fd45e3a7c6edb0ab3e1449
--- /dev/null
+++ b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
@@ -0,0 +1,92 @@
+shared_examples 'authenticates sessionless user' do |path, format, params|
+ params ||= {}
+
+ before do
+ stub_authentication_activity_metrics(debug: false)
+ end
+
+ let(:user) { create(:user) }
+ let(:personal_access_token) { create(:personal_access_token, user: user) }
+ let(:default_params) { { format: format }.merge(params.except(:public) || {}) }
+
+ context "when the 'personal_access_token' param is populated with the personal access token" do
+ it 'logs the user in' do
+ expect(authentication_metrics)
+ .to increment(:user_authenticated_counter)
+ .and increment(:user_session_override_counter)
+ .and increment(:user_sessionless_authentication_counter)
+
+ get path, default_params.merge(private_token: personal_access_token.token)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(controller.current_user).to eq(user)
+ end
+
+ it 'does not log the user in if page is public', if: params[:public] do
+ get path, default_params
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(controller.current_user).to be_nil
+ end
+ end
+
+ context 'when the personal access token has no api scope', unless: params[:public] do
+ it 'does not log the user in' do
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+
+ personal_access_token.update(scopes: [:read_user])
+
+ get path, default_params.merge(private_token: personal_access_token.token)
+
+ expect(response).not_to have_gitlab_http_status(200)
+ end
+ end
+
+ context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
+ it 'logs the user in' do
+ expect(authentication_metrics)
+ .to increment(:user_authenticated_counter)
+ .and increment(:user_session_override_counter)
+ .and increment(:user_sessionless_authentication_counter)
+
+ @request.headers['PRIVATE-TOKEN'] = personal_access_token.token
+ get path, default_params
+
+ expect(response).to have_gitlab_http_status(200)
+ end
+ end
+
+ context "when the 'feed_token' param is populated with the feed token", if: format == :rss do
+ it "logs the user in" do
+ expect(authentication_metrics)
+ .to increment(:user_authenticated_counter)
+ .and increment(:user_session_override_counter)
+ .and increment(:user_sessionless_authentication_counter)
+
+ get path, default_params.merge(feed_token: user.feed_token)
+
+ expect(response).to have_gitlab_http_status 200
+ end
+ end
+
+ context "when the 'feed_token' param is populated with an invalid feed token", if: format == :rss, unless: params[:public] do
+ it "logs the user" do
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+
+ get path, default_params.merge(feed_token: 'token')
+
+ expect(response.status).not_to eq 200
+ end
+ end
+
+ it "doesn't log the user in otherwise", unless: params[:public] do
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+
+ get path, default_params.merge(private_token: 'token')
+
+ expect(response.status).not_to eq(200)
+ end
+end