[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #846: GUACAMOLE-1780: Fix TOTP interaction with SAML and SSL

["jmuehlner (via GitHub)" <gi...@apache.org>]

jmuehlner commented on code in PR #846:
URL: https://github.com/apache/guacamole-client/pull/846#discussion_r1181948531


##########
guacamole-ext/src/main/java/org/apache/guacamole/net/auth/AuthenticationProvider.java:
##########
@@ -247,5 +247,42 @@ UserContext redecorate(UserContext decorated, UserContext context,
      * shutting down.
      */
     void shutdown();
+
+    /**
+     * Called when the overall authentication attempt failed, despite a
+     * successful authentication or decoration for this authentication provider.
+     * Implementations may add custom behavior here - by default this method
+     * does not do anything.
+     *
+     * @param authenticatedUser
+     *      The user who successfully authenticated with this authentication
+     *      provider.
+     *
+     * @param credentials
+     *     The credentials which were most recently submitted for the given
+     *     AuthenticatedUser. These are not guaranteed to be the same as the
+     *     credentials associated with the AuthenticatedUser object, which are
+     *     the credentials provided when the user originally authenticated.
+     */
+    default void notifyAuthenticationFailure(
+                AuthenticatedUser authenticatedUser, Credentials credentials) {};
+
+    /**
+     * Called when the overall authentication attempt succeeded.
+     * Implementations may add custom behavior here - by default this method
+     * does not do anything.
+     *
+     * @param authenticatedUser
+     *      The user who successfully authenticated with this authentication
+     *      provider.
+     *
+     * @param credentials
+     *     The credentials which were most recently submitted for the given
+     *     AuthenticatedUser. These are not guaranteed to be the same as the
+     *     credentials associated with the AuthenticatedUser object, which are
+     *     the credentials provided when the user originally authenticated.
+     */
+    default void notifyAuthenticationSuccess(
+                AuthenticatedUser authenticatedUser, Credentials credentials) {};

Review Comment:
   I had some ideas when I was first working on this that it was important that failures should only be reported to auth providers that had actually succeeded, but that turned out to not really matter for this feature - and it's pretty easy for auth providers to keep track of that manually if required. 
   
   I've switched over to the standard `Listener` approach now. Much simpler.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org