Skip to content

Fix As NON-ADMIN user not possible to manage own resource#11421

Merged
allyoucanmap merged 3 commits intogeosolutions-it:masterfrom
stefanocudini:11371-non-admin-user-not-manage-own-resource
Sep 10, 2025
Merged

Fix As NON-ADMIN user not possible to manage own resource#11421
allyoucanmap merged 3 commits intogeosolutions-it:masterfrom
stefanocudini:11371-non-admin-user-not-manage-own-resource

Conversation

@stefanocudini
Copy link
Copy Markdown
Member

@stefanocudini stefanocudini commented Sep 1, 2025 •
edited
Loading

Description

fix #11371
fix lose of user owner on save resource in GeoStore module. The fix tries to keep the user attribute fixed and not lost in case the group permissions are changed, so the resource owner can still modify it.

here is defined the bug replication conditions and other considerations about server side:
#11371 (comment)

And the result or PR:
https://github.com/user-attachments/assets/bb3e922f-e324-4f09-9c33-b6bfb27661ae

The bug of resource permissions is solved, but some tests fail that I couldn't fix:

  • computePendingChanges: in utils/__tests__/GeostoreUtils-js?:349:76
  • getPendingChanges in ResourcesCatalog/selectors/__tests__/save-test.js?:34:9

The test payload is probably wrong in new conditions.

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

@stefanocudini stefanocudini requested a review from MV88 September 1, 2025 17:09
@stefanocudini stefanocudini self-assigned this Sep 1, 2025
@tdipisa tdipisa requested review from allyoucanmap and removed request for MV88 September 2, 2025 08:18
@tdipisa tdipisa added this to the 2025.01.02 milestone Sep 2, 2025
@stefanocudini stefanocudini marked this pull request as draft September 2, 2025 08:21
@stefanocudini stefanocudini added bug BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch labels Sep 2, 2025
@stefanocudini stefanocudini force-pushed the 11371-non-admin-user-not-manage-own-resource branch from b54786d to f7a8c1a Compare September 2, 2025 14:37
@stefanocudini stefanocudini marked this pull request as ready for review September 2, 2025 14:38
@stefanocudini stefanocudini marked this pull request as draft September 2, 2025 14:55
@stefanocudini stefanocudini force-pushed the 11371-non-admin-user-not-manage-own-resource branch from f7a8c1a to 5fb24f1 Compare September 3, 2025 08:14
@tdipisa tdipisa assigned allyoucanmap and unassigned stefanocudini Sep 3, 2025
@stefanocudini stefanocudini marked this pull request as ready for review September 3, 2025 13:06
allyoucanmap
allyoucanmap requested changes Sep 8, 2025
View reviewed changes
@tdipisa
Copy link
Copy Markdown
Member

tdipisa commented Sep 8, 2025

Dear @stefanocudini it seems I can't reproduce the problem either by following steps in issue description here nor the ones in this comment. I can only suppose some strange race condition in managing permissions.
Anyway, the review of @allyoucanmap above seems consistent with the problem investigated by @rowheat02 here. It make sense to follow @allyoucanmap indications definitely. I saw you only need to fix for failing checks. Please go ahead and finalize the PR. Thank you.

@MV88 MV88 assigned stefanocudini and unassigned allyoucanmap Sep 9, 2025
@stefanocudini stefanocudini force-pushed the 11371-non-admin-user-not-manage-own-resource branch from b78809d to 32eb2d4 Compare September 9, 2025 14:57
@stefanocudini
Copy link
Copy Markdown
Member Author

stefanocudini commented Sep 9, 2025 •
edited
Loading

Dear @stefanocudini it seems I can't reproduce the problem either by following steps in issue description here nor the ones in this comment. I can only suppose some strange race condition in managing permissions. Anyway, the review of @allyoucanmap above seems consistent with the problem investigated by @rowheat02 here. It make sense to follow @allyoucanmap indications definitely. I saw you only need to fix for failing checks. Please go ahead and finalize the PR. Thank you.

last commit apply the review suggestions by @allyoucanmap, testing in local this fix the original bug #11371

the CI Fails is caused only by this #11441 just fixed

@stefanocudini stefanocudini mentioned this pull request Sep 9, 2025
Closed
1 task
allyoucanmap
allyoucanmap approved these changes Sep 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@allyoucanmap allyoucanmap left a comment

Choose a reason for hiding this comment

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

@tdipisa reported an additional problem on the save workflow where the permissions were not restored. There is already a commit to fix this here 77f6e8e

  • steps to reproduce the problem:
    • login as an user
    • create a map
    • open the resource details panel and select the permissions panel
    • click on edit properties button
    • edit permissions adding everyone group
    • click again on edit properties button to exit editing
    • click on save
    • Current: No permission available message
    • Expected: The permissions are still visible in the panel

@stefanocudini
Copy link
Copy Markdown
Member Author

stefanocudini commented Sep 10, 2025 •
edited
Loading

@tdipisa reported an additional problem on the save workflow where the permissions were not restored. There is already a commit to fix this here 77f6e8e

Dear @allyoucanmap I was investigating issue #11436 as noted below to be a related to this issue #11421
Actually, trying this additional attribute withPermissions also resolves the issue #11436!

@tdipisa this commit could solve also #11436 in this same PR

@tdipisa
Copy link
Copy Markdown
Member

tdipisa commented Sep 10, 2025

No worries guys @allyoucanmap @stefanocudini I see we are aligned. @stefanocudini you can simply connect the issue #11436 to this PR then. Thank you.

@stefanocudini stefanocudini linked an issue Sep 10, 2025 that may be closed by this pull request
One case where the Permissions panel is not properly updated #11436
Closed
1 task
@allyoucanmap allyoucanmap merged commit 4e168bc into geosolutions-it:master Sep 10, 2025
6 checks passed
@allyoucanmap
Copy link
Copy Markdown
Contributor

allyoucanmap commented Sep 10, 2025

@ElenaGallo please test this fix on dev and let us know when we can backport, thanks

@ElenaGallo
Copy link
Copy Markdown
Contributor

ElenaGallo commented Sep 10, 2025

Test passed, @stefanocudini please backport to 2025.01.xx. Thanks

stefanocudini added a commit to stefanocudini/MapStore2 that referenced this pull request Sep 11, 2025
@stefanocudini @allyoucanmap
Fix geosolutions-it#11371 & geosolutions-it#11436 As NON-ADMIN user n…
714e214 
…ot possible to manage own resource (geosolutions-it#11421)

---------

Co-authored-by: allyoucanmap <stefano.bovio@geosolutionsgroup.com>
MV88 pushed a commit that referenced this pull request Sep 11, 2025
@stefanocudini @allyoucanmap
Fix #11371 & #11436 As NON-ADMIN user not possible to manage own reso…
8bcd0ab 
…urce (#11421) (#11462)

---------

Co-authored-by: allyoucanmap <stefano.bovio@geosolutionsgroup.com>
@stefanocudini
Copy link
Copy Markdown
Member Author

stefanocudini commented Sep 11, 2025

Test passed, @stefanocudini please backport to 2025.01.xx. Thanks

@ElenaGallo backport merged now

@tdipisa tdipisa removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@allyoucanmap allyoucanmap allyoucanmap approved these changes

Assignees

@stefanocudini stefanocudini

Labels

bug

Projects

None yet

Milestone

2025.01.02

5 participants

@stefanocudini @tdipisa @allyoucanmap @ElenaGallo @MariaBanti