Perform private token and personal access token authentication in the same `before_action`.

- So that the check for valid personal access tokens happens only if
  private token auth fails.

app/controllers/application_controller.rb

@@ -7,8 +7,7 @@ class ApplicationController < ActionController::Base
   include GitlabRoutingHelper
   include PageLayoutHelper
 
-  before_action :authenticate_user_from_private_token!
-  before_action :authenticate_user_from_personal_access_token!
+  before_action :authenticate_user_from_token!
   before_action :authenticate_user!
   before_action :validate_user_service_ticket!
   before_action :reject_blocked!
@@ -64,26 +63,8 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example
-  # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6
-  def authenticate_user_from_private_token!
-    user_token = params[:private_token].presence  || request.headers['PRIVATE-TOKEN'].presence
-    user = user_token && User.find_by_authentication_token(user_token.to_s)
-
-    if user
-      # 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
-    end
-  end
-
-  def authenticate_user_from_personal_access_token!
-    token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
-    personal_access_token = PersonalAccessToken.active.find_by_token(token_string)
-    user = personal_access_token && personal_access_token.user
-
+  def authenticate_user_from_token!
+    user = get_user_from_private_token || get_user_from_personal_access_token
     if user
       # Notice we are passing store false, so the user is not
       # actually stored in the session and a token is needed
@@ -383,4 +364,17 @@ class ApplicationController < ActionController::Base
     (controller_name == 'groups' && action_name == page_type) ||
     (controller_name == 'dashboard' && action_name == page_type)
   end
+
+  # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example
+  # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6
+  def get_user_from_private_token
+    user_token = params[:private_token].presence  || request.headers['PRIVATE-TOKEN'].presence
+    User.find_by_authentication_token(user_token.to_s) if user_token
+  end
+
+  def get_user_from_personal_access_token
+    token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
+    personal_access_token = PersonalAccessToken.active.find_by_token(token_string)
+    personal_access_token.user if personal_access_token
+  end
 end

spec/controllers/application_controller_spec.rb

@@ -31,63 +31,65 @@ describe ApplicationController do
     end
   end
 
-  describe "#authenticate_user_from_private_token!" do
-    controller(ApplicationController) do
-      def index
-        render text: "authenticated"
+  describe "#authenticate_user_from_token!" do
+    describe "authenticating a user from a private token" do
+      controller(ApplicationController) do
+        def index
+          render text: "authenticated"
+        end
+      end
+
+      let(:user) { create(:user) }
+
+      it "logs the user in when the 'private_token' param is populated with the private token" do
+        get :index, private_token: user.private_token
+        expect(response.status).to eq(200)
+        expect(response.body).to eq("authenticated")
+      end
+
+      it "logs the user in when the 'PRIVATE-TOKEN' header is populated with the private token" do
+        @request.headers['PRIVATE-TOKEN'] = user.private_token
+        get :index
+        expect(response.status).to eq(200)
+        expect(response.body).to eq("authenticated")
+      end
+
+      it "doesn't log the user in otherwise" do
+        @request.headers['PRIVATE-TOKEN'] = "token"
+        get :index, private_token: "token", authenticity_token: "token"
+        expect(response.status).to_not eq(200)
+        expect(response.body).to_not eq("authenticated")
       end
     end
 
-    let(:user) { create(:user) }
-
-    it "logs the user in when the 'private_token' param is populated with the private token" do
-      get :index, private_token: user.private_token
-      expect(response.status).to eq(200)
-      expect(response.body).to eq("authenticated")
-    end
-
-    it "logs the user in when the 'PRIVATE-TOKEN' header is populated with the private token" do
-      @request.headers['PRIVATE-TOKEN'] = user.private_token
-      get :index
-      expect(response.status).to eq(200)
-      expect(response.body).to eq("authenticated")
-    end
-
-    it "doesn't log the user in otherwise" do
-      @request.headers['PRIVATE-TOKEN'] = "token"
-      get :index, private_token: "token", authenticity_token: "token"
-      expect(response.status).to_not eq(200)
-      expect(response.body).to_not eq("authenticated")
-    end
-  end
-
-  describe "#authenticate_user_from_personal_access_token!" do
-    controller(ApplicationController) do
-      def index
-        render text: 'authenticated'
+    describe "authenticating a user from a personal access token" do
+      controller(ApplicationController) do
+        def index
+          render text: 'authenticated'
+        end
       end
-    end
 
-    let(:user) { create(:user) }
-    let(:personal_access_token) { create(:personal_access_token, user: user) }
+      let(:user) { create(:user) }
+      let(:personal_access_token) { create(:personal_access_token, user: user) }
 
-    it "logs the user in when the 'personal_access_token' param is populated with the personal access token" do
-      get :index, private_token: personal_access_token.token
-      expect(response.status).to eq(200)
-      expect(response.body).to eq('authenticated')
-    end
+      it "logs the user in when the 'personal_access_token' param is populated with the personal access token" do
+        get :index, private_token: personal_access_token.token
+        expect(response.status).to eq(200)
+        expect(response.body).to eq('authenticated')
+      end
 
-    it "logs the user in when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
-      @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
-      get :index
-      expect(response.status).to eq(200)
-      expect(response.body).to eq('authenticated')
-    end
+      it "logs the user in when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
+        @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
+        get :index
+        expect(response.status).to eq(200)
+        expect(response.body).to eq('authenticated')
+      end
 
-    it "doesn't log the user in otherwise" do
-      get :index, private_token: "token"
-      expect(response.status).to_not eq(200)
-      expect(response.body).to_not eq('authenticated')
+      it "doesn't log the user in otherwise" do
+        get :index, private_token: "token"
+        expect(response.status).to_not eq(200)
+        expect(response.body).to_not eq('authenticated')
+      end
     end
   end
 end