Enhance static page with group user accessibility #7707

wangf1122 · 2024-02-06T21:51:05Z

This enhanced feature gives the administrative user the capability to config the static page to some specific user in some groups.

I have two static page, one for the "sandbox" or "my_org" group users. One for all logged users

Either the sandbox_editor or org_editor logged in, the "org" page will display:

But when the "other_editor who has no access to sandbox or my_org group logged in, this user cannot see "org" page

So this feature is fully expandable to some other security measure scenario because the backend table column is named as access expression and will store JSON string by parsing from Java object AccessExpression. We can customize it by change the java business model itself.

ianwallen · 2024-02-07T10:47:05Z

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java

@@ -76,6 +81,9 @@ public class PagesAPI {

    private final PageRepository pageRepository;

+    @Autowired


Suggest to move autowire to the constructor

I don't find such practice within the whole Geonetwork project. Also the GN is using some old Spring version which I am little afraid of it does not support constructor type of injection. I would leave it for now to be consistent of overall geonetwork's coding standard.

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java

web-ui/src/main/resources/catalog/locales/en-v4.json

domain/src/main/java/org/fao/geonet/domain/page/Page.java

web-ui/src/main/resources/catalog/templates/admin/settings/static-pages.html

domain/src/main/java/org/fao/geonet/domain/page/Page.java

…nced' into main.page.user.accessbility.enhanced

wangf1122 · 2024-02-07T21:43:10Z

I am amending the feature to have multiple group selection option. Will post another version soon

domain/src/main/java/org/fao/geonet/domain/page/Page.java

web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v443/migrate-default.sql

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java

services/src/main/java/org/fao/geonet/api/pages/AccessExpression.java

josegar74 · 2024-03-04T15:47:00Z

@wangf1122 I'm testing the UI, but the groups panel doesn't seem enabled properly.

I create a new static page, the groups panel is disabled, but when selecting the value Visible to users belonging to the groups is not enabled, I have to select the value again.

The same when selecting other value, it's not disabled until I selected the value again.

wangf1122 · 2024-03-04T17:43:53Z

@wangf1122 I'm testing the UI, but the groups panel doesn't seem enabled properly.

I create a new static page, the groups panel is disabled, but when selecting the value Visible to users belonging to the groups is not enabled, I have to select the value again.

The same when selecting other value, it's not disabled until I selected the value again.

@josegar74

Did you refreshed your local CSS/JS cache? I dont seem to have such issue at my localhost. See below

josegar74 · 2024-03-05T08:47:49Z

@wangf1122 yes, I did that and cleanup also the browser cache.

See the attached screencast.

static-pages.webm

services/src/main/java/org/fao/geonet/api/pages/model/GroupAccessExpression.java

josegar74 · 2024-03-05T08:57:47Z

services/src/main/java/org/fao/geonet/api/pages/PageProperties.java

@@ -114,4 +123,12 @@ public String getIcon() {
    public void setIcon(String icon) {
        this.icon = icon;
    }
+
+    public List<String> getGroups() {


The main concern is about not using referential integrity with the groups table.

Could be maybe check as a similar example the User -> Address relationship:

core-geonetwork/domain/src/main/java/org/fao/geonet/domain/User.java

Lines 223 to 244 in 88cc425

/**

* Get all the user's addresses.

*

* @return all the user's addresses.

*/

@OneToMany(fetch = FetchType.EAGER, cascade = CascadeType.ALL, orphanRemoval = true)

@JoinTable(name = "UserAddress", joinColumns = @JoinColumn(name = "userid"), inverseJoinColumns = {@JoinColumn(name = "addressid",

referencedColumnName = "ID", unique = true)})

public Set<Address> getAddresses() {

return _addresses;

}

/**

* Set all the user's addresses.

*

* @param addresses all the user's addresses.

* @return this user object

*/

protected User setAddresses(Set<Address> addresses) {

this._addresses = addresses;

return this;

}

The challenge is the group is from JSON representation. We cannot just build the backend in pure JPA representation. The JSON representation will need to expand to future security representation not limited to groups but something else.

@wangf1122 - @josegar74 is suggesting that the json expression be replaced with a new pageGroup table that would be used to link the pages to the groups within the database. This will ensure that if a group or page is removed then the database will cascade the delete causing it to clean itself.

Otherwise with the json expression, there could be issues where it references groups that no longer exist.

@josegar74 @ianwallen

I have added the page/group joint table and the entity mapping. I got rid of the accessexpression JSON in this case. Please retest from your local.

…essExpression.java Co-authored-by: Jose García <[email protected]>

wangf1122 · 2024-03-05T16:53:03Z

@wangf1122 yes, I did that and cleanup also the browser cache.

See the attached screencast.

static-pages.webm

@josegar74

I see what you mean. I change the ui code from ng-click to ng-change. The javascript change will fit here better than click behavior

….com/wangf1122/core-geonetwork into main.page.user.accessbility.enhanced

ianwallen · 2024-03-28T15:48:18Z

web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v443/migrate-default.sql

@@ -4,3 +4,13 @@ INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system

 UPDATE Settings SET value='4.4.3' WHERE name='system/platform/version';
 UPDATE Settings SET value='0' WHERE name='system/platform/subVersion';
+
+CREATE TABLE spg_page_group


@wangf1122
I'm not sure if create tables are required in the migration as I think JPA will take care of creating any missing tables.

@josegar74
Can you confirm if this is required?

@josegar74 @ianwallen

I had confirmed myself. It is not required. The JPA will automatically create the table. I will update the script.

josegar74

@wangf1122 I tested the feature and looks fine. The only thing that was that I could delete a group that had pages linked to it, the group in the page it's still there.

It can be good when removing the group to delete the relation or just show some message when deleting the group to inform that has static pages related and need to remove that relation first.

domain/src/main/java/org/fao/geonet/domain/page/Page.java

josegar74 · 2024-04-30T10:27:55Z

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java

@@ -203,6 +227,30 @@ private ResponseEntity<Void> updatePageInternal(@NotNull String language,
        String newLabel = pageProperties.getLabel();
        String newIcon = pageProperties.getIcon();

+        Set<Group> _groups = new LinkedHashSet<>();


I see this variable with the _groups and another with the name groups. I would rename them to have more meaningful names and remove the _ from this one.

renamed to pageGroups. Please check

josegar74 · 2024-04-30T10:30:15Z

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java

+            for (String groupName : pageProperties.getGroups()) {
+                Group group = groupRepository.findByName(groupName);
+
+                Group groupToAdd= new Group();


What is the reason to do a copy of the group?

I had trouble with the JPA not adding group if not doing copy. Cannot guarantee if the code will work without the copying

Co-authored-by: Jose García <[email protected]>

wangf1122 · 2024-05-07T23:11:44Z

@wangf1122 I tested the feature and looks fine. The only thing that was that I could delete a group that had pages linked to it, the group in the page it's still there.

It can be good when removing the group to delete the relation or just show some message when deleting the group to inform that has static pages related and need to remove that relation first.

@josegar74

I have added the logic in group delete API to prevent the deletion if the group has associate with static pages. Similar logic like users and groups

josegar74

I notice also issues when saving the pages:

Create a page that has a status not related to groups.
Edit it and change to related to groups and assign it to 1 group --> it works
Edit it and change to related to groups and assign it to 2 group --> it doesn't work, it maintains the previous configuration. No error reported.

It seems happening also when creating a page assigned to 2 or more groups.

josegar74 · 2024-07-10T08:44:33Z

services/src/main/java/org/fao/geonet/api/groups/GroupsApi.java

@@ -541,6 +547,21 @@ public void deleteGroup(
                ));
            }

+            List<Page> staticPages = pageRepository.findAll();


I would add this method in the repository:

List<Page> findPageByStatus(Page.PageStatus status);

And update the code to the following to at least only process the pages that are assigned to groups:

List<Page> staticPages = pageRepository.findPageByStatus(Page.PageStatus.GROUPS); List<Page> staticPagesAssignedToGroup = staticPages.stream().filter(p -> !p.getGroups().stream().filter(g -> g.getId() == groupIdentifier).collect(Collectors.toList()).isEmpty()) .collect(Collectors.toList()); if (!staticPagesAssignedToGroup.isEmpty()) { throw new NotAllowedException(String.format( "Group %s is associated with '%s' static page(s). Please remove the static page(s) associated with that group first.", group.get().getName(), staticPagesAssignedToGroup.stream().map(p -> p.getLabel()).collect(Collectors.joining()) )); }

@josegar74

I have added the code. Also the issue you are experiencing "Edit it and change to related to groups and assign it to 2 group --> it doesn't work, it maintains the previous configuration. No error reported." was due to the backend entity relation. It's supposed to be ManyToMany relation. Otherwise, the groupId in the spg_page_group table will be unique. So once one page occupied groupId_1, this groupId_1 will not be available to other pages. Once I have the ManyToMany relation, it does. Here is some example of my local database after the fix:

josegar74

Tested the latest changes and look ok.

geonetworkbuild · 2024-11-07T12:24:14Z

The backport to 4.2.x failed:

The process '/usr/bin/git' failed with exit code 1

stderr

error: could not apply 6d1fd4c529... Add group/accessExpression field to static page feature
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

stdout

Auto-merging domain/src/main/java/org/fao/geonet/domain/page/Page.java
CONFLICT (content): Merge conflict in domain/src/main/java/org/fao/geonet/domain/page/Page.java
Auto-merging services/src/main/java/org/fao/geonet/api/pages/PageProperties.java
CONFLICT (content): Merge conflict in services/src/main/java/org/fao/geonet/api/pages/PageProperties.java
Auto-merging services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java
CONFLICT (content): Merge conflict in services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java
Auto-merging web-ui/src/main/resources/catalog/locales/en-v4.json
Auto-merging web-ui/src/main/resources/catalog/templates/admin/settings/static-pages.html
CONFLICT (modify/delete): web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v443/migrate-default.sql deleted in HEAD and modified in 6d1fd4c529 (Add group/accessExpression field to static page feature).  Version 6d1fd4c529 (Add group/accessExpression field to static page feature) of web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v443/migrate-default.sql left in tree.

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.2.x 4.2.x
# Navigate to the new working tree
cd .worktrees/backport-4.2.x
# Create a new branch
git switch --create backport-7707-to-4.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 6d1fd4c5294d5398d49d96c535782cd3f08879b7,99e1e5486e83818aae6b8ce96efbbfcc23d14990,04a25216fe531dddfb50b6fc724f123f7f394923,214af08ef35cde02b1660b4c219c57da49216a80,e09d0c204a39ce58727ed6cffd5f2e037239853b,47a5ee200daf9a878d12cf85baf17e01a2d09727,db1b7db3c8c4cb97d487bf1498b6363cbd4afb3f,fd7c7b125de114437e02e7e27c02e799513d7967,c98548ab4325c20b528ae71eacd5d6b5738dc363,de04d9a052e450c653c6c052f8acdb49ab29001d,30a0b2b6597488b2af57f835c8962564e6d446bd,bc3ffb344b42190682cbc5c9abcb5a3fc487cad1,246b184e4fb3a7e5c55fdc334eff00a64936f0cb,8b10f742f2155d833f631f0beb86939f0ec3711b,864c2cbba01c5959e76c53bd4be063e7355cb4a9,a0a5534b18b8699e237efaadf636e375d2e4cbdd,7d8e03d18482180ef63181732dff3ad207b473bb,c93bade75516e2d6af099faab3171ebb662ceb61,7c056cf2c4f2b4e20ccd50faadbf57679b22b58b,9ca02c8784176e0f1c437b4a57ed109f2c0bf27d,262fa60115fe62817a1671d529bcd77f5430a25c,2d82af005440808ccbc3b23d3036fd4b350dcd81,56ac85c552ae4e2cf0ecb23b99915ad48e4d2730,292d127de196f944ef5ac29390c2141d1edc517c
# Push it to GitHub
git push --set-upstream origin backport-7707-to-4.2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.2.x

Then, create a pull request where the base branch is 4.2.x and the compare/head branch is backport-7707-to-4.2.x.

* Add group/accessExpression field to static page feature * Filter none group member access to page belong to certain group * code improvement * multi selections for groups * multi selections for groups. * Refactoring group checking logic * Update services/src/main/java/org/fao/geonet/api/pages/model/GroupAccessExpression.java Co-authored-by: Jose García <[email protected]> * ui change menu behavior * Fixing ui issue * pageGroup joint relation in page entity * pageGroup joint relation in page entity (add SQL) * remove page creation script * Update domain/src/main/java/org/fao/geonet/domain/page/Page.java Co-authored-by: Jose García <[email protected]> * rename group * prevent group remove if it has association with static pages * JPA ManyToMany bug fix --------- Co-authored-by: Jose García <[email protected]>

* Add group/accessExpression field to static page feature * Filter none group member access to page belong to certain group * code improvement * multi selections for groups * multi selections for groups. * Refactoring group checking logic * Update services/src/main/java/org/fao/geonet/api/pages/model/GroupAccessExpression.java * ui change menu behavior * Fixing ui issue * pageGroup joint relation in page entity * pageGroup joint relation in page entity (add SQL) * remove page creation script * Update domain/src/main/java/org/fao/geonet/domain/page/Page.java * rename group * prevent group remove if it has association with static pages * JPA ManyToMany bug fix --------- Co-authored-by: Jose García <[email protected]>

wangf1122 and others added 3 commits February 2, 2024 21:35

Add group/accessExpression field to static page feature

6d1fd4c

Filter none group member access to page belong to certain group

99e1e54

Merge branch 'geonetwork:main' into main.page.user.accessbility.enhanced

04a2521

wangf1122 marked this pull request as ready for review February 6, 2024 22:05

ianwallen reviewed Feb 7, 2024

View reviewed changes

ianwallen added the backport 4.2.x label Feb 7, 2024

ianwallen added this to the 4.4.3 milestone Feb 7, 2024

ianwallen added the enhancement label Feb 7, 2024

wangf1122 added 2 commits February 7, 2024 12:24

code improvement

214af08

Merge remote-tracking branch 'origin/main.page.user.accessbility.enha…

e09d0c2

…nced' into main.page.user.accessbility.enhanced

wangf1122 marked this pull request as draft February 7, 2024 21:42

wangf1122 added 2 commits February 7, 2024 20:35

multi selections for groups

47a5ee2

multi selections for groups.

db1b7db

wangf1122 marked this pull request as ready for review February 9, 2024 15:05

ianwallen reviewed Feb 13, 2024

View reviewed changes

domain/src/main/java/org/fao/geonet/domain/page/Page.java Outdated Show resolved Hide resolved

ianwallen reviewed Feb 13, 2024

View reviewed changes

web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v443/migrate-default.sql Outdated Show resolved Hide resolved

ianwallen reviewed Feb 13, 2024

View reviewed changes

services/src/main/java/org/fao/geonet/api/pages/PagesAPI.java Outdated Show resolved Hide resolved

ianwallen reviewed Feb 13, 2024

View reviewed changes

services/src/main/java/org/fao/geonet/api/pages/AccessExpression.java Outdated Show resolved Hide resolved

wangf1122 and others added 2 commits February 13, 2024 14:29

Refactoring group checking logic

fd7c7b1

Merge branch 'main' into main.page.user.accessbility.enhanced

c98548a

ianwallen requested a review from josegar74 February 26, 2024 11:56

josegar74 reviewed Mar 5, 2024

View reviewed changes

wangf1122 and others added 2 commits March 5, 2024 09:11

Update services/src/main/java/org/fao/geonet/api/pages/model/GroupAcc…

de04d9a

…essExpression.java Co-authored-by: Jose García <[email protected]>

ui change menu behavior

30a0b2b

fxprunayre removed this from the 4.4.3 milestone Mar 13, 2024

wangf1122 added 2 commits March 27, 2024 22:57

Merge branch 'main.page.user.accessbility.enhanced' of https://github…

864c2cb

….com/wangf1122/core-geonetwork into main.page.user.accessbility.enhanced

pageGroup joint relation in page entity (add SQL)

a0a5534

ianwallen reviewed Mar 28, 2024

View reviewed changes

remove page creation script

7d8e03d

ianwallen mentioned this pull request Apr 2, 2024

Enhance the workflowAssistApps so that third party links can have a check access url #6994

Open

fxprunayre modified the milestones: 4.4.4, 4.4.5 Apr 16, 2024

josegar74 reviewed Apr 30, 2024

View reviewed changes

wangf1122 and others added 5 commits April 30, 2024 09:08

Merge branch 'geonetwork:main' into main.page.user.accessbility.enhanced

c93bade

Update domain/src/main/java/org/fao/geonet/domain/page/Page.java

7c056cf

Co-authored-by: Jose García <[email protected]>

rename group

9ca02c8

Merge branch 'geonetwork:main' into main.page.user.accessbility.enhanced

262fa60

prevent group remove if it has association with static pages

2d82af0

ianwallen requested a review from josegar74 May 8, 2024 10:42

josegar74 modified the milestones: 4.4.5, 4.4.6 Jun 3, 2024

josegar74 reviewed Jul 10, 2024

View reviewed changes

wangf1122 and others added 2 commits July 10, 2024 13:04

Merge branch 'geonetwork:main' into main.page.user.accessbility.enhanced

56ac85c

JPA ManyToMany bug fix

292d127

wangf1122 requested a review from josegar74 July 11, 2024 20:53

josegar74 approved these changes Sep 11, 2024

View reviewed changes

ianwallen modified the milestones: 4.4.6, 4.4.7 Oct 8, 2024

ianwallen merged commit bedfe3d into geonetwork:main Nov 7, 2024
6 checks passed

geonetworkbuild added the failed backport label Nov 7, 2024

wangf1122 mentioned this pull request Nov 7, 2024

[Backport 4.2.x] Enhance static page with group user accessibility #8491

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance static page with group user accessibility #7707

Enhance static page with group user accessibility #7707

wangf1122 commented Feb 6, 2024 •

edited

Loading

ianwallen Feb 7, 2024

wangf1122 Feb 7, 2024

wangf1122 commented Feb 7, 2024

josegar74 commented Mar 4, 2024

wangf1122 commented Mar 4, 2024

josegar74 commented Mar 5, 2024

josegar74 Mar 5, 2024

wangf1122 Mar 5, 2024

ianwallen Mar 24, 2024

wangf1122 Mar 28, 2024

wangf1122 commented Mar 5, 2024

ianwallen Mar 28, 2024

wangf1122 Apr 1, 2024

josegar74 left a comment

josegar74 Apr 30, 2024

wangf1122 Apr 30, 2024 •

edited

Loading

josegar74 Apr 30, 2024

wangf1122 Apr 30, 2024

wangf1122 commented May 7, 2024

josegar74 left a comment

josegar74 Jul 10, 2024

wangf1122 Jul 11, 2024

josegar74 left a comment

geonetworkbuild commented Nov 7, 2024

		@@ -76,6 +81,9 @@ public class PagesAPI {

		private final PageRepository pageRepository;

		@Autowired

	/**
	* Get all the user's addresses.
	*
	* @return all the user's addresses.
	*/
	@OneToMany(fetch = FetchType.EAGER, cascade = CascadeType.ALL, orphanRemoval = true)
	@JoinTable(name = "UserAddress", joinColumns = @JoinColumn(name = "userid"), inverseJoinColumns = {@JoinColumn(name = "addressid",
	referencedColumnName = "ID", unique = true)})
	public Set<Address> getAddresses() {
	return _addresses;
	}

	/**
	* Set all the user's addresses.
	*
	* @param addresses all the user's addresses.
	* @return this user object
	*/
	protected User setAddresses(Set<Address> addresses) {
	this._addresses = addresses;
	return this;
	}

Enhance static page with group user accessibility #7707

Enhance static page with group user accessibility #7707

Conversation

wangf1122 commented Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangf1122 commented Feb 7, 2024

josegar74 commented Mar 4, 2024

wangf1122 commented Mar 4, 2024

josegar74 commented Mar 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangf1122 commented Mar 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josegar74 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangf1122 Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangf1122 commented May 7, 2024

josegar74 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josegar74 left a comment

Choose a reason for hiding this comment

geonetworkbuild commented Nov 7, 2024

wangf1122 commented Feb 6, 2024 •

edited

Loading

wangf1122 Apr 30, 2024 •

edited

Loading