Bug with interaction between ResourceHooks and DefaultEntityRepository when updating entities

[maurei]

This is a bug resulting from complex interaction between the repository and resource hooks.

See PersonDefinition in JsonApiDotNetCoreExample and the Patch_Entity_With_HasMany_Does_Not_Included_Relationships test in the corresponding test project. If we add a BeforeUpate resource hook to PersonDefinition with database values enabled, this test will fail.

This happens because todo-items will be included in the person that is being loaded by the database value loader in Resource hooks. This person is then tracked by EF Core in that DbContext (which is shared over the entire request scope, because the repositories are scoped services), so when the query from the Repo is executed and returned, the todo-items that ought to be excluded are already populated because it is the same instance being tracked by EF Core. They are then returned from the API, even though they weren't included.

Although I feel it is unlikely, this bug could potentially result in a security leak in your application because data could be exposed that you might not want to expose.

Using AsNoTracking() in the database-value loading might seem like the evident solution to this problem. The problem with this however that

• IEntityReadRepository is being used by the database value loader, not dbContext directly. This is by design, because else hooks wouldn't be supported when EF Core isn't used
• the AsNoTracking is a EF Core specific thing and shouldn't be configurable on the IEntityReadRepository API.

A workaround is making sure you return data doesn't expose any sensitive data by implementing the OnReturn hook with related authorization/filtering logic. If there is a risk of sensitive data being exposed, you probably (should) have implemented this hook in the first place, so in that case you won't run into any problems

[maurei]

closed in #518