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

Fixed AuthenticaionFailureHandler to utilize messages from custom exceptions #650

Merged
merged 1 commit into from
May 5, 2019
Merged

Conversation

EresDev
Copy link

@EresDev EresDev commented May 2, 2019

Fixed #588

@chalasr
Copy link
Collaborator

chalasr commented May 5, 2019

Thank you @EresDev.

@chalasr chalasr merged commit 9414427 into lexik:master May 5, 2019
chalasr added a commit that referenced this pull request May 5, 2019
…ustom exceptions (EresDev)

This PR was squashed before being merged into the 2.x-dev branch (closes #650).

Discussion
----------

Fixed AuthenticaionFailureHandler to utilize messages from custom exceptions

Fixed #588

Commits
-------

9414427 Fixed AuthenticaionFailureHandler to utilize messages from custom exceptions
@teohhanhui
Copy link
Contributor

This is wrong. getMessage is not safe to be displayed to the user: symfony/symfony#18865 (comment)

@nik-lampe
Copy link

What about this? @chalasr

After looking at the issue @teohhanhui linked to me it seems like the code should use $exception->getMessageKey() instead of $exception->getMessage() to display a message which is safe to be displayed to the user.

chalasr added a commit that referenced this pull request Oct 26, 2019
…is build env, and .idea to gitignore (EresDev)

This PR was squashed before being merged into the 2.x-dev branch (closes #687).

Discussion
----------

Authentication Exception Message from its key, Explicit Travis build env, and .idea to gitignore

There are 3 issues addressed here. I would have used different PR for each issue but there was problem with travis build env that wouldn't have produced accurate results if not combined.

1. Updated Travis build env as you can see php 5.5 build are failing. Travis has recently [changed it default build env to Xenial](https://blog.travis-ci.com/2019-04-15-xenial-default-build-environment) and I have explicitly specified it to be "Trusty".

2. Addresses [the concern](#650 (comment)) of Authentication Exception message to be retrieved by getMessage. I have changed it to getMessageKey method to meet symfony convention, help translation and prevent possible leak of sensitive info to end user. Also addresses #684

3. Adds .idea to .gitignore
Most of use PHPStorm and it creates .idea folder that is an issue while working. It should be in .gitignore to prevent its accidental addition to repository. It also helps to work on this library quickly as you don't have to deal with .idea first.

Commits
-------

abc1a01 Added explicit build environment for travis. Because Travis has changed its default build enviorment to Xenial and it does not compile php 5.5. Making dist: trusty
e1fcdb5 Getting exception message via getMessageKey method to meet Symfony convention, to help with translation, and to prevent possible leak of sensitive information.
a5d5109 Adding .idea to .gitignore to prevent accidental commit/push of IDE files.
@nik-lampe
Copy link

Great, thanks for merging.

When will be the next release?

@chalasr
Copy link
Collaborator

chalasr commented Nov 22, 2019

@nik-lampe v2.6.5 has been tagged

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.

UserChecker Exception Messages are lost. It is only "bad credentials" when there is any issue with credentials
4 participants