Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix time tracker permissions #150

Merged
merged 4 commits into from
Oct 15, 2024

Conversation

CuddlyBunion341
Copy link
Contributor

@CuddlyBunion341 CuddlyBunion341 commented Oct 9, 2024

Testing on develop showed that timer sessions cannot be managed by regular user accounts.

In the refactoring PR #142, I forgot to update the controller permissions in the plugin initializer.
Because the tests used the admin user and because the local testing was also done with the admin account, this issue never showed up.

Todo

  • Update tests to use non-admin user
  • Add regression tests for missing role
  • Update views with new permissions

@CuddlyBunion341 CuddlyBunion341 self-assigned this Oct 9, 2024
@CuddlyBunion341 CuddlyBunion341 marked this pull request as draft October 9, 2024 07:46
Comment on lines 9 to 13
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the 403 get rendered because of the plugin initializer?

@@ -19,8 +19,8 @@ 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
@controller.logged_user = User.find(2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test on a user with an associated project, instead of the administrator.

@CuddlyBunion341 CuddlyBunion341 force-pushed the feature/19656-fix-time-tracker-permissions branch from 4c7f161 to b49fdfc Compare October 9, 2024 08:08
@CuddlyBunion341 CuddlyBunion341 marked this pull request as ready for review October 9, 2024 08:14
@schmijos schmijos merged commit 2a176f7 into main Oct 15, 2024
1 check passed
@schmijos schmijos deleted the feature/19656-fix-time-tracker-permissions branch October 15, 2024 13:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants