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

Ensure that UserID matches database when logging in #3178

Merged
merged 1 commit into from Oct 17, 2017

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Oct 12, 2017

The MySQL string comparison in SinglePointLogin was
authenticating in a case-insensitive manner, resulting
in various places in the code failing if they tried to
compare $_SESSION['State']->getUsername() (which has
the value from when the user logged in) with User::singleton()->getUsername()
(which has the value from the database) in PHP (which,
unlike MySQL, is case sensitive.)

This updates the SinglePointLogin class so that it uses
the username from the database, rather than the HTTP request
for the username in $_SESSION['State'].

@driusan driusan added Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) [branch] bugfix labels Oct 12, 2017
@driusan driusan added this to the 18.0.2 milestone Oct 12, 2017
Copy link
Collaborator

@ridz1208 ridz1208 left a comment

Choose a reason for hiding this comment

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

line 456 makes your change useless

The MySQL string comparison in SinglePointLogin was
authenticating in a case-insensitive manner, resulting
in various places in the code failing if they tried to
compare $_SESSION['State']->getUsername() (which has
the value from when the user logged in) with User::singleton()->getUsername()
(which has the value from the database) in PHP (which,
unlike MySQL, *is* case sensitive.)

This updates the SinglePointLogin class so that it uses
the username from the database, rather than the HTTP request
for the username in $_SESSION['State'].
@driusan
Copy link
Collaborator Author

driusan commented Oct 12, 2017

@ridz1208 Good catch, I updated it.

@ridz1208 ridz1208 added the Passed Manual Tests PR has undergone proper testing by at least one peer label Oct 12, 2017
@driusan driusan merged commit 89ddc81 into aces:bugfix Oct 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants