Browse Source

Maintenance: Unify permissions check in frontend and backend

This change allows to check if user has multiple permissions in one run. For example user.permissions?("permission1+permission2").
Mantas Masalskis 8 months ago
parent
commit
daad6ef75d

+ 1 - 1
app/graphql/gql/mutations/user/current/calendar_subscription/update.rb

@@ -10,7 +10,7 @@ module Gql::Mutations
     field :success, Boolean, null: false, description: 'Profile appearance settings updated successfully?'
 
     def self.authorize(_obj, ctx)
-      ctx.current_user.permissions?('user_preferences.calendar') && ctx.current_user.permissions?('ticket.agent')
+      ctx.current_user.permissions?('user_preferences.calendar+ticket.agent')
     end
 
     def resolve(input:)

+ 1 - 1
app/graphql/gql/mutations/user/current/notification_preferences_reset.rb

@@ -7,7 +7,7 @@ module Gql::Mutations
     field :user, Gql::Types::UserType, null: false, description: 'Updated user object'
 
     def self.authorize(_obj, ctx)
-      ctx.current_user.permissions?('user_preferences.notifications') && ctx.current_user.permissions?('ticket.agent')
+      ctx.current_user.permissions?('user_preferences.notifications+ticket.agent')
     end
 
     def resolve

+ 1 - 1
app/graphql/gql/mutations/user/current/notification_preferences_update.rb

@@ -11,7 +11,7 @@ module Gql::Mutations
     field :user, Gql::Types::UserType, null: false, description: 'Updated user object'
 
     def self.authorize(_obj, ctx)
-      ctx.current_user.permissions?('user_preferences.notifications') && ctx.current_user.permissions?('ticket.agent')
+      ctx.current_user.permissions?('user_preferences.notifications+ticket.agent')
     end
 
     def resolve(matrix:, sound:, group_ids: nil)

+ 1 - 1
app/graphql/gql/mutations/user/current/out_of_office.rb

@@ -9,7 +9,7 @@ module Gql::Mutations
     field :success, Boolean, description: 'Profile out of office settings updated successfully?'
 
     def self.authorize(_obj, ctx)
-      ctx.current_user.permissions?('user_preferences.out_of_office') && ctx.current_user.permissions?('ticket.agent')
+      ctx.current_user.permissions?('user_preferences.out_of_office+ticket.agent')
     end
 
     def resolve(input:)

+ 1 - 1
app/graphql/gql/queries/user/current/calendar_subscription/list.rb

@@ -8,7 +8,7 @@ module Gql::Queries
     type Gql::Types::User::PersonalSettings::CalendarSubscriptionsConfigType, null: false
 
     def self.authorize(_obj, ctx)
-      ctx.current_user.permissions?('user_preferences.calendar') && ctx.current_user.permissions?('ticket.agent')
+      ctx.current_user.permissions?('user_preferences.calendar+ticket.agent')
     end
 
     def resolve

+ 2 - 2
app/models/organization/search.rb

@@ -27,7 +27,7 @@ returns if user has no permissions to search
 =end
 
       def search_preferences(current_user)
-        return false if !current_user.permissions?('ticket.agent') && !current_user.permissions?('ticket.customer') && !current_user.permissions?('admin.organization')
+        return false if !current_user.permissions?(['ticket.agent', 'ticket.customer', 'admin.organization'])
 
         {
           prio:                1500,
@@ -36,7 +36,7 @@ returns if user has no permissions to search
       end
 
       def customer_only?(current_user)
-        return true if current_user.permissions?('ticket.customer') && !current_user.permissions?('admin.organization') && !current_user.permissions?('ticket.agent')
+        return true if current_user.permissions?('ticket.customer') && !current_user.permissions?(['admin.organization', 'ticket.agent'])
 
         false
       end

+ 1 - 1
app/models/ticket/search.rb

@@ -26,7 +26,7 @@ returns if user has no permissions to search
 =end
 
     def search_preferences(current_user)
-      return false if !current_user.permissions?('ticket.agent') && !current_user.permissions?('ticket.customer')
+      return false if !current_user.permissions?(['ticket.agent', 'ticket.customer'])
 
       {
         prio:                3000,

+ 1 - 20
app/models/token.rb

@@ -1,6 +1,7 @@
 # Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
 class Token < ApplicationModel
+  include Token::Permissions
   include Token::TriggersSubscriptions
 
   before_create :generate_token
@@ -105,26 +106,6 @@ cleanup old token
     true
   end
 
-  def permissions
-    Permission.where(
-      name:   Array(preferences[:permission]),
-      active: true,
-    )
-  end
-
-  def permissions?(permissions)
-    permissions!(permissions)
-    true
-  rescue Exceptions::Forbidden
-    false
-  end
-
-  def permissions!(auth_query)
-    return true if effective_user.permissions?(auth_query) && Auth::RequestCache.permissions?(self, auth_query)
-
-    raise Exceptions::Forbidden, __('Not authorized (token)!')
-  end
-
   # allows to evaluate token permissions in context of given user instead of owner
   # @param [User] user to use as context for the given block
   # @param block to evaluate in given context

+ 22 - 0
app/models/token/permissions.rb

@@ -0,0 +1,22 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+module Token::Permissions
+  extend ActiveSupport::Concern
+
+  def permissions
+    Permission.where(
+      name:   Array(preferences[:permission]),
+      active: true,
+    )
+  end
+
+  def permissions?(query)
+    effective_user.permissions?(query) && Auth::Permissions.authorized?(self, query)
+  end
+
+  def permissions!(query)
+    return true if permissions?(query)
+
+    raise Exceptions::Forbidden, __('Token authorization failed.')
+  end
+end

+ 2 - 134
app/models/user.rb

@@ -24,6 +24,7 @@ class User < ApplicationModel
   include User::PerformsGeoLookup
   include User::UpdatesTicketOrganization
   include User::OutOfOffice
+  include User::Permissions
 
   has_and_belongs_to_many :organizations,          after_add: %i[cache_update create_organization_add_history], after_remove: %i[cache_update create_organization_remove_history], class_name: 'Organization'
   has_and_belongs_to_many :overviews,              dependent: :nullify
@@ -41,7 +42,6 @@ class User < ApplicationModel
   has_many                :owner_tickets,          class_name: 'Ticket', foreign_key: :owner_id, inverse_of: :owner
   has_many                :overview_sortings,      dependent: :destroy
   has_many                :created_recent_views,   class_name: 'RecentView', foreign_key: :created_by_id, dependent: :destroy, inverse_of: :created_by
-  has_many                :permissions,            -> { where(roles: { active: true }, active: true) }, through: :roles
   has_many                :data_privacy_tasks,     as: :deletable
   belongs_to              :organization,           inverse_of: :members, optional: true
 
@@ -81,8 +81,7 @@ class User < ApplicationModel
                                  :chat_agents,
                                  :data_privacy_tasks,
                                  :overviews,
-                                 :mentions,
-                                 :permissions
+                                 :mentions
 
   activity_stream_permission 'admin.user'
 
@@ -322,104 +321,6 @@ returns
     end
   end
 
-=begin
-
-returns all accessable permission ids of user
-
-  user = User.find(123)
-  user.permissions_with_child_ids
-
-returns
-
-  [permission1_id, permission2_id, permission3_id]
-
-=end
-
-  def permissions_with_child_ids
-    permissions_with_child_elements.pluck(:id)
-  end
-
-=begin
-
-returns all accessable permission names of user
-
-  user = User.find(123)
-  user.permissions_with_child_names
-
-returns
-
-  [permission1_name, permission2_name, permission3_name]
-
-=end
-
-  def permissions_with_child_names
-    permissions_with_child_elements.pluck(:name)
-  end
-
-  def permissions?(permissions)
-    permissions!(permissions)
-    true
-  rescue Exceptions::Forbidden
-    false
-  end
-
-  def permissions!(auth_query)
-    return true if Auth::RequestCache.permissions?(self, auth_query)
-
-    raise Exceptions::Forbidden, __('Not authorized (user)!')
-  end
-
-=begin
-
-get all users with permission
-
-  users = User.with_permissions('ticket.agent')
-
-get all users with permission "admin.session" or "ticket.agent"
-
-  users = User.with_permissions(['admin.session', 'ticket.agent'])
-
-returns
-
-  [user1, user2, ...]
-
-=end
-
-  def self.with_permissions(keys)
-    if keys.class != Array
-      keys = [keys]
-    end
-    permission_ids = []
-
-    total_role_ids = keys
-      .filter_map do |key|
-        ::Permission.with_parents(key).each do |local_key|
-          permission = ::Permission.lookup(name: local_key)
-          next if !permission
-
-          permission_ids.push permission.id
-        end
-        next if permission_ids.blank?
-
-        Role
-          .joins(:permissions_roles)
-          .joins(:permissions)
-          .where(permissions_roles: { permission_id: permission_ids }, roles: { active: true }, permissions: { active: true })
-          .distinct
-          .pluck(:id)
-      end
-      .flatten
-
-    return [] if total_role_ids.blank?
-
-    User
-      .joins(:roles_users)
-      .where(users: { active: true })
-      .where(roles_users: { role_id: total_role_ids })
-      .distinct
-      .reorder(:id)
-  end
-
   # Find a user by mobile number, either directly or by number variants stored in the Cti::CallerIds.
   def self.by_mobile(number:)
     direct_lookup = User.where(mobile: number).reorder(:updated_at).first
@@ -796,25 +697,6 @@ try to find correct name
     preferences[:notification_config][:matrix] = Setting.get('ticket_agent_default_notifications')
   end
 
-  def permissions_with_child_and_parent_elements
-    permission_names         = permissions.pluck(:name)
-    names_including_ancestor = permission_names.flat_map { |name| Permission.with_parents(name) }.uniq
-
-    base_query = Permission.reorder(:name).where(active: true)
-
-    permission_names
-      .reduce(base_query.where(name: names_including_ancestor)) do |memo, name|
-        memo.or(base_query.where('permissions.name LIKE ?', "#{SqlHelper.quote_like(name)}.%"))
-      end
-      .tap do |permissions|
-        ancestor_names = names_including_ancestor - permission_names
-
-        permissions
-          .select { |permission| permission.name.in?(ancestor_names) }
-          .each { |permission| permission.preferences['disabled'] = true }
-      end
-  end
-
   private
 
   def organization_history_log(org, type)
@@ -963,20 +845,6 @@ try to find correct name
     errors.add :base, __('More than 250 secondary organizations are not allowed.')
   end
 
-  def permissions_with_child_elements
-    where = ''
-    where_bind = [true]
-    permissions.pluck(:name).each do |permission_name|
-      where += ' OR ' if where != ''
-      where += 'permissions.name = ? OR permissions.name LIKE ?'
-      where_bind.push permission_name
-      where_bind.push "#{SqlHelper.quote_like(permission_name)}.%"
-    end
-    return [] if where == ''
-
-    ::Permission.where("permissions.active = ? AND (#{where})", *where_bind)
-  end
-
   def validate_roles(role)
     return true if !role_ids # we need role_ids for checking in role_ids below, in this method
     return true if role.preferences[:not].blank?

Some files were not shown because too many files changed in this diff