Skip to content

Grant access to MapStore resources by IP#447

Merged
tdipisa merged 17 commits intomasterfrom
ip_range_sec
Aug 11, 2025
Merged

Grant access to MapStore resources by IP#447
tdipisa merged 17 commits intomasterfrom
ip_range_sec

Conversation

@axl8713
Copy link
Copy Markdown
Contributor

@axl8713 axl8713 commented Aug 8, 2025

These changes implements the functionality nedeed on MapStore to limit read/write permission on resources by reqesting user's IP address.

The endpoint that permits to specify the new constraint on resources is /rest/resources/resource/{resourceId}/permissions, that now is capable to handle a permission model like the following:

<SecurityRuleList>
    <SecurityRule>
        <canRead>true</canRead>
        <canWrite>true</canWrite>
        <user>
            <id>7</id>
        </user>
        <ipRanges>
            <ipRange>
                <cidr>127.0.0.0/8</cidr>
                <description>home</description>
            </ipRange>
            <ipRange>
                <cidr>192.168.1.1/32</cidr>
                <description>single host</description>
            </ipRange>
        </ipRanges>
    </SecurityRule>
[...]
</SecurityRuleList>

As per the example, an user with an IP address like 127.1.2.3 would have complete access to the resource, since the canRead and canWrite values are true.
Conversely, an user with IP 192.168.1.2, even with the read and write permissions, would not be able to access the resource, so a call to GET rest/extjs/resource/{resourceId}?includePermissions=true will return 403 error.

Documentation updated to describe this new functionality:

@axl8713 axl8713 requested a review from afabiani August 8, 2025 11:11
@axl8713 axl8713 self-assigned this Aug 8, 2025
@axl8713 axl8713 requested a review from tdipisa August 8, 2025 11:13
@axl8713 axl8713 marked this pull request as draft August 8, 2025 11:23
@axl8713 axl8713 marked this pull request as ready for review August 8, 2025 11:34
Copy link
Copy Markdown
Member

@afabiani afabiani left a comment

Choose a reason for hiding this comment

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

Other than the other inline comments, can we add tests both direct and rest, demonstrating that it is not possible to inject invalid ip ranges on the security rules? Put particular attention to SQL injection.

@axl8713
Copy link
Copy Markdown
Contributor Author

axl8713 commented Aug 11, 2025

Other than the other inline comments, can we add tests both direct and rest, demonstrating that it is not possible to inject invalid ip ranges on the security rules? Put particular attention to SQL injection.

The CIDR validation is performed during the conversion of the ReST DAO from the incoming request. Additionally, an extra CIDR validation has been added inside the security rule update logic in the ResourceService.

There is no SQL injection risk here because the DAO’s persist method delegates to JPA’s EntityManager.persist, which uses parameter binding internally and does not execute raw SQL constructed from user input.

@tdipisa tdipisa removed their request for review August 11, 2025 14:37
@tdipisa tdipisa added this to the 2025.02.00 milestone Aug 11, 2025
@tdipisa tdipisa linked an issue Aug 11, 2025 that may be closed by this pull request
2 tasks
@tdipisa tdipisa merged commit 20c3ed6 into master Aug 11, 2025
2 checks passed
@afabiani afabiani deleted the ip_range_sec branch August 25, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grant access to MapStore resources by IP

3 participants