spring-projects
Allow multiple repositories per entity (only one should be exported) [DATAREST-923]
Tim Meißner opened DATAREST-923 and commented
My case is that i have an Entity "User" and two Repositories:
- UserRepository extends CrudRepository,
@RepositoryRestResource(exported = false): meant for internal usage by services etc. - UserRestRepository extends Repository,
@RepositoryRestResource(exported = true): meant for external usage, restricted access, etc.
This exports a user resource in 50% of the times i start up the application. I tried to work around this by annotating them with @Order(0/1) for the UserRepository and @Order(1/0) for the UserRestRepository, but it doesn't work.
So I would like to have the possibility to have multiple Repositories for one entity (only one exported but it should not conflict with other normal Repositories that are not exported).
Affects: 2.5.4 (Hopper SR4)
Reference URL: http://stackoverflow.com/questions/36112451/multiple-repositories-for-the-same-entity-in-spring-data-rest
Issue Links:
- DATACMNS-1448 Repositories should select primary repository in case multiple are available for a domain type ("depends on")
Referenced from: pull request https://github.com/spring-projects/spring-data-commons/pull/465, and commits https://github.com/spring-projects/spring-data-commons/commit/de7a7caa49308e6687f60d0909794df9d921b420, https://github.com/spring-projects/spring-data-commons/commit/d8705c1cca3908284abd15d972152d9720f59fee
40 votes, 40 watchers
Oliver Drotbohm commented
Generally speaking SD REST expects one repository per domain type. You can disambiguate the situation by making one @Primary. Would you mind elaborating what's behind the idea to use two different types?
Tim Meißner commented
Sure, so basically I am implementing a User Account Settings page - a user should be able to edit his own entity, so I use method security to disallow saving user entities with a different username than the principal. On the other hand there might be an admin user which can trigger service methods over a custom controller that might change other user entities than himself. If i use the same repository the access control will prevent the admin from doing that, so it's good to have two in this case. A complicated security expression / service call might not be enough in this situation, because i could also need to change other users as a normal user by calling other business related services.
@Primary doesn't bring the desired result, i tried with one Repository but as soon as i have @PreAuthorize it requires me to have an Authentication in the SecurityContext which means i can't use the Repo in a CommandLineRunner for example
Daniel Wegener commented
Imo not a minor - once you provide more than one repository for an entity type, the behaviour of the application changes randomly, depending on which repository is found in the bean factory first (and this seem to change from compile to compile run). Took me a day to figure out why this is happening (I'd second a comment on Stackoverflow, it should at least yield a warning on the log).
My use case:
I have two distinct JpaRepository for a single type. One to be exposed by spring data rest (using @PreAuthorize annotations) and one to be used by other internal controllers (which have no spring security context (yet) - in my case the spring security oauth ClientDetailsService)
Daniel Wegener commented
Hi Oliver. The workaround does not work in this case since they are distinct repositories:
@RepositoryRestResource
public interface ClientRepository extends JpaRepository<Client, String> { // with @PreAuthorize overwritten accessors
and
@RepositoryRestResource(exported = false)
public interface InternalClientRepository extends JpaRepository<Client, String> {}
From what I understand: Since none of them is a subtype of the other, there is no ambiguity that could be resolved with @Primary. Both pass org/springframework/data/spring-data-commons/1.12.2.RELEASE/spring-data-commons-1.12.2.RELEASE-sources.jar!/org/springframework/data/repository/support/Repositories.java:93 and the last one for the same entity name simply wins. The @Primary does not seem to affect the order in which they pass through it.
Oliver Drotbohm commented
I see. I guess we need Repositories to leniently collect all repositories first, then allow a lookup with a dedicated selection criteria with the current implementations expecting either a canonical reference and throwing an exception on ambiguities. Spring Data REST could then use a custom RepositoryInvoker that applies a criteria (e.g. the presence of @RepositoryRestResource) to select the interface it's interested in
Johannes Hiemer commented
Having exactly the same issue and the same requirements:
- One set of repositories where you have all the security annotations for the public REST endpoint
- One set of repositories for internal handling of data access
Is there any update, when this will be solved?
Istvan Ratkai commented
Exactly the same issue here. One repository for public usage, one for internal usage without security + some extra methods
Johannes Hiemer commented
I would pick up again on this issue, as I don't see why it is prioritised with "minor". To secure an application is substantially important to have mechanism of not maintaining the same repositories two times for different purposes - one of applications with external access to the data via REST and one for the internal access.
So my question: if this is not possible on the repository implementation level, is there any useful workaround on the SpelEvaluationContext?
Will Faithfull commented
I also think this is the simplest solution to secure repositories - I would also agree that it should be more than "minor". Relying on @PreAuthorize on repository methods to secure requests is causing me no end of headaches. Not least when I bootstrap the database in a CommandLineRunner with no authentication in the context. I am currently exploring an ugly hack to get the control I want over security, with Aspects around the spring-data-rest controllers. That is too ugly and brittle to move forward with though
Will Faithfull commented
Inspired by the suggestion by Oliver above, I have arrived at an acceptably solid hack for the time being. Rather than using the RepositoryRestConfigurerAdapter for my configuration, I subclass RepositoryRestMvcConfiguration to the same end. That allows me to override the repositories() factory method with a subclassed implementation of Repositories, which prioritises repositories which have @RepositoryRestResource(exported=true) on the repository interface.
This means I can define, for example
@RepositoryRestResource(exported = false)
public interface InternalUserRepository extends BaseRepository<User, UUID> {
...
}
@RepositoryRestResource(path = "users")
@PreAuthorize("hasRole('ADMINISTRATOR')")
public interface UserRepository extends BaseRepository<User, UUID> {
...
}
etc. If I want to allow a signup by an anonymous principal, I can handle that insertion through a custom route, with my InternalUserRepository. Due to the correct prioritization in Repositories, all the exported repositories are correctly managed by spring data rest
Johannes Hiemer commented
@Will would you mind sharing your workaround? I had no time to look into it, but I would be highly interested.
Oliver Drotbohm commented
I guess UserRepository in his example is additionally annotated with @Primary. Note, that you can just drop public from InternalUserRepository to prevent it from being exported, too. If it's for internal use, you most likely would want to use it from outside the package anyway but rather expose some more high-level service
Will Faithfull commented
@Johannes I've thrown together a sample on github here. It feels like quite a dirty workaround, but it does the job in the mean time. @Oliver yes, I presume it will follow whatever the configured repository detection strategy is. I would be intending to use it as part of a UserService or such anyway
Nguyen commented
I decided not to use spring data rest until this issue is fixed. The priority should not just minor
Marc Friedman commented
I just upvoted this issue after spending a day debugging through the bowels of Spring Data Rest trying to determine why my unit tests were randomly passing and failing. I agree that the priority needs to be raised as there is currently no warning and the behavior is unintuitive - if a repository is marked exported=false in an @RepositoryRestResource annotation the expectation is that it will be ignored and another repository for the same entity without that annotation will be used.
My use case is slightly different. I have a microservice which is managing entities that support soft delete (entities are marked as deleted but remain in the table). My internal repository needs extends CrudRepository and works with all entities. I was attempting to define a REST repository for use by other microservices which exported methods annotated with @Query (such as findOne) with constraints to include only active entities when I ran into this issue
Max Mumford commented
I also wasted a very frustrating day on this with unit tests randomly failing. My use case is similar to that of the other guys here - UserRepository needs to be secured when accessed via REST (eg to stop users from loading a complete list of other users in the system), but accessible without security restrictions internally (ie to search for another user by their email address). If it wasn't for Will's workaround I'd without a doubt have to drop Spring Data, and my use case is by no means unusual. Definitely warrants a higher priority than minor imo :/
Marc Friedman commented
I was able to use @Will's workaround to get this working, but not before losing another half day getting it to play nice with Spring Boot. The workaround provides an @Configuration bean which overrides RepositoryRestMvcConfiguration. Presuming this is under your package hierarchy Spring Boot creates this bean and that results in suppression of the RepositoryRestMvcAutoConfiguration which is conditional on RepositoryRestMvcConfiguration not existing (as it imports it). RepositoryRestMvcAutoConfiguration is responsible for creating the SpringBootRepositoryRestConfigurer bean which does things like load spring.data.rest properties from property files.
As SpringBootRepositoryRestConfigurer is not publlic, I ended up duplicating it into my own RepositoryRestConfigurer and that finally worked.
Max Mumford commented
Just found another issue with having multiple repositories - secured repository methods are sometimes called during the deserialization of a json post request to a related entity, ie:
- Create an entity User
- Create two repositories for User as per Will's workaround
- Secure the "find" method on the exported repository so users can only "find" their own user
- Create an entity Meeting
- Create a single, unsecured and exported repository for Meeting
- Create a property "owner" on Meeting that is a
@ManyToOneto User - Make a JSON post request to meeting repository to create a meeting with the "owner" property pointing to a User object that is not the logged in user (ie {"owner": "/users/2"})
- Access denied error is raised when the json is deserialized and spring attempts to load the user by calling the "find" method on the secured repository. Returned response is status 400 with no error messages or content
I'm fairly new to Spring so not really sure how to even go about fixing this... I've tried adding @Primary to my unsecured repository but still getting the same error. Any ideas?
Will Faithfull commented
@Max, I don't believe what you are talking about is anything to do with the workaround. In this context, the Repositories bean exists to keep track of the repositories Spring Data Rest is exporting. The 'unsecured' repositories that this workaround allows are effectively invisible to Spring Data Rest, as they are selectively kept out of the Repositories bean. The purpose of this workaround is to allow you to bypass your security annotations, by defining your own controller method, which ultimately uses the 'unsecured' repository directly.
Access denied error is raised when the json is deserialized and spring attempts to load the user by calling the "find" method on the secured repository. Returned response is status 400 with no error messages or content
Do you mean you are making a request to /users/2? Or is this on the request to /meetings/1? If it is the former, then that sounds like the intended behaviour to me. Otherwise, if a repository is exported for User, then that should be rendered as a link on the meeting anyway, and findOne on User shouldn't be called..
Max Mumford commented
Hi @Will, I didn't mean that what I'm experiencing is a result of your workaround, I meant it is another problem I'm facing when trying to implement multiple repositories with SD.
The problem occurs when I do the following:
- I have security rules on my secured, exported Users repository that prevents users from loading other user's data
- So when logged in as "users/1", if I post to "/meetings" to create a new meeting, and set the "owner" property to "/users/2", I get access denied because during deserialization, the secured "findOne('/users/2')" method of the users repository is called
- So I essentially need to make spring use, by default, internally, the unsecured repository, when deserializing requests for related entities
Hope that clarifies things?
Piotr Żmudziński commented
Isn't it major, rather than minor ticket?
Currently there's no way of using @RepositoryRestResource together with spring-security annonations, such as @PreAuthorize, if you want to have some custom permission evaluators.
If you need to implement UserDetailsService and do a user lookup through userRepository you want to talk to unsecured version of repository (the same goes with MethodSecurityExpressionHandler and custom PermissionEvaluator) . If it's a client-side it should be secured with whatever is needed. That issue seems to be major.
Is there any plan of resolving it?
Burkhard Graves commented
Multiple repositories for entities are also problematic if request parameters or path variables are converted to instances of repository managed domain classes (done by DomainClassConverter respectively its inner class ToEntityConverter). In this case it's totally ambiguous which repository is used for a given domain class. If an unsecured repository is used, everything works fine, if a secured (by using @PreAuthorize) one is used - bang, see DATACMNS-1142
Andrii Neverov commented
+1 on Piotr's use-case
Having composite repositories which would do a security check and delegate work to the non-secure version while maintaining the same interface is a very basic and essential requirement for enterprise apps.
The issue has been open for more than a year. Any chance this would be prioritized and looked at any time soon?
Also would be nice to put a notice into the reference so that people don't spend countless hours debugging
Norbert Somlai commented
I'm trying to work this around by merging my repositories, but I need to be able to handle saving records from POST requests (@PreAuthorize) and in the backend (without authentication). How can I create two save() methods in one repository?
Dario Seidl commented
I was also facing this issue with exactly the same use-case as the OP. Having two repositories, one exported and secured with method security expressions and another non-exported one for internal usage looked like the perfect solution. I hope this will become possible in the future.
@Norbert Somlai
What we ended up doing, was to create a custom base repository implementation, extended from SimpleJpaRepository, as explained here: https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#repositories.customize-base-repository with an implementation that provides aliases for the common CRUD methods, like this:
@NoRepositoryBean
public interface InternalRepository<T, ID extends Serializable> extends Repository<T, ID> {
@Transactional(readOnly = true)
List<T> internalFindAll();
@Transactional(readOnly = false)
<S extends T> S internalSave(S entity);
// ...
}
public class InternalRepositoryImpl<T, ID extends Serializable> extends SimpleJpaRepository<T, ID> implements InternalRepository<T, ID> {
public InternalRepositoryImpl(JpaEntityInformation<T, ?> entityInformation, EntityManager entityManager) {
super(entityInformation, entityManager);
}
@Override
@Transactional(readOnly = true)
public List<T> internalFindAll() {
return super.findAll();
}
@Override
@Transactional(readOnly = false)
public <S extends T> S internalSave(S entity) {
return super.save(entity);
}
// ...
}
Then all repositories implement a common repository that implements the InternalRepository interface as well as CrudRepository (or whatever you need to export), and overrides the exported methods with method security expressions.
This way, we can use the internalFind... and internalSave... methods in our services, and have the default CrudRepository find... and save... exported and secured.
This works well, but I still think having the repositories separated would be cleaner and more convenient.
Norbert Somlai commented
@Dario Seidl : Good idea. I have also received a suggestion on Stackoverflow to secure save() in the repository and use saveAndFlush() internally. Does not help with other methods though
Yves Galante commented
Simple question...
Why Spring Data does not simply ignore repositories annoted at class level with exposed false ?
Actually Spring data keep a referance on that generate issues.
That will allow to have one unsecue with few change on Spring data ...
Istvan Ratkai commented
I created an extension for Spring Boot Data JPA which resolves this issue by adding the following methods automatically:
<S extends T> S saveWithoutPermissionCheck(S entity);
void deleteWithoutPermissionCheck(T entity);
T findOneWithoutPermissionCheck(ID id);
Actually, these methods are only a side-effects of the tons of other useful features I created for securing Data Rest endpoints.