Merge branch 'feature/geo-single-signout' into 'master'

Geo: Single Sign Out

Implements Single Sign Out for Geo (#76).

Initial proposal was to generate a hash based on the `access_token`, but that created a O(N) cost against a desirable O(1), as a new `access_token` is generated for each new login. To overcome that cost we would need to send a "public identifier" to help retrieve the correct `access_token` and provide that during login process.

This is also how most Single Sign On implementations works (they provide some sort of session_id, that we notify every node to invalidate, during sign out process).

As I don't want to modify our OAuth table (that is managed by doorkeeper) nor change the way our login process work, the solution is to encrypt the `access_token` using a symmetric key known by both nodes, and expire the `access_token` after the logout to prevent replay attacks (otherwise we would need to send a `nounce` and store that on primary). 

The key is based on `Gitlab::Application.secrets.db_key_base` which we already use to encrypt database attributes and is synced between both nodes. We communicate sending a `state` parameter which is known terminology in OAuth protocol.

Although this is implemented with Geo only in mind, we can backport to CE (with minimal changes) and provide as a "non-standard" way of single sign off for applications that integrate with GitLab.

Fixes #522 

See merge request !380

CHANGELOG-EE

@@ -14,6 +14,7 @@ v 8.8.0 (unreleased)
   - Set KRB5 as default clone protocol when Kerberos is enabled and user is logged in (Borja Aparicio)
   - Reduce emails-on-push HTML size by using a simple monospace font
   - API requests to /internal/authorized_keys are now tagged properly
+  - Geo: Single Sign Out support !380
 
 v 8.7.5
   - No EE-specific changes

app/controllers/application_controller.rb

@@ -118,7 +118,7 @@ class ApplicationController < ActionController::Base
 
   def after_sign_out_path_for(resource)
     if Gitlab::Geo.secondary?
-      Gitlab::Geo.primary_node.url
+      Gitlab::Geo.primary_node.oauth_logout_url(@geo_logout_state)
     else
       current_application_settings.after_sign_out_path.presence || new_user_session_path
     end

app/controllers/oauth/geo_auth_controller.rb

@@ -15,16 +15,17 @@ class Oauth::GeoAuthController < ActionController::Base
   def callback
     oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
     unless oauth.is_oauth_state_valid?
-      redirect_to new_user_sessions_path
+      redirect_to new_user_session_path
       return
     end
 
     token = oauth.get_token(params[:code], redirect_uri: oauth_geo_callback_url)
     remote_user = oauth.authenticate_with_gitlab(token)
 
-    user = User.find(remote_user['id'])
+    user = User.find_by(id: remote_user['id'])
 
     if user && sign_in(user, bypass: true)
+      session[:access_token] = token
       return_to = oauth.get_oauth_state_return_to
       redirect_to(return_to || root_path)
     else
@@ -32,10 +33,31 @@ class Oauth::GeoAuthController < ActionController::Base
     end
   end
 
+  def logout
+    logout = Oauth2::LogoutTokenValidationService.new(current_user, params)
+    result = logout.execute
+    if result[:status] == :success
+      sign_out current_user
+      redirect_to root_path
+    else
+      access_token_error(result[:message])
+    end
+  end
+
   private
 
+  def invalid_credentials
+    @error = 'Cannot find user to login. Your account may have been deleted.'
+    render :error, layout: 'errors'
+  end
+
   def undefined_oauth_application
     @error = 'There are no OAuth application defined for this Geo node. Please ask your administrator to visit "Geo Nodes" on admin screen and click on "Repair authentication".'
     render :error, layout: 'errors'
   end
+
+  def access_token_error(status)
+    @error = "There is a problem with the OAuth access_token: #{status}"
+    render :error, layout: 'errors'
+  end
 end

app/controllers/sessions_controller.rb

@@ -9,6 +9,7 @@ class SessionsController < Devise::SessionsController
     if: :two_factor_enabled?, only: [:create]
   prepend_before_action :store_redirect_path, only: [:new]
   before_action :gitlab_geo_login, only: [:new]
+  before_action :gitlab_geo_logout, only: [:destroy]
   before_action :auto_sign_in_with_provider, only: [:new]
   before_action :load_recaptcha
 
@@ -111,15 +112,23 @@ class SessionsController < Devise::SessionsController
   end
 
   def gitlab_geo_login
-    if !signed_in? && Gitlab::Geo.enabled? && Gitlab::Geo.secondary?
-      oauth = Gitlab::Geo::OauthSession.new
+    return unless Gitlab::Geo.secondary?
+    return if signed_in?
 
-      # share full url with primary node by shared session
-      user_return_to = URI.join(root_url, session[:user_return_to].to_s).to_s
-      oauth.return_to = @redirect_to || user_return_to
+    oauth = Gitlab::Geo::OauthSession.new
 
-      redirect_to oauth_geo_auth_url(state: oauth.generate_oauth_state)
-    end
+    # share full url with primary node by oauth state
+    user_return_to = URI.join(root_url, session[:user_return_to].to_s).to_s
+    oauth.return_to = @redirect_to || user_return_to
+
+    redirect_to oauth_geo_auth_url(state: oauth.generate_oauth_state)
+  end
+
+  def gitlab_geo_logout
+    return unless Gitlab::Geo.secondary?
+
+    oauth = Gitlab::Geo::OauthSession.new(access_token: session[:access_token])
+    @geo_logout_state = oauth.generate_logout_state
   end
 
   def auto_sign_in_with_provider

app/models/geo_node.rb

@@ -69,6 +69,12 @@ class GeoNode < ActiveRecord::Base
     URI.join(uri, "#{uri.path}/", 'oauth/geo/callback').to_s
   end
 
+  def oauth_logout_url(state)
+    logout_uri = URI.join(uri, "#{uri.path}/", 'oauth/geo/logout')
+    logout_uri.query = "state=#{state}"
+    logout_uri.to_s
+  end
+
   def missing_oauth_application?
     self.primary? ? false : !oauth_application.present?
   end

app/services/oauth2/logout_token_validation_service.rb

+module Oauth2
+  class LogoutTokenValidationService < ::BaseService
+    attr_reader :status
+
+    def initialize(user, params={})
+      @params = params
+      @current_user = user
+    end
+
+    def execute
+      return error('access token not found') 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
+          success
+        end
+      else
+        error(status)
+      end
+    end
+
+    def access_token
+      @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
+
+    end
+  end
+end

config/routes.rb

@@ -53,9 +53,10 @@ Rails.application.routes.draw do
                 authorizations: 'oauth/authorizations'
   end
 
-  namespace :oauth do
-    get 'geo/auth' => 'geo_auth#auth'
-    get 'geo/callback' => 'geo_auth#callback'
+  namespace :oauth, path: 'geo', controller: 'geo_auth', as: 'oauth_geo' do
+    get 'auth'
+    get 'callback'
+    get 'logout'
   end
 
   # Autocomplete

lib/gitlab/geo/oauth_session.rb

@@ -3,6 +3,7 @@ module Gitlab
     class OauthSession
       include ActiveModel::Model
 
+      attr_accessor :access_token
       attr_accessor :state
       attr_accessor :return_to
 
@@ -16,8 +17,29 @@ module Gitlab
 
       def generate_oauth_state
         return unless return_to
+
         hmac = generate_oauth_hmac(oauth_salt, return_to)
-        "#{oauth_salt}:#{hmac}:#{return_to}"
+        self.state = "#{oauth_salt}:#{hmac}:#{return_to}"
+      end
+
+      def generate_logout_state
+        return unless access_token
+
+        cipher = logout_token_cipher(oauth_salt, :encrypt)
+        encrypted = cipher.update(access_token) + cipher.final
+        self.state = "#{oauth_salt}:#{Base64.urlsafe_encode64(encrypted)}"
+      rescue OpenSSL::OpenSSLError
+        return false
+      end
+
+      def extract_logout_token
+        return unless state
+
+        salt, encrypted = state.split(':', 2)
+        decipher = logout_token_cipher(salt, :decrypt)
+        decipher.update(Base64.urlsafe_decode64(encrypted)) + decipher.final
+      rescue OpenSSL::OpenSSLError
+        return false
       end
 
       def get_oauth_state_return_to
@@ -43,11 +65,21 @@ module Gitlab
 
       def generate_oauth_hmac(salt, return_to)
         return false unless return_to
+
         digest = OpenSSL::Digest.new('sha256')
         key = Gitlab::Application.secrets.secret_key_base + salt
         OpenSSL::HMAC.hexdigest(digest, key, return_to)
       end
 
+      def logout_token_cipher(salt, operation)
+        cipher = OpenSSL::Cipher::AES.new(128, :CBC)
+        cipher.send(operation)
+        cipher.iv = salt
+        cipher.key = Gitlab::Application.secrets.db_key_base
+        cipher.auth_data = ''
+        cipher
+      end
+
       def oauth_salt
         @salt ||= SecureRandom.hex(16)
       end

spec/controllers/oauth/geo_auth_controller_spec.rb

+require 'spec_helper'
+
+describe Oauth::GeoAuthController do
+  let(:user) { create(:user) }
+  let(:oauth_app) { create(:doorkeeper_application) }
+  let(:access_token) { create(:doorkeeper_access_token, resource_owner_id: user.id).token }
+  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 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
+
+  describe 'GET auth' do
+    let(:primary_node_oauth_endpoint) { Gitlab::Geo::OauthSession.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: auth_state) }
+
+    it 'redirects to root_url when state is invalid' do
+      allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:is_oauth_state_valid?) { false }
+      get :auth, state: auth_state
+
+      expect(response).to redirect_to(root_url)
+    end
+
+    it "redirects to primary node's oauth endpoint" do
+      get :auth, state: auth_state
+
+      expect(response).to redirect_to(primary_node_oauth_endpoint)
+    end
+  end
+
+  describe 'GET callback' do
+    let(:callback_state) { Gitlab::Geo::OauthSession.new(access_token: access_token, return_to: projects_url).generate_oauth_state }
+    let(:primary_node_oauth_endpoint) { Gitlab::Geo::OauthSession.new.authorize_url(redirect_uri: oauth_geo_callback_url, state: callback_state) }
+
+    context 'redirection' 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
+
+      it 'redirects to login screen if state is invalid' do
+        allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:is_oauth_state_valid?) { false }
+        get :callback, state: callback_state
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+
+      it 'redirects to redirect_url if state is valid' do
+        get :callback, state: callback_state
+
+        expect(response).to redirect_to(projects_url)
+      end
+    end
+
+    context 'invalid credentials' do
+      let(:fake_response) { double('Faraday::Response', headers: {}, body: '', status: 403) }
+      let(:oauth_error) { OAuth2::Error.new(OAuth2::Response.new(fake_response)) }
+
+      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
+
+      it 'handles invalid credentials error' do
+        get :callback, state: callback_state
+
+        expect(response).to redirect_to(primary_node_oauth_endpoint)
+      end
+    end
+
+    context 'inexistent local user' do
+      render_views
+
+      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
+
+      it 'handles inexistent local user error' do
+        get :callback, state: callback_state
+
+        expect(response.code).to eq '200'
+        expect(response.body).to include('Your account may have been deleted')
+      end
+    end
+  end
+
+  describe 'GET logout' do
+    let(:logout_state) { Gitlab::Geo::OauthSession.new(access_token: access_token).generate_logout_state }
+
+    context 'access_token error' do
+      render_views
+
+      before do
+        allow(controller).to receive(:current_user) { user }
+      end
+
+      it 'logs out when correct access_token is informed' do
+        get :logout, state: logout_state
+
+        expect(response).to redirect_to root_url
+      end
+
+      it 'handles access token problems' do
+        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}")
+      end
+    end
+  end
+end

spec/lib/gitlab/geo/oauth_session_spec.rb

@@ -37,8 +37,7 @@ describe Gitlab::Geo::OauthSession do
 
   describe '#generate_oauth_state' do
     it 'returns nil when return_to is not present' do
-      state = subject.generate_oauth_state
-      expect(state).to be_nil
+      expect(subject.generate_oauth_state).to be_nil
     end
 
     context 'when return_to is present' do
@@ -61,6 +60,49 @@ describe Gitlab::Geo::OauthSession do
     end
   end
 
+  describe '#generate_logout_state' do
+    it 'returns nil when access_token is not defined' do
+      expect(described_class.new.generate_logout_state).to be_nil
+    end
+
+    it 'returns false when encryptation fails' do
+      allow_any_instance_of(OpenSSL::Cipher::AES).to receive(:final) { raise OpenSSL::OpenSSLError }
+      expect(subject.generate_logout_state).to be_falsey
+    end
+
+    it 'returns a string with salt and encrypted access token colon separated' do
+      state = described_class.new(access_token: access_token).generate_logout_state
+      expect(state).to be_a String
+      expect(state).not_to be_blank
+
+      salt, encrypted = state.split(':', 2)
+      expect(salt).not_to be_blank
+      expect(encrypted).not_to be_blank
+    end
+  end
+
+  describe '#extract_logout_token' do
+    subject { described_class.new(access_token: access_token) }
+
+    it 'returns nil when state is not defined' do
+      expect(subject.extract_logout_token).to be_nil
+    end
+
+    it 'returns false when decryptation fails' do
+      subject.generate_logout_state
+      allow_any_instance_of(OpenSSL::Cipher::AES).to receive(:final) { raise OpenSSL::OpenSSLError }
+
+      expect(subject.extract_logout_token).to be_falsey
+    end
+
+    it 'encrypted access token is recoverable' do
+      subject.generate_logout_state
+
+      access_token = subject.extract_logout_token
+      expect(access_token).to eq access_token
+    end
+  end
+
   describe '#authorized_url' do
     subject { described_class.new(return_to: oauth_return_to) }
 

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 }
+  let(:logout_state) { Gitlab::Geo::OauthSession.new(access_token: access_token).generate_logout_state }
+
+  context '#execute' do
+    it 'return error when params are empty' do
+      result = described_class.new(user, {}).execute
+      expect(result[:status]).to eq(:error)
+    end
+
+    it 'returns error when state param is empty' do
+      result = described_class.new(user, { state: nil }).execute
+      expect(result[:status]).to eq(:error)
+
+      result = described_class.new(user, { state: '' }).execute
+      expect(result[:status]).to eq(:error)
+    end
+
+    it 'returns error when incorrect encoding' do
+      invalid_token = "\xD800\xD801\xD802"
+      allow_any_instance_of(Gitlab::Geo::OauthSession).to receive(:extract_logout_token) { invalid_token }
+
+      result = described_class.new(user, { state: logout_state }).execute
+      expect(result[:status]).to eq(:error)
+    end
+
+    it 'returns true when token is valid' do
+      result = described_class.new(user, { state: logout_state }).execute
+      expect(result[:status]).to eq(:success)
+    end
+  end
+end