Skip to content

Conversation

@SamFleming
Copy link

When binding the the POST_BIND event I found the need to have access to the security token.

I'm not sure if this now makes passing in the user as the first parameter redundant due to being able to access it from the token.

@johnkary
Copy link
Contributor

Yes, passing the User is redundant when passing the Token. Changing the constructor to only pass the Token would break backwards compatibility. So if we decide this is a good idea (I'm +1) we should only do it across a minor version bump (2.3 to 2.4) and document in a changelog.

@SamFleming
Copy link
Author

Breaking backwards-compatibility did cross my mind. Although I assumed that consumers of the bundle don't instantiate the LdapUserEvent class directly. Its only use is within the bundle.

What would definitely break the backwards-compatibility is removing the LdapUserEvent::getUser() methods. Unless it was used as a convenience function to return $this->getToken()->getUser() - not sure if that's a the right thing to do or just a code smell.

Othewise, 👍 for a minor version bump.

@SamFleming
Copy link
Author

Closing as it's now been covered by @vtek21 here #151

@SamFleming SamFleming closed this Aug 9, 2016
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.

2 participants