diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 947a8e865..c5b173773 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,7 +5,7 @@ class ApplicationController < ActionController::Base include ApplicationController::HandlesTransitions include ApplicationController::Authenticates include ApplicationController::SetsHeaders - include ApplicationController::ChecksMaintainance + include ApplicationController::ChecksMaintenance include ApplicationController::RendersModels include ApplicationController::HasUser include ApplicationController::HasResponseExtentions diff --git a/app/controllers/application_controller/authenticates.rb b/app/controllers/application_controller/authenticates.rb index 6df649d19..ef3ab3eff 100644 --- a/app/controllers/application_controller/authenticates.rb +++ b/app/controllers/application_controller/authenticates.rb @@ -50,15 +50,6 @@ module ApplicationController::Authenticates return authentication_check_prerequesits(user, 'session', auth_param) if user end - # check sso based authentication - sso_user = User.sso(params) - if sso_user - if authentication_check_prerequesits(sso_user, 'session', auth_param) - session[:persistent] = true - return sso_user - end - end - # check http basic based authentication authenticate_with_http_basic do |username, password| request.session_options[:skip] = true # do not send a session cookie @@ -135,21 +126,37 @@ module ApplicationController::Authenticates false end + def authenticate_with_password + user = User.authenticate(params[:username], params[:password]) + raise Exceptions::NotAuthorized, 'Wrong Username or Password combination.' if !user + + session.delete(:switched_from_user_id) + authentication_check_prerequesits(user, 'session', {}) + end + + def authenticate_with_sso + user = begin + login = request.env['REMOTE_USER'] || + request.env['HTTP_REMOTE_USER'] || + request.headers['X-Forwarded-User'] + + User.lookup(login: login&.downcase) + end + + raise Exceptions::NotAuthorized, 'no valid session' if !user + + session.delete(:switched_from_user_id) + authentication_check_prerequesits(user, 'session', {}) + end + def authentication_check_prerequesits(user, auth_type, auth_param) - if check_maintenance_only(user) - raise Exceptions::NotAuthorized, 'Maintenance mode enabled!' - end - + raise Exceptions::NotAuthorized, 'Maintenance mode enabled!' if in_maintenance_mode?(user) raise Exceptions::NotAuthorized, 'User is inactive!' if !user.active - - # check scopes / permission check - if auth_param[:permission] && !user.permissions?(auth_param[:permission]) - raise Exceptions::NotAuthorized, 'Not authorized (user)!' - end + raise Exceptions::NotAuthorized, 'Not authorized (user)!' if auth_param[:permission] && !user.permissions?(auth_param[:permission]) current_user_set(user, auth_type) user_device_log(user, auth_type) logger.debug { "#{auth_type} for '#{user.login}'" } - true + user end end diff --git a/app/controllers/application_controller/checks_maintainance.rb b/app/controllers/application_controller/checks_maintenance.rb similarity index 52% rename from app/controllers/application_controller/checks_maintainance.rb rename to app/controllers/application_controller/checks_maintenance.rb index 7d1ef0ae5..3cdaeaab4 100644 --- a/app/controllers/application_controller/checks_maintainance.rb +++ b/app/controllers/application_controller/checks_maintenance.rb @@ -1,16 +1,9 @@ -module ApplicationController::ChecksMaintainance +module ApplicationController::ChecksMaintenance extend ActiveSupport::Concern private - def check_maintenance(user) - return false if !check_maintenance_only(user) - - raise Exceptions::NotAuthorized, 'Maintenance mode enabled!' - end - - # check maintenance mode - def check_maintenance_only(user) + def in_maintenance_mode?(user) return false if Setting.get('maintenance_mode') != true return false if user.permissions?('admin.maintenance') diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 8376799bc..d242a4646 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -2,112 +2,32 @@ class SessionsController < ApplicationController prepend_before_action :authentication_check, only: %i[switch_to_user list delete] - skip_before_action :verify_csrf_token, only: %i[show destroy create_omniauth failure_omniauth create_sso] + skip_before_action :verify_csrf_token, only: %i[show destroy create_omniauth failure_omniauth] # "Create" a login, aka "log the user in" def create - - # in case, remove switched_from_user_id - session[:switched_from_user_id] = nil - - # authenticate user - user = User.authenticate(params[:username], params[:password]) - - # check maintenance mode - check_maintenance(user) - - # auth failed - raise Exceptions::NotAuthorized, 'Wrong Username or Password combination.' if !user - - # remember me - set session cookie to expire later - expire_after = nil - if params[:remember_me] - expire_after = 1.year - end - request.env['rack.session.options'][:expire_after] = expire_after - - # set session user - current_user_set(user) - - # log device - return if !user_device_log(user, 'session') - - # log new session - user.activity_stream_log('session started', user.id, true) - - # add session user assets - assets = {} - assets = user.assets(assets) - - # auto population of default collections - collections, assets = SessionHelper.default_collections(user, assets) - - # get models - models = SessionHelper.models(user) - - # sessions created via this - # controller are persistent - session[:persistent] = true + user = authenticate_with_password + initiate_session_for(user) # return new session data - render status: :created, - json: { - session: user, - config: config_frontend, - models: models, - collections: collections, - assets: assets, - } + render status: :created, + json: SessionHelper.json_hash(user).merge(config: config_frontend) end def show - - user_id = nil - - # no valid sessions - if session[:user_id] - user_id = session[:user_id] - end - - if !user_id || !User.exists?(user_id) - # get models - models = SessionHelper.models() - - render json: { - error: 'no valid session', - config: config_frontend, - models: models, - collections: { - Locale.to_app_model => Locale.where(active: true) - }, - } - return - end - - # Save the user ID in the session so it can be used in - # subsequent requests - user = User.find(user_id) - - # log device - return if !user_device_log(user, 'session') - - # add session user assets - assets = {} - assets = user.assets(assets) - - # auto population of default collections - collections, assets = SessionHelper.default_collections(user, assets) - - # get models - models = SessionHelper.models(user) + user = authentication_check_only || authenticate_with_sso + initiate_session_for(user) # return current session + render json: SessionHelper.json_hash(user).merge(config: config_frontend) + rescue Exceptions::NotAuthorized => e + raise if e.message != 'no valid session' + render json: { - session: user, + error: e.message, config: config_frontend, - models: models, - collections: collections, - assets: assets, + models: SessionHelper.models, + collections: { Locale.to_app_model => Locale.where(active: true) } } end @@ -146,8 +66,7 @@ class SessionsController < ApplicationController authorization = Authorization.create_from_hash(auth, current_user) end - # check maintenance mode - if check_maintenance_only(authorization.user) + if in_maintenance_mode?(authorization.user) redirect_to '/#' return end @@ -169,36 +88,6 @@ class SessionsController < ApplicationController raise Exceptions::UnprocessableEntity, "Message from #{params[:strategy]}: #{params[:message]}" end - def create_sso - - # in case, remove switched_from_user_id - session[:switched_from_user_id] = nil - - user = User.sso(params) - - # Log the authorizing user in. - if user - - # check maintenance mode - if check_maintenance_only(user) - redirect_to '/#' - return - end - - # set current session user - current_user_set(user) - - # log new session - user.activity_stream_log('session started', user.id, true) - - # remember last login date - user.update_last_login - end - - # redirect to app - redirect_to '/#' - end - # "switch" to user def switch_to_user permission_check(['admin.session', 'admin.user']) @@ -308,6 +197,12 @@ class SessionsController < ApplicationController private + def initiate_session_for(user) + request.env['rack.session.options'][:expire_after] = 1.year if params[:remember_me] + session[:persistent] = true + user.activity_stream_log('session started', user.id, true) + end + def config_frontend # config @@ -333,4 +228,5 @@ class SessionsController < ApplicationController config end + end diff --git a/app/models/user.rb b/app/models/user.rb index 36481bcd1..d8a550efa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -333,27 +333,6 @@ returns =begin -authenticate user again sso - - result = User.sso(sso_params) - -returns - - result = user_model # user model if authentication was successfully - -=end - - def self.sso(params) - - # try to login against configure auth backends - user_auth = Sso.check(params) - return if !user_auth - - user_auth - end - -=begin - create user from from omni auth hash result = User.create_from_hash!(hash) diff --git a/config/routes/auth.rb b/config/routes/auth.rb index 92aa9fdd5..701bc6c75 100644 --- a/config/routes/auth.rb +++ b/config/routes/auth.rb @@ -5,9 +5,6 @@ Zammad::Application.routes.draw do match '/auth/:provider/callback', to: 'sessions#create_omniauth', via: %i[post get puts delete] match '/auth/failure', to: 'sessions#failure_omniauth', via: %i[post get] - # sso - match '/auth/sso', to: 'sessions#create_sso', via: %i[post get] - # sessions match api_path + '/signin', to: 'sessions#create', via: :post match api_path + '/signshow', to: 'sessions#show', via: %i[get post] diff --git a/lib/session_helper.rb b/lib/session_helper.rb index d459e3426..ce1e41774 100644 --- a/lib/session_helper.rb +++ b/lib/session_helper.rb @@ -1,8 +1,20 @@ module SessionHelper - def self.default_collections(user, assets = {}) + def self.json_hash(user) + collections, assets = default_collections(user) + + { + session: user, + models: models(user), + collections: collections, + assets: assets, + } + end + + def self.default_collections(user) # auto population collections, store all here default_collection = {} + assets = user.assets({}) # load collections to deliver from external files dir = File.expand_path('..', __dir__) diff --git a/lib/sso.rb b/lib/sso.rb deleted file mode 100644 index 6acae6579..000000000 --- a/lib/sso.rb +++ /dev/null @@ -1,53 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ - -class Sso - include ApplicationLib - -=begin - -authenticate user via username and password - - result = Sso.check( params ) - -returns - - result = user_model # if authentication was successfully - -=end - - def self.check(params) - - # use std. auth backends - config = [ - { - adapter: 'Sso::Env', - }, - ] - - # added configured backends - Setting.where( area: 'Security::SSO' ).each do |setting| - if setting.state_current[:value] - config.push setting.state_current[:value] - end - end - - # try to login against configure auth backends - user_auth = nil - config.each do |config_item| - next if !config_item[:adapter] - - user_auth = config_item[:adapter].constantize.check( params, config_item ) - - # auth not ok - next if !user_auth - - Rails.logger.info "Authentication against #{config_item[:adapter]} for user #{user_auth.login} ok." - - # remember last login date - user_auth.update_last_login - - return user_auth - end - nil - end -end diff --git a/lib/sso/env.rb b/lib/sso/env.rb deleted file mode 100644 index 974891041..000000000 --- a/lib/sso/env.rb +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ - -module Sso::Env - def self.check( _params, _config_item ) - - # try to find user based on login - if ENV['REMOTE_USER'] - user = User.where( login: ENV['REMOTE_USER'], active: true ).first - return user if user - end - - if ENV['HTTP_REMOTE_USER'] - user = User.where( login: ENV['HTTP_REMOTE_USER'], active: true ).first - return user if user - end - - false - end -end diff --git a/spec/requests/session_spec.rb b/spec/requests/session_spec.rb new file mode 100644 index 000000000..31edec401 --- /dev/null +++ b/spec/requests/session_spec.rb @@ -0,0 +1,178 @@ +require 'rails_helper' + +RSpec.describe 'Sessions endpoints', type: :request do + # The frontend sends a device fingerprint in the request parameters during authentication + # (as part of App.Auth.loginCheck() and App.WebSocket.auth()). + # + # Without this parameter, the controller will raise a 422 Unprocessable Entity error + # (in ApplicationController::HandlesDevices#user_device_log). + let(:fingerprint) { { fingerprint: 'foo' } } + + describe 'GET /api/v1/signshow (single sign-on)' do + context 'with invalid user login' do + let(:login) { User.pluck(:login).max.next } + + context 'in "REMOTE_USER" request env var' do + let(:env) { { 'REMOTE_USER' => login } } + + it 'returns invalid session response' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(response).to have_http_status(:ok) + expect(json_response) + .to include('error' => 'no valid session') + .and not_include('session') + end + end + + context 'in "HTTP_REMOTE_USER" request env var' do + let(:env) { { 'HTTP_REMOTE_USER' => login } } + + it 'returns invalid session response' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(response).to have_http_status(:ok) + expect(json_response) + .to include('error' => 'no valid session') + .and not_include('session') + end + end + + context 'in "X-Forwarded-User" request header' do + let(:headers) { { 'X-Forwarded-User' => login } } + + it 'returns invalid session response' do + get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint + + expect(response).to have_http_status(:ok) + expect(json_response) + .to include('error' => 'no valid session') + .and not_include('session') + end + end + end + + context 'with valid user login' do + let(:user) { User.last } + let(:login) { user.login } + + context 'in Maintenance Mode' do + before { Setting.set('maintenance_mode', true) } + + context 'in "REMOTE_USER" request env var' do + let(:env) { { 'REMOTE_USER' => login } } + + it 'returns 401 unauthorized' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(response).to have_http_status(:unauthorized) + expect(json_response).to include('error' => 'Maintenance mode enabled!') + end + end + + context 'in "HTTP_REMOTE_USER" request env var' do + let(:env) { { 'HTTP_REMOTE_USER' => login } } + + it 'returns 401 unauthorized' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(response).to have_http_status(:unauthorized) + expect(json_response).to include('error' => 'Maintenance mode enabled!') + end + end + + context 'in "X-Forwarded-User" request header' do + let(:headers) { { 'X-Forwarded-User' => login } } + + it 'returns 401 unauthorized' do + get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint + + expect(response).to have_http_status(:unauthorized) + expect(json_response).to include('error' => 'Maintenance mode enabled!') + end + end + end + + context 'in "REMOTE_USER" request env var' do + let(:env) { { 'REMOTE_USER' => login } } + + it 'returns a new user-session response' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(json_response) + .to include('session' => hash_including('login' => login)) + .and not_include('error') + end + + it 'sets the :user_id session parameter' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change { request&.session&.fetch(:user_id) }.to(user.id) + end + + it 'sets the :persistent session parameter' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change { request&.session&.fetch(:persistent) }.to(true) + end + + it 'adds an activity stream entry for the user’s session' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change(ActivityStream, :count).by(1) + end + end + + context 'in "HTTP_REMOTE_USER" request env var' do + let(:env) { { 'HTTP_REMOTE_USER' => login } } + + it 'returns a new user-session response' do + get '/api/v1/signshow', as: :json, env: env, params: fingerprint + + expect(json_response) + .to include('session' => hash_including('login' => login)) + .and not_include('error') + end + + it 'sets the :user_id session parameter' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change { request&.session&.fetch(:user_id) }.to(user.id) + end + + it 'sets the :persistent session parameter' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change { request&.session&.fetch(:persistent) }.to(true) + end + + it 'adds an activity stream entry for the user’s session' do + expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint } + .to change(ActivityStream, :count).by(1) + end + end + + context 'in "X-Forwarded-User" request header' do + let(:headers) { { 'X-Forwarded-User' => login } } + + it 'returns a new user-session response' do + get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint + + expect(json_response) + .to include('session' => hash_including('login' => login)) + .and not_include('error') + end + + it 'sets the :user_id session parameter on the client' do + expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint } + .to change { request&.session&.fetch(:user_id) }.to(user.id) + end + + it 'sets the :persistent session parameter' do + expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint } + .to change { request&.session&.fetch(:persistent) }.to(true) + end + + it 'adds an activity stream entry for the user’s session' do + expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint } + .to change(ActivityStream, :count).by(1) + end + end + end + end +end