Added missing permissions filter for group owner

[joachimnielandt]

The following situation breaks due to this missing functionality:

• have a record with groupOwner A and owner X
• group A has users X and Y
• Y can see the edit button, but upon clicking it the UI reports the record cannot be found

I have added the missing check (marked as TODO) on groupOwner which solves the issue on our end. @fxprunayre , does this approach make sense, am I missing a potential case in the permissions filter?

Checklist

• I have read the contribution guidelines
• Pull request provided for main branch, backports managed with label
• Good housekeeping of code, cleaning up comments, tests, and documentation
• Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
• Clean commit messages, longer verbose messages are encouraged
• API Changes are identified in commit messages
• Testing provided for features or enhancements using automatic tests
• User documentation provided for new features or enhancements in manual
• Build documentation provided for development instructions in README.md files
• Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

[fxprunayre]

I can't reproduce @joachimnielandt , what are the privileges of the record? I have

• record "X RECORD" created by user X in group A
• user X and Y member of A as editor
• Y can edit and save the record with the following privileges

[joachimnielandt]

I think in this case (for some reason) there were no privileges attached to the record. However, I'd argue that the semantics of 'groupOwner' would act the same way as 'owner' for a record, giving those entities implicit privileges on the record?

In our case, ownership of a record is not defined by the user but through the group to which the user belongs to. It makes sense then to always have the record available to you, even if the privileges were modified.

If this change would not be relevant to the core, perhaps the TODOES should be removed instead :).

[fxprunayre]

I think in this case (for some reason) there were no privileges attached to the record.

If there were no privileges for the record, then user Y can't see the record currently. So I can't reproduce the case were the edit button is present and editor fails.

However, I'd argue that the semantics of 'groupOwner' would act the same way as 'owner' for a record, giving those entities implicit privileges on the record?

Checking
https://docs.geonetwork-opensource.org/4.2/user-guide/publishing/managing-privileges/#assigning-privileges

"A content reviewer can view a metadata if: The metadata is part of a group that the user is a member of." so here "is part" probably means groupOwner and indeed your fix applies.

"A user administrator or an editor can view: All metadata that has the view privilege selected for one of the groups they are a member of." and with your fix, the record will also be displayed in this case.

So the changes compared to current situation with this fix are:

• Reviewer will be able to view and edit all records in their groups (even if view/edit privileges are not set)
• Editor will be able to view all records in their groups (even if view privilege is not set)

@josegar74 any opinion about this change?

[joachimnielandt]

If there were no privileges for the record, then user Y can't see the record currently. So I can't reproduce the case were the edit button is present and editor fails.

I could get this situation (with core/main), @fxprunayre :

• create a record with an admin account for group A and publish it
• remove all privileges on the record, keep all:publish, all:interactivemap, all:download
• now have a user X in group A with reviewer right
• X can see the record and has an edit button enabled
• upon clicking the edit button as X, the draft copy is created but an error is shown afterwards to indicate the record cannot be accessed

For our own fork we have made some modifications with regards to the edit button display logic, but that doesn't seem to be the reason here.

[fxprunayre]

upon clicking the edit button as X, the draft copy is created but an error is shown afterwards to indicate the record cannot be accessed

Indeed the editor fails to find the draft because the draft is not publish to all - so is not returned for the reviewer search.

So the changes compared to current situation with this fix are:

• Reviewer user will be able to view and edit all records in their groups (even if view/edit privileges are not set)
• Editor user will be able to view all records in their groups (even if view privilege is not set)
• Editor will properly open for reviewer when a record is published to all and no edit privilege is set.

[josegar74]

@fxprunayre

• Reviewer user will be able to view and edit all records in their groups (even if view/edit privileges are not set)
• Editor user will be able to view all records in their groups (even if view privilege is not set)

So this means that Reviewer profile will be able to edit even if no Edit privilege, but Editor profile no?

• Editor user will be able to view all records in their groups (even if view privilege is not set)

It looks strange to me that a metadata has no view privileges in it's group owner.

• Editor will properly open for reviewer when a record is published to all and no edit privilege is set.

Is this similar to the 1st point?

---

Not for this pull request, but I think we should try to improve / simplify the way to assign permissions, currently some configurations that are not very orthodox can be created:

1. Does it make sense to assign the Edit privilege only without the View privilege?
2. Does it make sense to assign the Download / Interactive map privilege without the View privilege?
3. Does it make sense that the group owner doesn't have at least the View privilege?

Another question that would be good to clarify:

1. Does it make sense the Download / Interactive map privileges other than for the special groups?

[josegar74]

Tested the changes and works as described, but I don't fully understand why a Reviewer would be allowed to edit the metadata, but not the Editor if both are in the metadata group owner.

[joachimnielandt]

As we (Flanders) have customised the workflow / status handling, the above questions are not really applicable to our case: we use the concept of a groupOwner to specify ownership, then consider Editor and Reviewer to check what they can do (in our case, the Reviewer has some extra power in the workflow).

This PR would solve the base behaviour of what it means to be part of the groupOwner, as there is some ambiguity here currently. Our interpretation is that it is functionally equivalent to being the owner. We do limit the ability to modify the workflow, depending on what role you have. Privileges are used to extend access beyond owner and groupOwner.

[fxprunayre]

Yes quite some questions about the privileges panels @josegar74!

While working in Flanders workflow, I also quite liked the way it manage the privileges by using the workflow steps (and the privilege panel is less needed).

For now, I would merge this PR which solve the base behaviour as @joachimnielandt said.

On the long run, maybe we should have a demo of the Flanders workflow and decide if it is not a better way to simplify publication of records and use something like this by default.

Does it make sense to assign the Edit privilege only without the View privilege?

Probably not.

Does it make sense to assign the Download / Interactive map privilege without the View privilege?

No.

Does it make sense the Download / Interactive map privileges other than for the special groups?

Some users are using this but it is quite rare.

Does it make sense that the group owner doesn't have at least the View privilege?

This was used sometimes so than only the author (and admin) can find/edit the record until ready. But maybe we can drop this.

Another question:

• Anyone added new operation ? (I know one catalogue only)

There is room for improvements!