diff --git a/Gemfile b/Gemfile index 4325682f4..14e914d52 100644 --- a/Gemfile +++ b/Gemfile @@ -52,6 +52,7 @@ gem 'mime-types' gem 'biz' +gem 'composite_primary_keys' gem 'delayed_job_active_record' gem 'daemons' diff --git a/Gemfile.lock b/Gemfile.lock index 138fb1c44..94c3b3615 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -75,6 +75,8 @@ GEM coffee-script execjs json + composite_primary_keys (8.1.5) + activerecord (~> 4.2.0) concurrent-ruby (1.0.2) coveralls (0.8.16) json (>= 1.8, < 3) @@ -430,6 +432,7 @@ DEPENDENCIES coffee-rails coffee-script-source coffeelint + composite_primary_keys coveralls daemons delayed_job_active_record diff --git a/app/assets/javascripts/app/controllers/_profile/notification.coffee b/app/assets/javascripts/app/controllers/_profile/notification.coffee index d037b58b3..244a8681e 100644 --- a/app/assets/javascripts/app/controllers/_profile/notification.coffee +++ b/app/assets/javascripts/app/controllers/_profile/notification.coffee @@ -75,13 +75,14 @@ class Index extends App.ControllerSubContent groups = [] group_ids = @Session.get('group_ids') if group_ids - for group_id in group_ids - group = App.Group.find(group_id) - groups.push group - if !user_group_config - if !config['group_ids'] - config['group_ids'] = [] - config['group_ids'].push group_id.toString() + for group_id, access of group_ids + if _.contains(access, 'full') + group = App.Group.find(group_id) + groups.push group + if !user_group_config + if !config['group_ids'] + config['group_ids'] = [] + config['group_ids'].push group_id.toString() for sound in @sounds sound.selected = sound.file is App.OnlineNotification.soundFile() ? true : false @@ -90,7 +91,7 @@ class Index extends App.ControllerSubContent groups: groups config: config sounds: @sounds - notification_sound_enabled: App.OnlineNotification.soundEnabled() + notificationSoundEnabled: App.OnlineNotification.soundEnabled() update: (e) => diff --git a/app/assets/javascripts/app/controllers/_ui_element/permission.coffee b/app/assets/javascripts/app/controllers/_ui_element/permission.coffee index 96c996467..ec1430a4d 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/permission.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/permission.coffee @@ -4,10 +4,25 @@ class App.UiElement.permission extends App.UiElement.ApplicationUiElement permissions = App.Permission.search(sortBy: 'name') + # get selectable groups and selected groups + groups = [] + groupsSelected = {} + groupsRaw = App.Group.search(sortBy: 'name') + for group in groupsRaw + if group.active + groups.push group + if params.group_ids + for group_id in params.group_ids + if group_id.toString() is group.id.toString() + groupsSelected[group.id] = true + item = $( App.view('generic/permission')( attribute: attribute params: params permissions: permissions + groups: groups + groupsSelected: groupsSelected + groupAccesses: App.Group.accesses() ) ) # show/hide trees @@ -37,4 +52,4 @@ class App.UiElement.permission extends App.UiElement.ApplicationUiElement ) - item \ No newline at end of file + item diff --git a/app/assets/javascripts/app/controllers/_ui_element/user_permission.coffee b/app/assets/javascripts/app/controllers/_ui_element/user_permission.coffee index 936d6885a..79cd7215e 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/user_permission.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/user_permission.coffee @@ -72,6 +72,7 @@ class App.UiElement.user_permission rolesSelected: rolesSelected groupsSelected: groupsSelected hideGroups: hideGroups + groupAccesses: App.Group.accesses() ) ) # if customer, remove admin and agent @@ -105,7 +106,7 @@ class App.UiElement.user_permission # select groups if only one is available if hideGroups - item.find('.js-groupList [name=group_ids]').prop('checked', false) + item.find('.js-groupList .js-groupListItem[value=full]').prop('checked', false) return # if role with groups plugin is selected, show group selection @@ -114,7 +115,7 @@ class App.UiElement.user_permission # select groups if only one is available if hideGroups - item.find('.js-groupList [name=group_ids]').prop('checked', true) + item.find('.js-groupList .js-groupListItem[value=full]').prop('checked', true) for trigger in triggers trigger.trigger('change') diff --git a/app/assets/javascripts/app/controllers/agent_ticket_create.coffee b/app/assets/javascripts/app/controllers/agent_ticket_create.coffee index 80c5fe9f0..38b2228ca 100644 --- a/app/assets/javascripts/app/controllers/agent_ticket_create.coffee +++ b/app/assets/javascripts/app/controllers/agent_ticket_create.coffee @@ -476,10 +476,11 @@ class App.TicketCreate extends App.Controller ui.scrollTo() # access to group - group_ids = _.map(App.Session.get('group_ids'), (id) -> id.toString()) - if group_ids && _.contains(group_ids, @group_id.toString()) - ui.navigate "#ticket/zoom/#{@id}" - return + for group_id, access of App.Session.get('group_ids') + if @group_id.toString() is group_id.toString() + if _.contains(access, 'read') || _.contains(access, 'full') + ui.navigate "#ticket/zoom/#{@id}" + return # if not, show start screen ui.navigate '#' diff --git a/app/assets/javascripts/app/models/group.coffee b/app/assets/javascripts/app/models/group.coffee index 0ad754349..e58634828 100644 --- a/app/assets/javascripts/app/models/group.coffee +++ b/app/assets/javascripts/app/models/group.coffee @@ -34,4 +34,12 @@ class App.Group extends App.Model cssClass.push("avatar--group-color-#{@id % 3}") return App.view('avatar_group') - cssClass: cssClass.join(' ') \ No newline at end of file + cssClass: cssClass.join(' ') + + @accesses: -> + read: 'Read' + create: 'Create' + change: 'Change' + delete: 'Delete' + overview: 'Overview' + full: 'Full' diff --git a/app/assets/javascripts/app/models/role.coffee b/app/assets/javascripts/app/models/role.coffee index b929e85d8..994f64199 100644 --- a/app/assets/javascripts/app/models/role.coffee +++ b/app/assets/javascripts/app/models/role.coffee @@ -1,5 +1,5 @@ class App.Role extends App.Model - @configure 'Role', 'name', 'permission_ids', 'default_at_signup', 'note', 'active', 'updated_at' + @configure 'Role', 'name', 'permission_ids', 'group_ids', 'default_at_signup', 'note', 'active', 'updated_at' @extend Spine.Model.Ajax @url: @apiPath + '/roles' @configure_attributes = [ diff --git a/app/assets/javascripts/app/views/generic/permission.jst.eco b/app/assets/javascripts/app/views/generic/permission.jst.eco index 4ebbce9b3..55e00e7da 100644 --- a/app/assets/javascripts/app/views/generic/permission.jst.eco +++ b/app/assets/javascripts/app/views/generic/permission.jst.eco @@ -15,6 +15,36 @@ <%- @Icon('checkbox-checked', 'icon-checked') %> <%= permission.displayName().replace(/^.+?\./, '') %> - <%- @T.apply(@, [permission.note].concat(permission.preferences.translations)) %> + <% if _.contains(permission.preferences.plugin, 'groups'): %> +
+ + + + <% for group in @groups: %> + <% accesses = [] %> + <% if @params.group_ids && @params.group_ids[group.id]: %> + <% accesses = @params.group_ids[group.id] %> + <% end %> + + + <% end %> +
<%- @T('Group') %> + <% for key, text of @groupAccesses: %> + <%- @T(text) %> + <% end %> +
+ <%= group.displayName() %> + <% for key, text of @groupAccesses: %> + + + <% end %> +
+
+ <% end %> <% end %> <% end %> diff --git a/app/assets/javascripts/app/views/generic/user_permission.jst.eco b/app/assets/javascripts/app/views/generic/user_permission.jst.eco index 7bd6b89b7..b8dd7fd64 100644 --- a/app/assets/javascripts/app/views/generic/user_permission.jst.eco +++ b/app/assets/javascripts/app/views/generic/user_permission.jst.eco @@ -1,4 +1,18 @@
+<% showGroups = false %> +<% for role in @roles: %> +<% if role.permissions: %> +<% for permission in role.permissions: %> +<% if _.contains(permission.preferences.plugin, 'groups'): %> +<% if showGroups is true: %> +<% showGroups = false %> +<% break %> +<% end %> +<% showGroups = true %> +<% end %> +<% end %> +<% end %> +<% end %> <% for role in @roles: %> <% if role.permissions: %> <% for permission in role.permissions: %> - <% if _.contains(permission.preferences.plugin, 'groups'): %> -
+ <% if showGroups is true && _.contains(permission.preferences.plugin, 'groups'): %> +
+ + + <% for group in @groups: %> - + <% permissions = [] %> + <% if @params.group_ids && @params.group_ids[group.id]: %> + <% permissions = @params.group_ids[group.id] %> + <% end %> + + <% end %> +
<%- @T('Group') %> + <% for key, text of @groupAccesses: %> + <%- @T(text) %> + <% end %> +
+ <%= group.displayName() %> + <% for key, text of @groupAccesses: %> + + + <% end %> +
<% break %> <% end %> <% end %> <% end %> <% end %> -
\ No newline at end of file +
diff --git a/app/assets/javascripts/app/views/profile/notification.jst.eco b/app/assets/javascripts/app/views/profile/notification.jst.eco index 66ff16384..188fff03c 100644 --- a/app/assets/javascripts/app/views/profile/notification.jst.eco +++ b/app/assets/javascripts/app/views/profile/notification.jst.eco @@ -92,7 +92,7 @@
- \ No newline at end of file + diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f9a6e8475..1d37f3f60 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,4 +12,5 @@ class ApplicationController < ActionController::Base include ApplicationController::HasUser include ApplicationController::PreventsCsrf include ApplicationController::LogsHttpAccess + include ApplicationController::ChecksAccess end diff --git a/app/controllers/application_controller/checks_access.rb b/app/controllers/application_controller/checks_access.rb new file mode 100644 index 000000000..7d246e541 --- /dev/null +++ b/app/controllers/application_controller/checks_access.rb @@ -0,0 +1,9 @@ +module ApplicationController::ChecksAccess + extend ActiveSupport::Concern + + private + + def access!(instance, access) + instance.access!(current_user, access) + end +end diff --git a/app/controllers/concerns/accesses_tickets.rb b/app/controllers/concerns/accesses_tickets.rb deleted file mode 100644 index d8cd734ad..000000000 --- a/app/controllers/concerns/accesses_tickets.rb +++ /dev/null @@ -1,10 +0,0 @@ -module AccessesTickets - extend ActiveSupport::Concern - - private - - def ticket_permission(ticket) - return true if ticket.permission(current_user: current_user) - raise Exceptions::NotAuthorized - end -end diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index 9ef12fe24..43cb987f1 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/app/controllers/ticket_articles_controller.rb @@ -1,7 +1,6 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TicketArticlesController < ApplicationController - include AccessesTickets include CreatesTicketArticles prepend_before_action :authentication_check @@ -15,7 +14,7 @@ class TicketArticlesController < ApplicationController # GET /articles/1 def show article = Ticket::Article.find(params[:id]) - article_permission(article) + access!(article, 'read') if params[:expand] result = article.attributes_with_association_names @@ -35,7 +34,7 @@ class TicketArticlesController < ApplicationController # GET /ticket_articles/by_ticket/1 def index_by_ticket ticket = Ticket.find(params[:id]) - ticket_permission(ticket) + access!(ticket, 'read') articles = [] @@ -82,7 +81,7 @@ class TicketArticlesController < ApplicationController # POST /articles def create ticket = Ticket.find(params[:ticket_id]) - ticket_permission(ticket) + access!(ticket, 'create') article = article_create(ticket, params) if params[:expand] @@ -103,7 +102,7 @@ class TicketArticlesController < ApplicationController # PUT /articles/1 def update article = Ticket::Article.find(params[:id]) - article_permission(article) + access!(article, 'change') if !current_user.permissions?('ticket.agent') && !current_user.permissions?('admin') raise Exceptions::NotAuthorized, 'Not authorized (ticket.agent or admin permission required)!' @@ -132,7 +131,7 @@ class TicketArticlesController < ApplicationController # DELETE /articles/1 def destroy article = Ticket::Article.find(params[:id]) - article_permission(article) + access!(article, 'delete') if current_user.permissions?('admin') article.destroy! @@ -209,9 +208,8 @@ class TicketArticlesController < ApplicationController # GET /ticket_attachment/:ticket_id/:article_id/:id def attachment ticket = Ticket.lookup(id: params[:ticket_id]) - if !ticket_permission(ticket) - raise Exceptions::NotAuthorized, 'No such ticket.' - end + access!(ticket, 'read') + article = Ticket::Article.find(params[:article_id]) if ticket.id != article.ticket_id @@ -221,9 +219,7 @@ class TicketArticlesController < ApplicationController end ticket = article.ticket - if !ticket_permission(ticket) - raise Exceptions::NotAuthorized, "No access, for ticket_id '#{ticket.id}'." - end + access!(ticket, 'read') end list = article.attachments || [] @@ -251,7 +247,7 @@ class TicketArticlesController < ApplicationController # GET /ticket_article_plain/1 def article_plain article = Ticket::Article.find(params[:id]) - article_permission(article) + access!(article, 'read') file = article.as_raw @@ -268,15 +264,6 @@ class TicketArticlesController < ApplicationController private - def article_permission(article) - if current_user.permissions?('ticket.customer') - raise Exceptions::NotAuthorized if article.internal == true - end - ticket = Ticket.lookup(id: article.ticket_id) - return true if ticket.permission(current_user: current_user) - raise Exceptions::NotAuthorized - end - def sanitized_disposition disposition = params.fetch(:disposition, 'inline') valid_disposition = %w(inline attachment) diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 81506f393..69aa617ac 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -1,7 +1,6 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TicketsController < ApplicationController - include AccessesTickets include CreatesTicketArticles include TicketStats @@ -21,7 +20,7 @@ class TicketsController < ApplicationController per_page = 100 end - access_condition = Ticket.access_condition(current_user) + access_condition = Ticket.access_condition(current_user, 'read') tickets = Ticket.where(access_condition).order(id: 'ASC').offset(offset).limit(per_page) if params[:expand] @@ -52,10 +51,8 @@ class TicketsController < ApplicationController # GET /api/v1/tickets/1 def show - - # permission check ticket = Ticket.find(params[:id]) - ticket_permission(ticket) + access!(ticket, 'read') if params[:expand] result = ticket.attributes_with_association_names @@ -180,10 +177,8 @@ class TicketsController < ApplicationController # PUT /api/v1/tickets/1 def update - - # permission check ticket = Ticket.find(params[:id]) - ticket_permission(ticket) + access!(ticket, 'change') clean_params = Ticket.association_name_to_id_convert(params) clean_params = Ticket.param_cleanup(clean_params, true) @@ -218,10 +213,8 @@ class TicketsController < ApplicationController # DELETE /api/v1/tickets/1 def destroy - - # permission check ticket = Ticket.find(params[:id]) - ticket_permission(ticket) + access!(ticket, 'delete') raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' if !current_user.permissions?('admin') @@ -247,9 +240,7 @@ class TicketsController < ApplicationController # get ticket data ticket = Ticket.find(params[:id]) - - # permission check - ticket_permission(ticket) + access!(ticket, 'read') # get history of ticket history = ticket.history_get(true) @@ -265,7 +256,7 @@ class TicketsController < ApplicationController assets = ticket.assets({}) # open tickets by customer - access_condition = Ticket.access_condition(current_user) + access_condition = Ticket.access_condition(current_user, 'read') ticket_lists = Ticket .where( @@ -328,9 +319,7 @@ class TicketsController < ApplicationController } return end - - # permission check - ticket_permission(ticket_master) + access!(ticket_master, 'full') # check slave ticket ticket_slave = Ticket.find_by(id: params[:slave_ticket_id]) @@ -341,11 +330,9 @@ class TicketsController < ApplicationController } return end + access!(ticket_slave, 'full') - # permission check - ticket_permission(ticket_slave) - - # check diffetent ticket ids + # check different ticket ids if ticket_slave.id == ticket_master.id render json: { result: 'failed', @@ -370,10 +357,8 @@ class TicketsController < ApplicationController # GET /api/v1/ticket_split def ticket_split - - # permission check ticket = Ticket.find(params[:ticket_id]) - ticket_permission(ticket) + access!(ticket, 'read') assets = ticket.assets({}) # get related articles @@ -390,7 +375,7 @@ class TicketsController < ApplicationController # get attributes to update attributes_to_change = Ticket::ScreenOptions.attributes_to_change( - user: current_user, + current_user: current_user, ) render json: attributes_to_change end @@ -483,7 +468,7 @@ class TicketsController < ApplicationController # lookup open user tickets limit = 100 assets = {} - access_condition = Ticket.access_condition(current_user) + access_condition = Ticket.access_condition(current_user, 'read') user_tickets = {} if params[:user_id] @@ -578,7 +563,10 @@ class TicketsController < ApplicationController def ticket_all(ticket) # get attributes to update - attributes_to_change = Ticket::ScreenOptions.attributes_to_change(user: current_user, ticket: ticket) + attributes_to_change = Ticket::ScreenOptions.attributes_to_change( + current_user: current_user, + ticket: ticket + ) # get related users assets = attributes_to_change[:assets] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index c29663960..8376b1c68 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -75,25 +75,22 @@ class UsersController < ApplicationController # @response_message 200 [User] User record matching the requested identifier. # @response_message 401 Invalid session. def show - - # access deny - permission_check_local + user = User.find(params[:id]) + access!(user, 'read') if params[:expand] - user = User.find(params[:id]).attributes_with_association_names - render json: user, status: :ok - return + result = user.attributes_with_association_names + elsif params[:full] + result = { + id: params[:id], + assets: user.assets({}), + } + else + result = user.attributes_with_association_ids + result.delete('password') end - if params[:full] - full = User.full(params[:id]) - render json: full - return - end - - user = User.find(params[:id]).attributes_with_association_ids - user.delete('password') - render json: user + render json: result end # @path [POST] /users @@ -108,8 +105,6 @@ class UsersController < ApplicationController def create clean_params = User.association_name_to_id_convert(params) clean_params = User.param_cleanup(clean_params, true) - user = User.new(clean_params) - user.associations_from_param(params) # check if it's first user, the admin user # inital admin account @@ -131,6 +126,8 @@ class UsersController < ApplicationController if admin_account_exists && !params[:signup] raise Exceptions::UnprocessableEntity, 'Only signup with not authenticate user possible!' end + user = User.new(clean_params) + user.associations_from_param(params) user.updated_by_id = 1 user.created_by_id = 1 @@ -164,12 +161,8 @@ class UsersController < ApplicationController # permission check permission_check_by_permission(params) - if params[:role_ids] - user.role_ids = params[:role_ids] - end - if params[:group_ids] - user.group_ids = params[:group_ids] - end + user = User.new(clean_params) + user.associations_from_param(params) end # check if user already exists @@ -245,28 +238,25 @@ class UsersController < ApplicationController # @response_message 200 [User] Updated User record. # @response_message 401 Invalid session. def update - - # access deny - permission_check_local + permission_check_by_permission(params) user = User.find(params[:id]) - clean_params = User.association_name_to_id_convert(params) - clean_params = User.param_cleanup(clean_params, true) + access!(user, 'change') # permission check permission_check_by_permission(params) user.with_lock do + clean_params = User.association_name_to_id_convert(params) + clean_params = User.param_cleanup(clean_params, true) user.update_attributes(clean_params) # only allow Admin's if current_user.permissions?('admin.user') && (params[:role_ids] || params[:roles]) - user.role_ids = params[:role_ids] user.associations_from_param({ role_ids: params[:role_ids], roles: params[:roles] }) end # only allow Admin's if current_user.permissions?('admin.user') && (params[:group_ids] || params[:groups]) - user.group_ids = params[:group_ids] user.associations_from_param({ group_ids: params[:group_ids], groups: params[:groups] }) end @@ -298,7 +288,9 @@ class UsersController < ApplicationController # @response_message 200 User successfully deleted. # @response_message 401 Invalid session. def destroy - permission_check('admin.user') + user = User.find(params[:id]) + access!(user, 'delete') + model_references_check(User, params) model_destroy_render(User, params) end @@ -1006,30 +998,25 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content def permission_check_by_permission(params) return true if current_user.permissions?('admin.user') - if !current_user.permissions?('admin.user') && params[:role_ids] - if params[:role_ids].class != Array - params[:role_ids] = [params[:role_ids]] - end - params[:role_ids].each { |role_id| - role_local = Role.lookup(id: role_id) - if !role_local - logger.info "Invalid role_ids for current_user_id: #{current_user.id} role_ids #{role_id}" - raise Exceptions::NotAuthorized, 'Invalid role_ids!' - end - role_name = role_local.name - # TODO: check role permissions - next if role_name != 'Admin' && role_name != 'Agent' - logger.info "This role assignment is only allowed by admin! current_user_id: #{current_user.id} assigned to #{role_name}" + %i(role_ids roles).each do |key| + next if !params[key] + if current_user.permissions?('ticket.agent') + params.delete(key) + else + logger.info "Role assignment is only allowed by admin! current_user_id: #{current_user.id} assigned to #{params[key].inspect}" raise Exceptions::NotAuthorized, 'This role assignment is only allowed by admin!' - } + end + end + if current_user.permissions?('ticket.agent') && !params[:role_ids] && !params[:roles] && params[:id].blank? + params[:role_ids] = Role.signup_role_ids end - if !current_user.permissions?('admin.user') && params[:group_ids] - if params[:group_ids].class != Array - params[:group_ids] = [params[:group_ids]] - end - if !params[:group_ids].empty? - logger.info "Group relation is only allowed by admin! current_user_id: #{current_user.id} group_ids #{params[:group_ids].inspect}" + %i(group_ids groups).each do |key| + next if !params[key] + if current_user.permissions?('ticket.agent') + params.delete(key) + else + logger.info "Group relation assignment is only allowed by admin! current_user_id: #{current_user.id} assigned to #{params[key].inspect}" raise Exceptions::NotAuthorized, 'Group relation is only allowed by admin!' end end @@ -1039,16 +1026,4 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content response_access_deny false end - - def permission_check_local - return true if current_user.permissions?('admin.user') - return true if current_user.permissions?('ticket.agent') - - # allow to update any by him self - # TODO check certain attributes like roles_ids and group_ids - return true if params[:id].to_i == current_user.id - - raise Exceptions::NotAuthorized - end - end diff --git a/app/models/activity_stream.rb b/app/models/activity_stream.rb index 42c0df00d..957e7ff2a 100644 --- a/app/models/activity_stream.rb +++ b/app/models/activity_stream.rb @@ -97,7 +97,7 @@ return all activity entries of an user return [] if !user.permissions?('ticket.agent') && !user.permissions?('admin') permission_ids = user.permissions_with_child_ids - group_ids = user.group_ids + group_ids = user.group_ids_access('read') stream = if group_ids.empty? ActivityStream.where('(permission_id IN (?) AND group_id is NULL)', permission_ids) diff --git a/app/models/application_model/can_associations.rb b/app/models/application_model/can_associations.rb index 812217b26..488073ce3 100644 --- a/app/models/application_model/can_associations.rb +++ b/app/models/application_model/can_associations.rb @@ -17,9 +17,21 @@ returns def associations_from_param(params) + # special handling for group access association + { + groups: :group_names_access_map=, + group_ids: :group_ids_access_map= + }.each do |param, setter| + map = params[param] + next if map.blank? + next if !respond_to?(setter) + send(setter, map) + end + # set relations by id/verify if ref exists self.class.reflect_on_all_associations.map { |assoc| assoc_name = assoc.name + next if association_attributes_ignored.include?(assoc_name) real_ids = assoc_name[0, assoc_name.length - 1] + '_ids' real_ids = real_ids.to_sym next if !params.key?(real_ids) @@ -44,6 +56,7 @@ returns # set relations by name/lookup self.class.reflect_on_all_associations.map { |assoc| assoc_name = assoc.name + next if association_attributes_ignored.include?(assoc_name) real_ids = assoc_name[0, assoc_name.length - 1] + '_ids' next if !respond_to?(real_ids) real_values = assoc_name[0, assoc_name.length - 1] + 's' @@ -95,17 +108,20 @@ returns cache = Cache.get(key) return cache if cache - ignored_attributes = self.class.instance_variable_get(:@association_attributes_ignored) || [] - # get relations attributes = self.attributes self.class.reflect_on_all_associations.map { |assoc| + next if association_attributes_ignored.include?(assoc.name) real_ids = assoc.name.to_s[0, assoc.name.to_s.length - 1] + '_ids' - next if ignored_attributes.include?(real_ids.to_sym) next if !respond_to?(real_ids) attributes[real_ids] = send(real_ids) } + # special handling for group access associations + if respond_to?(:group_ids_access_map) + attributes['group_ids'] = send(:group_ids_access_map) + end + filter_attributes(attributes) Cache.write(key, attributes) @@ -131,6 +147,7 @@ returns attributes = attributes_with_association_ids self.class.reflect_on_all_associations.map { |assoc| next if !respond_to?(assoc.name) + next if association_attributes_ignored.include?(assoc.name) ref = send(assoc.name) next if !ref if ref.respond_to?(:first) @@ -156,6 +173,11 @@ returns attributes[assoc.name.to_s] = ref[:name] } + # special handling for group access associations + if respond_to?(:group_names_access_map) + attributes['groups'] = send(:group_names_access_map) + end + # fill created_by/updated_by { 'created_by_id' => 'created_by', @@ -214,6 +236,12 @@ returns true end + private + + def association_attributes_ignored + @association_attributes_ignored ||= self.class.instance_variable_get(:@association_attributes_ignored) || [] + end + # methods defined here are going to extend the class, not the instance of it class_methods do @@ -223,13 +251,14 @@ serve methode to ignore model attribute associations class Model < ApplicationModel include AssociationConcern - association_attributes_ignored :user_ids + association_attributes_ignored :users end =end def association_attributes_ignored(*attributes) - @association_attributes_ignored = attributes + @association_attributes_ignored ||= [] + @association_attributes_ignored |= attributes end =begin diff --git a/app/models/application_model/checks_import.rb b/app/models/application_model/checks_import.rb index 2e5e65868..82e7e6f9a 100644 --- a/app/models/application_model/checks_import.rb +++ b/app/models/application_model/checks_import.rb @@ -13,7 +13,7 @@ module ApplicationModel::ChecksImport # do noting, use id as it is return if !Setting.get('system_init_done') return if Setting.get('import_mode') && import_class_list.include?(self.class.to_s) - + return if !has_attribute?(:id) self[:id] = nil end end diff --git a/app/models/application_model/has_cache.rb b/app/models/application_model/has_cache.rb index 9446cfe29..aea7ef61e 100644 --- a/app/models/application_model/has_cache.rb +++ b/app/models/application_model/has_cache.rb @@ -14,6 +14,7 @@ module ApplicationModel::HasCache def cache_update(o) cache_delete if respond_to?('cache_delete') o.cache_delete if o.respond_to?('cache_delete') + true end def cache_delete @@ -52,6 +53,7 @@ module ApplicationModel::HasCache Cache.delete(key) end end + true end # methods defined here are going to extend the class, not the instance of it diff --git a/app/models/concerns/has_groups.rb b/app/models/concerns/has_groups.rb new file mode 100644 index 000000000..57fe9dbe2 --- /dev/null +++ b/app/models/concerns/has_groups.rb @@ -0,0 +1,331 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +module HasGroups + extend ActiveSupport::Concern + + included do + before_destroy :destroy_group_relations + + attr_accessor :group_access_buffer + + after_create :check_group_access_buffer + after_update :check_group_access_buffer + + association_attributes_ignored :groups + + has_many group_through_identifier + has_many :groups, through: group_through_identifier do + + # A helper to join the :through table into the result of groups to access :through attributes + # + # @param [String, Array] access Limiting to one or more access verbs. 'full' gets added automatically + # + # @example All access groups + # user.groups.access + # #=> [#, ...] + # + # @example Groups for given access(es) plus 'full' + # user.groups.access('read') + # #=> [#, ...] + # + # @example Groups for given access(es)es plus 'full' + # user.groups.access('read', 'write') + # #=> [#, ...] + # + # @return [ActiveRecord::AssociationRelation<[] List of Groups with :through attributes + def access(*access) + table_name = proxy_association.owner.class.group_through.table_name + query = select("groups.*, #{table_name}.*") + return query if access.blank? + + access.push('full') if !access.include?('full') + + query.where("#{table_name}.access" => access) + end + end + end + + # Checks a given Group( ID) for given access(es) for the instance. + # Checks indirect access via Roles if instance has Roles, too. + # + # @example Group ID param + # user.group_access?(1, 'read') + # #=> true + # + # @example Group param + # user.group_access?(group, 'read') + # #=> true + # + # @example Access list + # user.group_access?(group, ['read', 'create']) + # #=> true + # + # @return [Boolean] + def group_access?(group_id, access) + group_id = self.class.ensure_group_id_parameter(group_id) + access = self.class.ensure_group_access_list_parameter(access) + + # check direct access + return true if group_through.klass.includes(:group).exists?( + group_through.foreign_key => id, + group_id: group_id, + access: access, + groups: { + active: true + } + ) + + # check indirect access through Roles if possible + return false if !respond_to?(:role_access?) + role_access?(group_id, access) + end + + # Lists the Group IDs the instance has the given access(es) plus 'full' to. + # Adds indirect accessable Group IDs via Roles if instance has Roles, too. + # + # @example Single access + # user.group_ids_access('read') + # #=> [1, 3, ...] + # + # @example Access list + # user.group_ids_access(['read', 'create']) + # #=> [1, 3, ...] + # + # @return [Array] Group IDs the instance has the given access(es) to. + def group_ids_access(access) + access = self.class.ensure_group_access_list_parameter(access) + + foreign_key = group_through.foreign_key + klass = group_through.klass + + # check direct access + ids = klass.includes(:group).where(foreign_key => id, access: access, groups: { active: true }).pluck(:group_id) + ids ||= [] + + # check indirect access through roles if possible + return ids if !respond_to?(:role_ids) + + role_group_ids = RoleGroup.includes(:group).where(role_id: role_ids, access: access, groups: { active: true }).pluck(:group_id) + + # combines and removes duplicates + # and returns them in one statement + ids | role_group_ids + end + + # Lists Groups the instance has the given access(es) plus 'full' to. + # Adds indirect accessable Groups via Roles if instance has Roles, too. + # + # @example Single access + # user.groups_access('read') + # #=> [#, ...] + # + # @example Access list + # user.groups_access(['read', 'create']) + # #=> [#, ...] + # + # @return [Array] Groups the instance has the given access(es) to. + def groups_access(access) + group_ids = group_ids_access(access) + Group.where(id: group_ids) + end + + # Returns a map of Group name to access + # + # @example + # user.group_names_access_map + # #=> {'Users' => 'full', 'Support' => ['read', 'write']} + # + # @return [HashString,Array>] The map of Group name to access + def group_names_access_map + groups_access_map(:name) + end + + # Stores a map of Group ID to access. Deletes all other relations. + # + # @example + # user.group_names_access_map = {'Users' => 'full', 'Support' => ['read', 'write']} + # #=> {'Users' => 'full', 'Support' => ['read', 'write']} + # + # @return [HashString,Array>] The given map + def group_names_access_map=(name_access_map) + groups_access_map_store(name_access_map) do |group_name| + Group.where(name: group_name).pluck(:id).first + end + end + + # Returns a map of Group ID to access + # + # @example + # user.group_ids_access_map + # #=> {1 => 'full', 42 => ['read', 'write']} + # + # @return [HashString,Array>] The map of Group ID to access + def group_ids_access_map + groups_access_map(:id) + end + + # Stores a map of Group ID to access. Deletes all other relations. + # + # @example + # user.group_ids_access_map = {1 => 'full', 42 => ['read', 'write']} + # #=> {1 => 'full', 42 => ['read', 'write']} + # + # @return [HashString,Array>] The given map + def group_ids_access_map=(id_access_map) + groups_access_map_store(id_access_map) + end + + # An alias to .groups class method + def group_through + @group_through ||= self.class.group_through + end + + private + + def groups_access_map(key) + {}.tap do |hash| + groups.access.where(active: true).pluck(key, :access).each do |entry| + hash[ entry[0] ] ||= [] + hash[ entry[0] ].push(entry[1]) + end + end + end + + def groups_access_map_store(map) + map.each do |group_identifier, accesses| + # use given key as identifier or look it up + # via the given block which returns the identifier + group_id = block_given? ? yield(group_identifier) : group_identifier + + if !accesses.is_a?(Array) + accesses = [accesses] + end + + accesses.each do |access| + push_group_access_buffer( + group_id: group_id, + access: access + ) + + Rails.logger.error "TE DEBUG group_access_buffer = #{group_access_buffer.inspect}" + end + end + + check_group_access_buffer if id + end + + def push_group_access_buffer(entry) + @group_access_buffer ||= [] + @group_access_buffer.push(entry) + end + + def check_group_access_buffer + return if group_access_buffer.blank? + destroy_group_relations + + foreign_key = group_through.foreign_key + entries = group_access_buffer.collect do |entry| + entry[foreign_key] = id + entry + end + + group_through.klass.create!(entries) + + group_access_buffer = nil + + cache_delete + true + end + + def destroy_group_relations + group_through.klass.destroy_all(group_through.foreign_key => id) + end + + # methods defined here are going to extend the class, not the instance of it + class_methods do + + # Lists IDs of instances having the given access(es) to the given Group. + # + # @example Group ID param + # User.group_access_ids(1, 'read') + # #=> [1, 3, ...] + # + # @example Group param + # User.group_access_ids(group, 'read') + # #=> [1, 3, ...] + # + # @example Access list + # User.group_access_ids(group, ['read', 'create']) + # #=> [1, 3, ...] + # + # @return [Array] + def group_access_ids(group_id, access) + group_id = ensure_group_id_parameter(group_id) + access = ensure_group_access_list_parameter(access) + + # check direct access + ids = group_through.klass.includes(name.downcase).where(group_id: group_id, access: access, table_name => { active: true }).pluck(group_through.foreign_key) + ids ||= [] + + # check indirect access through roles if possible + return ids if !respond_to?(:role_access_ids) + role_instance_ids = role_access_ids(group_id, access) + + # combines and removes duplicates + # and returns them in one statement + ids | role_instance_ids + end + + # Lists instances having the given access(es) to the given Group. + # + # @example Group ID param + # User.group_access(1, 'read') + # #=> [#, ...] + # + # @example Group param + # User.group_access(group, 'read') + # #=> [#, ...] + # + # @example Access list + # User.group_access(group, ['read', 'create']) + # #=> [#, ...] + # + # @return [Array] + def group_access(group_id, access) + instance_ids = group_access_ids(group_id, access) + where(id: instance_ids) + end + + # The reflection instance containing the association data + # + # @example + # User.group_through + # #=> + # + # @return [ActiveRecord::Reflection::HasManyReflection] The given map + def group_through + @group_through ||= reflect_on_association(group_through_identifier) + end + + # The identifier of the has_many :through relation + # + # @example + # User.group_through_identifier + # #=> :user_groups + # + # @return [Symbol] The relation identifier + def group_through_identifier + "#{name.downcase}_groups".to_sym + end + + def ensure_group_id_parameter(group_or_id) + return group_or_id if group_or_id.is_a?(Integer) + group_or_id.id + end + + def ensure_group_access_list_parameter(access) + access = [access] if access.is_a?(String) + access.push('full') if !access.include?('full') + access + end + end +end diff --git a/app/models/concerns/has_roles.rb b/app/models/concerns/has_roles.rb new file mode 100644 index 000000000..bc3c75b72 --- /dev/null +++ b/app/models/concerns/has_roles.rb @@ -0,0 +1,75 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +module HasRoles + extend ActiveSupport::Concern + + # Checks a given Group( ID) for given access(es) for the instance associated roles. + # + # @example Group ID param + # user.role_access?(1, 'read') + # #=> true + # + # @example Group param + # user.role_access?(group, 'read') + # #=> true + # + # @example Access list + # user.role_access?(group, ['read', 'create']) + # #=> true + # + # @return [Boolean] + def role_access?(group_id, access) + group_id = self.class.ensure_group_id_parameter(group_id) + access = self.class.ensure_group_access_list_parameter(access) + + RoleGroup.includes(:group, :role).exists?( + role_id: roles.pluck(:id), + group_id: group_id, + access: access, + groups: { + active: true + }, + roles: { + active: true + } + ) + end + + # methods defined here are going to extend the class, not the instance of it + class_methods do + + # Lists IDs of instances having the given access(es) to the given Group through Roles. + # + # @example Group ID param + # User.role_access_ids(1, 'read') + # #=> [1, 3, ...] + # + # @example Group param + # User.role_access_ids(group, 'read') + # #=> [1, 3, ...] + # + # @example Access list + # User.role_access_ids(group, ['read', 'create']) + # #=> [1, 3, ...] + # + # @return [Array] + def role_access_ids(group_id, access) + group_id = ensure_group_id_parameter(group_id) + access = ensure_group_access_list_parameter(access) + + role_ids = RoleGroup.includes(:role).where(group_id: group_id, access: access, roles: { active: true }).pluck(:role_id) + join_table = reflect_on_association(:roles).join_table + includes(:roles).where(active: true, join_table => { role_id: role_ids }).distinct.pluck(:id) + end + + def ensure_group_id_parameter(group_or_id) + return group_or_id if group_or_id.is_a?(Integer) + group_or_id.id + end + + def ensure_group_access_list_parameter(access) + access = [access] if access.is_a?(String) + access.push('full') if !access.include?('full') + access + end + end +end diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 750128734..6f05f26ff 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -49,7 +49,7 @@ add a new attribute entry for an object data_type: 'select', data_option: { relation: 'Group', - relation_condition: { access: 'rw' }, + relation_condition: { access: 'full' }, multiple: false, null: true, translate: false, diff --git a/app/models/organization.rb b/app/models/organization.rb index 18cb20eba..a68b8505e 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -6,9 +6,8 @@ class Organization < ApplicationModel include ChecksLatestChangeObserved include HasHistory include HasSearchIndexBackend + include Organization::ChecksAccess - load 'organization/permission.rb' - include Organization::Permission load 'organization/assets.rb' include Organization::Assets extend Organization::Search diff --git a/app/models/organization/checks_access.rb b/app/models/organization/checks_access.rb new file mode 100644 index 000000000..33c17d489 --- /dev/null +++ b/app/models/organization/checks_access.rb @@ -0,0 +1,48 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +class Organization + module ChecksAccess + extend ActiveSupport::Concern + + # Checks the given access of a given user for an organization. + # + # @param [User] The user that will be checked for given access. + # @param [String] The access that should get checked. + # + # @example + # organization.access?(user, 'read') + # #=> true + # + # @return [Boolean] + def access?(user, access) + + # check customer + if user.permissions?('ticket.customer') + + # access ok if its own organization + return false if access != 'read' + return false if !user.organization_id + return id == user.organization_id + end + + # check agent + return true if user.permissions?('admin') + return true if user.permissions?('ticket.agent') + false + end + + # Checks the given access of a given user for an organization and fails with an exception. + # + # @param (see Organization#access?) + # + # @example + # organization.access!(user, 'read') + # + # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. + # + # @return [nil] + def access!(user, access) + return if access?(user, access) + raise Exceptions::NotAuthorized + end + end +end diff --git a/app/models/organization/permission.rb b/app/models/organization/permission.rb deleted file mode 100644 index e81eb158b..000000000 --- a/app/models/organization/permission.rb +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ - -class Organization - module Permission - -=begin - -check if user has access to user - - user = Organization.find(123) - result = organization.permission(type: 'rw', current_user: User.find(123)) - -returns - - result = true|false - -=end - - def permission (data) - - # check customer - if data[:current_user].permissions?('ticket.customer') - - # access ok if its own organization - return false if data[:type] != 'ro' - return false if !data[:current_user].organization_id - return true if id == data[:current_user].organization_id - - # no access - return false - end - - # check agent - return true if data[:current_user].permissions?('admin') - return true if data[:current_user].permissions?('ticket.agent') - false - end - end -end diff --git a/app/models/recent_view.rb b/app/models/recent_view.rb index 8cb226f90..3e44d79f9 100644 --- a/app/models/recent_view.rb +++ b/app/models/recent_view.rb @@ -105,8 +105,8 @@ class RecentView < ApplicationModel end # check permission - return if !record.respond_to?(:permission) - record.permission(current_user: user) + return if !record.respond_to?(:access?) + record.access?(user, 'read') end =begin diff --git a/app/models/role.rb b/app/models/role.rb index 0ff38b23b..174ea005d 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -4,6 +4,10 @@ class Role < ApplicationModel include HasActivityStreamLog include ChecksClientNotification include ChecksLatestChangeObserved + include HasGroups + + load 'role/assets.rb' + include Role::Assets has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update has_and_belongs_to_many :permissions, after_add: :cache_update, after_remove: :cache_update, before_add: :validate_agent_limit @@ -13,7 +17,7 @@ class Role < ApplicationModel before_create :validate_permissions before_update :validate_permissions - association_attributes_ignored :user_ids + association_attributes_ignored :users activity_stream_permission 'admin.role' diff --git a/app/models/role/assets.rb b/app/models/role/assets.rb new file mode 100644 index 000000000..8b023a303 --- /dev/null +++ b/app/models/role/assets.rb @@ -0,0 +1,57 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +class Role + module Assets + +=begin + +get all assets / related models for this roles + + role = Role.find(123) + result = role.assets(assets_if_exists) + +returns + + result = { + :Role => { + 123 => role_model_123, + 1234 => role_model_1234, + } + } + +=end + + def assets(data) + + app_model = self.class.to_app_model + + if !data[ app_model ] + data[ app_model ] = {} + end + if !data[ app_model ][ id ] + local_attributes = attributes_with_association_ids + + # set temp. current attributes to assets pool to prevent + # loops, will be updated with lookup attributes later + data[ app_model ][ id ] = local_attributes + + local_attributes['group_ids'].each { |group_id, _access| + group = Group.lookup(id: group_id) + next if !group + data = group.assets(data) + } + end + + return data if !self['created_by_id'] && !self['updated_by_id'] + app_model_user = User.to_app_model + %w(created_by_id updated_by_id).each { |local_user_id| + next if !self[ local_user_id ] + next if data[ app_model_user ] && data[ app_model_user ][ self[ local_user_id ] ] + user = User.lookup(id: self[ local_user_id ]) + next if !user + data = user.assets(data) + } + data + end + end +end diff --git a/app/models/role_group.rb b/app/models/role_group.rb new file mode 100644 index 000000000..c684af14a --- /dev/null +++ b/app/models/role_group.rb @@ -0,0 +1,13 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +class RoleGroup < ApplicationModel + self.table_name = 'roles_groups' + self.primary_keys = :role_id, :group_id, :access + belongs_to :role + belongs_to :group + validates :access, presence: true + + def self.ref_key + :role_id + end +end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 668f43c86..d549e9561 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -10,11 +10,10 @@ class Ticket < ApplicationModel include HasOnlineNotifications include HasKarmaActivityLog include HasLinks + include Ticket::ChecksAccess include Ticket::Escalation include Ticket::Subject - load 'ticket/permission.rb' - include Ticket::Permission load 'ticket/assets.rb' include Ticket::Assets load 'ticket/search_index.rb' @@ -75,33 +74,9 @@ class Ticket < ApplicationModel =begin -list of agents in group of ticket - - ticket = Ticket.find(123) - result = ticket.agent_of_group - -returns - - result = [user1, user2, ...] - -=end - - def agent_of_group - roles = Role.with_permissions('ticket.agent') - role_ids = roles.map(&:id) - Group.find(group_id) - .users.where(active: true) - .joins(:roles) - .where('roles.id' => role_ids, 'roles.active' => true) - .order('users.login') - .uniq() - end - -=begin - get user access conditions - conditions = Ticket.access_condition( User.find(1) ) + conditions = Ticket.access_condition( User.find(1) , 'full') returns @@ -109,22 +84,14 @@ returns =end - def self.access_condition(user) - access_condition = [] + def self.access_condition(user, access) if user.permissions?('ticket.agent') - group_ids = Group.select('groups.id').joins(:users) - .where('groups_users.user_id = ?', user.id) - .where('groups.active = ?', true) - .map(&:id) - access_condition = [ 'group_id IN (?)', group_ids ] + ['group_id IN (?)', user.group_ids_access(access)] + elsif !user.organization || ( !user.organization.shared || user.organization.shared == false ) + ['tickets.customer_id = ?', user.id] else - access_condition = if !user.organization || ( !user.organization.shared || user.organization.shared == false ) - [ 'tickets.customer_id = ?', user.id ] - else - [ '(tickets.customer_id = ? OR tickets.organization_id = ?)', user.id, user.organization.id ] - end + ['(tickets.customer_id = ? OR tickets.organization_id = ?)', user.id, user.organization.id] end - access_condition end =begin @@ -393,11 +360,11 @@ returns get count of tickets and tickets which match on selector - ticket_count, tickets = Ticket.selectors(params[:condition], limit, current_user) + ticket_count, tickets = Ticket.selectors(params[:condition], limit, current_user, 'full') =end - def self.selectors(selectors, limit = 10, current_user = nil) + def self.selectors(selectors, limit = 10, current_user = nil, access = 'full') raise 'no selectors given' if !selectors query, bind_params, tables = selector2sql(selectors, current_user) return [] if !query @@ -408,7 +375,7 @@ get count of tickets and tickets which match on selector return [ticket_count, tickets] end - access_condition = Ticket.access_condition(current_user) + access_condition = Ticket.access_condition(current_user, access) ticket_count = Ticket.where(access_condition).where(query, *bind_params).joins(tables).count tickets = Ticket.where(access_condition).where(query, *bind_params).joins(tables).limit(limit) @@ -801,9 +768,9 @@ perform changes on ticket email = User.lookup(id: owner_id).email recipients_raw.push(email) elsif recipient == 'ticket_agents' - agent_of_group.each { |user| + User.group_access(group_id, 'full').order(:login).each do |user| recipients_raw.push(user.email) - } + end else logger.error "Unknown email notification recipient '#{recipient}'" next diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 738cf112f..546f614fa 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -4,6 +4,7 @@ class Ticket::Article < ApplicationModel include ChecksClientNotification include HasHistory include ChecksHtmlSanitized + include Ticket::Article::ChecksAccess load 'ticket/article/assets.rb' include Ticket::Article::Assets diff --git a/app/models/ticket/article/checks_access.rb b/app/models/ticket/article/checks_access.rb new file mode 100644 index 000000000..5bf3eac12 --- /dev/null +++ b/app/models/ticket/article/checks_access.rb @@ -0,0 +1,42 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +class Ticket + class Article + module ChecksAccess + extend ActiveSupport::Concern + + # Checks the given access of a given user for a ticket article. + # + # @param [User] The user that will be checked for given access. + # @param [String] The access that should get checked. + # + # @example + # article.access?(user, 'read') + # #=> true + # + # @return [Boolean] + def access?(user, access) + if user.permissions?('ticket.customer') + return false if internal == true + end + + ticket = Ticket.lookup(id: ticket_id) + ticket.access?(user, access) + end + + # Checks the given access of a given user for a ticket article and fails with an exception. + # + # @param (see Ticket::Article#access?) + # + # @example + # article.access!(user, 'read') + # + # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. + # + # @return [nil] + def access!(user, access) + return if access?(user, access) + raise Exceptions::NotAuthorized + end + end + end +end diff --git a/app/models/ticket/checks_access.rb b/app/models/ticket/checks_access.rb new file mode 100644 index 000000000..fe5366239 --- /dev/null +++ b/app/models/ticket/checks_access.rb @@ -0,0 +1,57 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +class Ticket + module ChecksAccess + extend ActiveSupport::Concern + + # Checks the given access of a given user for a ticket. + # + # @param [User] The user that will be checked for given access. + # @param [String] The access that should get checked. + # + # @example + # ticket.access?(user, 'read') + # #=> true + # + # @return [Boolean] + def access?(user, access) + + # check customer + if user.permissions?('ticket.customer') + + # access ok if its own ticket + return true if customer_id == user.id + + # access ok if its organization ticket + if user.organization_id && organization_id + return true if organization_id == user.organization_id + end + + # no access + return false + end + + # check agent + + # access if requestor is owner + return true if owner_id == user.id + + # access if requestor is in group + user.group_access?(group.id, access) + end + + # Checks the given access of a given user for a ticket and fails with an exception. + # + # @param (see Ticket#access?) + # + # @example + # ticket.access!(user, 'read') + # + # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. + # + # @return [nil] + def access!(user, access) + return if access?(user, access) + raise Exceptions::NotAuthorized + end + end +end diff --git a/app/models/ticket/overviews.rb b/app/models/ticket/overviews.rb index 6f9e0b4fd..bea92fced 100644 --- a/app/models/ticket/overviews.rb +++ b/app/models/ticket/overviews.rb @@ -89,7 +89,7 @@ returns return [] if overviews.blank? # get only tickets with permissions - access_condition = Ticket.access_condition(user) + access_condition = Ticket.access_condition(user, 'overview') ticket_attributes = Ticket.new.attributes list = [] diff --git a/app/models/ticket/permission.rb b/app/models/ticket/permission.rb deleted file mode 100644 index 774ca9fd4..000000000 --- a/app/models/ticket/permission.rb +++ /dev/null @@ -1,45 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ -module Ticket::Permission - -=begin - -check if user has access to ticket - - ticket = Ticket.find(123) - result = ticket.permission(current_user: User.find(123)) - -returns - - result = true|false - -=end - - def permission(data) - - # check customer - if data[:current_user].permissions?('ticket.customer') - - # access ok if its own ticket - return true if customer_id == data[:current_user].id - - # access ok if its organization ticket - if data[:current_user].organization_id && organization_id - return true if organization_id == data[:current_user].organization_id - end - - # no access - return false - end - - # check agent - - # access if requestor is owner - return true if owner_id == data[:current_user].id - - # access if requestor is in group - data[:current_user].groups.each { |group| - return true if self.group.id == group.id - } - false - end -end diff --git a/app/models/ticket/screen_options.rb b/app/models/ticket/screen_options.rb index 1b9104d03..40053bb72 100644 --- a/app/models/ticket/screen_options.rb +++ b/app/models/ticket/screen_options.rb @@ -10,6 +10,7 @@ list attributes article_id: 123, ticket: ticket_model, + current_user: User.find(123), ) returns @@ -26,6 +27,8 @@ returns =end def self.attributes_to_change(params) + raise 'current_user param needed' if !params[:current_user] + if params[:ticket_id] params[:ticket] = Ticket.find(params[:ticket_id]) end @@ -45,22 +48,22 @@ returns if state_type && !state_types.include?(state_type.name) state_ids.push params[:ticket].state.id end - state_types.each { |type| + state_types.each do |type| state_type = Ticket::StateType.find_by(name: type) next if !state_type - state_type.states.each { |state| + state_type.states.each do |state| assets = state.assets(assets) state_ids.push state.id - } - } + end + end filter[:state_id] = state_ids # get priorities priority_ids = [] - Ticket::Priority.where(active: true).each { |priority| + Ticket::Priority.where(active: true).each do |priority| assets = priority.assets(assets) priority_ids.push priority.id - } + end filter[:priority_id] = priority_ids type_ids = [] @@ -69,36 +72,45 @@ returns if params[:ticket].group.email_address_id types.push 'email' end - types.each { |type_name| + types.each do |type_name| type = Ticket::Article::Type.lookup( name: type_name ) - if type - type_ids.push type.id - end - } + next if type.blank? + type_ids.push type.id + end end filter[:type_id] = type_ids # get group / user relations agents = {} - User.with_permissions('ticket.agent').each { |user| + User.with_permissions('ticket.agent').each do |user| agents[ user.id ] = 1 - } + end dependencies = { group_id: { '' => { owner_id: [] } } } - Group.where(active: true).each { |group| + + filter[:group_id] = [] + groups = if params[:current_user].permissions?('ticket.agent') + params[:current_user].groups_access('create') + else + Group.where(active: true) + end + + groups.each do |group| + filter[:group_id].push group.id assets = group.assets(assets) dependencies[:group_id][group.id] = { owner_id: [] } - group.users.each { |user| + + User.group_access(group.id, 'full').each do |user| next if !agents[ user.id ] assets = user.assets(assets) dependencies[:group_id][ group.id ][ :owner_id ].push user.id - } - } + end + end { - assets: assets, + assets: assets, form_meta: { - filter: filter, + filter: filter, dependencies: dependencies, } } diff --git a/app/models/ticket/search.rb b/app/models/ticket/search.rb index 096d875bb..5979da11c 100644 --- a/app/models/ticket/search.rb +++ b/app/models/ticket/search.rb @@ -105,15 +105,9 @@ returns query_extention['bool']['must'] = [] if current_user.permissions?('ticket.agent') - groups = Group.joins(:users) - .where('groups_users.user_id = ?', current_user.id) - .where('groups.active = ?', true) - group_condition = [] - groups.each { |group| - group_condition.push group.id - } + group_ids = current_user.group_ids_access('read') access_condition = { - 'query_string' => { 'default_field' => 'group_id', 'query' => "\"#{group_condition.join('" OR "')}\"" } + 'query_string' => { 'default_field' => 'group_id', 'query' => "\"#{group_ids.join('" OR "')}\"" } } else access_condition = if !current_user.organization || ( !current_user.organization.shared || current_user.organization.shared == false ) @@ -151,7 +145,7 @@ returns end # fallback do sql query - access_condition = Ticket.access_condition(current_user) + access_condition = Ticket.access_condition(current_user, 'read') # do query # - stip out * we already search for *query* - diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb index 417e98fa9..f8880c359 100644 --- a/app/models/transaction/notification.rb +++ b/app/models/transaction/notification.rb @@ -46,36 +46,8 @@ class Transaction::Notification # find recipients recipients_and_channels = [] -=begin - # group of agents to work on - if data[:recipient] == 'group' - recipients = ticket.agent_of_group() - - # owner - elsif data[:recipient] == 'owner' - if ticket.owner_id != 1 - recipients.push ticket.owner - end - - # customer - elsif data[:recipient] == 'customer' - if ticket.customer_id != 1 - # temporarily disabled - # recipients.push ticket.customer - end - - # owner or group of agents to work on - elsif data[:recipient] == 'to_work_on' - if ticket.owner_id != 1 - recipients.push ticket.owner - else - recipients = ticket.agent_of_group() - end - end -=end - # loop through all users - possible_recipients = ticket.agent_of_group + possible_recipients = User.group_access(ticket.group_id, 'full').order(:login) if ticket.owner_id == 1 possible_recipients.push ticket.owner end diff --git a/app/models/user.rb b/app/models/user.rb index 6941fb1b4..aa06390d5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,9 +28,10 @@ class User < ApplicationModel include ChecksClientNotification include HasHistory include HasSearchIndexBackend + include HasGroups + include HasRoles + include User::ChecksAccess - load 'user/permission.rb' - include User::Permission load 'user/assets.rb' include User::Assets extend User::Search @@ -44,7 +45,6 @@ class User < ApplicationModel after_update :avatar_for_email_check after_destroy :avatar_destroy - has_and_belongs_to_many :groups, after_add: :cache_update, after_remove: :cache_update, class_name: 'Group' has_and_belongs_to_many :roles, after_add: [:cache_update, :check_notifications], after_remove: :cache_update, before_add: :validate_agent_limit, before_remove: :last_admin_check, class_name: 'Role' has_and_belongs_to_many :organizations, after_add: :cache_update, after_remove: :cache_update, class_name: 'Organization' #has_many :permissions, class_name: 'Permission', through: :roles, class_name: 'Role' diff --git a/app/models/user/assets.rb b/app/models/user/assets.rb index 59d12abc2..1d64c4f8b 100644 --- a/app/models/user/assets.rb +++ b/app/models/user/assets.rb @@ -32,7 +32,7 @@ returns local_attributes = attributes_with_association_ids # do not transfer crypted pw - local_attributes['password'] = '' + local_attributes.delete('password') # set temp. current attributes to assets pool to prevent # loops, will be updated with lookup attributes later @@ -65,7 +65,7 @@ returns # get groups if local_attributes['group_ids'] - local_attributes['group_ids'].each { |group_id| + local_attributes['group_ids'].each { |group_id, _access| group = Group.lookup(id: group_id) next if !group data = group.assets(data) diff --git a/app/models/user/checks_access.rb b/app/models/user/checks_access.rb new file mode 100644 index 000000000..3dfdd48dc --- /dev/null +++ b/app/models/user/checks_access.rb @@ -0,0 +1,46 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ +class User + module ChecksAccess + extend ActiveSupport::Concern + + # Checks the given access of a given user for another user. + # + # @param [User] The user that will be checked for given access. + # @param [String] The access that should get checked. + # + # @example + # user.access?(user, 'read') + # #=> true + # + # @return [Boolean] + def access?(user, _access) + + # check agent + return true if user.permissions?('admin.user') + return true if user.permissions?('ticket.agent') + + # check customer + if user.permissions?('ticket.customer') + # access ok if its own user + return id == user.id + end + + false + end + + # Checks the given access of a given user for another user and fails with an exception. + # + # @param (see User#access?) + # + # @example + # user.access!(user, 'read') + # + # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. + # + # @return [nil] + def access!(user, access) + return if access?(user, access) + raise Exceptions::NotAuthorized + end + end +end diff --git a/app/models/user/permission.rb b/app/models/user/permission.rb deleted file mode 100644 index 9b443c943..000000000 --- a/app/models/user/permission.rb +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ - -class User - module Permission - -=begin - -check if user has access to user - - user = User.find(123) - result = user.permission(type: 'rw', current_user: User.find(123)) - -returns - - result = true|false - -=end - - def permission (data) - - # check customer - if data[:current_user].permissions?('ticket.customer') - - # access ok if its own user - return true if id == data[:current_user].id - - # no access - return false - end - - # check agent - return true if data[:current_user].permissions?('admin.user') - return true if data[:current_user].permissions?('ticket.agent') - false - end - end -end diff --git a/app/models/user_group.rb b/app/models/user_group.rb new file mode 100644 index 000000000..795051803 --- /dev/null +++ b/app/models/user_group.rb @@ -0,0 +1,13 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +class UserGroup < ApplicationModel + self.table_name = 'groups_users' + self.primary_keys = :user_id, :group_id, :access + belongs_to :user + belongs_to :group + validates :access, presence: true + + def self.ref_key + :user_id + end +end diff --git a/config/environments/test.rb b/config/environments/test.rb index 0177c1bcf..99f46bed0 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -46,4 +46,8 @@ Rails.application.configure do # format log config.log_formatter = Logger::Formatter.new + config.after_initialize do + ActiveRecord::Base.logger = Rails.logger.clone + ActiveRecord::Base.logger.level = Logger::INFO + end end diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index bc746804a..ea3b18ee9 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -161,14 +161,27 @@ class CreateBase < ActiveRecord::Migration add_foreign_key :roles_users, :roles create_table :groups_users, id: false do |t| - t.references :user - t.references :group + t.references :user, null: false + t.references :group, null: false + t.string :access, limit: 50, null: false, default: 'full' end add_index :groups_users, [:user_id] add_index :groups_users, [:group_id] + add_index :groups_users, [:access] add_foreign_key :groups_users, :users add_foreign_key :groups_users, :groups + create_table :roles_groups, id: false do |t| + t.references :role, null: false + t.references :group, null: false + t.string :access, limit: 50, null: false, default: 'full' + end + add_index :roles_groups, [:role_id] + add_index :roles_groups, [:group_id] + add_index :roles_groups, [:access] + add_foreign_key :roles_groups, :roles + add_foreign_key :roles_groups, :groups + create_table :organizations_users, id: false do |t| t.references :user t.references :organization diff --git a/db/migrate/20170403000001_fixed_admin_user_permission_920.rb b/db/migrate/20170403000001_fixed_admin_user_permission_920.rb index 970463353..7c8b131ba 100644 --- a/db/migrate/20170403000001_fixed_admin_user_permission_920.rb +++ b/db/migrate/20170403000001_fixed_admin_user_permission_920.rb @@ -84,7 +84,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration data_option: { default: '', relation: 'Group', - relation_condition: { access: 'rw' }, + relation_condition: { access: 'full' }, nulloption: true, multiple: false, null: false, diff --git a/db/migrate/20170608151442_enhanced_permissions.rb b/db/migrate/20170608151442_enhanced_permissions.rb new file mode 100644 index 000000000..70748b515 --- /dev/null +++ b/db/migrate/20170608151442_enhanced_permissions.rb @@ -0,0 +1,23 @@ +class EnhancedPermissions < ActiveRecord::Migration + def up + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + change_column_null :groups_users, :user_id, false + change_column_null :groups_users, :group_id, false + add_column :groups_users, :access, :string, limit: 50, null: false, default: 'full' + add_index :groups_users, [:access] + UserGroup.connection.schema_cache.clear! + UserGroup.reset_column_information + + create_table :roles_groups, id: false do |t| + t.references :role, null: false + t.references :group, null: false + t.string :access, limit: 50, null: false, default: 'full' + end + add_index :roles_groups, [:role_id] + add_index :roles_groups, [:group_id] + add_index :roles_groups, [:access] + end +end diff --git a/db/seeds/object_manager_attributes.rb b/db/seeds/object_manager_attributes.rb index 6e70e8ed6..420d4f3ee 100644 --- a/db/seeds/object_manager_attributes.rb +++ b/db/seeds/object_manager_attributes.rb @@ -106,7 +106,7 @@ ObjectManager::Attribute.add( data_option: { default: '', relation: 'Group', - relation_condition: { access: 'rw' }, + relation_condition: { access: 'full' }, nulloption: true, multiple: false, null: false, diff --git a/lib/sessions/backend/base.rb b/lib/sessions/backend/base.rb index a807c64f8..afbb67aa8 100644 --- a/lib/sessions/backend/base.rb +++ b/lib/sessions/backend/base.rb @@ -1,6 +1,6 @@ class Sessions::Backend::Base - def initialize(user, asset_lookup, client, client_id, ttl = 30) + def initialize(user, asset_lookup, client, client_id, ttl = 10) @user = user @client = client @client_id = client_id diff --git a/lib/sessions/backend/ticket_create.rb b/lib/sessions/backend/ticket_create.rb index b4b098e3e..c8e757ad7 100644 --- a/lib/sessions/backend/ticket_create.rb +++ b/lib/sessions/backend/ticket_create.rb @@ -4,7 +4,7 @@ class Sessions::Backend::TicketCreate < Sessions::Backend::Base # get attributes to update ticket_create_attributes = Ticket::ScreenOptions.attributes_to_change( - user: @user.id, + current_user: @user, ) # no data exists diff --git a/lib/stats/ticket_channel_distribution.rb b/lib/stats/ticket_channel_distribution.rb index b75b35e18..06da940b7 100644 --- a/lib/stats/ticket_channel_distribution.rb +++ b/lib/stats/ticket_channel_distribution.rb @@ -8,7 +8,7 @@ class Stats::TicketChannelDistribution time_range = 7.days # get users groups - group_ids = user.groups.map(&:id) + group_ids = user.group_ids_access('full') # get channels channels = [ diff --git a/lib/stats/ticket_escalation.rb b/lib/stats/ticket_escalation.rb index 6c0662f33..3358e3df5 100644 --- a/lib/stats/ticket_escalation.rb +++ b/lib/stats/ticket_escalation.rb @@ -7,7 +7,7 @@ class Stats::TicketEscalation open_state_ids = Ticket::State.by_category(:open).pluck(:id) # get users groups - group_ids = user.groups.map(&:id) + group_ids = user.group_ids_access('full') # owned tickets own_escalated = Ticket.where( diff --git a/lib/stats/ticket_load_measure.rb b/lib/stats/ticket_load_measure.rb index 67c33836d..8f9b992ba 100644 --- a/lib/stats/ticket_load_measure.rb +++ b/lib/stats/ticket_load_measure.rb @@ -10,7 +10,7 @@ class Stats::TicketLoadMeasure count = Ticket.where(owner_id: user.id, state_id: open_state_ids).count # get total open - total = Ticket.where(group_id: user.groups.map(&:id), state_id: open_state_ids).count + total = Ticket.where(group_id: user.group_ids_access('full'), state_id: open_state_ids).count average = '-' state = 'good' diff --git a/lib/stats/ticket_waiting_time.rb b/lib/stats/ticket_waiting_time.rb index efcc10427..7a474dcb8 100644 --- a/lib/stats/ticket_waiting_time.rb +++ b/lib/stats/ticket_waiting_time.rb @@ -5,7 +5,7 @@ class Stats::TicketWaitingTime def self.generate(user) # get users groups - group_ids = user.groups.map(&:id) + group_ids = user.group_ids_access('full') own_waiting = Ticket.where( 'owner_id = ? AND group_id IN (?) AND updated_at > ?', user.id, group_ids, Time.zone.today diff --git a/spec/factories/role.rb b/spec/factories/role.rb new file mode 100644 index 000000000..6a8cd742f --- /dev/null +++ b/spec/factories/role.rb @@ -0,0 +1,14 @@ +FactoryGirl.define do + sequence :test_role_name do |n| + "TestRole#{n}" + end +end + +FactoryGirl.define do + + factory :role do + name { generate(:test_role_name) } + created_by_id 1 + updated_by_id 1 + end +end diff --git a/spec/models/concerns/has_groups_examples.rb b/spec/models/concerns/has_groups_examples.rb new file mode 100644 index 000000000..0fd9b3f43 --- /dev/null +++ b/spec/models/concerns/has_groups_examples.rb @@ -0,0 +1,523 @@ +RSpec.shared_examples 'HasGroups' do + + context 'group' do + + let(:factory_name) { described_class.name.downcase.to_sym } + let(:instance) { create(factory_name) } + let(:instance_inactive) { create(factory_name, active: false) } + let(:group_full) { create(:group) } + let(:group_read) { create(:group) } + let(:group_inactive) { create(:group, active: false) } + + context '.group_through_identifier' do + + it 'responds to group_through_identifier' do + expect(described_class).to respond_to(:group_through_identifier) + end + + it 'returns a Symbol as identifier' do + expect(described_class.group_through_identifier).to be_a(Symbol) + end + + it 'instance responds to group_through_identifier method' do + expect(instance).to respond_to(described_class.group_through_identifier) + end + end + + context '.group_through' do + + it 'responds to group_through' do + expect(described_class).to respond_to(:group_through) + end + + it 'returns the Reflection instance of the has_many :through relation' do + expect(described_class.group_through).to be_a(ActiveRecord::Reflection::HasManyReflection) + end + end + + context '#groups' do + + it 'responds to groups' do + expect(instance).to respond_to(:groups) + end + + context '#groups.access' do + + it 'responds to groups.access' do + expect(instance.groups).to respond_to(:access) + end + + context 'result' do + + before(:each) do + instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + group_inactive.name => 'write', + } + end + + it 'returns all related Groups' do + expect(instance.groups.access.size).to eq(3) + end + + it 'adds join table attribute(s like) access' do + expect(instance.groups.access.first).to respond_to(:access) + end + + it 'filters for given access parameter' do + expect(instance.groups.access('read')).to include(group_read) + end + + it 'filters for given access list parameter' do + expect(instance.groups.access('read', 'write')).to include(group_read, group_inactive) + end + + it 'always includes full access groups' do + expect(instance.groups.access('read')).to include(group_full) + end + end + end + end + + context '#group_access?' do + + before(:each) do + instance.group_names_access_map = { + group_read.name => 'read', + } + end + + it 'responds to group_access?' do + expect(instance).to respond_to(:group_access?) + end + + context 'Group ID parameter' do + include_examples '#group_access? call' do + let(:group_parameter) { group_read.id } + end + end + + context 'Group parameter' do + include_examples '#group_access? call' do + let(:group_parameter) { group_read } + end + end + + it 'prevents inactive Group' do + instance.group_names_access_map = { + group_inactive.name => 'read', + } + + expect(instance.group_access?(group_inactive.id, 'read')).to be false + end + end + + context '#group_ids_access' do + + before(:each) do + instance.group_names_access_map = { + group_read.name => 'read', + } + end + + it 'responds to group_ids_access' do + expect(instance).to respond_to(:group_ids_access) + end + + it 'lists only active Group IDs' do + instance.group_names_access_map = { + group_read.name => 'read', + group_inactive.name => 'read', + } + + result = instance.group_ids_access('read') + expect(result).not_to include(group_inactive.id) + end + + context 'single access' do + + it 'lists access Group IDs' do + result = instance.group_ids_access('read') + expect(result).to include(group_read.id) + end + + it "doesn't list for no access" do + result = instance.group_ids_access('write') + expect(result).not_to include(group_read.id) + end + end + + context 'access list' do + + it 'lists access Group IDs' do + result = instance.group_ids_access(%w(read write)) + expect(result).to include(group_read.id) + end + + it "doesn't list for no access" do + result = instance.group_ids_access(%w(write create)) + expect(result).not_to include(group_read.id) + end + end + end + + context '#groups_access' do + + it 'responds to groups_access' do + expect(instance).to respond_to(:groups_access) + end + + it 'wraps #group_ids_access' do + expect(instance).to receive(:group_ids_access) + instance.groups_access('read') + end + + it 'returns Groups' do + instance.group_names_access_map = { + group_read.name => 'read', + } + result = instance.groups_access('read') + expect(result).to include(group_read) + end + end + + context '#group_names_access_map=' do + + it 'responds to group_names_access_map=' do + expect(instance).to respond_to(:group_names_access_map=) + end + + context 'Group name => access relation storage' do + + it 'stores Hash with String values' do + expect do + instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + } + end.to change { + described_class.group_through.klass.count + }.by(2) + end + + it 'stores Hash with String values' do + expect do + instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => %w(read write), + } + end.to change { + described_class.group_through.klass.count + }.by(3) + end + + context 'new instance' do + let(:new_instance) { build(factory_name) } + + it "doesn't store directly" do + expect do + new_instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + } + end.not_to change { + described_class.group_through.klass.count + } + end + + it 'stores after save' do + expect do + new_instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + } + + new_instance.save + end.to change { + described_class.group_through.klass.count + }.by(2) + end + end + end + end + + context '#group_names_access_map' do + + it 'responds to group_names_access_map' do + expect(instance).to respond_to(:group_names_access_map) + end + + it 'returns instance Group name => access relations as Hash' do + expected = { + group_full.name => ['full'], + group_read.name => ['read'], + } + + instance.group_names_access_map = expected + + expect(instance.group_names_access_map).to eq(expected) + end + end + + context '#group_ids_access_map=' do + + it 'responds to group_ids_access_map=' do + expect(instance).to respond_to(:group_ids_access_map=) + end + + context 'Group ID => access relation storage' do + + it 'stores Hash with String values' do + expect do + instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => 'read', + } + end.to change { + described_class.group_through.klass.count + }.by(2) + end + + it 'stores Hash with String values' do + expect do + instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => %w(read write), + } + end.to change { + described_class.group_through.klass.count + }.by(3) + end + + context 'new instance' do + let(:new_instance) { build(factory_name) } + + it "doesn't store directly" do + expect do + new_instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => 'read', + } + end.not_to change { + described_class.group_through.klass.count + } + end + + it 'stores after save' do + expect do + new_instance.group_ids_access_map = { + group_full.id => 'full', + group_read.id => 'read', + } + + new_instance.save + end.to change { + described_class.group_through.klass.count + }.by(2) + end + end + end + end + + context '#group_ids_access_map' do + + it 'responds to group_ids_access_map' do + expect(instance).to respond_to(:group_ids_access_map) + end + + it 'returns instance Group ID => access relations as Hash' do + expected = { + group_full.id => ['full'], + group_read.id => ['read'], + } + + instance.group_ids_access_map = expected + + expect(instance.group_ids_access_map).to eq(expected) + end + end + + context '#associations_from_param' do + + it 'handles group_ids parameter as group_ids_access_map' do + expected = { + group_full.id => ['full'], + group_read.id => ['read'], + } + + instance.associations_from_param(group_ids: expected) + expect(instance.group_ids_access_map).to eq(expected) + end + + it 'handles groups parameter as group_names_access_map' do + expected = { + group_full.name => ['full'], + group_read.name => ['read'], + } + + instance.associations_from_param(groups: expected) + expect(instance.group_names_access_map).to eq(expected) + end + end + + context '#attributes_with_association_ids' do + + it 'includes group_ids as group_ids_access_map' do + expected = { + group_full.id => ['full'], + group_read.id => ['read'], + } + + instance.group_ids_access_map = expected + + result = instance.attributes_with_association_ids + expect(result['group_ids']).to eq(expected) + end + end + + context '#attributes_with_association_names' do + + it 'includes group_ids as group_ids_access_map' do + expected = { + group_full.id => ['full'], + group_read.id => ['read'], + } + + instance.group_ids_access_map = expected + + result = instance.attributes_with_association_names + expect(result['group_ids']).to eq(expected) + end + + it 'includes groups as group_names_access_map' do + expected = { + group_full.name => ['full'], + group_read.name => ['read'], + } + + instance.group_names_access_map = expected + + result = instance.attributes_with_association_names + expect(result['groups']).to eq(expected) + end + end + + context '.group_access_ids' do + + before(:each) do + instance.group_names_access_map = { + group_read.name => 'read', + } + end + + it 'responds to group_access_ids' do + expect(described_class).to respond_to(:group_access_ids) + end + + it 'lists only active instance IDs' do + instance_inactive.group_names_access_map = { + group_read.name => 'read', + } + + result = described_class.group_access_ids(group_read.id, 'read') + expect(result).not_to include(instance_inactive.id) + end + + context 'Group ID parameter' do + include_examples '.group_access_ids call' do + let(:group_parameter) { group_read.id } + end + end + + context 'Group parameter' do + include_examples '.group_access_ids call' do + let(:group_parameter) { group_read.id } + end + end + end + + context '.group_access' do + + it 'responds to group_access' do + expect(described_class).to respond_to(:group_access) + end + + it 'wraps .group_access_ids' do + expect(described_class).to receive(:group_access_ids) + described_class.group_access(group_read, 'read') + end + + it 'returns class instances' do + instance.group_names_access_map = { + group_read.name => 'read', + } + + result = described_class.group_access(group_read, 'read') + expect(result).to include(instance) + end + end + + it 'destroys relations before instance gets destroyed' do + + instance.group_names_access_map = { + group_full.name => 'full', + group_read.name => 'read', + group_inactive.name => 'write', + } + expect do + instance.destroy + end.to change { + described_class.group_through.klass.count + }.by(-3) + end + end +end + +RSpec.shared_examples '#group_access? call' do + context 'single access' do + + it 'checks positive' do + expect(instance.group_access?(group_parameter, 'read')).to be true + end + + it 'checks negative' do + expect(instance.group_access?(group_parameter, 'write')).to be false + end + end + + context 'access list' do + + it 'checks positive' do + expect(instance.group_access?(group_parameter, %w(read write))).to be true + end + + it 'checks negative' do + expect(instance.group_access?(group_parameter, %w(write create))).to be false + end + end +end + +RSpec.shared_examples '.group_access_ids call' do + context 'single access' do + + it 'lists access IDs' do + expect(described_class.group_access_ids(group_parameter, 'read')).to include(instance.id) + end + + it 'excludes non access IDs' do + expect(described_class.group_access_ids(group_parameter, 'write')).not_to include(instance.id) + end + end + + context 'access list' do + + it 'lists access IDs' do + expect(described_class.group_access_ids(group_parameter, %w(read write))).to include(instance.id) + end + + it 'excludes non access IDs' do + expect(described_class.group_access_ids(group_parameter, %w(write create))).not_to include(instance.id) + end + end +end diff --git a/spec/models/concerns/has_roles_examples.rb b/spec/models/concerns/has_roles_examples.rb new file mode 100644 index 000000000..83f5fe29c --- /dev/null +++ b/spec/models/concerns/has_roles_examples.rb @@ -0,0 +1,268 @@ +RSpec.shared_examples 'HasRoles' do + + context 'role' do + + let(:factory_name) { described_class.name.downcase.to_sym } + let(:instance) { create(factory_name) } + let(:instance_inactive) { create(factory_name, active: false) } + let(:role) { create(:role) } + let(:group_instance) { create(:group) } + let(:group_role) { create(:group) } + let(:group_inactive) { create(:group, active: false) } + + context '#role_access?' do + + before(:each) do + role.group_names_access_map = { + group_role.name => 'read', + } + + instance.roles = [role] + end + + it 'responds to role_access?' do + expect(instance).to respond_to(:role_access?) + end + + context 'Group ID parameter' do + include_examples '#role_access? call' do + let(:group_parameter) { group_role.id } + end + end + + context 'Group parameter' do + include_examples '#role_access? call' do + let(:group_parameter) { group_role } + end + end + + it 'prevents inactive Group' do + role.group_names_access_map = { + group_inactive.name => 'read', + } + + expect(instance.group_access?(group_inactive.id, 'read')).to be false + end + + it 'prevents inactive Role' do + role_inactive = create(:role, active: false) + role_inactive.group_names_access_map = { + group_role.name => 'read', + } + + instance.roles = [role_inactive] + + expect(instance.group_access?(group_role.id, 'read')).to be false + end + end + + context '.role_access_ids' do + + before(:each) do + role.group_names_access_map = { + group_role.name => 'read', + } + + instance.roles = [role] + end + + it 'responds to role_access_ids' do + expect(described_class).to respond_to(:role_access_ids) + end + + it 'lists only active instance IDs' do + role.group_names_access_map = { + group_role.name => 'read', + } + + result = described_class.group_access_ids(group_role.id, 'read') + expect(result).not_to include(instance_inactive.id) + end + + it 'lists only active instance IDs' do + role.group_names_access_map = { + group_role.name => 'read', + } + + instance_inactive.roles = [role] + + result = described_class.role_access_ids(group_role.id, 'read') + expect(result).not_to include(instance_inactive.id) + end + + context 'Group ID parameter' do + include_examples '.role_access_ids call' do + let(:group_parameter) { group_role.id } + end + end + + context 'Group parameter' do + include_examples '.role_access_ids call' do + let(:group_parameter) { group_role.id } + end + end + end + + context 'group' do + + before(:each) do + role.group_names_access_map = { + group_role.name => 'read', + } + + instance.roles = [role] + + instance.group_names_access_map = { + group_instance.name => 'read', + } + end + + context '#group_access?' do + + it 'falls back to #role_access?' do + expect(instance).to receive(:role_access?) + instance.group_access?(group_role, 'read') + end + + it "doesn't fall back to #role_access? if not needed" do + expect(instance).not_to receive(:role_access?) + instance.group_access?(group_instance, 'read') + end + end + + context '#group_ids_access' do + + before(:each) do + role.group_names_access_map = { + group_role.name => 'read', + } + + instance.roles = [role] + + instance.group_names_access_map = { + group_instance.name => 'read', + } + end + + it 'lists only active Group IDs' do + role.group_names_access_map = { + group_role.name => 'read', + group_inactive.name => 'read', + } + + result = instance.group_ids_access('read') + expect(result).not_to include(group_inactive.id) + end + + context 'single access' do + + it 'lists access Group IDs' do + result = instance.group_ids_access('read') + expect(result).to include(group_role.id) + end + + it "doesn't list for no access" do + result = instance.group_ids_access('write') + expect(result).not_to include(group_role.id) + end + + it "doesn't contain duplicate IDs" do + instance.group_names_access_map = { + group_role.name => 'read', + } + + result = instance.group_ids_access('read') + expect(result.uniq).to eq(result) + end + end + + context 'access list' do + + it 'lists access Group IDs' do + result = instance.group_ids_access(%w(read write)) + expect(result).to include(group_role.id) + end + + it "doesn't list for no access" do + result = instance.group_ids_access(%w(write create)) + expect(result).not_to include(group_role.id) + end + + it "doesn't contain duplicate IDs" do + instance.group_names_access_map = { + group_role.name => 'read', + } + + result = instance.group_ids_access(%w(read create)) + expect(result.uniq).to eq(result) + end + end + end + + context '.group_access_ids' do + + it 'includes the result of .role_access_ids' do + result = described_class.group_access_ids(group_role, 'read') + expect(result).to include(instance.id) + end + + it "doesn't contain duplicate IDs" do + instance.group_names_access_map = { + group_role.name => 'read', + } + + result = described_class.group_access_ids(group_role, 'read') + expect(result.uniq).to eq(result) + end + end + end + end +end + +RSpec.shared_examples '#role_access? call' do + context 'single access' do + + it 'checks positive' do + expect(instance.role_access?(group_parameter, 'read')).to be true + end + + it 'checks negative' do + expect(instance.role_access?(group_parameter, 'write')).to be false + end + end + + context 'access list' do + + it 'checks positive' do + expect(instance.role_access?(group_parameter, %w(read write))).to be true + end + + it 'checks negative' do + expect(instance.role_access?(group_parameter, %w(write create))).to be false + end + end +end + +RSpec.shared_examples '.role_access_ids call' do + context 'single access' do + + it 'lists access IDs' do + expect(described_class.role_access_ids(group_parameter, 'read')).to include(instance.id) + end + + it 'excludes non access IDs' do + expect(described_class.role_access_ids(group_parameter, 'write')).not_to include(instance.id) + end + end + + context 'access list' do + + it 'lists access IDs' do + expect(described_class.role_access_ids(group_parameter, %w(read write))).to include(instance.id) + end + + it 'excludes non access IDs' do + expect(described_class.role_access_ids(group_parameter, %w(write create))).not_to include(instance.id) + end + end +end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb new file mode 100644 index 000000000..9e9d307d7 --- /dev/null +++ b/spec/models/role_spec.rb @@ -0,0 +1,6 @@ +require 'rails_helper' +require 'models/concerns/has_groups_examples' + +RSpec.describe Role do + include_examples 'HasGroups' +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7c932eebb..de8985e17 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' +require 'models/concerns/has_groups_examples' +require 'models/concerns/has_roles_examples' RSpec.describe User do + include_examples 'HasGroups' + include_examples 'HasRoles' let(:new_password) { 'N3W54V3PW!' } diff --git a/test/browser/abb_one_group_test.rb b/test/browser/abb_one_group_test.rb index e42349985..13de33b8a 100644 --- a/test/browser/abb_one_group_test.rb +++ b/test/browser/abb_one_group_test.rb @@ -59,10 +59,10 @@ class AgentTicketActionLevel0Test < TestCase ) exists( displayed: false, - css: '.modal [name="group_ids"]', + css: '.modal .js-groupList', ) exists( - css: '.modal [name="group_ids"]:checked', + css: '.modal .js-groupListItem[value=full]:checked', ) click( css: '.modal button.btn.btn--primary', @@ -105,10 +105,10 @@ class AgentTicketActionLevel0Test < TestCase exists( displayed: false, - css: '.modal [name="group_ids"]', + css: '.modal .js-groupList', ) exists_not( - css: '.modal [name="group_ids"]:checked', + css: '.modal .js-groupListItem[value=full]:checked', ) # enable agent role @@ -117,10 +117,11 @@ class AgentTicketActionLevel0Test < TestCase ) exists( - css: '.modal [name="group_ids"]', + displayed: false, + css: '.modal .js-groupList', ) exists( - css: '.modal [name="group_ids"]:checked', + css: '.modal .js-groupListItem[value=full]:checked', ) click( @@ -214,8 +215,14 @@ class AgentTicketActionLevel0Test < TestCase data: { name: "some group #{rand(999_999_999)}", member: [ - 'master@example.com', - 'agent1@example.com', + { + login: 'master@example.com', + access: 'full', + }, + { + login: 'agent1@example.com', + access: 'full', + }, ], }, ) diff --git a/test/browser/agent_ticket_attachment_test.rb b/test/browser/agent_ticket_attachment_test.rb index b91a4f6c2..2e35f5f56 100644 --- a/test/browser/agent_ticket_attachment_test.rb +++ b/test/browser/agent_ticket_attachment_test.rb @@ -233,6 +233,7 @@ class AgentTicketAttachmentTest < TestCase sleep 2 set(browser: browser1, css: '.modal [name="address"]', value: 'some new address') click(browser: browser1, css: '.modal .js-submit') + modal_disappear(browser: browser1) # verify is customer has chnaged other browser too click(browser: browser2, css: '.content.active .tabsSidebar-tab[data-tab="customer"]') @@ -255,6 +256,7 @@ class AgentTicketAttachmentTest < TestCase click(browser: browser1, css: '.modal .js-option') click(browser: browser1, css: '.modal .js-submit') + modal_disappear(browser: browser1) # check if org has changed in second browser sleep 3 diff --git a/test/browser/agent_ticket_email_signature_test.rb b/test/browser/agent_ticket_email_signature_test.rb index e4165af8f..3e8ad43cb 100644 --- a/test/browser/agent_ticket_email_signature_test.rb +++ b/test/browser/agent_ticket_email_signature_test.rb @@ -45,7 +45,10 @@ class AgentTicketEmailSignatureTest < TestCase name: group_name1, signature: signature_name1, member: [ - 'master@example.com' + { + login: 'master@example.com', + access: 'full', + }, ], } ) @@ -54,7 +57,10 @@ class AgentTicketEmailSignatureTest < TestCase name: group_name2, signature: signature_name2, member: [ - 'master@example.com' + { + login: 'master@example.com', + access: 'full', + }, ], } ) @@ -62,10 +68,14 @@ class AgentTicketEmailSignatureTest < TestCase data: { name: group_name3, member: [ - 'master@example.com' + { + login: 'master@example.com', + access: 'full', + }, ], } ) + sleep 6 # # check signature in new ticket diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index e16878901..3385b6dca 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -112,7 +112,7 @@ class AgentTicketOverviewLevel0Test < TestCase css: '.modal input[value="article_count"]', ) click(css: '.modal .js-submit') - sleep 6 + modal_disappear # check if number and article count is shown match( @@ -160,7 +160,7 @@ class AgentTicketOverviewLevel0Test < TestCase css: '.modal input[value="article_count"]', ) click(css: '.modal .js-submit') - sleep 6 + modal_disappear # check if number and article count is gone match_not( diff --git a/test/browser/agent_ticket_tag_test.rb b/test/browser/agent_ticket_tag_test.rb index a298304f4..337d2ab13 100644 --- a/test/browser/agent_ticket_tag_test.rb +++ b/test/browser/agent_ticket_tag_test.rb @@ -264,7 +264,7 @@ class AgentTicketTagTest < TestCase browser: browser2, css: '.modal .js-submit', ) - sleep 4 + modal_disappear(browser: browser2) ticket_open_by_search( browser: browser2, number: ticket3[:number], @@ -313,7 +313,7 @@ class AgentTicketTagTest < TestCase browser: browser2, css: '.modal .js-submit', ) - sleep 4 + modal_disappear(browser: browser2) ticket_open_by_search( browser: browser2, number: ticket3[:number], diff --git a/test/browser/chat_test.rb b/test/browser/chat_test.rb index 1d8954bb4..5fa781d37 100644 --- a/test/browser/chat_test.rb +++ b/test/browser/chat_test.rb @@ -471,6 +471,7 @@ class ChatTest < TestCase browser: agent, css: '.modal .js-submit', ) + modal_disappear(browser: agent) customer = browser_instance location( diff --git a/test/browser/first_steps_test.rb b/test/browser/first_steps_test.rb index 167703b23..b6316c865 100644 --- a/test/browser/first_steps_test.rb +++ b/test/browser/first_steps_test.rb @@ -36,7 +36,7 @@ class FirstStepsTest < TestCase css: '.modal [name="email"]', value: "#{agent}@example.com", ) - check(css: '.modal [name="group_ids"]') + check(css: '.modal .js-groupListItem[value=full]') click( css: '.modal button.btn.btn--primary', fast: true, diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 55e527f3b..1e221f193 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -472,13 +472,14 @@ class TestCase < Test::Unit::TestCase if params[:position] == 'botton' position = 'false' end - + screenshot(browser: instance, comment: 'scroll_to_before') execute( browser: instance, js: "\$('#{params[:css]}').get(0).scrollIntoView(#{position})", mute_log: params[:mute_log] ) sleep 0.3 + screenshot(browser: instance, comment: 'scroll_to_after') end =begin @@ -495,7 +496,9 @@ class TestCase < Test::Unit::TestCase instance = params[:browser] || @browser + screenshot(browser: instance, comment: 'modal_ready_before') sleep 3 + screenshot(browser: instance, comment: 'modal_ready_after') end =begin @@ -513,11 +516,13 @@ class TestCase < Test::Unit::TestCase instance = params[:browser] || @browser + screenshot(browser: instance, comment: 'modal_disappear_before') watch_for_disappear( browser: instance, css: '.modal', timeout: params[:timeout] || 8, ) + screenshot(browser: instance, comment: 'modal_disappear_after') end =begin @@ -1864,17 +1869,31 @@ wait untill text in selector disabppears # check if owner selection exists count = instance.find_elements(css: '.content.active .newTicket select[name="group_id"] option').count + if count.nonzero? + instance.find_elements(css: '.content.active .newTicket select[name="group_id"] option').each { |element| + log('ticket_create invalid group count', text: element.text) + } + end assert_equal(0, count, 'owner selection should not be showm') # check count of agents, should be only 3 / - selection + master + agent on init screen count = instance.find_elements(css: '.content.active .newTicket select[name="owner_id"] option').count + if count != 3 + instance.find_elements(css: '.content.active .newTicket select[name="owner_id"] option').each { |element| + log('ticket_create invalid owner count', text: element.text) + } + end assert_equal(3, count, 'check if owner selection is - selection + master + agent per default') - else # check count of agents, should be only 1 / - selection on init screen if !params[:disable_group_check] count = instance.find_elements(css: '.content.active .newTicket select[name="owner_id"] option').count + if count != 1 + instance.find_elements(css: '.content.active .newTicket select[name="owner_id"] option').each { |element| + log('ticket_create invalid owner count', text: element.text) + } + end assert_equal(1, count, 'check if owner selection is empty per default') end select( @@ -2869,7 +2888,10 @@ wait untill text in selector disabppears name: 'some sla' + random, signature: 'some signature bame', member: [ - 'some_user_login', + { + login: 'some_user_login', + access: 'all', + }, ], }, ) @@ -2922,20 +2944,21 @@ wait untill text in selector disabppears # add member if data[:member] - data[:member].each { |login| + data[:member].each { |member| instance.find_elements(css: 'a[href="#manage"]')[0].click sleep 1 instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click sleep 3 element = instance.find_elements(css: '.content.active [name="search"]')[0] element.clear - element.send_keys(login) + element.send_keys(member[:login]) sleep 3 #instance.find_elements(:css => '.content.active table [data-id]')[0].click instance.execute_script('$(".content.active table [data-id] td").first().click()') - sleep 3 + modal_ready(browser: instance) #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click - instance.execute_script('$(\'label:contains(" ' + data[:name] + '")\').first().click()') + instance.execute_script('$(".js-groupList tr:contains(\"' + data[:name] + '\") .js-groupListItem[value=' + member[:access] + ']").prop("checked", true)') + screenshot(browser: instance, comment: 'group_create_member') instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) } diff --git a/test/controllers/user_organization_controller_test.rb b/test/controllers/user_organization_controller_test.rb index 71159adc6..1107547ce 100644 --- a/test/controllers/user_organization_controller_test.rb +++ b/test/controllers/user_organization_controller_test.rb @@ -12,6 +12,18 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest groups = Group.all UserInfo.current_user_id = 1 + + @backup_admin = User.create_or_update( + login: 'backup-admin', + firstname: 'Backup', + lastname: 'Agent', + email: 'backup-admin@example.com', + password: 'adminpw', + active: true, + roles: roles, + groups: groups, + ) + @admin = User.create_or_update( login: 'rest-admin', firstname: 'Rest', @@ -384,17 +396,29 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest role = Role.lookup(name: 'Admin') params = { firstname: "Admin#{firstname}", lastname: 'Admin Last', email: 'new_admin_by_agent@example.com', role_ids: [ role.id ] } post '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials) - assert_response(401) - result = JSON.parse(@response.body) - assert(result) + assert_response(201) + result_user1 = JSON.parse(@response.body) + assert(result_user1) + user = User.find(result_user1['id']) + assert_not(user.role?('Admin')) + assert_not(user.role?('Agent')) + assert(user.role?('Customer')) + assert_equal('new_admin_by_agent@example.com', result_user1['login']) + assert_equal('new_admin_by_agent@example.com', result_user1['email']) # create user with agent role role = Role.lookup(name: 'Agent') params = { firstname: "Agent#{firstname}", lastname: 'Agent Last', email: 'new_agent_by_agent@example.com', role_ids: [ role.id ] } post '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials) - assert_response(401) - result = JSON.parse(@response.body) - assert(result) + assert_response(201) + result_user1 = JSON.parse(@response.body) + assert(result_user1) + user = User.find(result_user1['id']) + assert_not(user.role?('Admin')) + assert_not(user.role?('Agent')) + assert(user.role?('Customer')) + assert_equal('new_agent_by_agent@example.com', result_user1['login']) + assert_equal('new_agent_by_agent@example.com', result_user1['email']) # create user with customer role role = Role.lookup(name: 'Customer') diff --git a/test/integration/otrs_import_test.rb b/test/integration/otrs_import_test.rb index 307a949ae..818727393 100644 --- a/test/integration/otrs_import_test.rb +++ b/test/integration/otrs_import_test.rb @@ -67,15 +67,15 @@ class OtrsImportTest < ActiveSupport::TestCase assert_equal( true, user1.active ) assert( user1.roles.include?( role_agent ) ) - assert( !user1.roles.include?( role_admin ) ) - assert( !user1.roles.include?( role_customer ) ) - #assert( !user1.roles.include?( role_report ) ) + assert_not( user1.roles.include?( role_admin ) ) + assert_not( user1.roles.include?( role_customer ) ) + #assert_not( user1.roles.include?( role_report ) ) group_dasa = Group.where( name: 'dasa' ).first group_raw = Group.where( name: 'Raw' ).first - assert( !user1.groups.include?( group_dasa ) ) - assert( user1.groups.include?( group_raw ) ) + assert_not( user1.groups_access('full').include?( group_dasa ) ) + assert( user1.groups_access('full').include?( group_raw ) ) user2 = User.find(3) assert_equal( 'agent-2 firstname äöüß', user2.firstname ) @@ -86,11 +86,11 @@ class OtrsImportTest < ActiveSupport::TestCase assert( user2.roles.include?( role_agent ) ) assert( user2.roles.include?( role_admin ) ) - assert( !user2.roles.include?( role_customer ) ) + assert_not( user2.roles.include?( role_customer ) ) #assert( user2.roles.include?( role_report ) ) - assert( user2.groups.include?( group_dasa ) ) - assert( user2.groups.include?( group_raw ) ) + assert( user2.groups_access('full').include?( group_dasa ) ) + assert( user2.groups_access('full').include?( group_raw ) ) user3 = User.find(7) assert_equal( 'invalid', user3.firstname ) @@ -100,12 +100,12 @@ class OtrsImportTest < ActiveSupport::TestCase assert_equal( false, user3.active ) assert( user3.roles.include?( role_agent ) ) - assert( !user3.roles.include?( role_admin ) ) - assert( !user3.roles.include?( role_customer ) ) + assert_not( user3.roles.include?( role_admin ) ) + assert_not( user3.roles.include?( role_customer ) ) #assert( user3.roles.include?( role_report ) ) - assert( !user3.groups.include?( group_dasa ) ) - assert( !user3.groups.include?( group_raw ) ) + assert_not( user3.groups_access('full').include?( group_dasa ) ) + assert_not( user3.groups_access('full').include?( group_raw ) ) user4 = User.find(8) assert_equal( 'invalid-temp', user4.firstname ) @@ -115,12 +115,12 @@ class OtrsImportTest < ActiveSupport::TestCase assert_equal( false, user4.active ) assert( user4.roles.include?( role_agent ) ) - assert( !user4.roles.include?( role_admin ) ) - assert( !user4.roles.include?( role_customer ) ) + assert_not( user4.roles.include?( role_admin ) ) + assert_not( user4.roles.include?( role_customer ) ) #assert( user4.roles.include?( role_report ) ) - assert( !user4.groups.include?( group_dasa ) ) - assert( !user4.groups.include?( group_raw ) ) + assert_not( user4.groups_access('full').include?( group_dasa ) ) + assert_not( user4.groups_access('full').include?( group_raw ) ) end diff --git a/test/integration/zendesk_import_test.rb b/test/integration/zendesk_import_test.rb index 3e59fa1de..96191bd55 100644 --- a/test/integration/zendesk_import_test.rb +++ b/test/integration/zendesk_import_test.rb @@ -146,7 +146,7 @@ class ZendeskImportTest < ActiveSupport::TestCase end } assert_equal(check[:roles], user.roles.sort.to_a, "#{user.login} roles") - assert_equal(check[:groups], user.groups.sort.to_a, "#{user.login} groups") + assert_equal(check[:groups], user.groups_access('full').sort.to_a, "#{user.login} groups") } end diff --git a/test/test_helper.rb b/test/test_helper.rb index 24f283f45..7be5849fc 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -77,6 +77,8 @@ class ActiveSupport::TestCase ApplicationHandleInfo.current = 'unknown' Rails.logger.info '++++NEW++++TEST++++' + + travel_back end # Add more helper methods to be used by all tests here... diff --git a/test/unit/assets_test.rb b/test/unit/assets_test.rb index 95ed76cfa..52b065de7 100644 --- a/test/unit/assets_test.rb +++ b/test/unit/assets_test.rb @@ -62,7 +62,7 @@ class AssetsTest < ActiveSupport::TestCase user1 = User.find(user1.id) attributes = user1.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user1.id]), 'check assets' ) @@ -70,7 +70,7 @@ class AssetsTest < ActiveSupport::TestCase user2 = User.find(user2.id) attributes = user2.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user2.id]), 'check assets' ) @@ -78,7 +78,7 @@ class AssetsTest < ActiveSupport::TestCase user3 = User.find(user3.id) attributes = user3.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user3.id]), 'check assets' ) @@ -96,7 +96,7 @@ class AssetsTest < ActiveSupport::TestCase user1_new = User.find(user1.id) attributes = user1_new.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( !diff(attributes, assets[:User][user1_new.id]), 'check assets' ) @@ -110,7 +110,7 @@ class AssetsTest < ActiveSupport::TestCase user1 = User.find(user1.id) attributes = user1.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user1.id]), 'check assets' ) @@ -118,7 +118,7 @@ class AssetsTest < ActiveSupport::TestCase user2 = User.find(user2.id) attributes = user2.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user2.id]), 'check assets' ) @@ -126,7 +126,7 @@ class AssetsTest < ActiveSupport::TestCase user3 = User.find(user3.id) attributes = user3.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user3.id]), 'check assets' ) @@ -209,7 +209,7 @@ class AssetsTest < ActiveSupport::TestCase admin1 = User.find(admin1.id) attributes = admin1.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][admin1.id]), 'check assets' ) @@ -217,7 +217,7 @@ class AssetsTest < ActiveSupport::TestCase user1 = User.find(user1.id) attributes = user1.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user1.id]), 'check assets' ) @@ -225,7 +225,7 @@ class AssetsTest < ActiveSupport::TestCase user2 = User.find(user2.id) attributes = user2.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user2.id]), 'check assets' ) @@ -233,7 +233,7 @@ class AssetsTest < ActiveSupport::TestCase user3 = User.find(user3.id) attributes = user3.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert_nil( assets[:User][user3.id], 'check assets' ) @@ -251,7 +251,7 @@ class AssetsTest < ActiveSupport::TestCase attributes = user_new_2.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user_new_2.id]), 'check assets' ) @@ -264,7 +264,7 @@ class AssetsTest < ActiveSupport::TestCase attributes = user_new_2.attributes_with_association_ids attributes['accounts'] = {} - attributes['password'] = '' + attributes.delete('password') attributes.delete('token_ids') attributes.delete('authorization_ids') assert( diff(attributes, assets[:User][user_new_2.id]), 'check assets' ) diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index 89de3260e..c86389560 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -64,8 +64,8 @@ class ModelTest < ActiveSupport::TestCase test 'references test' do # create base - groups = Group.where( name: 'Users' ) - roles = Role.where( name: %w(Agent Admin) ) + groups = Group.where(name: 'Users') + roles = Role.where(name: %w(Agent Admin)) agent1 = User.create_or_update( login: 'model-agent1@example.com', firstname: 'Model', @@ -104,7 +104,7 @@ class ModelTest < ActiveSupport::TestCase updated_by_id: agent1.id, created_by_id: 1, ) - roles = Role.where( name: 'Customer' ) + roles = Role.where(name: 'Customer') customer1 = User.create_or_update( login: 'model-customer1@example.com', firstname: 'Model', @@ -153,10 +153,11 @@ class ModelTest < ActiveSupport::TestCase assert_equal(references1['User']['updated_by_id'], 3) assert_equal(references1['User']['created_by_id'], 1) assert_equal(references1['Organization']['updated_by_id'], 1) + assert_equal(references1['UserGroup']['user_id'], 1) assert(!references1['Group']) references_total1 = Models.references_total('User', agent1.id) - assert_equal(references_total1, 7) + assert_equal(references_total1, 8) # verify agent2 references2 = Models.references('User', agent2.id) @@ -164,10 +165,10 @@ class ModelTest < ActiveSupport::TestCase assert(!references2['User']) assert(!references2['Organization']) assert(!references2['Group']) - assert(references2.empty?) + assert_equal(references2['UserGroup']['user_id'], 1) references_total2 = Models.references_total('User', agent2.id) - assert_equal(references_total2, 0) + assert_equal(references_total2, 1) Models.merge('User', agent2.id, agent1.id) @@ -177,6 +178,7 @@ class ModelTest < ActiveSupport::TestCase assert(!references1['User']) assert(!references1['Organization']) assert(!references1['Group']) + assert(!references1['UserGroup']) assert(references1.empty?) references_total1 = Models.references_total('User', agent1.id) @@ -188,10 +190,11 @@ class ModelTest < ActiveSupport::TestCase assert_equal(references2['User']['updated_by_id'], 3) assert_equal(references2['User']['created_by_id'], 1) assert_equal(references2['Organization']['updated_by_id'], 1) + assert_equal(references2['UserGroup']['user_id'], 2) assert(!references2['Group']) references_total2 = Models.references_total('User', agent2.id) - assert_equal(references_total2, 7) + assert_equal(references_total2, 9) # org diff --git a/test/unit/object_cache_test.rb b/test/unit/object_cache_test.rb index 285b157d1..26d9b05ec 100644 --- a/test/unit/object_cache_test.rb +++ b/test/unit/object_cache_test.rb @@ -36,7 +36,7 @@ class ObjectCacheTest < ActiveSupport::TestCase test 'user cache' do roles = Role.where(name: %w(Agent Admin)) - groups = Group.all + groups = Group.all.order(:id) # be sure that minimum one admin is available User.create_or_update( @@ -65,7 +65,7 @@ class ObjectCacheTest < ActiveSupport::TestCase groups: groups, ) assets = user1.assets({}) - assert_equal(user1.group_ids.sort, assets[:User][user1.id]['group_ids'].sort) + assert_equal(user1.group_ids_access_map.sort, assets[:User][user1.id]['group_ids'].sort) # update group group1 = groups.first @@ -73,15 +73,16 @@ class ObjectCacheTest < ActiveSupport::TestCase group1.save assets = user1.assets({}) + assert(assets[:Group][group1.id]) assert_equal(group1.note, assets[:Group][group1.id]['note']) # update group - assert_equal(user1.group_ids.sort, assets[:User][user1.id]['group_ids'].sort) + assert_equal(user1.group_ids_access_map.sort, assets[:User][user1.id]['group_ids'].sort) user1.group_ids = [] user1.save assets = user1.assets({}) - assert_equal(user1.group_ids.sort, assets[:User][user1.id]['group_ids'].sort) + assert_equal(user1.group_ids_access_map.sort, assets[:User][user1.id]['group_ids'].sort) # update role assert_equal(user1.role_ids.sort, assets[:User][user1.id]['role_ids'].sort) diff --git a/test/unit/recent_view_test.rb b/test/unit/recent_view_test.rb index 912ca6931..6604b87ce 100644 --- a/test/unit/recent_view_test.rb +++ b/test/unit/recent_view_test.rb @@ -48,7 +48,7 @@ class RecentViewTest < ActiveSupport::TestCase ticket2.destroy list = RecentView.list(user1) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') travel_back end @@ -61,7 +61,7 @@ class RecentViewTest < ActiveSupport::TestCase # check if list is empty list = RecentView.list(user) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') # log entry of not existing record RecentView.user_log_destroy(user) @@ -69,7 +69,7 @@ class RecentViewTest < ActiveSupport::TestCase # check if list is empty list = RecentView.list(user) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') # log entry of not existing model with permission check RecentView.user_log_destroy(user) @@ -77,7 +77,7 @@ class RecentViewTest < ActiveSupport::TestCase # check if list is empty list = RecentView.list(user) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') end test 'permission tests' do @@ -103,6 +103,12 @@ class RecentViewTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1 ) + organization2 = Organization.create_if_not_exists( + name: 'Customer Organization Recent View 2', + note: 'some note', + updated_by_id: 1, + created_by_id: 1, + ) # no access for customer ticket1 = Ticket.create( @@ -122,7 +128,7 @@ class RecentViewTest < ActiveSupport::TestCase # check if list is empty list = RecentView.list(customer) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') # log entry of not existing object RecentView.user_log_destroy(agent) @@ -130,7 +136,7 @@ class RecentViewTest < ActiveSupport::TestCase # check if list is empty list = RecentView.list(agent) - assert(!list[0], 'check if recent view list is empty') + assert_not(list[0], 'check if recent view list is empty') # access for customer via customer id ticket1 = Ticket.create( @@ -152,27 +158,31 @@ class RecentViewTest < ActiveSupport::TestCase list = RecentView.list(customer) assert(list[0]['o_id'], ticket1.id) assert(list[0]['object'], 'Ticket') - assert(!list[1], 'check if recent view list is empty') + assert_not(list[1], 'check if recent view list is empty') # log entry - organization = Organization.find(1) + organization1 = Organization.find(1) RecentView.user_log_destroy(customer) - RecentView.log(organization.class.to_s, organization.id, customer) + RecentView.log(organization1.class.to_s, organization1.id, customer) + RecentView.log(organization2.class.to_s, organization2.id, customer) # check if list is empty list = RecentView.list(customer) - assert(!list[0], 'check if recent view list is empty') + assert(list[0], 'check if recent view list is empty') + assert_not(list[1], 'check if recent view list is empty') # log entry - organization = Organization.find(1) + organization1 = Organization.find(1) RecentView.user_log_destroy(agent) - RecentView.log(organization.class.to_s, organization.id, agent) + RecentView.log(organization1.class.to_s, organization1.id, agent) # check if list is empty list = RecentView.list(agent) - assert(list[0]['o_id'], organization.id) + assert(list[0]['o_id'], organization1.id) assert(list[0]['object'], 'Organization') - assert(!list[1], 'check if recent view list is empty') + assert_not(list[1], 'check if recent view list is empty') + + organization2.destroy end end diff --git a/test/unit/session_basic_test.rb b/test/unit/session_basic_test.rb index 3e239a2df..aa9e7f398 100644 --- a/test/unit/session_basic_test.rb +++ b/test/unit/session_basic_test.rb @@ -56,7 +56,7 @@ class SessionBasicTest < ActiveSupport::TestCase test 'c session create / update' do # create users - roles = Role.where(name: ['Agent']) + roles = Role.where(name: %w(Agent)) groups = Group.all UserInfo.current_user_id = 1 @@ -148,7 +148,10 @@ class SessionBasicTest < ActiveSupport::TestCase assert_nil(result2, 'check collections - after touch - recall') # change collection - group = Group.create(name: "SomeGroup::#{rand(999_999)}", active: true) + group = Group.create( + name: "SomeGroup::#{rand(999_999)}", + active: true + ) travel 4.seconds # get whole collections @@ -242,31 +245,52 @@ class SessionBasicTest < ActiveSupport::TestCase test 'c ticket_create' do - UserInfo.current_user_id = 2 - user = User.lookup(id: 1) - ticket_create_client1 = Sessions::Backend::TicketCreate.new(user, {}, false, '123-1', 3) + # create users + roles = Role.where(name: %w(Agent)) + groups = Group.all + + UserInfo.current_user_id = 1 + agent1 = User.create_or_update( + login: 'session-agent-1', + firstname: 'Session', + lastname: 'Agent 1', + email: 'session-agent-1@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: groups, + ) + agent1.save! + + ticket_create_client1 = Sessions::Backend::TicketCreate.new(agent1, {}, false, '123-1', 3) # get as stream result1 = ticket_create_client1.push assert(result1, 'check ticket_create') - sleep 0.6 + travel 1.second # next check should be empty result1 = ticket_create_client1.push assert(!result1, 'check ticket_create - recall') # next check should be empty - sleep 0.6 + travel 1.second result1 = ticket_create_client1.push assert(!result1, 'check ticket_create - recall 2') - Group.create(name: "SomeTicketCreateGroup::#{rand(999_999)}", active: true) + group = Group.create(name: "SomeTicketCreateGroup::#{rand(999_999)}", active: true) + agent1.groups = Group.all + agent1.save! + + # next check should be empty + result1 = ticket_create_client1.push travel 4.seconds # get as stream result1 = ticket_create_client1.push assert(result1, 'check ticket_create - recall 3') + travel_back end