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

[-] FO: auth error if $back is not defined #3553

Merged
merged 1 commit into from Aug 4, 2015

Conversation

@kermes
Copy link
Contributor

commented Jul 30, 2015

If $back is not set, when loggin never arrives to 'my-account' and falls into the last else, that is authentification_error and user goes to authentication page, logged in but without errors

[-] FO: auth error if $back is not defined
If $back is not set, when loggin never arrives to 'my-account' and falls into the last else, that is authentification_error and user goes to authentication page, logged in but without errors
jnadaud pushed a commit that referenced this pull request Aug 4, 2015
Jérôme Nadaud
Merge pull request #3553 from kermes/patch-5
[-] FO: auth error if $back is not defined

@jnadaud jnadaud merged commit 62abc46 into PrestaShop:1.6.1.x Aug 4, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jnadaud

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

Hi,

thank you for your contribution.

Best regards.

@jnadaud jnadaud added the Bug label Aug 4, 2015

@gRoussac

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

Hi,

I do not fully agree, you do not test $back anymore on line 330 in the first if so this can lead to another behavior than before and $back can be undefined on line 337.

Regards

@gRoussac

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

Sorry would you mind detailling again the exact way to reproduce your issue ?

Thank you Regards

@kermes

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2015

Before the modification, the way to reproduce is... just login without ajax and make sure $back is not defined (ie not in the form as a field or in the form action url).
You'll be redirected to the authentiaction page as if there were an error but you were correctly logged in.
before you made this commit (ba0d1d4#diff-798139a830d0ed90cc1972359a797c77), if $back was not defined, user got redirected to the default page 'my-account'.
After your modification, if back was not defined, the first test in line 316 will not pass, so the user will be redirected in line 336, even thoug that redirect is there as a fallback assuming that if you got there was because there was an error, but there wasn't because $back is not mandatory (that is why it has the default vaule 'my-account')
In line 326, you are testing again if $back is true. it is impossible that it is not true inside that If, where it was already tested, so it will never fall back to 'my-account'
after my modification i'm using line 331to test if $back is set and if it is not, then use the default value 'my-account' so this means that here the user will be redirected if there were no errors, either to the $back set page or to the default 'my-account' page (if this is not ajax, of course)

If you think I missed something, please let me know

@gRoussac

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

I am not used to PSR2 I missed the closing bracket of the first if.

I will test that right away. Many thanks for your explaination.

Regards

@kermes

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2015

hehehe,
I used to work in PSR2 before I started working with prestashop and now that I was getting used to the old norm, I have to adapt my mind again back to how it used to be, so I totally understand ;)

@gRoussac

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

It seems I just badly tested without back in url or input indeed, sorry about this.

Many thanks for that point.

Regards

jnadaud pushed a commit that referenced this pull request Aug 20, 2015
Jérôme Nadaud Jérôme Nadaud
Merge pull request #3553 from kermes/patch-5
[-] FO: auth error if $back is not defined
jnadaud pushed a commit that referenced this pull request Aug 20, 2015
Jérôme Nadaud Jérôme Nadaud
Merge pull request #3553 from kermes/patch-5
[-] FO: auth error if $back is not defined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.