From 6c3cbf706331988ce6c75cc6620048e40df8ec3a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 1 Jan 2021 15:10:11 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- changelogs/unreleased/unify-nuget-auth-1.yml | 6 + changelogs/unreleased/unify-nuget-auth-2.yml | 6 + lib/api/api_guard.rb | 13 +- lib/api/concerns/packages/nuget_endpoints.rb | 12 - lib/api/helpers/authentication.rb | 73 ++++++ lib/api/nuget_group_packages.rb | 10 +- lib/api/nuget_project_packages.rb | 23 +- lib/gitlab/api_authentication/builder.rb | 18 ++ .../sent_through_builder.rb | 19 ++ .../api_authentication/token_locator.rb | 37 ++++ .../api_authentication/token_resolver.rb | 87 ++++++++ .../api_authentication/token_type_builder.rb | 18 ++ lib/gitlab/auth/auth_finders.rb | 14 +- spec/lib/api/helpers/authentication_spec.rb | 207 ++++++++++++++++++ .../gitlab/api_authentication/builder_spec.rb | 76 +++++++ .../sent_through_builder_spec.rb | 17 ++ .../api_authentication/token_locator_spec.rb | 55 +++++ .../api_authentication/token_resolver_spec.rb | 117 ++++++++++ .../token_type_builder_spec.rb | 16 ++ .../api/nuget_project_packages_spec.rb | 16 +- .../api/nuget_endpoints_shared_examples.rb | 32 +-- 21 files changed, 808 insertions(+), 64 deletions(-) create mode 100644 changelogs/unreleased/unify-nuget-auth-1.yml create mode 100644 changelogs/unreleased/unify-nuget-auth-2.yml create mode 100644 lib/api/helpers/authentication.rb create mode 100644 lib/gitlab/api_authentication/builder.rb create mode 100644 lib/gitlab/api_authentication/sent_through_builder.rb create mode 100644 lib/gitlab/api_authentication/token_locator.rb create mode 100644 lib/gitlab/api_authentication/token_resolver.rb create mode 100644 lib/gitlab/api_authentication/token_type_builder.rb create mode 100644 spec/lib/api/helpers/authentication_spec.rb create mode 100644 spec/lib/gitlab/api_authentication/builder_spec.rb create mode 100644 spec/lib/gitlab/api_authentication/sent_through_builder_spec.rb create mode 100644 spec/lib/gitlab/api_authentication/token_locator_spec.rb create mode 100644 spec/lib/gitlab/api_authentication/token_resolver_spec.rb create mode 100644 spec/lib/gitlab/api_authentication/token_type_builder_spec.rb diff --git a/changelogs/unreleased/unify-nuget-auth-1.yml b/changelogs/unreleased/unify-nuget-auth-1.yml new file mode 100644 index 00000000000..e306f99994a --- /dev/null +++ b/changelogs/unreleased/unify-nuget-auth-1.yml @@ -0,0 +1,6 @@ +--- +title: Add a new Ruby API for specifying allowed authentication mechanisms + for REST API endpoints +merge_request: 38627 +author: Ethan Reesor (@firelizzard) +type: other diff --git a/changelogs/unreleased/unify-nuget-auth-2.yml b/changelogs/unreleased/unify-nuget-auth-2.yml new file mode 100644 index 00000000000..862111dc1a6 --- /dev/null +++ b/changelogs/unreleased/unify-nuget-auth-2.yml @@ -0,0 +1,6 @@ +--- +title: The NuGet endpoints will no longer ignore an invalid username when a personal + access token or deploy token is passed via HTTP Basic authentication +merge_request: 38627 +author: Ethan Reesor (@firelizzard) +type: security diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0a486307653..8641271f2df 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -69,10 +69,15 @@ module API def find_user_from_sources strong_memoize(:find_user_from_sources) do - deploy_token_from_request || - find_user_from_bearer_token || - find_user_from_job_token || - user_from_warden + if try(:namespace_inheritable, :authentication) + user_from_namespace_inheritable || + user_from_warden + else + deploy_token_from_request || + find_user_from_bearer_token || + find_user_from_job_token || + user_from_warden + end end end diff --git a/lib/api/concerns/packages/nuget_endpoints.rb b/lib/api/concerns/packages/nuget_endpoints.rb index dfc1d2957b0..70760bf048f 100644 --- a/lib/api/concerns/packages/nuget_endpoints.rb +++ b/lib/api/concerns/packages/nuget_endpoints.rb @@ -56,9 +56,6 @@ module API desc 'The NuGet Service Index' do detail 'This feature was introduced in GitLab 12.6' end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get 'index', format: :json do authorize_read_package!(project_or_group) track_package_event('cli_metadata', :nuget, category: 'API::NugetPackages') @@ -79,9 +76,6 @@ module API desc 'The NuGet Metadata Service - Package name level' do detail 'This feature was introduced in GitLab 12.8' end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get 'index', format: :json do present ::Packages::Nuget::PackagesMetadataPresenter.new(find_packages(params[:package_name])), with: ::API::Entities::Nuget::PackagesMetadata @@ -93,9 +87,6 @@ module API params do requires :package_version, type: String, desc: 'The NuGet package version', regexp: API::NO_SLASH_URL_PART_REGEX end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get '*package_version', format: :json do present ::Packages::Nuget::PackageMetadataPresenter.new(find_package(params[:package_name], params[:package_version])), with: ::API::Entities::Nuget::PackageMetadata @@ -117,9 +108,6 @@ module API desc 'The NuGet Search Service' do detail 'This feature was introduced in GitLab 12.8' end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get format: :json do search_options = { include_prerelease_versions: params[:prerelease], diff --git a/lib/api/helpers/authentication.rb b/lib/api/helpers/authentication.rb new file mode 100644 index 00000000000..a6cfe930190 --- /dev/null +++ b/lib/api/helpers/authentication.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module API + module Helpers + module Authentication + extend ActiveSupport::Concern + + class_methods do + def authenticate_with(&block) + strategies = ::Gitlab::APIAuthentication::Builder.new.build(&block) + namespace_inheritable :authentication, strategies + end + end + + included do + helpers ::Gitlab::Utils::StrongMemoize + + helpers do + def token_from_namespace_inheritable + strong_memoize(:token_from_namespace_inheritable) do + strategies = namespace_inheritable(:authentication) + next unless strategies&.any? + + # Extract credentials from the request + found = strategies.to_h { |location, _| [location, ::Gitlab::APIAuthentication::TokenLocator.new(location).extract(current_request)] } + found.filter! { |location, raw| raw } + next unless found.any? + + # Specifying multiple credentials is an error + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38627#note_475984136 + bad_request!('Found more than one set of credentials') if found.size > 1 + + location, raw = found.first + find_token_from_raw_credentials(strategies[location], raw) + end + + rescue ::Gitlab::Auth::UnauthorizedError + # TODO: this should be rescued and converted by the exception handling middleware + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38627#note_475174516 + unauthorized! + end + + def access_token_from_namespace_inheritable + token = token_from_namespace_inheritable + token if token.is_a? PersonalAccessToken + end + + def user_from_namespace_inheritable + token = token_from_namespace_inheritable + return token if token.is_a? DeployToken + + token&.user + end + + private + + def find_token_from_raw_credentials(token_types, raw) + token_types.each do |token_type| + # Resolve a token from the raw credentials + token = ::Gitlab::APIAuthentication::TokenResolver.new(token_type).resolve(raw) + return token if token + end + + # If a request provides credentials via an allowed transport, the + # credentials must be valid. If we reach this point, the credentials + # must not be valid credentials of an allowed type. + raise ::Gitlab::Auth::UnauthorizedError + end + end + end + end + end +end diff --git a/lib/api/nuget_group_packages.rb b/lib/api/nuget_group_packages.rb index fac4dd4780e..76bfc54f37a 100644 --- a/lib/api/nuget_group_packages.rb +++ b/lib/api/nuget_group_packages.rb @@ -9,13 +9,19 @@ # This is the group level API. module API class NugetGroupPackages < ::API::Base - helpers ::API::Helpers::PackagesManagerClientsHelpers + helpers ::API::Helpers::PackagesHelpers helpers ::API::Helpers::Packages::BasicAuthHelpers + include ::API::Helpers::Authentication feature_category :package_registry default_format :json + authenticate_with do |accept| + accept.token_types(:personal_access_token, :deploy_token, :job_token) + .sent_through(:http_basic_auth) + end + rescue_from ArgumentError do |e| render_api_error!(e.message, 400) end @@ -38,8 +44,6 @@ module API requires :id, type: String, desc: 'The ID of a group', regexp: ::API::Concerns::Packages::NugetEndpoints::POSITIVE_INTEGER_REGEX end - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do namespace ':id/packages/nuget' do after_validation do diff --git a/lib/api/nuget_project_packages.rb b/lib/api/nuget_project_packages.rb index 005493a90aa..2146f4d4b78 100644 --- a/lib/api/nuget_project_packages.rb +++ b/lib/api/nuget_project_packages.rb @@ -9,8 +9,9 @@ # This is the project level API. module API class NugetProjectPackages < ::API::Base - helpers ::API::Helpers::PackagesManagerClientsHelpers + helpers ::API::Helpers::PackagesHelpers helpers ::API::Helpers::Packages::BasicAuthHelpers + include ::API::Helpers::Authentication feature_category :package_registry @@ -18,6 +19,11 @@ module API default_format :json + authenticate_with do |accept| + accept.token_types(:personal_access_token, :deploy_token, :job_token) + .sent_through(:http_basic_auth) + end + rescue_from ArgumentError do |e| render_api_error!(e.message, 400) end @@ -35,9 +41,6 @@ module API params do requires :id, type: String, desc: 'The ID of a project', regexp: ::API::Concerns::Packages::NugetEndpoints::POSITIVE_INTEGER_REGEX end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do namespace ':id/packages/nuget' do include ::API::Concerns::Packages::NugetEndpoints @@ -50,9 +53,6 @@ module API params do requires :package, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)' end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - put do authorize_upload!(project_or_group) bad_request!('File is too large') if project_or_group.actual_limits.exceeded?(:nuget_max_file_size, params[:package].size) @@ -78,9 +78,6 @@ module API forbidden! end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - put 'authorize' do authorize_workhorse!( subject: project_or_group, @@ -101,9 +98,6 @@ module API desc 'The NuGet Content Service - index request' do detail 'This feature was introduced in GitLab 12.8' end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get 'index', format: :json do present ::Packages::Nuget::PackagesVersionsPresenter.new(find_packages(params[:package_name])), with: ::API::Entities::Nuget::PackagesVersions @@ -116,9 +110,6 @@ module API requires :package_version, type: String, desc: 'The NuGet package version', regexp: API::NO_SLASH_URL_PART_REGEX requires :package_filename, type: String, desc: 'The NuGet package filename', regexp: API::NO_SLASH_URL_PART_REGEX end - - route_setting :authentication, deploy_token_allowed: true, job_token_allowed: :basic_auth, basic_auth_personal_access_token: true - get '*package_version/*package_filename', format: :nupkg do filename = "#{params[:package_filename]}.#{params[:format]}" package_file = ::Packages::PackageFileFinder.new(find_package(params[:package_name], params[:package_version]), filename, with_file_name_like: true) diff --git a/lib/gitlab/api_authentication/builder.rb b/lib/gitlab/api_authentication/builder.rb new file mode 100644 index 00000000000..717c664826a --- /dev/null +++ b/lib/gitlab/api_authentication/builder.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Authentication Strategies Builder +# +# AuthBuilder and its child classes, TokenType and SentThrough, support +# declaring allowed authentication strategies with patterns like +# `accept.token_type(:job_token).sent_through(:http_basic)`. +module Gitlab + module APIAuthentication + class Builder + def build + strategies = Hash.new([]) + yield ::Gitlab::APIAuthentication::TokenTypeBuilder.new(strategies) + strategies + end + end + end +end diff --git a/lib/gitlab/api_authentication/sent_through_builder.rb b/lib/gitlab/api_authentication/sent_through_builder.rb new file mode 100644 index 00000000000..f66e5960019 --- /dev/null +++ b/lib/gitlab/api_authentication/sent_through_builder.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# See Gitlab::APIAuthentication::Builder +module Gitlab + module APIAuthentication + class SentThroughBuilder + def initialize(strategies, resolvers) + @strategies = strategies + @resolvers = resolvers + end + + def sent_through(*locators) + locators.each do |locator| + @strategies[locator] |= @resolvers + end + end + end + end +end diff --git a/lib/gitlab/api_authentication/token_locator.rb b/lib/gitlab/api_authentication/token_locator.rb new file mode 100644 index 00000000000..32a98908e5b --- /dev/null +++ b/lib/gitlab/api_authentication/token_locator.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module APIAuthentication + class TokenLocator + UsernameAndPassword = Struct.new(:username, :password) + + include ActiveModel::Validations + include ActionController::HttpAuthentication::Basic + + attr_reader :location + + validates :location, inclusion: { in: %i[http_basic_auth] } + + def initialize(location) + @location = location + validate! + end + + def extract(request) + case @location + when :http_basic_auth + extract_from_http_basic_auth request + end + end + + private + + def extract_from_http_basic_auth(request) + username, password = user_name_and_password(request) + return unless username.present? && password.present? + + UsernameAndPassword.new(username, password) + end + end + end +end diff --git a/lib/gitlab/api_authentication/token_resolver.rb b/lib/gitlab/api_authentication/token_resolver.rb new file mode 100644 index 00000000000..5b30777b6ec --- /dev/null +++ b/lib/gitlab/api_authentication/token_resolver.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module Gitlab + module APIAuthentication + class TokenResolver + include ActiveModel::Validations + + attr_reader :token_type + + validates :token_type, inclusion: { in: %i[personal_access_token job_token deploy_token] } + + def initialize(token_type) + @token_type = token_type + validate! + end + + # Existing behavior is known to be inconsistent across authentication + # methods with regards to whether to silently ignore present but invalid + # credentials or to raise an error/respond with 401. + # + # If a token can be located from the provided credentials, but the token + # or credentials are in some way invalid, this implementation opts to + # raise an error. + # + # For example, if the raw credentials include a username and password, and + # a token is resolved from the password, but the username does not match + # the token, an error will be raised. + # + # See https://gitlab.com/gitlab-org/gitlab/-/issues/246569 + + def resolve(raw) + case @token_type + when :personal_access_token + resolve_personal_access_token raw + + when :job_token + resolve_job_token raw + + when :deploy_token + resolve_deploy_token raw + end + end + + private + + def resolve_personal_access_token(raw) + # Check if the password is a personal access token + pat = ::PersonalAccessToken.find_by_token(raw.password) + return unless pat + + # Ensure that the username matches the token. This check is a subtle + # departure from the existing behavior of #find_personal_access_token_from_http_basic_auth. + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38627#note_435907856 + raise ::Gitlab::Auth::UnauthorizedError unless pat.user.username == raw.username + + pat + end + + def resolve_job_token(raw) + # Only look for a job if the username is correct + return if ::Gitlab::Auth::CI_JOB_USER != raw.username + + job = ::Ci::AuthJobFinder.new(token: raw.password).execute + + # Actively reject credentials with the username `gitlab-ci-token` if + # the password is not a valid job token. This replicates existing + # behavior of #find_user_from_job_token. + raise ::Gitlab::Auth::UnauthorizedError unless job + + job + end + + def resolve_deploy_token(raw) + # Check if the password is a deploy token + token = ::DeployToken.active.find_by_token(raw.password) + return unless token + + # Ensure that the username matches the token. This check is a subtle + # departure from the existing behavior of #deploy_token_from_request. + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38627#note_474826205 + raise ::Gitlab::Auth::UnauthorizedError unless token.username == raw.username + + token + end + end + end +end diff --git a/lib/gitlab/api_authentication/token_type_builder.rb b/lib/gitlab/api_authentication/token_type_builder.rb new file mode 100644 index 00000000000..4a57cdc2742 --- /dev/null +++ b/lib/gitlab/api_authentication/token_type_builder.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# See Gitlab::Auth::AuthBuilder +module Gitlab + module APIAuthentication + class TokenTypeBuilder + def initialize(strategies) + @strategies = strategies + end + + def token_types(*resolvers) + ::Gitlab::APIAuthentication::SentThroughBuilder.new(@strategies, resolvers) + end + + alias_method :token_type, :token_types + end + end +end diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index caa881eeeab..e23f4c8592d 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -194,11 +194,15 @@ module Gitlab def access_token strong_memoize(:access_token) do - # The token can be a PAT or an OAuth (doorkeeper) token - # It is also possible that a PAT is encapsulated in a `Bearer` OAuth token - # (e.g. NPM client registry auth), this case will be properly handled - # by find_personal_access_token - find_oauth_access_token || find_personal_access_token + if try(:namespace_inheritable, :authentication) + access_token_from_namespace_inheritable + else + # The token can be a PAT or an OAuth (doorkeeper) token + # It is also possible that a PAT is encapsulated in a `Bearer` OAuth token + # (e.g. NPM client registry auth), this case will be properly handled + # by find_personal_access_token + find_oauth_access_token || find_personal_access_token + end end end diff --git a/spec/lib/api/helpers/authentication_spec.rb b/spec/lib/api/helpers/authentication_spec.rb new file mode 100644 index 00000000000..461b0d2f6f9 --- /dev/null +++ b/spec/lib/api/helpers/authentication_spec.rb @@ -0,0 +1,207 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::Authentication do + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :public) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + describe 'class methods' do + subject { Class.new.include(described_class::ClassMethods).new } + + describe '.authenticate_with' do + it 'sets namespace_inheritable :authentication to correctly when body is empty' do + expect(subject).to receive(:namespace_inheritable).with(:authentication, {}) + + subject.authenticate_with { |allow| } + end + + it 'sets namespace_inheritable :authentication to correctly when body is not empty' do + expect(subject).to receive(:namespace_inheritable).with(:authentication, { basic: [:pat, :job], oauth: [:pat, :job] }) + + subject.authenticate_with { |allow| allow.token_type(:pat, :job).sent_through(:basic, :oauth) } + end + end + end + + describe 'helper methods' do + let(:object) do + cls = Class.new + + class << cls + def helpers(*modules, &block) + modules.each { |m| include m } + include Module.new.tap { |m| m.class_eval(&block) } if block_given? + end + end + + cls.define_method(:unauthorized!) { raise '401' } + cls.define_method(:bad_request!) { |m| raise "400 - #{m}" } + + # Include the helper class methods, as instance methods + cls.include described_class::ClassMethods + + # Include the methods under test + cls.include described_class + + cls.new + end + + describe '#token_from_namespace_inheritable' do + let(:object) do + o = super() + + o.instance_eval do + # It doesn't matter what this returns as long as the method is defined + def current_request + nil + end + + # Spoof Grape's namespace inheritable system + def namespace_inheritable(key, value = nil) + return unless key == :authentication + + if value + @authentication = value + else + @authentication + end + end + end + + o + end + + let(:authentication) do + object.authenticate_with { |allow| allow.token_types(*resolvers).sent_through(*locators) } + end + + subject { object.token_from_namespace_inheritable } + + before do + # Skip validation of token transports and types to simplify testing + allow(Gitlab::APIAuthentication::TokenLocator).to receive(:new) { |type| type } + allow(Gitlab::APIAuthentication::TokenResolver).to receive(:new) { |type| type } + + authentication + end + + shared_examples 'stops early' do |response_method| + it "calls ##{response_method}" do + errcls = Class.new(StandardError) + expect(object).to receive(response_method).and_raise(errcls) + expect { subject }.to raise_error(errcls) + end + end + + shared_examples 'an anonymous request' do + it 'returns nil' do + expect(subject).to be(nil) + end + end + + shared_examples 'an authenticated request' do + it 'returns the token' do + expect(subject).to be(token) + end + end + + shared_examples 'an unauthorized request' do + it_behaves_like 'stops early', :unauthorized! + end + + context 'with no allowed authentication strategies' do + let(:authentication) { nil } + + it_behaves_like 'an anonymous request' + end + + context 'with no located credentials' do + let(:locators) { [double(extract: nil)] } + let(:resolvers) { [] } + + it_behaves_like 'an anonymous request' + end + + context 'with one set of located credentials' do + let(:locators) { [double(extract: true)] } + + context 'when the credentials contain a valid token' do + let(:token) { double } + let(:resolvers) { [double(resolve: token)] } + + it_behaves_like 'an authenticated request' + end + + context 'when the credentials do not contain a valid token' do + let(:resolvers) { [double(resolve: nil)] } + + it_behaves_like 'an unauthorized request' + end + end + + context 'with multiple located credentials' do + let(:locators) { [double(extract: true), double(extract: true)] } + let(:resolvers) { [] } + + it_behaves_like 'stops early', :bad_request! + end + + context 'when a resolver raises UnauthorizedError' do + let(:locators) { [double(extract: true)] } + let(:resolvers) do + r = double + expect(r).to receive(:resolve).and_raise(Gitlab::Auth::UnauthorizedError) + r + end + + it_behaves_like 'an unauthorized request' + end + end + + describe '#access_token_from_namespace_inheritable' do + subject { object.access_token_from_namespace_inheritable } + + it 'returns #token_from_namespace_inheritable if it is a personal access token' do + expect(object).to receive(:token_from_namespace_inheritable).and_return(personal_access_token) + expect(subject).to be(personal_access_token) + end + + it 'returns nil if #token_from_namespace_inheritable is not a personal access token' do + token = double + expect(object).to receive(:token_from_namespace_inheritable).and_return(token) + expect(subject).to be(nil) + end + end + + describe '#user_from_namespace_inheritable' do + subject { object.user_from_namespace_inheritable } + + it 'returns #token_from_namespace_inheritable if it is a deploy token' do + expect(object).to receive(:token_from_namespace_inheritable).and_return(deploy_token) + expect(subject).to be(deploy_token) + end + + it 'returns #token_from_namespace_inheritable.user if the token is not a deploy token' do + user = double + token = double(user: user) + expect(object).to receive(:token_from_namespace_inheritable).and_return(token) + + expect(subject).to be(user) + end + + it 'falls back to #find_user_from_warden if #token_from_namespace_inheritable.user is nil' do + token = double(user: nil) + expect(object).to receive(:token_from_namespace_inheritable).and_return(token) + subject + end + + it 'falls back to #find_user_from_warden if #token_from_namespace_inheritable is nil' do + expect(object).to receive(:token_from_namespace_inheritable).and_return(nil) + subject + end + end + end +end diff --git a/spec/lib/gitlab/api_authentication/builder_spec.rb b/spec/lib/gitlab/api_authentication/builder_spec.rb new file mode 100644 index 00000000000..e241aa77805 --- /dev/null +++ b/spec/lib/gitlab/api_authentication/builder_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::APIAuthentication::Builder do + describe '#build' do + shared_examples 'builds the correct result' do |token_type:, sent_through:, builds:| + context "with #{token_type.size} token type(s) and #{sent_through.size} sent through(s)" do + it 'works when passed together' do + strategies = described_class.new.build { |allow| allow.token_types(*token_type).sent_through(*sent_through) } + + expect(strategies).to eq(builds) + end + + it 'works when token types are passed separately' do + strategies = described_class.new.build { |allow| token_type.each { |t| allow.token_types(t).sent_through(*sent_through) } } + + expect(strategies).to eq(builds) + end + + it 'works when sent throughs are passed separately' do + strategies = described_class.new.build { |allow| sent_through.each { |s| allow.token_types(*token_type).sent_through(s) } } + + expect(strategies).to eq(builds) + end + + it 'works when token types and sent throughs are passed separately' do + strategies = described_class.new.build { |allow| token_type.each { |t| sent_through.each { |s| allow.token_types(t).sent_through(s) } } } + + expect(strategies).to eq(builds) + end + end + end + + it_behaves_like 'builds the correct result', + token_type: [:pat], + sent_through: [:basic], + builds: { basic: [:pat] } + + it_behaves_like 'builds the correct result', + token_type: [:pat], + sent_through: [:basic, :oauth], + builds: { basic: [:pat], oauth: [:pat] } + + it_behaves_like 'builds the correct result', + token_type: [:pat, :job], + sent_through: [:basic], + builds: { basic: [:pat, :job] } + + it_behaves_like 'builds the correct result', + token_type: [:pat, :job], + sent_through: [:basic, :oauth], + builds: { basic: [:pat, :job], oauth: [:pat, :job] } + + context 'with a complex auth strategy' do + it 'builds the correct result' do + strategies = described_class.new.build do |allow| + allow.token_types(:pat, :job, :deploy).sent_through(:http_basic, :oauth) + allow.token_types(:pat).sent_through(:http_private, :query_private) + allow.token_types(:oauth2).sent_through(:http_bearer, :query_access) + end + + expect(strategies).to eq({ + http_basic: [:pat, :job, :deploy], + oauth: [:pat, :job, :deploy], + + http_private: [:pat], + query_private: [:pat], + + http_bearer: [:oauth2], + query_access: [:oauth2] + }) + end + end + end +end diff --git a/spec/lib/gitlab/api_authentication/sent_through_builder_spec.rb b/spec/lib/gitlab/api_authentication/sent_through_builder_spec.rb new file mode 100644 index 00000000000..845e317f3aa --- /dev/null +++ b/spec/lib/gitlab/api_authentication/sent_through_builder_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::APIAuthentication::SentThroughBuilder do + describe '#sent_through' do + let(:resolvers) { Array.new(3) { double } } + let(:locators) { Array.new(3) { double } } + + it 'adds a strategy for each of locators x resolvers' do + strategies = locators.to_h { |l| [l, []] } + described_class.new(strategies, resolvers).sent_through(*locators) + + expect(strategies).to eq(locators.to_h { |l| [l, resolvers] }) + end + end +end diff --git a/spec/lib/gitlab/api_authentication/token_locator_spec.rb b/spec/lib/gitlab/api_authentication/token_locator_spec.rb new file mode 100644 index 00000000000..68ce48a70ea --- /dev/null +++ b/spec/lib/gitlab/api_authentication/token_locator_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::APIAuthentication::TokenLocator do + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :public) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:ci_job) { create(:ci_build, project: project, user: user, status: :running) } + let_it_be(:ci_job_done) { create(:ci_build, project: project, user: user, status: :success) } + let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + describe '.new' do + context 'with a valid type' do + it 'creates a new instance' do + expect(described_class.new(:http_basic_auth)).to be_a(described_class) + end + end + + context 'with an invalid type' do + it 'raises ActiveModel::ValidationError' do + expect { described_class.new(:not_a_real_locator) }.to raise_error(ActiveModel::ValidationError) + end + end + end + + describe '#extract' do + let(:locator) { described_class.new(type) } + + subject { locator.extract(request) } + + context 'with :http_basic_auth' do + let(:type) { :http_basic_auth } + + context 'without credentials' do + let(:request) { double(authorization: nil) } + + it 'returns nil' do + expect(subject).to be(nil) + end + end + + context 'with credentials' do + let(:username) { 'foo' } + let(:password) { 'bar' } + let(:request) { double(authorization: "Basic #{::Base64.strict_encode64("#{username}:#{password}")}") } + + it 'returns the credentials' do + expect(subject.username).to eq(username) + expect(subject.password).to eq(password) + end + end + end + end +end diff --git a/spec/lib/gitlab/api_authentication/token_resolver_spec.rb b/spec/lib/gitlab/api_authentication/token_resolver_spec.rb new file mode 100644 index 00000000000..0028fb080ac --- /dev/null +++ b/spec/lib/gitlab/api_authentication/token_resolver_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::APIAuthentication::TokenResolver do + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :public) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:ci_job) { create(:ci_build, project: project, user: user, status: :running) } + let_it_be(:ci_job_done) { create(:ci_build, project: project, user: user, status: :success) } + let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + shared_examples 'an authorized request' do + it 'returns the correct token' do + expect(subject).to eq(token) + end + end + + shared_examples 'an unauthorized request' do + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + + shared_examples 'an anoymous request' do + it 'returns nil' do + expect(subject).to eq(nil) + end + end + + describe '.new' do + context 'with a valid type' do + it 'creates a new instance' do + expect(described_class.new(:personal_access_token)).to be_a(described_class) + end + end + + context 'with an invalid type' do + it 'raises a validation error' do + expect { described_class.new(:not_a_real_locator) }.to raise_error(ActiveModel::ValidationError) + end + end + end + + describe '#resolve' do + let(:resolver) { described_class.new(type) } + + subject { resolver.resolve(raw) } + + context 'with :personal_access_token' do + let(:type) { :personal_access_token } + let(:token) { personal_access_token } + + context 'with valid credentials' do + let(:raw) { username_and_password(user.username, token.token) } + + it_behaves_like 'an authorized request' + end + + context 'with an invalid username' do + let(:raw) { username_and_password("not-my-#{user.username}", token.token) } + + it_behaves_like 'an unauthorized request' + end + end + + context 'with :job_token' do + let(:type) { :job_token } + let(:token) { ci_job } + + context 'with valid credentials' do + let(:raw) { username_and_password(Gitlab::Auth::CI_JOB_USER, token.token) } + + it_behaves_like 'an authorized request' + end + + context 'when the job is not running' do + let(:raw) { username_and_password(Gitlab::Auth::CI_JOB_USER, ci_job_done.token) } + + it_behaves_like 'an unauthorized request' + end + + context 'with the wrong username' do + let(:raw) { username_and_password("not-#{Gitlab::Auth::CI_JOB_USER}", nil) } + + it_behaves_like 'an anoymous request' + end + + context 'with an invalid job token' do + let(:raw) { username_and_password(Gitlab::Auth::CI_JOB_USER, "not a valid CI job token") } + + it_behaves_like 'an unauthorized request' + end + end + + context 'with :deploy_token' do + let(:type) { :deploy_token } + let(:token) { deploy_token } + + context 'with a valid deploy token' do + let(:raw) { username_and_password(token.username, token.token) } + + it_behaves_like 'an authorized request' + end + + context 'with an invalid username' do + let(:raw) { username_and_password("not-my-#{token.username}", token.token) } + + it_behaves_like 'an unauthorized request' + end + end + end + + def username_and_password(username, password) + ::Gitlab::APIAuthentication::TokenLocator::UsernameAndPassword.new(username, password) + end +end diff --git a/spec/lib/gitlab/api_authentication/token_type_builder_spec.rb b/spec/lib/gitlab/api_authentication/token_type_builder_spec.rb new file mode 100644 index 00000000000..fbca62c9a42 --- /dev/null +++ b/spec/lib/gitlab/api_authentication/token_type_builder_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::APIAuthentication::TokenTypeBuilder do + describe '#token_types' do + it 'passes strategies and resolvers to SentThroughBuilder' do + strategies = double + resolvers = Array.new(3) { double } + retval = double + expect(Gitlab::APIAuthentication::SentThroughBuilder).to receive(:new).with(strategies, resolvers).and_return(retval) + + expect(described_class.new(strategies).token_types(*resolvers)).to be(retval) + end + end +end diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index c64ac6f3a94..813ebc35ede 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -49,12 +49,12 @@ RSpec.describe API::NugetProjectPackages do where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget download versions request' | :success 'PUBLIC' | :guest | true | true | 'process nuget download versions request' | :success - 'PUBLIC' | :developer | true | false | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | true | false | 'process nuget download versions request' | :success + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget download versions request' | :success 'PUBLIC' | :guest | false | true | 'process nuget download versions request' | :success - 'PUBLIC' | :developer | false | false | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | false | false | 'process nuget download versions request' | :success + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | 'process nuget download versions request' | :success 'PRIVATE' | :developer | true | true | 'process nuget download versions request' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden @@ -100,12 +100,12 @@ RSpec.describe API::NugetProjectPackages do where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget download content request' | :success 'PUBLIC' | :guest | true | true | 'process nuget download content request' | :success - 'PUBLIC' | :developer | true | false | 'process nuget download content request' | :success - 'PUBLIC' | :guest | true | false | 'process nuget download content request' | :success + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget download content request' | :success 'PUBLIC' | :guest | false | true | 'process nuget download content request' | :success - 'PUBLIC' | :developer | false | false | 'process nuget download content request' | :success - 'PUBLIC' | :guest | false | false | 'process nuget download content request' | :success + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | 'process nuget download content request' | :success 'PRIVATE' | :developer | true | true | 'process nuget download content request' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden diff --git a/spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb b/spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb index 689bbf5c3db..7b7d2a33e8c 100644 --- a/spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb @@ -10,12 +10,12 @@ RSpec.shared_examples 'handling nuget service requests' do |anonymous_requests_e where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget service index request' | :success 'PUBLIC' | :guest | true | true | 'process nuget service index request' | :success - 'PUBLIC' | :developer | true | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | true | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget service index request' | :success 'PUBLIC' | :guest | false | true | 'process nuget service index request' | :success - 'PUBLIC' | :developer | false | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | false | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | anonymous_requests_example_name | anonymous_requests_status 'PRIVATE' | :developer | true | true | 'process nuget service index request' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden @@ -109,12 +109,12 @@ RSpec.shared_examples 'handling nuget metadata requests with package name' do |a where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget metadata request at package name level' | :success 'PUBLIC' | :guest | true | true | 'process nuget metadata request at package name level' | :success - 'PUBLIC' | :developer | true | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | true | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget metadata request at package name level' | :success 'PUBLIC' | :guest | false | true | 'process nuget metadata request at package name level' | :success - 'PUBLIC' | :developer | false | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | false | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | anonymous_requests_example_name | anonymous_requests_status 'PRIVATE' | :developer | true | true | 'process nuget metadata request at package name level' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden @@ -171,12 +171,12 @@ RSpec.shared_examples 'handling nuget metadata requests with package name and pa where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget metadata request at package name and package version level' | :success 'PUBLIC' | :guest | true | true | 'process nuget metadata request at package name and package version level' | :success - 'PUBLIC' | :developer | true | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | true | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget metadata request at package name and package version level' | :success 'PUBLIC' | :guest | false | true | 'process nuget metadata request at package name and package version level' | :success - 'PUBLIC' | :developer | false | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | false | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | anonymous_requests_example_name | anonymous_requests_status 'PRIVATE' | :developer | true | true | 'process nuget metadata request at package name and package version level' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden @@ -235,12 +235,12 @@ RSpec.shared_examples 'handling nuget search requests' do |anonymous_requests_ex where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process nuget search request' | :success 'PUBLIC' | :guest | true | true | 'process nuget search request' | :success - 'PUBLIC' | :developer | true | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | true | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :developer | false | true | 'process nuget search request' | :success 'PUBLIC' | :guest | false | true | 'process nuget search request' | :success - 'PUBLIC' | :developer | false | false | anonymous_requests_example_name | anonymous_requests_status - 'PUBLIC' | :guest | false | false | anonymous_requests_example_name | anonymous_requests_status + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | anonymous_requests_example_name | anonymous_requests_status 'PRIVATE' | :developer | true | true | 'process nuget search request' | :success 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden