Commit fcf258ff authored by Gabriel Mazetto's avatar Gabriel Mazetto

Changed Single SignOut to use encrypted access_token as session id

parent fe1288ff
...@@ -118,7 +118,7 @@ class ApplicationController < ActionController::Base ...@@ -118,7 +118,7 @@ class ApplicationController < ActionController::Base
def after_sign_out_path_for(resource) def after_sign_out_path_for(resource)
if Gitlab::Geo.secondary? if Gitlab::Geo.secondary?
Gitlab::Geo.primary_node.oauth_logout_url(session[:access_token]) Gitlab::Geo.primary_node.oauth_logout_url(@geo_logout_state)
else else
current_application_settings.after_sign_out_path.presence || new_user_session_path current_application_settings.after_sign_out_path.presence || new_user_session_path
end end
......
...@@ -35,10 +35,18 @@ class Oauth::GeoAuthController < ActionController::Base ...@@ -35,10 +35,18 @@ class Oauth::GeoAuthController < ActionController::Base
def logout def logout
oauth = Gitlab::Geo::OauthSession.new(state: params[:state]) oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
access_token = oauth.extract_logout_token
access_token_status = Oauth2::AccessTokenValidationService.validate(access_token)
if oauth.is_logout_state_valid?(params[:token]) if access_token_status == Oauth2::AccessTokenValidationService::VALID
user = User.find(access_token.resource_owner_id)
if current_user == user
sign_out current_user sign_out current_user
end end
else
end
redirect_to root_path redirect_to root_path
end end
...@@ -49,4 +57,9 @@ class Oauth::GeoAuthController < ActionController::Base ...@@ -49,4 +57,9 @@ class Oauth::GeoAuthController < ActionController::Base
@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".' @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' render :error, layout: 'errors'
end end
def access_token_error(status)
@error = "There is a problem with the OAuth access_token: #{status}"
render :error, layout: 'errors'
end
end end
...@@ -9,6 +9,7 @@ class SessionsController < Devise::SessionsController ...@@ -9,6 +9,7 @@ class SessionsController < Devise::SessionsController
if: :two_factor_enabled?, only: [:create] if: :two_factor_enabled?, only: [:create]
prepend_before_action :store_redirect_path, only: [:new] prepend_before_action :store_redirect_path, only: [:new]
before_action :gitlab_geo_login, 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 :auto_sign_in_with_provider, only: [:new]
before_action :load_recaptcha before_action :load_recaptcha
...@@ -122,6 +123,11 @@ class SessionsController < Devise::SessionsController ...@@ -122,6 +123,11 @@ class SessionsController < Devise::SessionsController
end end
end end
def gitlab_geo_logout
oauth = Gitlab::Geo::OauthSession.new(access_token: session[:access_token])
@geo_logout_state = oauth.generate_logout_state
end
def auto_sign_in_with_provider def auto_sign_in_with_provider
provider = Gitlab.config.omniauth.auto_sign_in_with_provider provider = Gitlab.config.omniauth.auto_sign_in_with_provider
return unless provider.present? return unless provider.present?
......
...@@ -71,7 +71,7 @@ class GeoNode < ActiveRecord::Base ...@@ -71,7 +71,7 @@ class GeoNode < ActiveRecord::Base
def oauth_logout_url(access_token) def oauth_logout_url(access_token)
logout_uri = URI.join(uri, "#{uri.path}/", 'oauth/geo/logout') logout_uri = URI.join(uri, "#{uri.path}/", 'oauth/geo/logout')
logout_uri.query="token=#{access_token}" logout_uri.query="state=#{access_token}"
logout_uri.to_s logout_uri.to_s
end end
......
...@@ -3,6 +3,7 @@ module Gitlab ...@@ -3,6 +3,7 @@ module Gitlab
class OauthSession class OauthSession
include ActiveModel::Model include ActiveModel::Model
attr_accessor :access_token
attr_accessor :state attr_accessor :state
attr_accessor :return_to attr_accessor :return_to
...@@ -14,18 +15,25 @@ module Gitlab ...@@ -14,18 +15,25 @@ module Gitlab
hmac == generate_oauth_hmac(salt, return_to) hmac == generate_oauth_hmac(salt, return_to)
end end
def is_logout_state_valid?(access_token)
return false unless state
salt, hmac = state.split(':', 2)
hmac == generate_oauth_hmac(salt, access_token)
end
def generate_oauth_state def generate_oauth_state
return unless return_to return unless return_to
hmac = generate_oauth_hmac(oauth_salt, return_to) hmac = generate_oauth_hmac(oauth_salt, return_to)
"#{oauth_salt}:#{hmac}:#{return_to}" "#{oauth_salt}:#{hmac}:#{return_to}"
end end
def generate_logout_state
cipher = logout_token_cipher(oauth_salt, :encrypt)
encrypted = cipher.update(access_token) + cipher.final
"#{oauth_salt}:#{encrypted}"
end
def extract_logout_token
return unless state
salt, encrypted = state.split(':', 2)
decipher = logout_token_cipher(salt, :decrypt)
decipher.update(encrypted) + decipher.final
end
def get_oauth_state_return_to def get_oauth_state_return_to
state.split(':', 3)[2] if state state.split(':', 3)[2] if state
end end
...@@ -54,6 +62,14 @@ module Gitlab ...@@ -54,6 +62,14 @@ module Gitlab
OpenSSL::HMAC.hexdigest(digest, key, return_to) OpenSSL::HMAC.hexdigest(digest, key, return_to)
end end
def logout_token_cipher(salt, operation)
cipher = OpenSSL::Cipher::AES.new(128, :CBC)
cipher.send(operation)
cipher.iv=salt
cipher.key="#{oauth_app.uid}#{oauth_app.secret}"
cipher
end
def oauth_salt def oauth_salt
@salt ||= SecureRandom.hex(16) @salt ||= SecureRandom.hex(16)
end end
......
...@@ -61,6 +61,36 @@ describe Gitlab::Geo::OauthSession do ...@@ -61,6 +61,36 @@ describe Gitlab::Geo::OauthSession do
end end
end end
describe '#generate_logout_state' do
subject { described_class.new(access_token: access_token) }
it 'returns a string with salt and encrypted access token colon separated' do
state = subject.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 'encrypted access token is recoverable' do
state = subject.generate_logout_state
subject.state = state
access_token = subject.extract_logout_token
expect(access_token).to eq access_token
end
end
describe '#authorized_url' do describe '#authorized_url' do
subject { described_class.new(return_to: oauth_return_to) } subject { described_class.new(return_to: oauth_return_to) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment