Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: utilize access control when populating client-side routes in menu #2316

Merged
merged 3 commits into from
Apr 12, 2024

Conversation

taefi
Copy link
Contributor

@taefi taefi commented Apr 11, 2024

This enables filtering of Hilla views based on the security configurations exported from views, and uses the same access control API that could be optionally used Spring Security configuration for http level security.

It is important to note that filtering Hilla views in the menu is a usability feature not a security one.

This introduces a new API `RouteUtil::protectHillaViews` to be used while
configuring spring security to enable protection of Hilla views based on
the same access control logic that is used for http level access control.

The impact of calling `protectHillaViews` will manifest itself in the
population of the client-side views in the automatic menu.
@taefi taefi requested a review from cromoteca April 11, 2024 14:44
Copy link

codecov bot commented Apr 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.44%. Comparing base (16ab52f) to head (a3aa1d7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2316   +/-   ##
=======================================
  Coverage   95.44%   95.44%           
=======================================
  Files          67       67           
  Lines        4438     4438           
  Branches      633      633           
=======================================
  Hits         4236     4236           
  Misses        162      162           
  Partials       40       40           
Flag Coverage Δ
unittests 95.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Legioth
Copy link
Member

Legioth commented Apr 12, 2024

I would suggest having the filtering enabled by default with no way of customizing it for now (other than the existing application.properties option to completely disables the feature). The reason for this is that we want to introduce a filtering API that gives unified control over both Flow views and Hilla views rather than having solutions specific to each framework.

@taefi
Copy link
Contributor Author

taefi commented Apr 12, 2024

I would suggest having the filtering enabled by default with no way of customizing it for now (other than the existing application.properties option to completely disables the feature). The reason for this is that we want to introduce a filtering API that gives unified control over both Flow views and Hilla views rather than having solutions specific to each framework.

I think what's done in this PR is enabling the filtering by default but only for the client-side views. So is the requirement to to enabled it for the server-side views as well?

@Legioth
Copy link
Member

Legioth commented Apr 12, 2024

I think what's done in this PR is enabling the filtering by default but only for the client-side views. So is the requirement to to enabled it for the server-side views as well?

Only client-side views. The reason it seems like it's not enabled by default is that there's a line of code in application code that is needed to enable the filtering.

Copy link

sonarcloud bot commented Apr 12, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@taefi taefi merged commit 8b1d0db into main Apr 12, 2024
15 checks passed
@taefi taefi deleted the taefi/consider-aa-when-populating-clientside-menu branch April 12, 2024 13:28
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.4.0.alpha23 and is also targeting the upcoming stable 24.4.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants