Refactor logout token validation 💄

app/controllers/oauth/geo_auth_controller.rb

@@ -37,21 +37,11 @@ class Oauth::GeoAuthController < ActionController::Base
     oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
     token_string = oauth.extract_logout_token
 
-    unless token_string && token_string.is_utf8?
-      access_token_error('invalid')
-    end
-
-    access_token = Doorkeeper::AccessToken.by_token(token_string)
-    access_token_status = Oauth2::AccessTokenValidationService.validate(access_token)
-
-    if access_token_status == Oauth2::AccessTokenValidationService::VALID
-      user = User.find(access_token.resource_owner_id)
-
-      if current_user == user
+    logout = Oauth2::LogoutTokenValidationService.new(current_user, token_string)
+    if logout.valid?
       sign_out current_user
-      end
     else
-      access_token_error('invalid')
+      access_token_error(logout.status)
     end
 
     redirect_to root_path

app/services/oauth2/logout_token_validation_service.rb

+module Oauth2
+  class LogoutTokenValidationService
+    attr_reader :status, :current_user
+
+    def initialize(user, access_token_string)
+      @access_token_string = access_token_string
+      @current_user = user
+    end
+
+    def validate
+      return false unless access_token
+
+      @status = Oauth2::AccessTokenValidationService.validate(access_token)
+
+      if @status == Oauth2::AccessTokenValidationService::VALID
+        user = User.find(access_token.resource_owner_id)
+
+        if current_user == user
+          true
+        end
+      else
+        false
+      end
+    end
+
+    def access_token
+      return unless @access_token_string && @access_token_string.is_utf8?
+
+      @access_token ||= Doorkeeper::AccessToken.by_token(@access_token_string)
+    end
+  end
+end

spec/services/oauth2/logout_token_validation_service_spec.rb

+require 'spec_helper'
+
+describe Oauth2::LogoutTokenValidationService, services: true do
+  let(:user) { FactoryGirl.create(:user) }
+  let(:access_token) { FactoryGirl.create(:doorkeeper_access_token, resource_owner_id: user.id).token }
+
+  context '#validate' do
+    it 'returns false when empty' do
+      expect(described_class.new(user, nil).validate).to be_falsey
+    end
+
+    it 'returns false when incorrect encoding' do
+      invalid_token = "\xD800\xD801\xD802"
+      expect(described_class.new(user, invalid_token).validate).to be_falsey
+    end
+
+    it 'returns true when token is valid' do
+      expect(described_class.new(user, access_token).validate).to be_truthy
+    end
+  end
+end