Skip to content
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

Symfony::extractRawRoles() failed on security collector from Symfony >= 3.3 #4309

Merged
merged 5 commits into from
Jun 9, 2017
Merged

Symfony::extractRawRoles() failed on security collector from Symfony >= 3.3 #4309

merged 5 commits into from
Jun 9, 2017

Conversation

Basster
Copy link
Contributor

@Basster Basster commented Jun 7, 2017

When extracting roles from security collector, you can simply utilize the getValue method of Data class, when it exists.

…>= 3.3.

When extracting roles from security collector, you can simply utilize the getValue method of Data class, when it exists.
// prior to Symfony 3.3
if (method_exists($data, 'getValue')) {
$roles = $data->getValue();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected 1 space after closing brace; newline found

@Naktibalda
Copy link
Member

@Basster Does this patch fix #4306 or is it a different issue?

@@ -521,9 +521,15 @@ protected function debugResponse($url)
*/
private function extractRawRoles(Data $data)
{
$raw = $data->getRawData();
// prior to Symfony 3.3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this comment is correct?
getRawData method is deprecated since 3.3 and removed in 4.0.

@Basster
Copy link
Contributor Author

Basster commented Jun 7, 2017

@Naktibalda I didn't submitted an issue for this PR but it looks like this may fix #4306 as well. Everytime a user is somehow authenticated (even with an AnonymousToken) this branch of the debugResponse() gets executed. So, yes - without further investigation, this will fix #4306 as well.

*
* @return bool
*/
private function dataIsFromSymfony3_3(Data $data): bool

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method name Symfony::dataIsFromSymfony3_3 is not in camel caps format

*
* @return bool
*/
private function dataIsFromSymfony33(Data $data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this name because it will be confusing when Symfony 4.0 comes out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm with you...

There are only two hard things in Computer Science: cache invalidation and naming things. -- Phil Karlton

Made the name a little more generic

@HypeMC
Copy link

HypeMC commented Jun 7, 2017

@Naktibalda @Basster Confirmed, this fixes #4306 .

@Naktibalda Naktibalda merged commit 65b4f4f into Codeception:2.3 Jun 9, 2017
@Basster Basster deleted the bugfix/symfony-security-profiler-roles branch August 4, 2017 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants