mike-jumper commented on a change in pull request #254:
URL: https://github.com/apache/guacamole-client/pull/254#discussion_r444476526



##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java
##########
@@ -0,0 +1,264 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.onelogin.saml2.authn.AuthnRequest;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.SettingsException;
+import com.onelogin.saml2.exception.ValidationError;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.util.Util;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.xpath.XPathExpressionException;
+import org.apache.guacamole.auth.saml.conf.ConfigurationService;
+import org.apache.guacamole.auth.saml.user.SAMLAuthenticatedUser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.form.Field;
+import org.apache.guacamole.form.RedirectField;
+import org.apache.guacamole.language.TranslatableMessage;
+import org.apache.guacamole.net.auth.AuthenticatedUser;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import 
org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
+import 
org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.xml.sax.SAXException;
+
+/**
+ * Class that provides services for use by the SAMLAuthenticationProvider 
class.
+ */
+public class AuthenticationProviderService {
+
+    /**
+     * Logger for this class.
+     */
+    private final Logger logger = 
LoggerFactory.getLogger(AuthenticationProviderService.class);
+
+    /**
+     * Service for retrieving SAML configuration information.
+     */
+    @Inject
+    private ConfigurationService confService;
+
+    /**
+     * Provider for AuthenticatedUser objects.
+     */
+    @Inject
+    private Provider<SAMLAuthenticatedUser> authenticatedUserProvider;
+    
+    /**
+     * The map used to track active SAML responses.
+     */
+    @Inject
+    private SAMLResponseMap samlResponseMap;
+
+    /**
+     * Returns an AuthenticatedUser representing the user authenticated by the
+     * given credentials.
+     *
+     * @param credentials
+     *     The credentials to use for authentication.
+     *
+     * @return
+     *     An AuthenticatedUser representing the user authenticated by the
+     *     given credentials.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while authenticating the user, or if access is
+     *     denied.
+     */
+    public AuthenticatedUser authenticateUser(Credentials credentials)
+            throws GuacamoleException {
+        
+        HttpServletRequest request = credentials.getRequest();
+
+        // Initialize and configure SAML client.
+        Saml2Settings samlSettings = confService.getSamlSettings();
+
+        if (request != null) {
+            
+            // Look for the SAML Response parameter.
+            String responseHash = request.getParameter("responseHash");
+
+            if (responseHash != null) {
+
+                try {
+
+                    // Generate the response object
+                    if (!samlResponseMap.hasSamlResponse(responseHash)) {
+                        logger.warn("SAML response was not found.");
+                        logger.debug("SAML response hash {} not fonud in 
response map.", responseHash);
+                        throw new 
GuacamoleInvalidCredentialsException("Provided response was not found.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+                        
+                    SamlResponse samlResponse = 
samlResponseMap.getSamlResponse(responseHash);
+
+                    if (!samlResponse.validateNumAssertions()) {
+                        logger.warn("SAML response contained other than single 
assertion.");
+                        logger.debug("validateNumAssertions returned false.");
+                        throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+                    if (!samlResponse.validateTimestamps()) {
+                        logger.warn("SAML response timestamps were invalid.");
+                        logger.debug("validateTimestamps returned false.");
+                        throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+
+                    // Grab the username, and, if present, finish 
authentication.
+                    String username = samlResponse.getNameId().toLowerCase();
+                    if (username != null) {
+                        
+                        // Retrieve any provided attributes
+                        Map<String, List<String>> attributes =
+                                samlResponse.getAttributes();
+                        
+                        // Back-port the username to the credentials
+                        credentials.setUsername(username);
+                        
+                        // Configure the AuthenticatedUser and return it
+                        SAMLAuthenticatedUser authenticatedUser =
+                                authenticatedUserProvider.get();
+                        
+                        authenticatedUser.init(username, credentials,
+                                parseTokens(attributes),
+                                parseGroups(attributes, 
confService.getGroupAttribute()));
+                        
+                        return authenticatedUser;
+                    }
+                }
+
+                // Errors are logged and result in a normal username/password 
login box.
+                catch (IOException e) {
+                    logger.warn("Error during I/O while parsing SAML response: 
{}", e.getMessage());
+                    logger.debug("Received IOException when trying to parse 
SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (ParserConfigurationException e) {
+                    logger.warn("Error configuring XML parser: {}", 
e.getMessage());
+                    logger.debug("Received ParserConfigurationException when 
trying to parse SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (SAXException e) {
+                    logger.warn("Bad XML when parsing SAML response: {}", 
e.getMessage());
+                    logger.debug("Received SAXException while parsing SAML 
response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (SettingsException e) {
+                    logger.warn("Error with SAML settings while parsing 
response: {}", e.getMessage());
+                    logger.debug("Received SettingsException while parsing 
SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (ValidationError e) {
+                    logger.warn("Error validating SAML response: {}", 
e.getMessage());
+                    logger.debug("Received ValidationError while parsing SAML 
response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (XPathExpressionException e) {
+                    logger.warn("Problem with XML parsing response: {}", 
e.getMessage());
+                    logger.debug("Received XPathExpressionException while 
processing SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (Exception e) {
+                    logger.warn("Exception while getting name from SAML 
response: {}", e.getMessage());
+                    logger.debug("Received Exception while retrieving name 
from SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+            }
+        }
+
+        // No SAML Response is present, so generate a request.
+        AuthnRequest samlReq = new AuthnRequest(samlSettings);
+        URI authUri;
+        try {
+            authUri = new URI(samlSettings.getIdpSingleSignOnServiceUrl() + 
"?SAMLRequest=" +
+                    Util.urlEncoder(samlReq.getEncodedAuthnRequest()));
+        }
+        catch (IOException e) {
+            logger.error("Error encoding authentication request to string: 
{}", e.getMessage());
+            logger.debug("Got IOException encoding authentication request.", 
e);
+            throw new GuacamoleInvalidCredentialsException("Error during SAML 
login.",
+                    CredentialsInfo.USERNAME_PASSWORD);
+        }
+        catch(URISyntaxException e) {
+            logger.error("Error generating URI for authentication redirect: 
{}", e.getMessage());
+            logger.debug("Got URISyntaxException generating authentication 
URI", e);
+            throw new GuacamoleInvalidCredentialsException("Error during SAML 
login.",
+                    CredentialsInfo.USERNAME_PASSWORD);
+        }
+
+        // Redirect to SAML Identity Provider (IdP)
+        throw new GuacamoleInsufficientCredentialsException("Redirecting to 
SAML IdP.",
+                new CredentialsInfo(Arrays.asList(new Field[] {
+                    new RedirectField("samlRedirect", authUri, new 
TranslatableMessage("LOGIN.INFO_SAML_REDIRECT_PENDING"))
+                }))
+        );
+
+    }
+    
+    private Map<String, String> parseTokens(Map<String, List<String>> 
attributes)
+            throws GuacamoleException {
+        
+        Map<String, String> tokens = new HashMap<>();
+        for (Entry<String, List<String>> entry : attributes.entrySet()) {
+            
+            List<String> values = entry.getValue();
+            tokens.put(entry.getKey(), values.get(0));
+            
+        }
+        
+        return tokens;
+        
+    }
+    
+    private Set<String> parseGroups(Map<String, List<String>> attributes, 
String groupAttribute) throws GuacamoleException {

Review comment:
       Please document.

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java
##########
@@ -0,0 +1,264 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.onelogin.saml2.authn.AuthnRequest;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.SettingsException;
+import com.onelogin.saml2.exception.ValidationError;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.util.Util;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.xpath.XPathExpressionException;
+import org.apache.guacamole.auth.saml.conf.ConfigurationService;
+import org.apache.guacamole.auth.saml.user.SAMLAuthenticatedUser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.form.Field;
+import org.apache.guacamole.form.RedirectField;
+import org.apache.guacamole.language.TranslatableMessage;
+import org.apache.guacamole.net.auth.AuthenticatedUser;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import 
org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException;
+import 
org.apache.guacamole.net.auth.credentials.GuacamoleInsufficientCredentialsException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.xml.sax.SAXException;
+
+/**
+ * Class that provides services for use by the SAMLAuthenticationProvider 
class.
+ */
+public class AuthenticationProviderService {
+
+    /**
+     * Logger for this class.
+     */
+    private final Logger logger = 
LoggerFactory.getLogger(AuthenticationProviderService.class);
+
+    /**
+     * Service for retrieving SAML configuration information.
+     */
+    @Inject
+    private ConfigurationService confService;
+
+    /**
+     * Provider for AuthenticatedUser objects.
+     */
+    @Inject
+    private Provider<SAMLAuthenticatedUser> authenticatedUserProvider;
+    
+    /**
+     * The map used to track active SAML responses.
+     */
+    @Inject
+    private SAMLResponseMap samlResponseMap;
+
+    /**
+     * Returns an AuthenticatedUser representing the user authenticated by the
+     * given credentials.
+     *
+     * @param credentials
+     *     The credentials to use for authentication.
+     *
+     * @return
+     *     An AuthenticatedUser representing the user authenticated by the
+     *     given credentials.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while authenticating the user, or if access is
+     *     denied.
+     */
+    public AuthenticatedUser authenticateUser(Credentials credentials)
+            throws GuacamoleException {
+        
+        HttpServletRequest request = credentials.getRequest();
+
+        // Initialize and configure SAML client.
+        Saml2Settings samlSettings = confService.getSamlSettings();
+
+        if (request != null) {
+            
+            // Look for the SAML Response parameter.
+            String responseHash = request.getParameter("responseHash");
+
+            if (responseHash != null) {
+
+                try {
+
+                    // Generate the response object
+                    if (!samlResponseMap.hasSamlResponse(responseHash)) {
+                        logger.warn("SAML response was not found.");
+                        logger.debug("SAML response hash {} not fonud in 
response map.", responseHash);
+                        throw new 
GuacamoleInvalidCredentialsException("Provided response was not found.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+                        
+                    SamlResponse samlResponse = 
samlResponseMap.getSamlResponse(responseHash);
+
+                    if (!samlResponse.validateNumAssertions()) {
+                        logger.warn("SAML response contained other than single 
assertion.");
+                        logger.debug("validateNumAssertions returned false.");
+                        throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+                    if (!samlResponse.validateTimestamps()) {
+                        logger.warn("SAML response timestamps were invalid.");
+                        logger.debug("validateTimestamps returned false.");
+                        throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                    }
+
+                    // Grab the username, and, if present, finish 
authentication.
+                    String username = samlResponse.getNameId().toLowerCase();
+                    if (username != null) {
+                        
+                        // Retrieve any provided attributes
+                        Map<String, List<String>> attributes =
+                                samlResponse.getAttributes();
+                        
+                        // Back-port the username to the credentials
+                        credentials.setUsername(username);
+                        
+                        // Configure the AuthenticatedUser and return it
+                        SAMLAuthenticatedUser authenticatedUser =
+                                authenticatedUserProvider.get();
+                        
+                        authenticatedUser.init(username, credentials,
+                                parseTokens(attributes),
+                                parseGroups(attributes, 
confService.getGroupAttribute()));
+                        
+                        return authenticatedUser;
+                    }
+                }
+
+                // Errors are logged and result in a normal username/password 
login box.
+                catch (IOException e) {
+                    logger.warn("Error during I/O while parsing SAML response: 
{}", e.getMessage());
+                    logger.debug("Received IOException when trying to parse 
SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (ParserConfigurationException e) {
+                    logger.warn("Error configuring XML parser: {}", 
e.getMessage());
+                    logger.debug("Received ParserConfigurationException when 
trying to parse SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (SAXException e) {
+                    logger.warn("Bad XML when parsing SAML response: {}", 
e.getMessage());
+                    logger.debug("Received SAXException while parsing SAML 
response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (SettingsException e) {
+                    logger.warn("Error with SAML settings while parsing 
response: {}", e.getMessage());
+                    logger.debug("Received SettingsException while parsing 
SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (ValidationError e) {
+                    logger.warn("Error validating SAML response: {}", 
e.getMessage());
+                    logger.debug("Received ValidationError while parsing SAML 
response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (XPathExpressionException e) {
+                    logger.warn("Problem with XML parsing response: {}", 
e.getMessage());
+                    logger.debug("Received XPathExpressionException while 
processing SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+                catch (Exception e) {
+                    logger.warn("Exception while getting name from SAML 
response: {}", e.getMessage());
+                    logger.debug("Received Exception while retrieving name 
from SAML response.", e);
+                    throw new GuacamoleInvalidCredentialsException("Error 
during SAML login.",
+                                CredentialsInfo.USERNAME_PASSWORD);
+                }
+            }
+        }
+
+        // No SAML Response is present, so generate a request.
+        AuthnRequest samlReq = new AuthnRequest(samlSettings);
+        URI authUri;
+        try {
+            authUri = new URI(samlSettings.getIdpSingleSignOnServiceUrl() + 
"?SAMLRequest=" +
+                    Util.urlEncoder(samlReq.getEncodedAuthnRequest()));
+        }
+        catch (IOException e) {
+            logger.error("Error encoding authentication request to string: 
{}", e.getMessage());
+            logger.debug("Got IOException encoding authentication request.", 
e);
+            throw new GuacamoleInvalidCredentialsException("Error during SAML 
login.",
+                    CredentialsInfo.USERNAME_PASSWORD);
+        }
+        catch(URISyntaxException e) {
+            logger.error("Error generating URI for authentication redirect: 
{}", e.getMessage());
+            logger.debug("Got URISyntaxException generating authentication 
URI", e);
+            throw new GuacamoleInvalidCredentialsException("Error during SAML 
login.",
+                    CredentialsInfo.USERNAME_PASSWORD);
+        }
+
+        // Redirect to SAML Identity Provider (IdP)
+        throw new GuacamoleInsufficientCredentialsException("Redirecting to 
SAML IdP.",
+                new CredentialsInfo(Arrays.asList(new Field[] {
+                    new RedirectField("samlRedirect", authUri, new 
TranslatableMessage("LOGIN.INFO_SAML_REDIRECT_PENDING"))
+                }))
+        );
+
+    }
+    
+    private Map<String, String> parseTokens(Map<String, List<String>> 
attributes)
+            throws GuacamoleException {

Review comment:
       Please document.

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderResource.java
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Inject;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.SettingsException;
+import com.onelogin.saml2.exception.ValidationError;
+import com.onelogin.saml2.http.HttpRequest;
+import com.onelogin.saml2.servlet.ServletUtils;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.util.Util;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.FormParam;
+import javax.ws.rs.Path;
+import javax.ws.rs.POST;
+import javax.ws.rs.core.Context;
+import javax.xml.bind.DatatypeConverter;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.xpath.XPathExpressionException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.saml.conf.ConfigurationService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.xml.sax.SAXException;
+
+/**
+ * A class that implements the REST API necessary for the
+ * SAML Idp to POST back its response to Guacamole.
+ */
+public class SAMLAuthenticationProviderResource {
+
+    /**
+     * Logger for this class.
+     */
+    private final Logger logger =
+            LoggerFactory.getLogger(SAMLAuthenticationProviderResource.class);
+    
+    /**
+     * The configuration service for this module.
+     */
+    @Inject
+    private ConfigurationService confService;
+    
+    /**
+     * The map used to track active responses.
+     */
+    @Inject
+    private SAMLResponseMap samlResponseMap;
+
+    /**
+     * A REST endpoint that is POSTed to by the SAML IdP
+     * with the results of the SAML SSO Authentication.
+     * 
+     * @param samlResponseString
+     *     The encoded response returned by the SAML IdP.
+     * 
+     * @param consumedRequest
+     *     The HttpServletRequest associated with the SAML response. The
+     *     parameters of this request may not be accessible, as the request may
+     *     have been fully consumed by JAX-RS.
+     * 
+     * @return
+     *     A HTTP Response that will redirect the user back to the
+     *     Guacamole home page, with the SAMLResponse encoded in the
+     *     return URL.
+     * 
+     * @throws GuacamoleException
+     *     If the Guacamole configuration cannot be read or an error occurs
+     *     parsing a URI.
+     */
+    @POST
+    @Path("callback")
+    public Response processSamlResponse(
+            @FormParam("SAMLResponse") String samlResponseString,
+            @Context HttpServletRequest consumedRequest)
+            throws GuacamoleException {
+        
+        String guacBase = confService.getCallbackUrl().toString();
+        Saml2Settings samlSettings = confService.getSamlSettings();
+        try {
+            HttpRequest request = ServletUtils
+                    .makeHttpRequest(consumedRequest)
+                    .addParameter("SAMLResponse", samlResponseString);
+            SamlResponse samlResponse = new SamlResponse(samlSettings, 
request);
+            
+            String responseHash = hashSamlResponse(samlResponseString);
+            samlResponseMap.putSamlResponse(responseHash, samlResponse);
+            return Response.seeOther(new URI(guacBase 
+                    + "?responseHash="
+                    + Util.urlEncoder(responseHash))
+            ).build();
+
+        }
+        catch (IOException e) {
+            throw new GuacamoleServerException("I/O exception processing SAML 
response.", e);
+        }
+        catch (NoSuchAlgorithmException e) {
+            throw new GuacamoleServerException("Unexpected missing SHA-256 
support while generating SAML response hash.", e);
+        }
+        catch (ParserConfigurationException e) {
+            throw new GuacamoleServerException("Parser exception processing 
SAML response.", e);
+        }
+        catch (SAXException e) {
+            throw new GuacamoleServerException("SAX exception processing SAML 
response.", e);
+        }
+        catch (SettingsException e) {
+            throw new GuacamoleServerException("Settings exception processing 
SAML response.", e);
+        }
+        catch (URISyntaxException e) {
+            throw new GuacamoleServerException("URI exception process SAML 
response.", e);
+        }
+        catch (ValidationError e) {
+            throw new GuacamoleServerException("Exception validating SAML 
response.", e);
+        }
+        catch (XPathExpressionException e) {
+            throw new GuacamoleServerException("XML Xpath exception validating 
SAML response.", e);
+        }
+
+    }
+    
+    /**
+     * This is a utility method designed to generate a SHA-256 has for the

Review comment:
       hash*

##########
File path: extensions/guacamole-auth-saml/src/licenses/LICENSE
##########
@@ -0,0 +1,257 @@
+
+                                 Apache License
+                           Version 2.0, January 2004
+                        http://www.apache.org/licenses/
+
+   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
+
+   1. Definitions.
+
+      "License" shall mean the terms and conditions for use, reproduction,
+      and distribution as defined by Sections 1 through 9 of this document.
+
+      "Licensor" shall mean the copyright owner or entity authorized by
+      the copyright owner that is granting the License.
+
+      "Legal Entity" shall mean the union of the acting entity and all
+      other entities that control, are controlled by, or are under common
+      control with that entity. For the purposes of this definition,
+      "control" means (i) the power, direct or indirect, to cause the
+      direction or management of such entity, whether by contract or
+      otherwise, or (ii) ownership of fifty percent (50%) or more of the
+      outstanding shares, or (iii) beneficial ownership of such entity.
+
+      "You" (or "Your") shall mean an individual or Legal Entity
+      exercising permissions granted by this License.
+
+      "Source" form shall mean the preferred form for making modifications,
+      including but not limited to software source code, documentation
+      source, and configuration files.
+
+      "Object" form shall mean any form resulting from mechanical
+      transformation or translation of a Source form, including but
+      not limited to compiled object code, generated documentation,
+      and conversions to other media types.
+
+      "Work" shall mean the work of authorship, whether in Source or
+      Object form, made available under the License, as indicated by a
+      copyright notice that is included in or attached to the work
+      (an example is provided in the Appendix below).
+
+      "Derivative Works" shall mean any work, whether in Source or Object
+      form, that is based on (or derived from) the Work and for which the
+      editorial revisions, annotations, elaborations, or other modifications
+      represent, as a whole, an original work of authorship. For the purposes
+      of this License, Derivative Works shall not include works that remain
+      separable from, or merely link (or bind by name) to the interfaces of,
+      the Work and Derivative Works thereof.
+
+      "Contribution" shall mean any work of authorship, including
+      the original version of the Work and any modifications or additions
+      to that Work or Derivative Works thereof, that is intentionally
+      submitted to Licensor for inclusion in the Work by the copyright owner
+      or by an individual or Legal Entity authorized to submit on behalf of
+      the copyright owner. For the purposes of this definition, "submitted"
+      means any form of electronic, verbal, or written communication sent
+      to the Licensor or its representatives, including but not limited to
+      communication on electronic mailing lists, source code control systems,
+      and issue tracking systems that are managed by, or on behalf of, the
+      Licensor for the purpose of discussing and improving the Work, but
+      excluding communication that is conspicuously marked or otherwise
+      designated in writing by the copyright owner as "Not a Contribution."
+
+      "Contributor" shall mean Licensor and any individual or Legal Entity
+      on behalf of whom a Contribution has been received by Licensor and
+      subsequently incorporated within the Work.
+
+   2. Grant of Copyright License. Subject to the terms and conditions of
+      this License, each Contributor hereby grants to You a perpetual,
+      worldwide, non-exclusive, no-charge, royalty-free, irrevocable
+      copyright license to reproduce, prepare Derivative Works of,
+      publicly display, publicly perform, sublicense, and distribute the
+      Work and such Derivative Works in Source or Object form.
+
+   3. Grant of Patent License. Subject to the terms and conditions of
+      this License, each Contributor hereby grants to You a perpetual,
+      worldwide, non-exclusive, no-charge, royalty-free, irrevocable
+      (except as stated in this section) patent license to make, have made,
+      use, offer to sell, sell, import, and otherwise transfer the Work,
+      where such license applies only to those patent claims licensable
+      by such Contributor that are necessarily infringed by their
+      Contribution(s) alone or by combination of their Contribution(s)
+      with the Work to which such Contribution(s) was submitted. If You
+      institute patent litigation against any entity (including a
+      cross-claim or counterclaim in a lawsuit) alleging that the Work
+      or a Contribution incorporated within the Work constitutes direct
+      or contributory patent infringement, then any patent licenses
+      granted to You under this License for that Work shall terminate
+      as of the date such litigation is filed.
+
+   4. Redistribution. You may reproduce and distribute copies of the
+      Work or Derivative Works thereof in any medium, with or without
+      modifications, and in Source or Object form, provided that You
+      meet the following conditions:
+
+      (a) You must give any other recipients of the Work or
+          Derivative Works a copy of this License; and
+
+      (b) You must cause any modified files to carry prominent notices
+          stating that You changed the files; and
+
+      (c) You must retain, in the Source form of any Derivative Works
+          that You distribute, all copyright, patent, trademark, and
+          attribution notices from the Source form of the Work,
+          excluding those notices that do not pertain to any part of
+          the Derivative Works; and
+
+      (d) If the Work includes a "NOTICE" text file as part of its
+          distribution, then any Derivative Works that You distribute must
+          include a readable copy of the attribution notices contained
+          within such NOTICE file, excluding those notices that do not
+          pertain to any part of the Derivative Works, in at least one
+          of the following places: within a NOTICE text file distributed
+          as part of the Derivative Works; within the Source form or
+          documentation, if provided along with the Derivative Works; or,
+          within a display generated by the Derivative Works, if and
+          wherever such third-party notices normally appear. The contents
+          of the NOTICE file are for informational purposes only and
+          do not modify the License. You may add Your own attribution
+          notices within Derivative Works that You distribute, alongside
+          or as an addendum to the NOTICE text from the Work, provided
+          that such additional attribution notices cannot be construed
+          as modifying the License.
+
+      You may add Your own copyright statement to Your modifications and
+      may provide additional or different license terms and conditions
+      for use, reproduction, or distribution of Your modifications, or
+      for any such Derivative Works as a whole, provided Your use,
+      reproduction, and distribution of the Work otherwise complies with
+      the conditions stated in this License.
+
+   5. Submission of Contributions. Unless You explicitly state otherwise,
+      any Contribution intentionally submitted for inclusion in the Work
+      by You to the Licensor shall be under the terms and conditions of
+      this License, without any additional terms or conditions.
+      Notwithstanding the above, nothing herein shall supersede or modify
+      the terms of any separate license agreement you may have executed
+      with Licensor regarding such Contributions.
+
+   6. Trademarks. This License does not grant permission to use the trade
+      names, trademarks, service marks, or product names of the Licensor,
+      except as required for reasonable and customary use in describing the
+      origin of the Work and reproducing the content of the NOTICE file.
+
+   7. Disclaimer of Warranty. Unless required by applicable law or
+      agreed to in writing, Licensor provides the Work (and each
+      Contributor provides its Contributions) on an "AS IS" BASIS,
+      WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+      implied, including, without limitation, any warranties or conditions
+      of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A
+      PARTICULAR PURPOSE. You are solely responsible for determining the
+      appropriateness of using or redistributing the Work and assume any
+      risks associated with Your exercise of permissions under this License.
+
+   8. Limitation of Liability. In no event and under no legal theory,
+      whether in tort (including negligence), contract, or otherwise,
+      unless required by applicable law (such as deliberate and grossly
+      negligent acts) or agreed to in writing, shall any Contributor be
+      liable to You for damages, including any direct, indirect, special,
+      incidental, or consequential damages of any character arising as a
+      result of this License or out of the use or inability to use the
+      Work (including but not limited to damages for loss of goodwill,
+      work stoppage, computer failure or malfunction, or any and all
+      other commercial damages or losses), even if such Contributor
+      has been advised of the possibility of such damages.
+
+   9. Accepting Warranty or Additional Liability. While redistributing
+      the Work or Derivative Works thereof, You may choose to offer,
+      and charge a fee for, acceptance of support, warranty, indemnity,
+      or other liability obligations and/or rights consistent with this
+      License. However, in accepting such obligations, You may act only
+      on Your own behalf and on Your sole responsibility, not on behalf
+      of any other Contributor, and only if You agree to indemnify,
+      defend, and hold each Contributor harmless for any liability
+      incurred by, or claims asserted against, such Contributor by reason
+      of your accepting any such warranty or additional liability.
+
+   END OF TERMS AND CONDITIONS
+
+   APPENDIX: How to apply the Apache License to your work.
+
+      To apply the Apache License to your work, attach the following
+      boilerplate notice, with the fields enclosed by brackets "[]"
+      replaced with your own identifying information. (Don't include
+      the brackets!)  The text should be enclosed in the appropriate
+      comment syntax for the file format. We also recommend that a
+      file or class name and description of purpose be included on the
+      same "printed page" as the copyright notice for easier
+      identification within third-party archives.
+
+   Copyright [yyyy] [name of copyright owner]
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+
+
+==============================================================================
+
+APACHE GUACAMOLE SUBCOMPONENTS
+
+Apache Guacamole includes a number of subcomponents with separate copyright
+notices and license terms. Your use of these subcomponents is subject to the
+terms and conditions of the following licenses.

Review comment:
       I think some licenses are missing here and within 
`src/licenses/bundled/`. Checking with Maven:
   
   ```
   [mjumper@dev-mjumper guacamole-auth-saml]$ mvn dependency:tree 
-Dscope=runtime
   [INFO] Scanning for projects...
   [INFO] 
   [INFO] 
------------------------------------------------------------------------
   [INFO] Building guacamole-auth-saml 1.2.0
   [INFO] 
------------------------------------------------------------------------
   [INFO] 
   [INFO] --- maven-dependency-plugin:2.10:tree (default-cli) @ 
guacamole-auth-saml ---
   [INFO] org.apache.guacamole:guacamole-auth-saml:jar:1.2.0
   [INFO] +- com.google.inject:guice:jar:3.0:compile
   [INFO] |  +- javax.inject:javax.inject:jar:1:compile
   [INFO] |  \- aopalliance:aopalliance:jar:1.0:compile
   [INFO] +- com.google.inject.extensions:guice-multibindings:jar:3.0:compile
   [INFO] \- com.onelogin:java-saml:jar:2.5.0:compile
   [INFO]    +- com.onelogin:java-saml-core:jar:2.5.0:compile
   [INFO]    +- joda-time:joda-time:jar:2.10.3:compile
   [INFO]    +- org.apache.commons:commons-lang3:jar:3.4:compile
   [INFO]    +- org.apache.santuario:xmlsec:jar:2.1.4:compile
   [INFO]    |  \- com.fasterxml.woodstox:woodstox-core:jar:5.0.3:compile
   [INFO]    |     \- org.codehaus.woodstox:stax2-api:jar:3.1.4:compile
   [INFO]    \- commons-codec:commons-codec:jar:1.12:compile
   [INFO] 
------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] 
------------------------------------------------------------------------
   [INFO] Total time: 3.839 s
   [INFO] Finished at: 2020-06-23T14:00:00-07:00
   [INFO] Final Memory: 14M/174M
   [INFO] 
------------------------------------------------------------------------
   [mjumper@dev-mjumper guacamole-auth-saml]$ 
   ```
   
   So, looks like Apache Commons Codec, Apache Commons Lang, Apache Santuario, 
Joda-Time, and Woodstox?

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLResponseMap.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Singleton;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.ValidationError;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A class that handles mapping of hashes to SAMLResponse objects.
+ */
+@Singleton
+public class SAMLResponseMap {
+    
+    /**
+     * The internal data structure that holds a map of SHA-256 hashes to
+     * SAML responses.
+     */
+    private final ConcurrentMap<String, SamlResponse> samlResponseMap =
+        new ConcurrentHashMap<>();
+    
+    /**
+     * Executor service which runs the periodic cleanup task
+     */
+    private final ScheduledExecutorService executor =
+            Executors.newScheduledThreadPool(1);
+
+    /**
+     * Create a new instance of this response map and kick off the executor
+     * that schedules the response cleanup task to run every five minutes.
+     */
+    public SAMLResponseMap() {
+        // Cleanup unclaimed responses every five minutes
+        executor.scheduleAtFixedRate(new SAMLResponseCleanupTask(), 5, 5, 
TimeUnit.MINUTES);
+    }
+    
+    /**
+     * Retrieve the SamlResponse from the map that is represented by the
+     * provided hash, or null if no such object exists.
+     * 
+     * @param hash
+     *     The SHA-256 hash of the SamlResponse.
+     * 
+     * @return 
+     *     The SamlResponse object matching the hash provided.
+     */
+    protected SamlResponse getSamlResponse(String hash) {
+        return samlResponseMap.remove(hash);
+    }
+    
+    /**
+     * Place the provided mapping of hash to SamlResponse into the map.
+     * 
+     * @param hash
+     *     The hash that will be the lookup key for this SamlResponse.
+     * 
+     * @param samlResponse 
+     *     The SamlResponse object.
+     */
+    protected void putSamlResponse(String hash, SamlResponse samlResponse) {
+        samlResponseMap.put(hash, samlResponse);
+    }
+    
+    /**
+     * Return true if the provided hash key exists in the map, otherwise false.
+     * 
+     * @param hash
+     *     The hash key to look for in the map.
+     * 
+     * @return 
+     *     true if the provided hash is present, otherwise false.
+     */
+    protected boolean hasSamlResponse(String hash) {
+        return samlResponseMap.containsKey(hash);
+    }
+    
+    /**
+     * Task which runs every five minutes and cleans up any expired SAML
+     * responses that haven't been claimed and removed from the map.
+     */
+    private class SAMLResponseCleanupTask implements Runnable {
+        
+        @Override
+        public void run() {
+
+            // Loop through responses in map and remove ones that are no 
longer valid.
+            for (Entry<String, SamlResponse> entry : 
samlResponseMap.entrySet()) {
+                try {
+                    entry.getValue().validateTimestamps();
+                }
+                catch (ValidationError e) {
+                    samlResponseMap.remove(entry.getKey());
+                }

Review comment:
       Elsewhere in this extension, I see `validateTimestamps()` being checked 
like:
   
       if (!samlResponse.validateTimestamps()) {
           ...
       }
   
   but the call here seems to indicate that the return value should be ignored, 
relying instead on exceptions to communicate that a response has expired.
   
   Which is the correct way to use `validateTimestamps()`?

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/user/SAMLAuthenticatedUser.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml.user;
+
+import com.google.inject.Inject;
+import java.util.Map;
+import java.util.Set;
+import org.apache.guacamole.net.auth.AbstractAuthenticatedUser;
+import org.apache.guacamole.net.auth.AuthenticationProvider;
+import org.apache.guacamole.net.auth.Credentials;
+
+/**
+ * An SAML-specific implementation of AuthenticatedUser, associating a
+ * username and particular set of credentials with the SAML authentication
+ * provider.
+ */
+public class SAMLAuthenticatedUser extends AbstractAuthenticatedUser {
+
+    /**
+     * Reference to the authentication provider associated with this
+     * authenticated user.
+     */
+    @Inject
+    private AuthenticationProvider authProvider;
+
+    /**
+     * The credentials provided when this user was authenticated.
+     */
+    private Credentials credentials;
+    
+    /**
+     * The effective groups of the authenticated user.
+     */
+    private Set<String> effectiveGroups;
+    
+    /**
+     * Tokens associated with the authenticated user.
+     */
+    private Map<String, String> tokens;
+
+    /**
+     * Initializes this AuthenticatedUser using the given username and
+     * credentials.
+     *
+     * @param username
+     *     The username of the user that was authenticated.
+     *
+     * @param credentials
+     *     The credentials provided when this user was authenticated.
+     * 
+     * @param tokens
+     *     The tokens available from this authentication provider.
+     * 
+     * @param effectiveGroups
+     *     The groups of which this user is a member.
+     */
+    public void init(String username, Credentials credentials,
+            Map<String, String> tokens, Set<String> effectiveGroups) {
+        this.credentials = credentials;
+        this.effectiveGroups = effectiveGroups;
+        this.tokens = tokens;
+        setIdentifier(username);
+    }
+    
+    /**
+     * Get the tokens associated with this particular user.

Review comment:
       To match convention, this should be phrased as a statement of what the 
function does ("Returns the tokens ...", "Retrieves the tokens ...", etc.) 
rather than in the imperative ("Get the tokens ...").

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLResponseMap.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Singleton;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.ValidationError;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A class that handles mapping of hashes to SAMLResponse objects.
+ */
+@Singleton
+public class SAMLResponseMap {
+    
+    /**
+     * The internal data structure that holds a map of SHA-256 hashes to
+     * SAML responses.
+     */
+    private final ConcurrentMap<String, SamlResponse> samlResponseMap =
+        new ConcurrentHashMap<>();
+    
+    /**
+     * Executor service which runs the periodic cleanup task
+     */
+    private final ScheduledExecutorService executor =
+            Executors.newScheduledThreadPool(1);
+
+    /**
+     * Create a new instance of this response map and kick off the executor
+     * that schedules the response cleanup task to run every five minutes.
+     */
+    public SAMLResponseMap() {
+        // Cleanup unclaimed responses every five minutes
+        executor.scheduleAtFixedRate(new SAMLResponseCleanupTask(), 5, 5, 
TimeUnit.MINUTES);
+    }
+    
+    /**
+     * Retrieve the SamlResponse from the map that is represented by the
+     * provided hash, or null if no such object exists.
+     * 
+     * @param hash
+     *     The SHA-256 hash of the SamlResponse.
+     * 
+     * @return 
+     *     The SamlResponse object matching the hash provided.
+     */
+    protected SamlResponse getSamlResponse(String hash) {
+        return samlResponseMap.remove(hash);
+    }
+    
+    /**
+     * Place the provided mapping of hash to SamlResponse into the map.
+     * 
+     * @param hash
+     *     The hash that will be the lookup key for this SamlResponse.
+     * 
+     * @param samlResponse 
+     *     The SamlResponse object.
+     */
+    protected void putSamlResponse(String hash, SamlResponse samlResponse) {
+        samlResponseMap.put(hash, samlResponse);
+    }
+    
+    /**
+     * Return true if the provided hash key exists in the map, otherwise false.
+     * 
+     * @param hash
+     *     The hash key to look for in the map.
+     * 
+     * @return 
+     *     true if the provided hash is present, otherwise false.
+     */
+    protected boolean hasSamlResponse(String hash) {
+        return samlResponseMap.containsKey(hash);
+    }
+    
+    /**
+     * Task which runs every five minutes and cleans up any expired SAML
+     * responses that haven't been claimed and removed from the map.
+     */
+    private class SAMLResponseCleanupTask implements Runnable {
+        
+        @Override
+        public void run() {
+
+            // Loop through responses in map and remove ones that are no 
longer valid.
+            for (Entry<String, SamlResponse> entry : 
samlResponseMap.entrySet()) {
+                try {
+                    entry.getValue().validateTimestamps();
+                }
+                catch (ValidationError e) {
+                    samlResponseMap.remove(entry.getKey());

Review comment:
       I don't think `remove()` can be safely called while iterating over 
`entrySet()`. From [the documentation for 
`entrySet()`](https://docs.oracle.com/javase/8/docs/api/java/util/Map.html#entrySet--):
   
   > ... If the map is modified while an iteration over the set is in progress 
(except through the iterator's own `remove` operation, or through the 
`setValue` operation on a map entry returned by the iterator) the results of 
the iteration are undefined. ...
   
   I think you would need to directly use an `Iterator` and call `remove()` on 
that, or leverage new hotness like 
[`removeIf()`](https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html#removeIf-java.util.function.Predicate-).
   
   BTW - changes to the `values()` view of a Java `Map` will affect the 
contents of the `Map` just as `entrySet()` would. If you are only operating on 
values here, iterating through `values()` may be easier on the soul.

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml.conf;
+
+import com.google.inject.Inject;
+import com.onelogin.saml2.settings.IdPMetadataParser;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.settings.SettingsBuilder;
+import com.onelogin.saml2.util.Constants;
+import java.io.File;
+import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.properties.BooleanGuacamoleProperty;
+import org.apache.guacamole.properties.FileGuacamoleProperty;
+import org.apache.guacamole.properties.StringGuacamoleProperty;
+import org.apache.guacamole.properties.URIGuacamoleProperty;
+
+/**
+ * Service for retrieving configuration information regarding the SAML
+ * authentication module.
+ */
+public class ConfigurationService {
+
+    /**
+     * The file containing the XML Metadata associated with the SAML IdP.
+     */
+    private static final FileGuacamoleProperty SAML_IDP_METADATA =
+            new FileGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-idp-metadata"; }
+
+    };
+
+    /**
+     * The URL of the SAML IdP.
+     */
+    private static final URIGuacamoleProperty SAML_IDP_URL =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-idp-url"; }
+
+    };
+
+    /**
+     * The URL identifier for this SAML client.
+     */
+    private static final URIGuacamoleProperty SAML_ENTITY_ID =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-entity-id"; }
+
+    };
+
+    /**
+     * The callback URL to use for SAML IdP, normally the base
+     * of the Guacamole install. The SAML extensions callback
+     * endpoint will be appended to this value.
+     */
+    private static final URIGuacamoleProperty SAML_CALLBACK_URL =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-callback-url"; }
+
+    };
+    
+    /**
+     * Whether or not debugging should be enabled in the SAML library to help
+     * track down errors.
+     */
+    private static final BooleanGuacamoleProperty SAML_DEBUG =
+            new BooleanGuacamoleProperty() {
+    
+        @Override
+        public String getName() { return "saml-debug"; }
+                
+    };
+    
+    /**
+     * Whether or not to enable compression for the SAML request.
+     */
+    private static final BooleanGuacamoleProperty SAML_COMPRESS_REQUEST =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-compress-request"; }
+                
+    };
+    
+    /**
+     * Whether or not to enable compression for the SAML response.
+     */
+    private static final BooleanGuacamoleProperty SAML_COMPRESS_RESPONSE =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-compress-response"; }
+                
+    };
+    
+    /**
+     * Whether or not to enforce strict SAML security during processing.
+     */
+    private static final BooleanGuacamoleProperty SAML_STRICT =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-strict"; }
+        
+    };
+    
+    /**
+     * The property that defines what attribute the SAML provider will return
+     * that contains group membership for the authenticated user.
+     */
+    private static final StringGuacamoleProperty SAML_GROUP_ATTRIBUTE =
+            new StringGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-group-attribute"; }
+                
+    };
+
+    /**
+     * The Guacamole server environment.
+     */
+    @Inject
+    private Environment environment;
+
+    /**
+     * Returns the URL to be submitted as the client ID to the SAML IdP, as
+     * configured in guacamole.properties.
+     *
+     * @return
+     *     The URL to send to the SAML IdP as the Client Identifier.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the
+     *     property is missing.
+     */
+    private URI getEntityId() throws GuacamoleException {
+        return environment.getRequiredProperty(SAML_ENTITY_ID);
+    }
+
+    /**
+     * The file that contains the metadata that the SAML client should
+     * use to communicate with the SAML IdP. This is generated by the
+     * SAML IdP and should be uploaded to the system where the Guacamole
+     * client is running.
+     *
+     * @return
+     *     The file containing the metadata used by the SAML client
+     *     when it communicates with the SAML IdP.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the client
+     *     metadata is missing.
+     */
+    private File getIdpMetadata() throws GuacamoleException {
+        return environment.getProperty(SAML_IDP_METADATA);
+    }
+
+    /**
+     * Return the URL used to log in to the SAML IdP.
+     *
+     * @return
+     *     The URL used to log in to the SAML IdP.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed.
+     */
+    private URI getIdpUrl() throws GuacamoleException {
+        return environment.getProperty(SAML_IDP_URL);
+    }
+
+    /**
+     * The callback URL used for the SAML IdP to POST a response
+     * to upon completion of authentication, normally the base
+     * of the Guacamole install.
+     *
+     * @return
+     *     The callback URL to be sent to the SAML IdP that will
+     *     be POSTed to upon completion of SAML authentication.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the
+     *     callback parameter is missing.
+     */
+    public URI getCallbackUrl() throws GuacamoleException {
+        return environment.getRequiredProperty(SAML_CALLBACK_URL);
+    }
+    
+    /**
+     * Return the Boolean value that indicates whether SAML client debugging
+     * will be enabled, as configured in guacamole.properties. The default is
+     * false, and debug information will not be generated or logged.
+     * 
+     * @return
+     *     True if debugging should be enabled in the SAML library, otherwise
+     *     false.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    private Boolean getDebug() throws GuacamoleException {
+        return environment.getProperty(SAML_DEBUG, false);
+    }

Review comment:
       For the various functions here returning boolean values, if `null` will 
never be returned, primitive `boolean` should be returned rather than `Boolean` 
objects.

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml.conf;
+
+import com.google.inject.Inject;
+import com.onelogin.saml2.settings.IdPMetadataParser;
+import com.onelogin.saml2.settings.Saml2Settings;
+import com.onelogin.saml2.settings.SettingsBuilder;
+import com.onelogin.saml2.util.Constants;
+import java.io.File;
+import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.properties.BooleanGuacamoleProperty;
+import org.apache.guacamole.properties.FileGuacamoleProperty;
+import org.apache.guacamole.properties.StringGuacamoleProperty;
+import org.apache.guacamole.properties.URIGuacamoleProperty;
+
+/**
+ * Service for retrieving configuration information regarding the SAML
+ * authentication module.
+ */
+public class ConfigurationService {
+
+    /**
+     * The file containing the XML Metadata associated with the SAML IdP.
+     */
+    private static final FileGuacamoleProperty SAML_IDP_METADATA =
+            new FileGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-idp-metadata"; }
+
+    };
+
+    /**
+     * The URL of the SAML IdP.
+     */
+    private static final URIGuacamoleProperty SAML_IDP_URL =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-idp-url"; }
+
+    };
+
+    /**
+     * The URL identifier for this SAML client.
+     */
+    private static final URIGuacamoleProperty SAML_ENTITY_ID =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-entity-id"; }
+
+    };
+
+    /**
+     * The callback URL to use for SAML IdP, normally the base
+     * of the Guacamole install. The SAML extensions callback
+     * endpoint will be appended to this value.
+     */
+    private static final URIGuacamoleProperty SAML_CALLBACK_URL =
+            new URIGuacamoleProperty() {
+
+        @Override
+        public String getName() { return "saml-callback-url"; }
+
+    };
+    
+    /**
+     * Whether or not debugging should be enabled in the SAML library to help
+     * track down errors.
+     */
+    private static final BooleanGuacamoleProperty SAML_DEBUG =
+            new BooleanGuacamoleProperty() {
+    
+        @Override
+        public String getName() { return "saml-debug"; }
+                
+    };
+    
+    /**
+     * Whether or not to enable compression for the SAML request.
+     */
+    private static final BooleanGuacamoleProperty SAML_COMPRESS_REQUEST =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-compress-request"; }
+                
+    };
+    
+    /**
+     * Whether or not to enable compression for the SAML response.
+     */
+    private static final BooleanGuacamoleProperty SAML_COMPRESS_RESPONSE =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-compress-response"; }
+                
+    };
+    
+    /**
+     * Whether or not to enforce strict SAML security during processing.
+     */
+    private static final BooleanGuacamoleProperty SAML_STRICT =
+            new BooleanGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-strict"; }
+        
+    };
+    
+    /**
+     * The property that defines what attribute the SAML provider will return
+     * that contains group membership for the authenticated user.
+     */
+    private static final StringGuacamoleProperty SAML_GROUP_ATTRIBUTE =
+            new StringGuacamoleProperty() {
+            
+        @Override
+        public String getName() { return "saml-group-attribute"; }
+                
+    };
+
+    /**
+     * The Guacamole server environment.
+     */
+    @Inject
+    private Environment environment;
+
+    /**
+     * Returns the URL to be submitted as the client ID to the SAML IdP, as
+     * configured in guacamole.properties.
+     *
+     * @return
+     *     The URL to send to the SAML IdP as the Client Identifier.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the
+     *     property is missing.
+     */
+    private URI getEntityId() throws GuacamoleException {
+        return environment.getRequiredProperty(SAML_ENTITY_ID);
+    }
+
+    /**
+     * The file that contains the metadata that the SAML client should
+     * use to communicate with the SAML IdP. This is generated by the
+     * SAML IdP and should be uploaded to the system where the Guacamole
+     * client is running.
+     *
+     * @return
+     *     The file containing the metadata used by the SAML client
+     *     when it communicates with the SAML IdP.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the client
+     *     metadata is missing.
+     */
+    private File getIdpMetadata() throws GuacamoleException {
+        return environment.getProperty(SAML_IDP_METADATA);
+    }
+
+    /**
+     * Return the URL used to log in to the SAML IdP.
+     *
+     * @return
+     *     The URL used to log in to the SAML IdP.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed.
+     */
+    private URI getIdpUrl() throws GuacamoleException {
+        return environment.getProperty(SAML_IDP_URL);
+    }
+
+    /**
+     * The callback URL used for the SAML IdP to POST a response
+     * to upon completion of authentication, normally the base
+     * of the Guacamole install.
+     *
+     * @return
+     *     The callback URL to be sent to the SAML IdP that will
+     *     be POSTed to upon completion of SAML authentication.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed, or if the
+     *     callback parameter is missing.
+     */
+    public URI getCallbackUrl() throws GuacamoleException {
+        return environment.getRequiredProperty(SAML_CALLBACK_URL);
+    }
+    
+    /**
+     * Return the Boolean value that indicates whether SAML client debugging
+     * will be enabled, as configured in guacamole.properties. The default is
+     * false, and debug information will not be generated or logged.
+     * 
+     * @return
+     *     True if debugging should be enabled in the SAML library, otherwise
+     *     false.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    private Boolean getDebug() throws GuacamoleException {
+        return environment.getProperty(SAML_DEBUG, false);
+    }
+    
+    /**
+     * Return the Boolean value that indicates whether or not compression of
+     * SAML requests to the IdP should be enabled or not, as configured in
+     * guacamole.properties. The default is to enable compression.
+     * 
+     * @return
+     *     True if compression should be enabled when sending the SAML request,
+     *     otherwise false.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    private Boolean getCompressRequest() throws GuacamoleException {
+        return environment.getProperty(SAML_COMPRESS_REQUEST, true);
+    }
+    
+    /**
+     * Return a Boolean value that indicates whether or not the SAML login
+     * should enforce strict security controls, as configured in
+     * guacamole.properties. By default this is true, and should be set to
+     * true in any production environment.
+     * 
+     * @return
+     *     True if the SAML login should enforce strict security checks,
+     *     otherwise false.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    private Boolean getStrict() throws GuacamoleException {
+        return environment.getProperty(SAML_STRICT, true);
+    }
+    
+    /**
+     * Return a Boolean value that indicates whether or not compression should
+     * be requested from the server when the SAML response is returned, as
+     * configured in guacamole.properties. The default is to request that the
+     * response be compressed.
+     * 
+     * @return
+     *     True if compression should be requested from the server for the SAML
+     *     response.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    private Boolean getCompressResponse() throws GuacamoleException {
+        return environment.getProperty(SAML_COMPRESS_RESPONSE, true);
+    }
+    
+    /**
+     * Return the name of the attribute that will be supplied by the identity
+     * provider that contains the groups of which this user is a member.
+     * 
+     * @return
+     *     The name of the attribute that contains the user groups.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed.
+     */
+    public String getGroupAttribute() throws GuacamoleException {
+        return environment.getProperty(SAML_GROUP_ATTRIBUTE, "groups");
+    }
+
+    /**
+     * Returns the collection of SAML settings used to initialize the client.
+     *
+     * @return
+     *     The collection of SAML settings used to initialize the SAML client.
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties cannot be parsed or if required parameters
+     *     are missing.
+     */
+    public Saml2Settings getSamlSettings() throws GuacamoleException {
+
+        // Try to get the XML file, first.
+        File idpMetadata = getIdpMetadata();
+        Map<String, Object> samlMap;
+        if (idpMetadata != null) {
+            try {
+                samlMap = 
IdPMetadataParser.parseFileXML(idpMetadata.getAbsolutePath());
+            }
+            catch (Exception e) {
+                throw new GuacamoleServerException(
+                        "Could not parse SAML IdP Metadata file.", e);
+            }
+        }
+
+        // If no XML metadata is provided, fall-back to individual values.
+        else {
+            samlMap = new HashMap<>();
+            samlMap.put(SettingsBuilder.IDP_ENTITYID_PROPERTY_KEY
+                    , getIdpUrl().toString());

Review comment:
       Is the placement of this comma on its own line intentional?

##########
File path: 
extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLResponseMap.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.saml;
+
+import com.google.inject.Singleton;
+import com.onelogin.saml2.authn.SamlResponse;
+import com.onelogin.saml2.exception.ValidationError;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A class that handles mapping of hashes to SAMLResponse objects.
+ */
+@Singleton
+public class SAMLResponseMap {
+    
+    /**
+     * The internal data structure that holds a map of SHA-256 hashes to
+     * SAML responses.
+     */
+    private final ConcurrentMap<String, SamlResponse> samlResponseMap =
+        new ConcurrentHashMap<>();
+    
+    /**
+     * Executor service which runs the periodic cleanup task
+     */
+    private final ScheduledExecutorService executor =
+            Executors.newScheduledThreadPool(1);

Review comment:
       This `ScheduledExecutorService` will need to be shutdown when the 
extension is unloaded so that its thread can be cleaned up. If this isn't done, 
the thread remains running and attempts to stop Tomcat will hang indefinitely 
(you can test this by ensuring this executor is exercised, such as by making a 
SAML auth attempt, and then attempting to shutdown Tomcat).
   
   The place to do this would be from the `shutdown()` function of 
`AuthenticationProvider`:
   
   
http://guacamole.apache.org/doc/guacamole-ext/org/apache/guacamole/net/auth/AuthenticationProvider.html#shutdown--
   
   There's some examples of this in the TOTP auth. The `shutdown()` function of 
`CodeUsageTrackingService` handles shutdown of its underlying executor:
   
   
https://github.com/apache/guacamole-client/blob/3912439472ba5d617fa30f42daedaa32ad5fa790/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/CodeUsageTrackingService.java#L255-L262
   
   and is ultimately invoked within `shutdown()` of the 
`AuthenticationProvider`:
   
   
https://github.com/apache/guacamole-client/blob/3912439472ba5d617fa30f42daedaa32ad5fa790/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/TOTPAuthenticationProvider.java#L91-L94

##########
File path: 
extensions/guacamole-auth-saml/src/main/resources/translations/en.json
##########
@@ -0,0 +1,12 @@
+{
+
+    "DATA_SOURCE_SAML" : {
+        "NAME" : "SAML Authentication Extension"
+    },
+
+    "LOGIN" : {
+        "FIELD_HEADER_SAML"     : "",
+        "INFO_SAML_REDIRECT_PENDING" : "Please wait, redirecting for SAML 
authentication..."

Review comment:
       Not a deal-breaker for me, but I am always uncomfortable with directly 
exposing the nature of authentication implementations to non-administrative 
users.
   
   If you think it's best to note that SAML is used, I'm fine with this as it 
stands. Otherwise, I suggest adopting generic wording similar to that of the 
OpenID support:
   
   
https://github.com/apache/guacamole-client/blob/3912439472ba5d617fa30f42daedaa32ad5fa790/extensions/guacamole-auth-openid/src/main/resources/translations/en.json#L9




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to