From 7710c1232ebf5e0b96389ac0208f5bf42a3822eb Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 30 Nov 2015 11:50:02 +0100 Subject: [PATCH] Fixed race condition in asset generation of session "/show". --- .gitlab-ci.yml | 4 --- app/controllers/application_controller.rb | 42 +++++++++++------------ app/controllers/sessions_controller.rb | 40 +++++++++++---------- app/controllers/tickets_controller.rb | 4 +-- app/models/chat.rb | 2 +- lib/session_helper.rb | 3 +- lib/sessions/event/chat_session_close.rb | 2 +- lib/sessions/event/chat_session_start.rb | 2 +- 8 files changed, 48 insertions(+), 51 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0296fe792..68a0a8a8d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -235,10 +235,6 @@ job_integration_browser_ff_3: - browser-ff script: - export BROWSER_URL=http://$IP:3031 - - unset MAILBOX_AUTO1 - - unset MAILBOX_AUTO2 - - unset MAILBOX_MANUAL1 - - unset MAILBOX_MANUAL2 - script/build/test_db_config.sh production ci_zammad_browser_ff_3 - script/build/test_db_config.sh test ci_zammad_browser_ff_3_test - script/build/test_slice_tests.sh 3 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a1aa3874e..571dfebb2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -61,7 +61,7 @@ class ApplicationController < ActionController::Base def current_user return @_current_user if @_current_user return if !session[:user_id] - @_current_user = User.find( session[:user_id] ) + @_current_user = User.lookup(id: session[:user_id]) end def current_user_set(user) @@ -86,7 +86,7 @@ class ApplicationController < ActionController::Base # check if remote ip need to be updated if !session[:remote_id] || session[:remote_id] != request.remote_ip session[:remote_id] = request.remote_ip - session[:geo] = Service::GeoIp.location( request.remote_ip ) + session[:geo] = Service::GeoIp.location(request.remote_ip) end # fill user agent @@ -159,7 +159,7 @@ class ApplicationController < ActionController::Base # already logged in, early exit if session.id && session[:user_id] - userdata = User.find(session[:user_id]) + userdata = User.lookup(id: session[:user_id]) current_user_set(userdata) return { @@ -183,7 +183,7 @@ class ApplicationController < ActionController::Base authenticate_with_http_basic do |username, password| logger.debug "http basic auth check '#{username}'" - userdata = User.authenticate( username, password ) + userdata = User.authenticate(username, password) next if !userdata @@ -223,7 +223,7 @@ class ApplicationController < ActionController::Base } end - def authentication_check( auth_param = {} ) + def authentication_check(auth_param = {} ) result = authentication_check_only(auth_param) # check if basic_auth fallback is possible @@ -247,19 +247,19 @@ class ApplicationController < ActionController::Base true end - def role?( role_name ) + def role?(role_name) return false if !current_user - current_user.role?( role_name ) + current_user.role?(role_name) end def ticket_permission(ticket) - return true if ticket.permission( current_user: current_user ) + return true if ticket.permission(current_user: current_user) response_access_deny false end - def deny_if_not_role( role_name ) - return false if role?( role_name ) + def deny_if_not_role(role_name) + return false if role?(role_name) response_access_deny true end @@ -282,7 +282,7 @@ class ApplicationController < ActionController::Base # config config = {} - Setting.select('name').where( frontend: true ).each { |setting| + Setting.select('name').where(frontend: true ).each { |setting| config[setting.name] = Setting.get(setting.name) } @@ -301,13 +301,13 @@ class ApplicationController < ActionController::Base def model_create_render (object, params) # create object - generic_object = object.new( object.param_cleanup( params[object.to_app_model_url], true ) ) + generic_object = object.new(object.param_cleanup(params[object.to_app_model_url], true )) # save object generic_object.save! # set relations - generic_object.param_set_associations( params ) + generic_object.param_set_associations(params) model_create_render_item(generic_object) rescue => e @@ -323,15 +323,15 @@ class ApplicationController < ActionController::Base def model_update_render (object, params) # find object - generic_object = object.find( params[:id] ) + generic_object = object.find(params[:id]) # save object - generic_object.update_attributes!( object.param_cleanup( params[object.to_app_model_url] ) ) + generic_object.update_attributes!(object.param_cleanup(params[object.to_app_model_url])) # set relations - generic_object.param_set_associations( params ) + generic_object.param_set_associations(params) - model_update_render_item( generic_object ) + model_update_render_item(generic_object) rescue => e logger.error e.message logger.error e.backtrace.inspect @@ -343,7 +343,7 @@ class ApplicationController < ActionController::Base end def model_destory_render (object, params) - generic_object = object.find( params[:id] ) + generic_object = object.find(params[:id]) generic_object.destroy model_destory_render_item() rescue => e @@ -359,12 +359,12 @@ class ApplicationController < ActionController::Base def model_show_render (object, params) if params[:full] - generic_object_full = object.full( params[:id] ) + generic_object_full = object.full(params[:id]) render json: generic_object_full, status: :ok return end - generic_object = object.find( params[:id] ) + generic_object = object.find(params[:id]) model_show_render_item(generic_object) rescue => e logger.error e.message @@ -378,7 +378,7 @@ class ApplicationController < ActionController::Base def model_index_render (object, _params) generic_objects = object.all - model_index_render_result( generic_objects ) + model_index_render_result(generic_objects) rescue => e logger.error e.message logger.error e.backtrace.inspect diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f84a57c91..82708c8b9 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -9,7 +9,7 @@ class SessionsController < ApplicationController session[:switched_from_user_id] = nil # authenticate user - user = User.authenticate( params[:username], params[:password] ) + user = User.authenticate(params[:username], params[:password]) # auth failed if !user @@ -34,14 +34,15 @@ class SessionsController < ApplicationController return if !user_device_log(user, 'session') # log new session - user.activity_stream_log( 'session started', user.id, true ) - - # auto population of default collections - collections, assets = SessionHelper.default_collections(user) + 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) @@ -78,7 +79,7 @@ class SessionsController < ApplicationController config: config_frontend, models: models, collections: { - Locale.to_app_model => Locale.where( active: true ) + Locale.to_app_model => Locale.where(active: true) }, } return @@ -86,17 +87,18 @@ class SessionsController < ApplicationController # Save the user ID in the session so it can be used in # subsequent requests - user = User.find( user_id ) + user = User.find(user_id) # log device return if !user_device_log(user, 'session') - # auto population of default collections - collections, assets = SessionHelper.default_collections(user) - # 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) @@ -148,7 +150,7 @@ class SessionsController < ApplicationController current_user_set(authorization.user) # log new session - authorization.user.activity_stream_log( 'session started', authorization.user.id, true ) + authorization.user.activity_stream_log('session started', authorization.user.id, true) # remember last login date authorization.user.update_last_login @@ -171,7 +173,7 @@ class SessionsController < ApplicationController current_user_set(user) # log new session - user.activity_stream_log( 'session started', user.id, true ) + user.activity_stream_log('session started', user.id, true) # remember last login date user.update_last_login @@ -194,7 +196,7 @@ class SessionsController < ApplicationController return false end - user = User.lookup( id: params[:id] ) + user = User.find(params[:id]) if !user render( json: {}, @@ -207,7 +209,7 @@ class SessionsController < ApplicationController session[:switched_from_user_id] = current_user.id # log new session - user.activity_stream_log( 'switch to', current_user.id, true ) + user.activity_stream_log('switch to', current_user.id, true) # set session user current_user_set(user) @@ -224,7 +226,7 @@ class SessionsController < ApplicationController return false end - user = User.lookup( id: session[:switched_from_user_id] ) + user = User.lookup(id: session[:switched_from_user_id]) if !user render( json: {}, @@ -243,7 +245,7 @@ class SessionsController < ApplicationController current_user_set(user) # log end session - current_session_user.activity_stream_log( 'ended switch to', user.id, true ) + current_session_user.activity_stream_log('ended switch to', user.id, true) redirect_to '/#' end @@ -256,8 +258,8 @@ class SessionsController < ApplicationController next if !session.data['user_id'] sessions_clean.push session if session.data['user_id'] - user = User.lookup( id: session.data['user_id'] ) - assets = user.assets( assets ) + user = User.lookup(id: session.data['user_id']) + assets = user.assets(assets) end } render json: { @@ -268,7 +270,7 @@ class SessionsController < ApplicationController def delete return if deny_if_not_role(Z_ROLENAME_ADMIN) - SessionHelper.destroy( params[:id] ) + SessionHelper.destroy(params[:id]) render json: {} end diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 1cf61bb72..86b009d6b 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -370,13 +370,13 @@ class TicketsController < ApplicationController # lookup open user tickets limit = 100 assets = {} - access_condition = Ticket.access_condition( current_user ) + access_condition = Ticket.access_condition(current_user) now = Time.zone.now user_tickets_open_ids = [] user_tickets_closed_ids = [] user_ticket_volume_by_year = [] if params[:user_id] - user = User.find( params[:user_id] ) + user = User.lookup(id: params[:user_id]) condition = { 'ticket.state_id' => { operator: 'is', diff --git a/app/models/chat.rb b/app/models/chat.rb index dc2704c97..58e65eec9 100644 --- a/app/models/chat.rb +++ b/app/models/chat.rb @@ -15,7 +15,7 @@ class Chat < ApplicationModel if chat_session.state == 'running' user = nil if chat_session.user_id - chat_user = User.find(chat_session.user_id) + chat_user = User.lookup(id: chat_session.user_id) url = nil if chat_user.image && chat_user.image != 'none' url = "#{Setting.get('http_type')}://#{Setting.get('fqdn')}/api/v1/users/image/#{chat_user.image}" diff --git a/lib/session_helper.rb b/lib/session_helper.rb index 9322a231f..f355e153c 100644 --- a/lib/session_helper.rb +++ b/lib/session_helper.rb @@ -1,9 +1,8 @@ module SessionHelper - def self.default_collections(user) + def self.default_collections(user, assets = {}) # auto population collections, store all here default_collection = {} - assets = {} # load collections to deliver from external files dir = File.expand_path('../../', __FILE__) diff --git a/lib/sessions/event/chat_session_close.rb b/lib/sessions/event/chat_session_close.rb index c5e945432..a2b7fae01 100644 --- a/lib/sessions/event/chat_session_close.rb +++ b/lib/sessions/event/chat_session_close.rb @@ -6,7 +6,7 @@ class Sessions::Event::ChatSessionClose < Sessions::Event::ChatBase realname = 'Anonymous' if @session && @session['id'] - realname = User.find(@session['id']).fullname + realname = User.lookup(id: @session['id']).fullname end # check count of participents diff --git a/lib/sessions/event/chat_session_start.rb b/lib/sessions/event/chat_session_start.rb index 0c40bc0e0..ab95d6b93 100644 --- a/lib/sessions/event/chat_session_start.rb +++ b/lib/sessions/event/chat_session_start.rb @@ -20,7 +20,7 @@ class Sessions::Event::ChatSessionStart < Sessions::Event::ChatBase chat_session.save # send chat_session_init to client - chat_user = User.find(chat_session.user_id) + chat_user = User.lookup(id: chat_session.user_id) url = nil if chat_user.image && chat_user.image != 'none' url = "#{Setting.get('http_type')}://#{Setting.get('fqdn')}/api/v1/users/image/#{chat_user.image}"