Skip to content

[SecurityBundle] unhide debug security voter services #29169

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

Merged
merged 1 commit into from
Nov 11, 2018
Merged

[SecurityBundle] unhide debug security voter services #29169

merged 1 commit into from
Nov 11, 2018

Conversation

fmata
Copy link
Contributor

@fmata fmata commented Nov 10, 2018

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

#27914 introduces testThatVotersAreNotDecoratedWithoutDebugMode() which tests if decorated services exist but uses a bad service name without starting dot.

Definition in the compiler pass :

if ($debug) {
// Decorate original voters with TraceableVoter
$debugVoterServiceId = '.debug.security.voter.'.$voterServiceId;
$container
->register($debugVoterServiceId, TraceableVoter::class)
->setDecoratedService($voterServiceId)
->addArgument(new Reference($debugVoterServiceId.'.inner'))
->addArgument(new Reference('event_dispatcher'));
}

The expected services are hidden and their name start with a dot. So the test will always pass, now it can fails :)

@nicolas-grekas
Copy link
Member

I'd prefer to "unhide" them instead, can you do that, please?
We hide services only when their name is garbage (eg when generating them using hashes)

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Nov 11, 2018
@fmata
Copy link
Contributor Author

fmata commented Nov 11, 2018

Indeed, it makes more sense. Done.

@fmata fmata changed the title [SecurityBundle] fix AddSecurityVotersPassTest::testThatVotersAreNotDecoratedWithoutDebugMode() using hidden service name [SecurityBundle] unhide debug security voter services Nov 11, 2018
@nicolas-grekas
Copy link
Member

Thank you @fmata.

@nicolas-grekas nicolas-grekas merged commit 4677bb4 into symfony:master Nov 11, 2018
nicolas-grekas added a commit that referenced this pull request Nov 11, 2018
…mata)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[SecurityBundle] unhide debug security voter services

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

#27914 introduces `testThatVotersAreNotDecoratedWithoutDebugMode()` which tests if decorated services exist but uses a bad service name without starting dot.

Definition in the compiler pass :
https://github.com/symfony/symfony/blob/a4204cd685c02377e6e2fbfc7ece98b5563644d9/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php#L58-L66

The expected services are hidden and their name start with a dot. So the test will always pass, now it can fails :)

Commits
-------

4677bb4 [SecurityBundle] unhide debug security voter services
@fmata fmata deleted the 27914-followup branch November 11, 2018 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants