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

Use a UserChecker #56

Merged
merged 3 commits into from Dec 6, 2018
Merged

Use a UserChecker #56

merged 3 commits into from Dec 6, 2018

Conversation

sandermarechal
Copy link
Contributor

Use a UserChecker during authentication. This fixes #52

@sandermarechal
Copy link
Contributor Author

sandermarechal commented Mar 16, 2017

Perhaps the documentation should even go as far as recommending (or requiring?) to set a user provider? Not doing so is a security issue. The default user checker checks for inactive or locked accounts, but users generated by this bundles user_provider generally pass all those checks, even when they should not.

Copy link

@xthiago xthiago left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@juliusstoerrle
Copy link

What's the plan for this merge request? We need this feature in order to use this Bundle and would appreciate if those changes would be merged.

@lashae
Copy link

lashae commented Dec 21, 2017

Is there any progress or ETA about merging this PR? It is fixing a severe security flaw and it is 9 months old.

@xthiago
Copy link

xthiago commented Dec 22, 2017

I'm guess the project was abandoned. The author does not reply any pull-request. I'm considering creating a fork project.

@markitosgv
Copy link
Owner

@xthiago Im searching for some collaborators to help me with this repository, do you want to participate?

@markitosgv markitosgv closed this Dec 1, 2018
@markitosgv markitosgv reopened this Dec 5, 2018
@markitosgv
Copy link
Owner

markitosgv commented Dec 5, 2018

@sandermarechal could you please up to date this PR and resolve conflicts for merging? thanks

@sandermarechal
Copy link
Contributor Author

I have rebased on master and resolved all the conflicts. Only, I could not run the tests locally because I do not have the mongodb extension installed and composer refuses to install. Perhaps the mongodb extension should be optional for testing, just skipping the tests?

@markitosgv markitosgv merged commit 604b70c into markitosgv:master Dec 6, 2018
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.

Security issue: Loadable users are always authenticated
5 participants