#10923, #10924: Disabling GeoFence Rules and Add filter by IP for GeoFence rules#10925
Conversation
geosolutions-it#10924: Disabling GeoFence Rules Description: - adding IPAddress filter in rules manage grid - adding Date filter in rules manage grid for validity period fields - add validity period fields for rule insertion/update - add unit tests - add translations
geosolutions-it#10924: Disabling GeoFence Rules Description: - fix FE failure unit test
|
@tdipisa by they way. Disappeared on dev https://dev-mapstore.geosolutionsgroup.com/mapstore/docs/api/plugins#pages.RulesManager Trying to apply tthe same config, I have this result
So we have to update the doc properly to explain how to setup rule-manager. The new correct config, copied from dev instance, is this: If documented somewhere, it should be linked in rules-manager plugin. I had to create an instance by sending the following post request curl --location 'http://localhost:8080/geofence/rest/instances' \
--header 'Content-Type: application/json' \
' \
--data '{
"name": "GeoServer",
"description": "",
"baseURL": "http://localhost:8080/geoserver",
"username": "admin",
"password": "geoserver"
}'It returns the ID of the instance in plain text (e.g. You can get the current configured instaces with this call: The ID should be used in configrued |
There was a problem hiding this comment.
Testing this PR widely is very hard by per-se, because we should need to test many use cases:
- Standalone new version (nightly)
- Embedded new version (nightly)
- Standalone old version (I suppose this tool needs to work with current and old versions of geoserver/geofence too)
- Embedded old version (I suppose this tool needs to work with current and old versions of geoserver/geofence too)
@mahmoudadel54 did you have a chance to test all these use cases?
Moreover configuring everything is not easy. I had the opporunity, in the time I had dedicated on it, to test only the standalone version.
I had a lot of crashes to report, so I stopped reviewing after reporting 3 different crashes, waiting for a more stable version.
Testing all the version should not be up to me, but we could provide a structure to switch and test easely the various version to testers/develoers. If we want this to be maintaiable, a good idea can be to create a docker dedicated, or nice instructions to setup the test env.
Here the points I report.
- See the comment for configuration and documentation above.
- Because geofence improvemnts are still on nightly, we have to ensure that it will work also for existing version of geoserver with compatibility, I didn't test it, but we have to proof this change will not be distruptive. Maybe add an opt-in ?
- I noticed this bug, not sure it is related to the PR, but on dev is not reroducible.
MapStore.mp4
StandardRouter.jsx:88 TypeError: Cannot read properties of null (reading 'scrollTop')
at DataGrid.eval [as scrollListener] (DataGrid.jsx:29:35)
at RulesGrid.eval [as componentDidUpdate] (RulesGrid.jsx:46:19)
- The
ipAddressAnyparameter do not have any effect. I'd expect it to filter by precise IP (range) vs if the rule is applied. Verify if it makes sense, or if it doesn't show it checked and disable. The same looks to be
MapStore.1.mp4
- See the video above (for ipAddressAny, I had a crash while recording .
StandardRouter.jsx:88 TypeError: Cannot read properties of undefined (reading 'id')
at eval (react-data-grid-addons.js:9672:39)
at Array.filter (<anonymous>)
at CustomDragLayer.isDraggedRowSelected (react-data-grid-addons.js:9671:30)
at CustomDragLayer.getDraggedRows (react-data-grid-addons.js:9690:29)
at CustomDragLayer.renderDraggedRows (react-data-grid-addons.js:9702:20)
at CustomDragLayer.render (react-data-grid-addons.js:9745:31)
at finishClassComponent (react-dom.development.js:18407:31)
at updateClassComponent (react-dom.development.js:18360:24)
at beginWork$1 (react-dom.development.js:20108:16)
at HTMLUnknownElement.callCallback (react-dom.development.js:362:14)
- Another crash while playing on date filter.
MapStore.2.mp4
StandardRouter.jsx:88 TypeError: Cannot destructure 'state.filters' as it is undefined.
at rulesmanager (rulesmanager.js:161:15)
at combination (combineReducers.js:126:29)
at reduce (StateUtils.js:256:14)
at standardRootReducerFunc (defaultOptions.js:79:8)
at rootReducer (StandardStore.js:90:12)
thank you for informing about that. I think we have to check why and restore, then. Please take care of this as part of this PR @mahmoudadel54, if possible. |
|
geosolutions-it#10924: Disabling GeoFence Rules Description: - fix bug of change date filter manually - add tooltip for date fields in insert new rule or date filter in rules manager - fix the crashing app issue of remove add rules in the grid - add translations
|
@tdipisa @offtherailz after the changes and clean up of JSDoc (#10814) that includes the automated script to list plugins now only plugins will checked and go in the plugins doc. The RulesManager is a page and not a plugin so afterward I moved it in the framework doc (#10877) so in the new doc for this page is here https://dev-mapstore.geosolutionsgroup.com/mapstore/docs/api/framework#pages.RulesManager. If we want to move it again inside the plugins doc we need to improve a bit the jsDocUtils.js to include pages automatically inside the plugins documentation |
|
So @mahmoudadel54 for the moment linking (with a relative path) the plugin to the page doc is enough. Something like: "This Plugin requires the configuration of the rules-manager page (link) to be activated. " |
There was a problem hiding this comment.
LGTM. I didn't tested on GeoServer integrated version, but it can be tested as well as deployed on dev.
There is only 1 bug I noticed that have to be fixed ON BACKEND. @etj
- There is a bug on filtering checkboxes. It happens only on IP and Date columns. Here in the video I show the IP's one, but the same can be done on date column
bug_rule_cb.mp4
The backend replies on dateAny params true or false in the same way, but for ipAddressAny or dateAny = true, it doesn't return any value. For the other parameters (e.g. requestAny = true) it simply ignores the param and works by returning all the values (because we don't have the corrispondent parameter to check).
basically this do not return anything:
http://localhost:8081/geofence/rest/rules/count?dateAny=true&layerAny=true&ipAddressAny=true&requestAny=true
and this returns everything
http://localhost:8081/geofence/rest/rules/count?layerAny=true&requestAny=true
That is unconsistent.
- the
datefilter withoutdateAnydo not have any difference. In this video you can see that I should be able to search "all dates that match the criteria" vs search by criteria. But both the request look only to work to match the criteria.
MapStore.3.mp4
|
@etj we need you to double check the above and address it. |
|
I will perform more test at the end of this week, anyway the "any" boolean does not work the way you think:
|
|
@etj I was reading the actual tooltips for the checkbox to deduce functionality. In fact I think that while for the other rules this can be the same, in this case setting the velue Any to true has not the same meaning (because of range nature of these fields). |
|
@offtherailz pls report
|
|
@offtherailz and @etj thank you so much for clarifying the above. @mahmoudadel54 we need you now to provide:
see above for more details. |
…ltips of any field checkbox in rule managers, handles cases of check/uncheck Description: - handles these cases in check/uncheck any field in rules manager grid: * with no value, the flag should not be send * When value is set: - When checked, the flag should be send with value false - When unchecked the flag should be sent with value true. - add unit tests - edit tooltips + translations for check/uncheck in rules manager
offtherailz
left a comment
There was a problem hiding this comment.
@tdipisa I had to fix tooltips initially achieved because translation where a lot misleading.
Can you take a look to have a second opinion. Thank you.
(I also fixed tootip position that was making the tooltip go out of screen for some fields.
tdipisa
left a comment
There was a problem hiding this comment.
@offtherailz I would write like this (see my change). If you agreed, please translate the same for other languages.
Co-authored-by: Tobia Di Pisa <tobia.dipisa@geosolutionsgroup.com>
|
@ElenaGallo, could you please test this on DEV ? Thank you
Actually on dev we can test only embedded old version. Let's check if the functionalitries break something (e.g. adding an ip rule or a date rule, trying to filter by ip or by date). |
|
@mahmoudadel54 @tdipisa The Date functionality works, but the Date filter is not clear to me. |
The docs here: https://docs.geoserver.org/latest/en/user/extensions/geofence-server/rest.html#filter-parameters
So I think the filtered date you entered not included. |
|
@mahmoudadel54 exactly. @ElenaGallo only single date value is allowed in the filter. Please check and let me know asap if we can mark the issue as Accepted. |








Description
This PR includes:
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
#10923 #10924
What is the current behavior?
#10923 #10924
What is the new behavior?
IPAddress filter is added to rules manager grid and date filter as well.
Now to add rule user can enter validity period fields 'validAfter', 'validBefore' in insert new rule or update existing rule
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information
Testing:
Can be tested on nightly geoserver/geofence.
GeoFence:
https://build.geoserver.org/geofence/nightly/main/geofence-main-latest-war.zip
GeoServer
https://build.geoserver.org/geoserver/main/geoserver-main-latest-war.zip
https://build.geoserver.org/geoserver/main/ext-latest/geoserver-2.27-SNAPSHOT-geofence-server-h2-plugin.zip