Fix time tracker permissions (#150)

* Update plugin permissions

* Update verify_permission! method

* Refactor specs to use non-admin account

* Add regression tests

app/controllers/tracky_controller.rb

@@ -7,8 +7,7 @@ class TrackyController < ApplicationController
   helper_method :offset_for_time_zone
 
   def verify_permission!
-    return unless User.current
-    return if User.current.allowed_to_globally?(action: action_name.to_sym, controller: controller_name.to_s)
+    return if User.current&.allowed_to_globally?(action: action_name.to_sym, controller: controller_name.to_s)
 
     render_403(flash: { error: t('timer_sessions.messages.errors.permission.no_access') })
   end

app/views/timer_sessions/_timer_container.html.erb

@@ -68,20 +68,20 @@
       <div class="mb-3"></div>
       <% if timer_session.persisted? %>
         <div data-ending-action-buttons>
-          <% if User.current.allowed_to_globally?(action: :stop, controller: 'time_tracker') %>
+          <% if User.current.allowed_to_globally?(action: :update, controller: 'time_tracker') %>
             <%= f.button :stop, type: :submit, data: { name: 'timer-stop', form_target: 'stopButton' }, name: :stop do %>
               <%= t('timer_sessions.timer.stop') %>
               <i class="icon icon-error"></i>
             <% end %>
           <% end %>
-          <% if User.current.allowed_to_globally?(action: :cancel, controller: 'time_tracker') %>
+          <% if User.current.allowed_to_globally?(action: :destroy, controller: 'time_tracker') %>
             <%= f.button :cancel, type: :submit, data: { name: 'timer-cancel', confirm: l(:text_are_you_sure) }, class: 'ml-3', name: :cancel, value: :cancel, form: "timer-cancel-form" do %>
               <%= t('timer_sessions.timer.cancel') %>
               <i class="icon icon-cancel"></i>
             <% end %>
           <% end %>
         </div>
-      <% elsif !timer_session.persisted? && User.current.allowed_to_globally?(action: :start, controller: 'time_tracker') %>
+      <% elsif !timer_session.persisted? && User.current.allowed_to_globally?(action: :create, controller: 'time_tracker') %>
         <%= f.button :start, type: :submit, value: :start, data: { name: 'timer-start' }, name: :commit do %>
           <%= t('timer_sessions.timer.start') %>
           <i class="icon icon-add new-issue"></i>

init.rb

@@ -37,23 +37,23 @@
     permission :manage_timer_sessions, {
       timer_sessions: %i[index create continue update edit patch
                          destroy report time_error report time_error rebalance],
-      time_tracker: %i[start stop update]
+      time_tracker: %i[create update destroy]
     }, require: :loggedin
 
     permission :index_timer_sessions, {
       timer_sessions: %i[index]
     }, require: :loggedin
 
     permission :create_timer_sessions, {
-      time_tracker: %i[start update]
+      time_tracker: %i[create]
     }, require: :loggedin
 
     permission :stop_timer_sessions, {
-      time_tracker: %i[stop]
+      time_tracker: %i[update]
     }, require: :loggedin
 
     permission :cancel_timer_sessions, {
-      time_tracker: %i[cancel]
+      time_tracker: %i[destroy]
     }, require: :loggedin
 
     permission :query_report, {

test/functional/time_tracker_controller_test.rb

@@ -19,8 +19,12 @@ class TimeTrackerControllerTest < ActionController::TestCase
            :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values
 
   def setup
-    @controller.logged_user = User.find(1)
-    @request.session[:user_id] = 1
+    user = User.find(2)
+    user.roles.first.add_permission! :create_timer_sessions
+    user.roles.first.add_permission! :stop_timer_sessions
+    user.roles.first.add_permission! :cancel_timer_sessions
+    @controller.logged_user = user
+    @request.session[:user_id] = user.id
   end
 
   test '#create - without login' do
@@ -75,7 +79,7 @@ def setup
   end
 
   test '#create - with existing session' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
     assert_equal 1, TimerSession.count
     post :create, params: { timer_session: {
       timer_start: Time.zone.now - 1.hour,
@@ -101,7 +105,7 @@ def setup
   end
 
   test '#create - from last session' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: true, timer_start: Time.zone.now - 2.hours,
+    FactoryBot.create(:timer_session, user: User.find(2), finished: true, timer_start: Time.zone.now - 2.hours,
                                       timer_end: Time.zone.now - 1.hour)
     assert_equal 1, TimerSession.count
     post :create, params: { timer_session: {
@@ -118,7 +122,7 @@ def setup
   end
 
   test '#update - with end time' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
 
     recorded_time = Time.zone.now - 1.hour
     post :update, params: { timer_session: {
@@ -131,7 +135,7 @@ def setup
   end
 
   test '#update - with no end time' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
 
     post :update, params: { timer_session: {
       timer_start: Time.zone.now - 1.hours,
@@ -143,7 +147,7 @@ def setup
   end
 
   test '#update - with invalid params' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
 
     post :update, params: { timer_session: {
       timer_start: Time.zone.now + 1.hours,
@@ -155,7 +159,7 @@ def setup
   end
 
   test '#update - with end time and invalid params' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
 
     post :update, params: { timer_session: {
       timer_start: Time.zone.now,
@@ -181,7 +185,7 @@ def setup
   end
 
   test '#destroy - with existing session' do
-    FactoryBot.create(:timer_session, user: User.find(1), finished: false)
+    FactoryBot.create(:timer_session, user: User.find(2), finished: false)
 
     delete :destroy, xhr: true
     assert_response 200
@@ -195,4 +199,28 @@ def setup
 
     assert TimerSession.count, 0
   end
+
+  test '#create - without create_timer_sessions permission' do
+    user = User.find(2)
+    user.roles.first.remove_permission! :create_timer_sessions
+    @controller.logged_user = user
+    post :create, params: { timer_session: { comments: 'Very interesting' } }, xhr: true
+    assert_response 403
+  end
+
+  test '#update - without edit_timer_sessions permission' do
+    user = User.find(2)
+    user.roles.first.remove_permission! :stop_timer_sessions
+    @controller.logged_user = user
+    patch :update, params: { timer_session: { comments: 'Very interesting' } }, xhr: true
+    assert_response 403
+  end
+
+  test '#destroy - without delete_timer_sessions permission' do
+    user = User.find(2)
+    user.roles.first.remove_permission! :cancel_timer_sessions
+    @controller.logged_user = user
+    delete :destroy, params: { timer_session: { comments: 'Very interesting' } }, xhr: true
+    assert_response 403
+  end
 end