More codestyle changes 💄

app/controllers/oauth/geo_auth_controller.rb

@@ -35,7 +35,7 @@ class Oauth::GeoAuthController < ActionController::Base
 
   def logout
     logout = Oauth2::LogoutTokenValidationService.new(current_user, params)
-    result = logout.validate
+    result = logout.execute
     if result[:status] == :success
       sign_out current_user
       redirect_to root_path
@@ -47,7 +47,7 @@ class Oauth::GeoAuthController < ActionController::Base
   private
 
   def invalid_credentials
-    @error = 'Cannot find user to login. Your account must have been deleted.'
+    @error = 'Cannot find user to login. Your account may have been deleted.'
     render :error, layout: 'errors'
   end
 

app/controllers/sessions_controller.rb

@@ -112,7 +112,9 @@ class SessionsController < Devise::SessionsController
   end
 
   def gitlab_geo_login
-    return if signed_in? || !Gitlab::Geo.secondary?
+    return unless Gitlab::Geo.secondary?
+    return if signed_in?
+    
     oauth = Gitlab::Geo::OauthSession.new
 
     # share full url with primary node by oauth state

app/services/oauth2/logout_token_validation_service.rb

 module Oauth2
   class LogoutTokenValidationService < ::BaseService
-    attr_reader :status, :current_user
+    attr_reader :status
 
     def initialize(user, params={})
-      if params && params[:state] && !params[:state].empty?
-        oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
-        @access_token_string = oauth.extract_logout_token
-      end
+      @params = params
       @current_user = user
     end
 
@@ -26,9 +23,17 @@ module Oauth2
     end
 
     def access_token
-      return unless @access_token_string && @access_token_string.is_utf8?
+      @access_token ||= begin
+        return unless params[:state] && !params[:state].empty?
+
+        oauth_session = Gitlab::Geo::OauthSession.new(state: params[:state])
+
+        logout_token = oauth_session.extract_logout_token
+        return unless logout_token && logout_token.is_utf8?
+
+        Doorkeeper::AccessToken.by_token(logout_token)
+      end
 
-      @access_token ||= Doorkeeper::AccessToken.by_token(@access_token_string)
     end
   end
 end

spec/controllers/oauth/geo_auth_controller_spec.rb

@@ -7,7 +7,7 @@ describe Oauth::GeoAuthController do
   let(:auth_state) { Gitlab::Geo::OauthSession.new(access_token: access_token, return_to: projects_url).generate_oauth_state }
   let(:primary_node_url) { 'http://localhost:3001/' }
 
-  before(:each) do
+  before do
     allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:oauth_app) { oauth_app }
     allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:primary_node_url) { primary_node_url }
   end
@@ -34,7 +34,7 @@ describe Oauth::GeoAuthController do
     let(:primary_node_oauth_endpoint) { Gitlab::Geo::OauthSession.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: callback_state) }
 
     context 'redirection' do
-      before(:each) do
+      before do
         allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:get_token) { 'token' }
         allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:authenticate_with_gitlab) { user.attributes }
       end
@@ -57,7 +57,7 @@ describe Oauth::GeoAuthController do
       let(:fake_response) { double('Faraday::Response', headers: {}, body: '', status: 403) }
       let(:oauth_error) { OAuth2::Error.new(OAuth2::Response.new(fake_response)) }
 
-      before(:each) do
+      before do
         expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:get_token) { access_token }
         expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:authenticate_with_gitlab).and_raise(oauth_error)
       end
@@ -72,7 +72,7 @@ describe Oauth::GeoAuthController do
     context 'inexistent local user' do
       render_views
 
-      before(:each) do
+      before do
         expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:get_token) { 'token' }
         expect_any_instance_of(Gitlab::Geo::OauthSession).to receive(:authenticate_with_gitlab) { User.new(id: 999999) }
       end
@@ -92,7 +92,7 @@ describe Oauth::GeoAuthController do
     context 'access_token error' do
       render_views
 
-      before(:each) do
+      before do
         allow(controller).to receive(:current_user) { user }
       end
 
@@ -103,7 +103,7 @@ describe Oauth::GeoAuthController do
       end
 
       it 'handles access token problems' do
-        allow_any_instance_of(Oauth2::LogoutTokenValidationService).to receive(:validate) { { status: :error, message: :expired } }
+        allow_any_instance_of(Oauth2::LogoutTokenValidationService).to receive(:execute) { { status: :error, message: :expired } }
         get :logout, state: logout_state
 
         expect(response.body).to include("There is a problem with the OAuth access_token: #{:expired}")