Skip to content

Conversation

camrrx
Copy link
Member

@camrrx camrrx commented Sep 22, 2025

Proposed changes

  • Delete the checkUserAccess and checkOrganizationAccess methods to manage all permissions in the permissionService
  • If we don't have permissions on platform-settings, it's not possible to create new organizations

Testing Instructions

  1. Login to user with bypass capability
  2. Try to create/update and delete a player

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality
  • For bug fix -> I implemented a test that covers the bug

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@camrrx camrrx changed the title Issue/375 manage players rbac [backend/frontend] Manage players Sep 23, 2025
@camrrx camrrx changed the title [backend/frontend] Manage players [backend/frontend] Manage players - permissions Sep 23, 2025
@camrrx camrrx marked this pull request as ready for review September 23, 2025 12:31
checkUserAccess(userRepository, userId);
User user = userRepository.findById(userId).orElseThrow(ElementNotFoundException::new);
if (!currentUser().isAdmin() && user.isManager()) {
if (!userService.currentUser().isAdminOrBypass() && user.isManager()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

that should be removed also, the rbac is enough

public Page<Organization> organizationPagination(
@NotNull SearchPaginationInput searchPaginationInput) {
User currentUser = userService.currentUser();
if (currentUser.isAdminOrBypass()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can also remove the if here

Copy link
Contributor

Choose a reason for hiding this comment

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

there is some logic to add in PermissionService also -> if resourceType - PLAYER or USER and userId is resourceId -> allow the call

Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.23%. Comparing base (b27632c) to head (0f9eeac).
⚠️ Report is 7 commits behind head on release/current.

Files with missing lines Patch % Lines
...nbas/service/organization/OrganizationService.java 0.00% 3 Missing ⚠️
.../io/openbas/rest/organization/OrganizationApi.java 0.00% 2 Missing ⚠️
...n/java/io/openbas/rest/statistic/StatisticApi.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##             release/current    #4059      +/-   ##
=====================================================
- Coverage              47.23%   47.23%   -0.01%     
+ Complexity              3252     3247       -5     
=====================================================
  Files                    840      840              
  Lines                  25187    25148      -39     
  Branches                1819     1814       -5     
=====================================================
- Hits                   11898    11878      -20     
+ Misses                 12592    12578      -14     
+ Partials                 697      692       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@camrrx camrrx requested a review from heditar September 25, 2025 14:52
@camrrx camrrx added the filigran team use to identify PR from the Filigran team label Sep 26, 2025
@camrrx camrrx merged commit 4a0f1bd into release/current Sep 29, 2025
10 checks passed
@camrrx camrrx deleted the issue/375-manage-players-rbac branch September 29, 2025 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team use to identify PR from the Filigran team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants