-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
SEC-2409: Spring Security / Spring Data Acl Integration #2629
Comments
Ian Duffy said: Any ETA on this? |
Rob Winch said: Spring Security integration is blocked on DATACMNS-293 Last I heard this issues is to be a priority in Spring Data's next release train so I am hopeful that we will start making headway soon. |
Rob Winch said: Not as far as I am aware (I haven't looked into this in any detail though) short of passing in the user information into the repository as an argument. Spring Security exposes the current user with SecurityContextHolder, so you would need to adapt Spring Data to be aware of this in some form or another. This is probably a better question for the Spring Data team. |
Ian Duffy said: Ok, Thank you. Any idea when the next release is due? |
Rob Winch said: Spring Data or Spring Security? |
Ian Duffy said: Spring data. Sorry I should of been clearer. |
Rob Winch said: No problem...but unfortunately I am not certain of the date. This is probably a better question for the Spring Data team. |
Rob Winch said: FYI...we are starting to play around with the first phase which is allowing modification of queries using SpEL to access Spring Security information. See https://github.com/rwinch/spring-security-data/tree/spel In the long term we how to have much more advanced support. |
Ian Duffy said: That looks pretty cool. Thanks Rob :-) |
Pedro Vilaça said: Any news about this issue? I noticed that it was planned for Spring Security 4.0 but as the first RC is already out and this is still open and without any recent comments, will it be included before the final release? Thanks |
Rob Winch said: We are still blocked on DATACMNS-293 so it will not be coming in 4.0 |
Pedro Vilaça said: Thanks Rob. Do you already know how will you do it or you're waiting to see the support that spring data will provide? |
Rob Winch said: I'm coordinating with the Spring Data team to get our requirements. The problem, as usual, is that we don't have enough resources to do everything. Please do vote on the Spring Data issue to ensure it gets prioritized. NOTE: We are including SEC-2676 in 4.0.x which will allow custom queries based on Spring Security's current user. |
Pedro Vilaça said: I've been thinking about this problem for a while and I'm not sure if this feature won't point us to the wrong direction when we're designing a system with ACLs. Is it correct to write code that will completely depend on the ACLs system? Let's talk about a real example.. Is it correct that a method that is responsible to retrieve all the "Objects" (for a given entity) that a User can access, use the ACLs to deliver those objects and not all the objects that exist in the database? I really appreciate the separation of concerns that is described on the docs but if we decide to implement the described behaviour, that separation doesn't exist anymore. So, I'm wondering if we shouldn't consider that ACLs are just an extra level of security and not something that is used as part of the business logic. What do you think? |
Rob Winch said:
It really depends on how you want to define the interface. If you create an interface called SecuredMessageRepository it might be defined up front that only messages that the current user that has access to will be returned.
There still is a clean separation because the ACLs query modifications will be implemented in an Aspect. Keep in mind this is not much different than the current method level security where one can use something like:
The biggest gain we will be getting by modifying queries is that if we need to support paging this can now be done in an efficient manner. Without modifying the query, how would you return the first 10 results a user has access to when there are 1 million entries? This is something that needs to be addressed in an efficient manner. Of course, just because we add support for modifying the query it doesn't mean you must use it. There will always be the ability to do permissions separately. |
Pedro Vilaça said: Thanks Rob! I understand that it'll have the same effect as the method level security but much more efficient because a join at the database level will solve the problem that you described. What I'm thinking is if it's correct to use the ACLs system as the foundation for the application. If you decide to write the code based on that, if you decide to remove the ACL system (it's an hypothetical scenario but it's just to explain what I'm trying to say) you'll end with an application that will return all the entries from the database because the ACL system is the only place where you define who has access to . If that isn't a valid concern, is it possible at the moment to use the ACL system to retrieve all the ObjectIdentities of a given Object that a "sid" has access to? |
Rob Winch said: I think this is the advantage of Aspects in general. If you need to change the aspect (i.e. remove it, modify it, rewrite it) you only need to change it in a single place.
Since this is an absolute question, this sounds like a good StackOverflow question. I understand it may be annoying to switch "venues", but this is important so others can find answers too. If you don't get a response in a few days feel free to ping me on Twitter. |
Pedro Vilaça said: Yep, I agree that it has some advantages. Regarding the other question, sorry for the off-topic.. I'll move it to StackOverflow. Thanks again for your comments! I'm looking forward to see this issue implemented |
This issue depends on https://jira.spring.io/browse/DATACMNS-293 |
Looks like some of the code here: https://github.com/rwinch/spring-security-data is to address this issue. Is there any plan to bring that into |
@gazal-k Thanks for your feedback! You are right that the playground is in spring-security-data project. However, it is far from complete at the moment. What's more it is depending on a feature branch of Spring Data. For this issue to progress we need more work on https://jira.spring.io/browse/DATACMNS-293 to be completed. So please vote (and have your friends vote) on https://jira.spring.io/browse/DATACMNS-293 |
👍 shall do @rwinch |
This would be especially nice for integrations with NoSQL databases where the number of records are in the millions/billions. We're using a similar approach to address these issues as the AclService+AclCache approach is not feasible. Looking forward to this being integrated into Spring. |
Hi there. couldn't wait any longer for this to be fixed... so here's my proposal : a Spring Security extension with beans defining ACL strategies ; easy to plug with Post/PreAuthorize annotations, and able to inject ACL restrictions inside JPA queries (thx to Spring data JPA). noSQL databases are not yet supported, but that shouldn't hurt much. I'm interested in your feedback : |
Having checked the related threads on Jira and github, seem the issue is not progressing. Since the assigned milestone 4.0 Backlog is already closed, would this feature be planned for inclussion in any next release? Thanks in advance. |
Hi, maybe it is too late, but do we have any alternative to this? |
Any update on this? I tracked this down through @marcusdacoregio's issue. What's happening with Spring ACLs? |
@rwinch maybe you can enlighten us, but why are tickets related to ACL always pushed back in priority. There are some useful, much needed features that are opened 10 years ago. Is there anything we can do to push this to the top of the priority list? |
Is there an update on this feature? This should be a priority feature request because not having pagination on large amounts is a significant issue. |
Rob Winch (Migrated from SEC-2409) said:
Spring Security's ACL implementation allows users to determine if a access is allowed after the results come back from the database. This works when there is a small number of results, but breaks down when paging is necessary. Now that we have Spring Data, it would be good to provide integration with Spring Data to ensure that the query's are automatically updated based upon the security restrictions. We could provide a default strategy that aligns with Spring Security's ACL model.
Blocked by spring-projects/spring-data-commons#766
The text was updated successfully, but these errors were encountered: