Allow multiple repositories per entity (only one should be exported) [DATAREST-923]

[spring-projects-issues]

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

[spring-projects-issues]

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?

[spring-projects-issues]

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

[spring-projects-issues]

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)

[spring-projects-issues]

Oliver Drotbohm commented

Daniel Wegener — Does the workaround with @Primary work for you?

[spring-projects-issues]

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.

[spring-projects-issues]

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

[spring-projects-issues]

benkuly commented

Is there any workaround?

[spring-projects-issues]

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?

[spring-projects-issues]

Istvan Ratkai commented

Exactly the same issue here. One repository for public usage, one for internal usage without security + some extra methods

[spring-projects-issues]

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?

[spring-projects-issues]

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

[spring-projects-issues]

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

[spring-projects-issues]

Johannes Hiemer commented

@Will would you mind sharing your workaround? I had no time to look into it, but I would be highly interested.

[spring-projects-issues]

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

[spring-projects-issues]

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

[spring-projects-issues]

Nguyen commented

I decided not to use spring data rest until this issue is fixed. The priority should not just minor

[spring-projects-issues]

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

[spring-projects-issues]

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 :/

[spring-projects-issues]

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.

[spring-projects-issues]

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 @ManyToOne to 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?

[spring-projects-issues]

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..

[spring-projects-issues]

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?

[spring-projects-issues]

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?

[spring-projects-issues]

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

[spring-projects-issues]

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

[spring-projects-issues]

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?

[spring-projects-issues]

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.

 

[spring-projects-issues]

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

[spring-projects-issues]

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 ... 

[spring-projects-issues]

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.

Spring Data JPA ACL extension

[spring-projects-issues]

Istvan Ratkai commented

I've found an other solution too.

 

The root of the problem is that you want an external and an internal method with the same functionality.

You can handle the externalization via @RestResource(exported=true/false) annotations on method level, so that part is solved..

The real problem is that how can you create the same query twice? (Because queries are generated from the method name, and of course you cannot define two methods with the same name)

The solution is quite simple: You can add any text between the 'find' and the 'By' keyword in the method name, because that part will be ignored when the actual SQL query is generated from the method name:

findByFirstnameAndLastname

and

findInternalByFirstnameAndLastname

or

findExternalByFirstnameAndLastname

or 

findAllThoseBastardsWithSameNamesByFirstnameAndLastname

 will have the exact same functionality. It's up to you how do you annotate those methods. (You can use @RestResource(path="...", rel="...") to rename the endpoint as you like)

The only problem is findAll, because the trick doesn't work here. But you can still create a similar query with the following name:

findAllByIdNotNull

Id cannot be null, so it will also return the whole table. (I also assume that 'id is not null' will be optimized in the DB level anyway so it won't even affect the performance.)

Oh, I only tested it for the latest Data Jpa version for Spring Boot 2, but maybe it also works in the older versions.

 

[spring-projects-issues]

Yves Galante commented

aisik The main issue isn't that this Sping data rest not support this feature.

It is that issue is not raise by Spring data rest and generate a random result.

Sprind data rest must detect this conflic and log an error or fatal.

 

Secondly the issue occur also when one of two repositories is mark as not exposed.

Repositories not expose must be ignored by sprind data rest

[spring-projects-issues]

Oliver Drotbohm commented

The reason the exposure flag doesn't make it into the selection is that Repositories, the API that exists to select repositories by domain type, lives in Spring Data Commons and that just does not and must not know about any Spring Data REST specifics.

I've just filed and fixed DATACMNS-1448 that puts the @Primary handling in place that I assumed we already had in place. Spring Data REST should now consistently use the interface annotated with @Primary as the one for it's entity interactions.

Would you mind giving the snapshots a spin? It's currently available in Moore snapshots. I might consider a backport to Lovelace and Kay if that helps

[spring-projects-issues]

Bartosz Kielczewski commented

@Primary approach in DATACMNS-1448 looks good to me, however there are currently at least two problems that prevents it from working (for me).

1) Spring Data REST in RepositoryRestMvcConfiguration passess whole ApplicationContext as a beanFactory for Repositories. This actually makes this check for @Primary not working because default contexts don't implement ConfigurableListableBeanFactory.

Work-around would be to override RepositoryRestMvcConfiguration so Repositories would get ConfigurableListableBeanFactory:

@Configuration
class HackRepositoryRestMvcConfiguration(
        private val context: ApplicationContext,
        @Qualifier("mvcConversionService") conversionService: ObjectFactory<ConversionService>
) : RepositoryRestMvcConfiguration(context, conversionService) {

    override fun repositories() =
            when (context) {
                is ConfigurableApplicationContext -> Repositories(context.beanFactory)
                else -> Repositories(context)
            }

}

2) RepositoryFactories when they make repository beans can ignore @Primary on repository interface, so any bean created out of it won't have isPrimary in its BeanDefinition.

Work-around would be to create BeanFactoryPostProcessor similar to this

[spring-projects-issues]

Nilesh Padwal commented

After applying both the workarounds mentioned above, this is still not working. Spring is still publishing the repository at random.

Following piece still returns two names in the for loop. One which has @Primary annotation and one which does not.

org.springframework.data.repository.support.Repositories class method 

private void populateRepositoryFactoryInformation(ListableBeanFactory factory) {
    for (String name : BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, RepositoryFactoryInformation.class, false, false)) {
            cacheRepositoryFactory(name);
    }
}

Any workarounds till this is fixed? 

[spring-projects-issues]

buckett commented

Ok, I've just lost a good few hours to this one, I've similar uses to others (one internal repo (no spring security) and one user facing repo (with spring security)). If nothing else it would be helpful to throw an error when this is detected so that it's clear what's going wrong

[spring-projects-issues]

cyril-gambis commented

Hi,

I would also need this same feature, for this exact same scenario.

Thanks

[spring-projects-issues]

Oliver Drotbohm commented

Would you guys mind trying the Moore snapshots or release candidate? I see we have DATACMNS-1448 fixed for quite a while already, we just forgot to mark it as resolved. That fix should make Repositories honor the @Primary repository and Spring Data REST uses exactly that then

[spring-projects-issues]

Bartosz Kielczewski commented

I'm currently on Spring Boot 2.2.0.M4, which uses RC1 of data-commons. @Primary works as expected only after applying both 'hacks' I've mentioned above.

The project is using spring-data-mongodb, spring-data-rest, though if you follow the link to SO thread, @Primary was ignored on JPA repositories as well when the beans were created out of them.

So looking at the commit you've made for DATACMNS-1448 it doesn't work because in cacheFirstOrPrimary() both:

• ConfigurableListableBeanFactory.class::isInstance is false
• BeanDefinition::isPrimary is false

Would providing you a minimal project help you debug?

[spring-projects-issues]

Chris McGuire commented

Just tried @Primary on a JPARepository with no other hacks.  It worked, though I can't be sure if I just got lucky or not since the bean/repo selection seems random.    So @Primary helps, but this was a pain to figure out until I stumbled on this thread/issue.  Please issue a proper fix.  Lost 2 days on this so far

[spring-projects-issues]

Chris McGuire commented

My apologies...  I spoke too soon.  @Primary still seems to be ignored on JPA Repositories,  producing a random result on which repo is exposed when multiple are defined.  

[spring-projects-issues]

Jan Zeppenfeld commented

Hey guys, because I stumbled upon the same issue in our current project, I found this thread and analyzed what's going on under the hood and tried to figure out how it can be fixed.

Actually, two independent problems already mentioned in the comments prevent this feature from working.

The @Primary issue was first tackled in DATACMNS-1448 by changing the Repositories code but because the 'primary' state didn't make it to the actual bean definition of the Spring Data Repositories, the actual 'primary' state never reached the Repositories object and therefore DATACMNS-1591 has been introduced in Spring Data 2.2.1 - I was about to analyze and submit a fix for this issue when I saw that it had already been fixed.

Now the 'primary' state reaches the Repositories object but as Bartosz Kielczewski mentioned in https://jira.spring.io/browse/DATAREST-923?focusedCommentId=182769&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-182769

ConfigurableListableBeanFactory.class::isInstance is false

This is the case because

• usually the ApplicationContext is directly passed to the Repositories constructor (as in RepositoryRestMvcConfiguration.repositories()) which is possible because all contexts implement ListableBeanFactory
• but there isn't any ApplicationContext implementing ConfigurableListableBeanFactory although all contexts contain such a bean factory (see below).

The implemented Test RepositoriesUnitTests.keepsPrimaryRepositoryInCaseOfMultipleOnes() also directly passes a DefaultListableBeanFactory to the Repositories object instead of the created GenericApplicationContext which obviously works as explained above.

So, there are four possible approaches to fix this issue from my point of view... from the cleanest and most invasive to the least invasive one:

1. From my point of view the root cause of this issue is that there are ApplicationContexts, especially ConfigurableApplicationContext, which just implement ListableBeanFactory directly or indrectly and contain a ConfigurableListableBeanFactory at the same time, which they return in methods and which they delegate to to fill out the ListableBeanFactory. This seems a bit contradictory because if it contains a ConfigurableListableBeanFactory, returns it explicitly via methods (public ConfigurableListableBeanFactory getBeanFactory()) and delegates to it to fill out the ListableBeanFactory, those contexts should implement ConfigurableListableBeanFactory... and not just ListableBeanFactory. But I guess that this happened to keep backward compatibility.
2. Change the Repositories constructor type from ListableBeanFactory to ConfigurableListableBeanFactory and explicitly pass a corresponding bean factory to the Repositories object instead of passing the ApplicationContext. This would be even simplier if the above issue was solved. But even without this change it would still be possible in most cases because all contexts implement ConfigurableApplicationContext which allows to get a ConfigurableListableBeanFactory via getBeanFactory(). However, in the end this change would be a breaking change and all depending packages/code would have to be updated.
3. Add an additional constructor with type ConfigurableListableBeanFactory and adapt dependent packages/code step by step. This would not introduce a breaking change but would only fix the current bug for cases in which this new constructor is used.
4. Adapt the Repositories.cacheFirstOrPrimary() method to check whether the given beanFactory implements ConfigurableApplicationContext and if so, get the ConfigurableListableBeanFactory from it.

Although I would prefer the cleaner solutions (1+2), I guess the fourth approach embracing backward compatibility would be the preferred one. So here is a pull request

[janzeppenfeld]

Hi @odrotbohm, I think, that this one can be closed, because

• it basically is a duplicate of https://github.com/spring-projects/spring-data-commons/issues/1583 and
• has been fixed by https://github.com/spring-projects/spring-data-commons/pull/465 in Feb. 2021.

Best regards