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

Pass user object to preprocess function. #483

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roelvanduijnhoven
Copy link
Contributor

Currently I am trying to let users logon to ZfcUser using legacy passwords. I was thinking of doing so by using the preprocess function. However it fails in that regard.

For this to work I need to have a case distinction between old and new users. I obviously have that state in my UserEntity model. There is however no way to access the user object at preprocessing time.

Therefore this PR adds a second paramter to the function passing in the user object. I find the API to still be easy usable and this change is BC because passing in a callback using a single parameter is still valid.

@@ -132,10 +133,10 @@ protected function updateUserPasswordHash(UserEntity $user, $password, Bcrypt $b
$this->getMapper()->update($user);
}

public function preprocessCredential($credential)
public function preprocessCredential($credential, UserEntity $user)
Copy link
Contributor

Choose a reason for hiding this comment

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

UserInterface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UserInterface is imported as UserEntity. I did not touch this, but this was already the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could resolve this in different PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry about that. ZfcUser\Mapper\UserInterface will be renamed to UserMapperInterface in 2.0 which will resolve this "collision".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see! So I guess I should leave it the way it is?

@Danielss89 Danielss89 added this to the 2.0.0 milestone Jul 4, 2014
@roelvanduijnhoven
Copy link
Contributor Author

I am currently using this to to log on with legacy passwords and it works as expected. Would be great to have this merged.

@stijnhau
Copy link

@Danielss89
Any comment on this PR, why isn't it yet merged?

@grizzm0
Copy link
Contributor

grizzm0 commented Jan 24, 2015

I guess it's OK to merge. The other function already pass this if I'm not mistaken. Can't check atm as I'm on the phone.

@stijnhau
Copy link

Any progress on this?

@Danielss89
Copy link
Member

This is for 2.0 which is still in the future, so i'll keep them as PR's until i'm ready to do more about it.

@stijnhau
Copy link

stijnhau commented Jul 6, 2019

Maybe this can be merged in now?###

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants