Skip to content

[DoctrineBridge] #[MapEntity] is "handled" before #[IsGranted] #58827

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.

Sign up for GitHub

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

Jump to bottom
Open
seb-jean opened this issue Nov 10, 2024 · 10 comments
Open

[DoctrineBridge] #[MapEntity] is "handled" before #[IsGranted] #58827

seb-jean opened this issue Nov 10, 2024 · 10 comments
Labels
Bug DoctrineBridge Status: Needs Review

Comments

@seb-jean
Copy link
Contributor

seb-jean commented Nov 10, 2024 •
edited
Loading

Symfony version(s) affected

7.1.1

Description

#[MapEntity] is "handled" before #[IsGranted].

How to reproduce

  1. Clone Symfony Demo Application
  2. In src/Controller/Admin/BlogController.php, replace index() function by
...
use Symfony\Bridge\Doctrine\Attribute\MapEntity;
...

#[Route('/', name: 'admin_index', methods: ['GET'])]
#[Route('/', name: 'admin_post_index', methods: ['GET'])]
public function index(
    #[MapEntity(class: Post::class, expr: 'repository.findBy({"author": 13}, {"publishedAt": "DESC"})')]
    iterable $authorPosts
): Response {

    return $this->render('admin/blog/index.html.twig', ['posts' => $authorPosts]);
}
  1. Go to https://127.0.0.1:8000/fr/admin/post.
  2. The application redirects the user to the login form so that's ok. But when we look at the profiler, there is the Doctrine tab which informs us that a query has been executed and it is indeed the query linked to MapEntity because there is the 13 which is in the query.

Capture d'écran 2024-11-10 161122

Capture d'écran 2024-11-10 160657

Possible Solution

No response

Additional Context

No response
@seb-jean seb-jean added the Bug label Nov 10, 2024
@derrabus
Copy link
Member

I fail to see the problem. Can you elaborate more?

@seb-jean
Copy link
Contributor Author

I did a test again and with the code copied in this issue, I have the error.

You have to click in the profiler once we are redirected. For the example, I clicked on the link "bc5bd7", and I see the Doctrine tab in the profiler.

@derrabus
Copy link
Member

I know how to use the profiler, thank you. But so far, you're only describing observed behavior and not why that behavior is a problem.

@nicolas-grekas
Copy link
Member

Sorry I'm not in front of my laptop: I think we already discussed this topic, can someone find anything?

@wouterj
Copy link
Member

wouterj commented Nov 10, 2024

I think you mean #50120

@seb-jean
Copy link
Contributor Author

Yes, it seems related to #50120.

@seb-jean
Copy link
Contributor Author

I know how to use the profiler, thank you. But so far, you're only describing observed behavior and not why that behavior is a problem.

This behavior is a problem because the query is executed while I am not authorized to execute the function because I am an anonymous user.

@wouterj
Copy link
Member

wouterj commented Nov 10, 2024

Other than that we are running a redundant query (thus causing unnecessary load on the database), there is not much wrong with running the query - given IsGranted will prevent Symfony or your app to do anything with its result. I think the latter is what @derrabus is referring too.

Still, I think it would be good if we can somehow fix this (or maybe the patch from the other issue already does?), as this opens the door to potential ddos-able endpoints.

@seb-jean
Copy link
Contributor Author

Still, I think it would be good if we can somehow fix this (or maybe the patch from the other issue already does?), as this opens the door to potential ddos-able endpoints.

Unfortunately, the patch from the other issue does not fix this.

@BenMakesGames
Copy link

if performance is the concern, it might be better to not use #[MapEntity] at all? using it isn't much better than SELECT *, and you can solve the "query before auth check" issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@nicolas-grekas @wouterj @derrabus @BenMakesGames @xabbuh @seb-jean @carsonbot