GUACAMOLE-103: License cleanup, fix comments, and minor code tweaks.

Includes implementation of executor shutdown, and correctly removing
items from the shared response map.
This commit is contained in:
Virtually Nick
2020-06-23 13:53:10 -04:00
parent 52318a99a8
commit 806ec964ff
16 changed files with 1199 additions and 151 deletions

View File

@@ -28,7 +28,8 @@ import com.onelogin.saml2.exception.ValidationError;
import com.onelogin.saml2.settings.Saml2Settings;
import com.onelogin.saml2.util.Util;
import java.io.IOException;
import java.util.ArrayList;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -41,10 +42,11 @@ 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.form.SAMLRedirectField;
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;
@@ -130,12 +132,9 @@ public class AuthenticationProviderService {
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);
}
// Validate timestamps, generating ValidationException if this fails.
samlResponse.validateTimestamps();
// Grab the username, and, if present, finish authentication.
String username = samlResponse.getNameId().toLowerCase();
@@ -208,10 +207,10 @@ public class AuthenticationProviderService {
// No SAML Response is present, so generate a request.
AuthnRequest samlReq = new AuthnRequest(samlSettings);
String reqString;
URI authUri;
try {
reqString = samlSettings.getIdpSingleSignOnServiceUrl() + "?SAMLRequest=" +
Util.urlEncoder(samlReq.getEncodedAuthnRequest());
authUri = new URI(samlSettings.getIdpSingleSignOnServiceUrl() + "?SAMLRequest=" +
Util.urlEncoder(samlReq.getEncodedAuthnRequest()));
}
catch (IOException e) {
logger.error("Error encoding authentication request to string: {}", e.getMessage());
@@ -219,18 +218,37 @@ public class AuthenticationProviderService {
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", reqString, "LOGIN.REDIRECT_PENDING")
new RedirectField("samlRedirect", authUri, new TranslatableMessage("LOGIN.INFO_SAML_REDIRECT_PENDING"))
}))
);
}
private Map<String, String> parseTokens(Map<String, List<String>> attributes)
throws GuacamoleException {
/**
* Generates Map of tokens that can be substituted within Guacamole
* parameters given a Map containing a List of attributes from the SAML IdP.
* Attributes that have multiple values will be reduced to a single value,
* taking the first available value and discarding the remaining values.
*
* @param attributes
* The Map containing the attributes retrieved from the SAML IdP.
*
* @return
* A Map of key and single value pairs that can be used as parameter
* tokens.
*/
private Map<String, String> parseTokens(Map<String,
List<String>> attributes) {
Map<String, String> tokens = new HashMap<>();
for (Entry<String, List<String>> entry : attributes.entrySet()) {
@@ -244,7 +262,22 @@ public class AuthenticationProviderService {
}
private Set<String> parseGroups(Map<String, List<String>> attributes, String groupAttribute) throws GuacamoleException {
/**
* Returns a list of groups found in the provided Map of attributes returned
* by the SAML IdP by searching the map for the provided group attribute.
*
* @param attributes
* The Map of attributes provided by the SAML IdP.
*
* @param groupAttribute
* The name of the attribute that may be present in the Map that
* will be used to parse group membership for the authenticated user.
*
* @return
* A Set of groups of which the user is a member.
*/
private Set<String> parseGroups(Map<String, List<String>> attributes,
String groupAttribute) {
List<String> samlGroups = attributes.get(groupAttribute);
if (samlGroups != null && !samlGroups.isEmpty())

View File

@@ -77,5 +77,10 @@ public class SAMLAuthenticationProvider extends AbstractAuthenticationProvider {
return authProviderService.authenticateUser(credentials);
}
@Override
public void shutdown() {
injector.getInstance(SAMLResponseMap.class).shutdown();
}
}

View File

@@ -145,7 +145,7 @@ public class SAMLAuthenticationProviderResource {
}
/**
* This is a utility method designed to generate a SHA-256 has for the
* This is a utility method designed to generate a SHA-256 hash for the
* given string representation of the SAMLResponse, throwing an exception
* if, for some reason, the Java implementation in use doesn't support
* SHA-256, and returning a hex-formatted hash value.

View File

@@ -22,7 +22,7 @@ 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.Collection;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
@@ -107,12 +107,13 @@ public class SAMLResponseMap {
public void run() {
// Loop through responses in map and remove ones that are no longer valid.
for (Entry<String, SamlResponse> entry : samlResponseMap.entrySet()) {
Collection<SamlResponse> samlResponses = samlResponseMap.values();
for (SamlResponse value : samlResponses) {
try {
entry.getValue().validateTimestamps();
value.validateTimestamps();
}
catch (ValidationError e) {
samlResponseMap.remove(entry.getKey());
samlResponses.remove(value);
}
}
@@ -120,4 +121,13 @@ public class SAMLResponseMap {
}
/**
* Shut down the executor service that periodically cleans out the
* SamlResponse Map. This must be invoked during webapp shutdown in order
* to avoid resource leaks.
*/
public void shutdown() {
executor.shutdownNow();
}
}

View File

@@ -77,7 +77,8 @@ public class ConfigurationService {
/**
* The callback URL to use for SAML IdP, normally the base
* of the Guacamole install.
* of the Guacamole install. The SAML extensions callback
* endpoint will be appended to this value.
*/
private static final URIGuacamoleProperty SAML_CALLBACK_URL =
new URIGuacamoleProperty() {
@@ -86,17 +87,6 @@ public class ConfigurationService {
public String getName() { return "saml-callback-url"; }
};
/**
* The single logout redirect URL.
*/
private static final URIGuacamoleProperty SAML_LOGOUT_URL =
new URIGuacamoleProperty() {
@Override
public String getName() { return "saml-logout-url"; }
};
/**
* Whether or not debugging should be enabled in the SAML library to help
@@ -111,7 +101,7 @@ public class ConfigurationService {
};
/**
* Whether or not to enabled compression for the SAML request.
* Whether or not to enable compression for the SAML request.
*/
private static final BooleanGuacamoleProperty SAML_COMPRESS_REQUEST =
new BooleanGuacamoleProperty() {
@@ -122,7 +112,7 @@ public class ConfigurationService {
};
/**
* Whether or not to enabled compression for the SAML response.
* Whether or not to enable compression for the SAML response.
*/
private static final BooleanGuacamoleProperty SAML_COMPRESS_RESPONSE =
new BooleanGuacamoleProperty() {
@@ -162,13 +152,11 @@ public class ConfigurationService {
private Environment environment;
/**
* Returns the URL to be used as the client ID which will be
* submitted to the SAML IdP as configured in
* guacamole.properties.
* Returns the URL to be submitted as the client ID to the SAML IdP, as
* configured in guacamole.properties.
*
* @return
* The URL to be used as the client ID sent to the
* SAML IdP.
* The URL to send to the SAML IdP as the Client Identifier.
*
* @throws GuacamoleException
* If guacamole.properties cannot be parsed, or if the
@@ -180,7 +168,7 @@ public class ConfigurationService {
/**
* The file that contains the metadata that the SAML client should
* use to communicate with the SAML IdP. This is generated by the
* 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.
*
@@ -197,7 +185,7 @@ public class ConfigurationService {
}
/**
* Retrieve the URL used to log in to the SAML IdP.
* Return the URL used to log in to the SAML IdP.
*
* @return
* The URL used to log in to the SAML IdP.
@@ -225,23 +213,11 @@ public class ConfigurationService {
public URI getCallbackUrl() throws GuacamoleException {
return environment.getRequiredProperty(SAML_CALLBACK_URL);
}
/**
* Return the URL used to log out from the SAML IdP.
*
* @return
* The URL used to log out from the SAML IdP.
*
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
private URI getLogoutUrl() throws GuacamoleException {
return environment.getProperty(SAML_LOGOUT_URL);
}
/**
* Return true if SAML debugging should be enabled, otherwise false. The
* default is false.
* 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
@@ -250,13 +226,14 @@ public class ConfigurationService {
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
private Boolean getDebug() throws GuacamoleException {
private boolean getDebug() throws GuacamoleException {
return environment.getProperty(SAML_DEBUG, false);
}
/**
* Return true if compression should be enabled when sending the SAML
* request, otherwise false. The default is to enable compression.
* 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,
@@ -265,14 +242,15 @@ public class ConfigurationService {
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
private Boolean getCompressRequest() throws GuacamoleException {
private boolean getCompressRequest() throws GuacamoleException {
return environment.getProperty(SAML_COMPRESS_REQUEST, true);
}
/**
* Returns whether or not the SAML login should enforce strict security
* controls. By default this is true, and should be set to true in any
* production environment.
* 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,
@@ -281,14 +259,15 @@ public class ConfigurationService {
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
private Boolean getStrict() throws GuacamoleException {
private boolean getStrict() throws GuacamoleException {
return environment.getProperty(SAML_STRICT, true);
}
/**
* Return true if compression should be requested from the server when the
* SAML response is returned, otherwise false. The default is to request
* that the response be compressed.
* 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
@@ -297,7 +276,7 @@ public class ConfigurationService {
* @throws GuacamoleException
* If guacamole.properties cannot be parsed.
*/
private Boolean getCompressResponse() throws GuacamoleException {
private boolean getCompressResponse() throws GuacamoleException {
return environment.getProperty(SAML_COMPRESS_RESPONSE, true);
}
@@ -316,19 +295,18 @@ public class ConfigurationService {
}
/**
* Returns the collection of SAML settings used to
* initialize the client.
* Returns the collection of SAML settings used to initialize the client.
*
* @return
* The collection of SAML settings used to
* initialize the SAML client.
* The collection of SAML settings used to initialize the SAML client.
*
* @throws GuacamoleException
* If guacamole.properties cannot be parsed or
* if parameters are missing.
* 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) {
@@ -341,20 +319,23 @@ public class ConfigurationService {
}
}
// If no XML metadata is provided, fall-back to individual values.
else {
samlMap = new HashMap<>();
samlMap.put(SettingsBuilder.SP_ENTITYID_PROPERTY_KEY,
getEntityId().toString());
samlMap.put(SettingsBuilder.SP_ASSERTION_CONSUMER_SERVICE_URL_PROPERTY_KEY,
getCallbackUrl().toString() + "/api/ext/saml/callback");
samlMap.put(SettingsBuilder.IDP_ENTITYID_PROPERTY_KEY
, getIdpUrl().toString());
samlMap.put(SettingsBuilder.IDP_ENTITYID_PROPERTY_KEY,
getIdpUrl().toString());
samlMap.put(SettingsBuilder.IDP_SINGLE_SIGN_ON_SERVICE_URL_PROPERTY_KEY,
getIdpUrl().toString());
samlMap.put(SettingsBuilder.IDP_SINGLE_SIGN_ON_SERVICE_BINDING_PROPERTY_KEY,
Constants.BINDING_HTTP_REDIRECT);
}
// Common settings, required with or without metadata file.
samlMap.put(SettingsBuilder.SP_ENTITYID_PROPERTY_KEY,
getEntityId().toString());
samlMap.put(SettingsBuilder.SP_ASSERTION_CONSUMER_SERVICE_URL_PROPERTY_KEY,
getCallbackUrl().toString() + "/api/ext/saml/callback");
SettingsBuilder samlBuilder = new SettingsBuilder();
Saml2Settings samlSettings = samlBuilder.fromValues(samlMap).build();
samlSettings.setStrict(getStrict());

View File

@@ -1,66 +0,0 @@
/*
* 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.form;
import org.apache.guacamole.form.Field;
/**
* Field definition which represents the data used to do redirects
* during SAML authentication.
*/
public class SAMLRedirectField extends Field {
/**
* The name of the parameter containing the redirect.
*/
public static final String PARAMETER_NAME = "samlRedirect";
/**
* The encoded URI of the redirect.
*/
private final String samlRedirect;
/**
* Creates a new field which facilitates redirection of the user
* during SAML SSO authentication.
*
* @param samlRedirect
* The URI to which the user should be redirected.
*/
public SAMLRedirectField(String samlRedirect) {
// Init base field properties
super(PARAMETER_NAME, "GUAC_SAML_REDIRECT");
this.samlRedirect = samlRedirect;
}
/**
* Returns the URI of the redirect.
*
* @return
* The URI of the redirect.
*/
public String getSamlRedirect() {
return samlRedirect;
}
}

View File

@@ -80,7 +80,7 @@ public class SAMLAuthenticatedUser extends AbstractAuthenticatedUser {
}
/**
* Get the tokens associated with this particular user.
* Returns a Map of tokens associated with this authenticated user.
*
* @return
* A map of token names and values available from this user account.

View File

@@ -1,12 +1,12 @@
{
"DATA_SOURCE_SAML" : {
"NAME" : "SAML SSO Backend"
"NAME" : "SAML Authentication Extension"
},
"LOGIN" : {
"FIELD_HEADER_SAML" : "",
"INFO_SAML_REDIRECT_PENDING" : "Please wait, redirecting for SAML authentication..."
"INFO_SAML_REDIRECT_PENDING" : "Please wait, redirecting to identity provider..."
}
}