Merge branch 'security-fix-pat-web-access' into 'master'

[master] Resolve "Personal access token with only `read_user` scope can be used to authenticate any web request"

See merge request gitlab/gitlabhq!2583

2 files changed, 78 insertions, 19 deletions

diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb
index 242ab4a91dd..3d979132880 100644
--- a/spec/lib/gitlab/auth/request_authenticator_spec.rb
+++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb
@@ -19,17 +19,17 @@ describe Gitlab::Auth::RequestAuthenticator do
       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 @@ describe Gitlab::Auth::RequestAuthenticator do
     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 454ad1589b9..5d3fbba264f 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 @@ describe Gitlab::Auth::UserAuthFinders do
       '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 @@ describe Gitlab::Auth::UserAuthFinders do
   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 @@ describe Gitlab::Auth::UserAuthFinders do
         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 @@ describe Gitlab::Auth::UserAuthFinders do
         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 @@ describe Gitlab::Auth::UserAuthFinders do
   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 @@ describe Gitlab::Auth::UserAuthFinders do
     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