Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/197#discussion_r145886535
--- Diff:
guacamole-ext/src/main/java/org/apache/guacamole/net/event/AuthenticationSuccessEvent.java
---
@@ -43,19 +44,19 @@
/**
* The credentials which passed authentication.
*/
- private Credentials credentials;
+ private AuthenticatedUser authenticatedUser;
/**
* Creates a new AuthenticationSuccessEvent which represents a
successful
* authentication attempt with the given credentials.
*
* @param context The UserContext created as a result of successful
* authentication.
- * @param credentials The credentials which passed authentication.
+ * @param authenticatedUser The user which passed authentication.
*/
- public AuthenticationSuccessEvent(UserContext context, Credentials
credentials) {
+ public AuthenticationSuccessEvent(UserContext context,
AuthenticatedUser authenticatedUser) {
--- End diff --
>
`authenticatedUser.getAuthenticationProvider().getUserContext(authenticatedUser)`
I'm worried about that approach.
The `UserContext` is effectively the data side of a user's session, and
typically is only created once, with that instance existing until the user logs
out. If an extension implementation needed to persist some sort of state
throughout the user's session, the lifecycle of that state would be tied to the
`UserContext`. There's also the new `invalidate()` function to consider, which
will need to be properly invoked for that short-lived `UserContext`.
This could have performance consequences down the line.
---
