play-authenticate icon indicating copy to clipboard operation
play-authenticate copied to clipboard

Add an event API?

Open smola opened this issue 11 years ago • 10 comments

While working on a remember me provider (see issue https://github.com/joscha/play-authenticate/issues/38) I created Resolver.onAuthSuccessHook(). This looks a bit forced to me so I looked up in SecureSocial and saw its Events API. Adding such API would help remember me and it would probably make code for other use cases cleaner.

What are your thoughts on this?

smola avatar Mar 20 '13 08:03 smola

An events API would be awesome! It would also allow users to subscribe to merge/creation events, etc.

joscha avatar Apr 04 '13 17:04 joscha

Just for the record, issue #104 could also benefit from this. It would be specially useful for my setup (since I use a custom mailer module and play-authenticate's emails are the only ones not getting through my mail system).

smola avatar Aug 26 '13 10:08 smola

I think my app would benefit from sth like this, too. Specifically, I have to carry out some additional tasks after a users logs in or out. Currently, I basically would have to duplicate PlayAuthenticate and add my custom operations to loginAndRedirect(). So I'd be willing to contribute to this.

@joscha Any rough sketch on how to implement this?

Which events should be emitted? For me it would be sth like afterLogin, afterSignup and afterLogout, although the latter would not really be necessary as the logout flow has got only one entry point and is easily overridable as of today.

jtammen avatar Sep 01 '13 15:09 jtammen

@jtammen afterMerge and afterLink would be nice as well. Regarding the implementation I am actually completely open for suggestions as long as it doesn't render the current API of the library backwards-incompatible.

joscha avatar Sep 01 '13 19:09 joscha

SecureSocial's implementation might be a good start. See http://securesocial.ws/guide/events.html

An EventListener would implement something like public void onEvent(Event event, Request request, Session session). Or we might want to do just public void onEvent(Event event, Context context), I'm not sure of the implications of either approach. We might pass the provider as an argument too?

Their events are LoginEvent, LogoutEvent, SignupEvent, PasswordResetEvent, PasswordChangeEvent. As @joscha points out, MergeEvent and LinkEvent might be useful too. I wonder if FailedAuthenticationEvent would make sense too.

There could be a method PlayAuthenticate.addEventListener(EventListener eventListener) for adding the listener during onStart. And all event listeners would be called on each event. That would be fully backwards-compatible.

Here are some use cases:

  • Sending verification emails -> SignupEvent.
  • I have to do changeLang (change the PLAY_LANG cookie) on login, logout or profile update). Profile update can be handled in a controller. Login and logout would be handled with LoginEvent and LogoutEvent.
  • On a successful login, I check for a parameter in the request asking for a "Remember Me" value, if that's true, I add a remember me cookie. -> LoginEvent.

What use cases do you have? I think, in my case, all of them would be implementable with such an event listener interface. But if we need to modify the behaviour of the authentication itself, we would need a hook interface instead of event listeners. For example, a hook would implement public LoginResult onLogin(Context context) and the authentication process would have to take the result into account for its behaviour. Both approaches are compatible anyway.

smola avatar Sep 01 '13 22:09 smola

I don't think it makes sense that we should mix events with hooks - a bus like interface like onEvent would not work well if you have to wait for listeners to respond and there would be problems especially if resulting actions are incompatible with each other. Also, the actions would need to be synchronised and the response would always take as long as the slowest listener. So I'd suggest let's focus on the events in this ticket here and if we need hooks we can open another one and think about the implementation independently of the eventing framework.

joscha avatar Sep 02 '13 06:09 joscha

@joscha Alright :+1:

smola avatar Sep 02 '13 07:09 smola

@smola and @joscha, thanks for your feedback!

My use cases are at the moment:

  • After a user has logged in: send a cookie containing a XSRF auth token that will be sent back to the server by my AngularJS based client. For that, I'd need access to the response – don't know atm whether it would be okay to pass the whole Context to the event listener.
  • After logout: invalidate the auth token. Again, access to response to be able to delete cookie would be needed.
  • After signup: subscribe the user to a newsletter list provided by an external service. In the case of the username/password authentication this would have to happen in the verify handler, so there is no need for the event handling in this case. Which also means it would be nice to have the authentication provider available to be able to ignore/treat differently SignupEvents for the username/password provider.

jtammen avatar Sep 03 '13 12:09 jtammen

@joscha and @smola, any thoughts on my use cases?

jtammen avatar Sep 17 '13 14:09 jtammen

@jtammen passing the context is okaz I think - in fact is probably even better than passing the response because as far as I remember the Request is a different Interface in Scala/Java Play, the context isn't. Also changing the API so an authentication providercan choose to ignore,etc. SignupEvents is fine I guess - I would love to make sure that the API stays backwards compatible though and that it doesn't get more complex for non-password providers.

joscha avatar Sep 20 '13 10:09 joscha