mike-jumper commented on a change in pull request #254: URL: https://github.com/apache/guacamole-client/pull/254#discussion_r385319801
########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java ########## @@ -0,0 +1,236 @@ +/* + * 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 java.io.File; +import java.net.URL; +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.FileGuacamoleProperty; +import org.apache.guacamole.properties.UrlGuacamoleProperty; + +/** + * 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 UrlGuacamoleProperty SAML_IDP_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-idp-url"; } + + }; + + /** + * The URL identifier for this SAML client. + */ + private static final UrlGuacamoleProperty SAML_ENTITY_ID = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-entity-id"; } + + }; + + /** + * The callback URL to use for SAML IdP, normally the base + * of the Guacamole install. + */ + private static final UrlGuacamoleProperty SAML_CALLBACK_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-callback-url"; } + + }; + + /** + * The single logout redirect URL. + */ + private static final UrlGuacamoleProperty SAML_LOGOUT_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-logout-url"; } + + }; + + /** + * The Guacamole server environment. + */ + @Inject + 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. + * + * @return + * The URL to be used as the client ID sent to the + * SAML IdP. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or if the + * property is missing. + */ + private URL 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); + } + + /** + * Retrieve 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 URL getIdpUrl() throws GuacamoleException { + return environment.getProperty( + SAML_IDP_URL, + null + ); Review comment: This is the behavior of `environment.getProperty(SAML_IDP_URL)`. ########## File path: extensions/guacamole-auth-saml/pom.xml ########## @@ -0,0 +1,260 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 + http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + <groupId>org.apache.guacamole</groupId> + <artifactId>guacamole-auth-saml</artifactId> + <packaging>jar</packaging> + <version>1.0.0</version> + <name>guacamole-auth-saml</name> + <url>http://guacamole.apache.org/</url> + + <properties> + <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> + </properties> + + <build> + <plugins> + + <!-- Written for 1.8 --> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + <version>3.3</version> + <configuration> + <source>1.8</source> + <target>1.8</target> + <compilerArgs> + <arg>-Xlint:all</arg> + <arg>-Werror</arg> + </compilerArgs> + <fork>true</fork> + </configuration> + </plugin> + + <!-- Pre-cache Angular templates with maven-angular-plugin --> + <plugin> + <groupId>com.keithbranton.mojo</groupId> + <artifactId>angular-maven-plugin</artifactId> + <version>0.3.2</version> + <executions> + <execution> + <phase>generate-resources</phase> + <goals> + <goal>html2js</goal> + </goals> + </execution> + </executions> + <configuration> + <sourceDir>${basedir}/src/main/resources</sourceDir> + <include>**/*.html</include> + <target>${basedir}/src/main/resources/generated/templates-main/templates.js</target> + <prefix>app/ext/guac-saml</prefix> + </configuration> + </plugin> + + <!-- JS/CSS Minification Plugin --> + <plugin> + <groupId>com.samaxes.maven</groupId> + <artifactId>minify-maven-plugin</artifactId> + <version>1.7.5</version> + <executions> + <execution> + <id>default-cli</id> + <configuration> + <charset>UTF-8</charset> + + <webappSourceDir>${basedir}/src/main/resources</webappSourceDir> + <webappTargetDir>${project.build.directory}/classes</webappTargetDir> + + <cssSourceDir>/</cssSourceDir> + <cssTargetDir>/</cssTargetDir> + <cssFinalFile>saml.css</cssFinalFile> + + <cssSourceFiles> + <cssSourceFile>license.txt</cssSourceFile> + </cssSourceFiles> + + <cssSourceIncludes> + <cssSourceInclude>**/*.css</cssSourceInclude> + </cssSourceIncludes> + + <jsSourceDir>/</jsSourceDir> + <jsTargetDir>/</jsTargetDir> + <jsFinalFile>saml.js</jsFinalFile> + + <jsSourceFiles> + <jsSourceFile>license.txt</jsSourceFile> + </jsSourceFiles> + + <jsSourceIncludes> + <jsSourceInclude>**/*.js</jsSourceInclude> + </jsSourceIncludes> + + <!-- Do not minify and include tests --> + <jsSourceExcludes> + <jsSourceExclude>**/*.test.js</jsSourceExclude> + </jsSourceExcludes> + <jsEngine>CLOSURE</jsEngine> + + <!-- Disable warnings for JSDoc annotations --> + <closureWarningLevels> + <misplacedTypeAnnotation>OFF</misplacedTypeAnnotation> + <nonStandardJsDocs>OFF</nonStandardJsDocs> + </closureWarningLevels> + + </configuration> + <goals> + <goal>minify</goal> + </goals> + </execution> + </executions> + </plugin> + + <!-- Copy dependencies prior to packaging --> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-dependency-plugin</artifactId> + <version>2.10</version> + <executions> + <execution> + <id>unpack-dependencies</id> + <phase>prepare-package</phase> + <goals> + <goal>unpack-dependencies</goal> + </goals> + <configuration> + <includeScope>runtime</includeScope> + <outputDirectory>${project.build.directory}/classes</outputDirectory> + <excludes>META-INF/*.SF,META-INF/*.DSA</excludes> + </configuration> + </execution> + </executions> + </plugin> + + <!-- Assembly plugin - for easy distribution --> + <plugin> + <artifactId>maven-assembly-plugin</artifactId> + <version>2.5.3</version> + <configuration> + <finalName>${project.artifactId}-${project.version}</finalName> + <appendAssemblyId>false</appendAssemblyId> + <descriptors> + <descriptor>src/main/assembly/dist.xml</descriptor> + </descriptors> + </configuration> + <executions> + <execution> + <id>make-dist-archive</id> + <phase>package</phase> + <goals> + <goal>single</goal> + </goals> + </execution> + </executions> + </plugin> + + <!-- Verify format using Apache RAT --> + <plugin> + <groupId>org.apache.rat</groupId> + <artifactId>apache-rat-plugin</artifactId> + <version>0.12</version> + + <configuration> + <excludes> + <exclude>**/*.json</exclude> + <exclude>src/licenses/**/*</exclude> + <exclude>src/main/resources/templates/*.html</exclude> + </excludes> + </configuration> + + <!-- Bind RAT to validate phase --> + <executions> + <execution> + <id>validate</id> + <phase>validate</phase> + <goals> + <goal>check</goal> + </goals> + </execution> + </executions> + + </plugin> + + </plugins> + </build> + + <dependencies> + + <!-- Guacamole Extension API --> + <dependency> + <groupId>org.apache.guacamole</groupId> + <artifactId>guacamole-ext</artifactId> + <version>1.0.0</version> Review comment: 1.2.0* ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java ########## @@ -0,0 +1,236 @@ +/* + * 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 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.FileGuacamoleProperty; +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. + */ + private static final URIGuacamoleProperty SAML_CALLBACK_URL = + new URIGuacamoleProperty() { + + @Override + 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"; } + + }; + + /** + * The Guacamole server environment. + */ + @Inject + 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. + * + * @return + * The URL to be used as the client ID sent to the + * SAML IdP. + * + * @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); + } + + /** + * Retrieve 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, + null + ); + } + + /** + * 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 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, + null + ); + } + + /** + * 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 parameters are missing. + */ + public Saml2Settings getSamlSettings() throws GuacamoleException { + + 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); + } + } + + else { + samlMap = new HashMap<>(); + samlMap.put("onelogin.saml2.sp.entityid", getEntityId().toString()); + samlMap.put("onelogin.saml2.sp.assertion_consumer_service.url", + getCallbackUrl().toString() + "/api/ext/saml/callback"); + samlMap.put("onelogin.saml2.idp.entityid", getIdpUrl().toString()); + samlMap.put("onelogin.saml2.idp.single_sign_on_service.url", + getIdpUrl().toString()); + samlMap.put("onelogin.saml2.idp.single_sign_on_sevice.binding", + "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"); Review comment: > Not sure if I should use constants here?? If possible, I'd say yes. It looks like the library in use defines its own: https://github.com/onelogin/java-saml/blob/58fc7925a2e1e3d222c811239ed483b8f8a042ae/core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java#L76 ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java ########## @@ -0,0 +1,208 @@ +/* + * 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.util.Arrays; +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.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 = Util.urlDecoder(request.getParameter("responseHash")); + + if (responseHash != null) { + + try { + + // Generate the response object + if (!samlResponseMap.hasSamlResponse(responseHash)) + throw new GuacamoleInvalidCredentialsException("Provided response has 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) { + credentials.setUsername(username); + SAMLAuthenticatedUser authenticatedUser = authenticatedUserProvider.get(); + authenticatedUser.init(username, credentials); + 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) { Review comment: Yikes - the library call(s) throw `Exception`? ########## File path: extensions/guacamole-auth-saml/pom.xml ########## @@ -0,0 +1,260 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 + http://maven.apache.org/maven-v4_0_0.xsd";> + + <modelVersion>4.0.0</modelVersion> + <groupId>org.apache.guacamole</groupId> + <artifactId>guacamole-auth-saml</artifactId> + <packaging>jar</packaging> + <version>1.0.0</version> Review comment: 1.2.0* ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java ########## @@ -0,0 +1,236 @@ +/* + * 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 java.io.File; +import java.net.URL; +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.FileGuacamoleProperty; +import org.apache.guacamole.properties.UrlGuacamoleProperty; + +/** + * 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 UrlGuacamoleProperty SAML_IDP_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-idp-url"; } + + }; + + /** + * The URL identifier for this SAML client. + */ + private static final UrlGuacamoleProperty SAML_ENTITY_ID = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-entity-id"; } + + }; + + /** + * The callback URL to use for SAML IdP, normally the base + * of the Guacamole install. + */ + private static final UrlGuacamoleProperty SAML_CALLBACK_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-callback-url"; } + + }; + + /** + * The single logout redirect URL. + */ + private static final UrlGuacamoleProperty SAML_LOGOUT_URL = + new UrlGuacamoleProperty() { + + @Override + public String getName() { return "saml-logout-url"; } + + }; + + /** + * The Guacamole server environment. + */ + @Inject + 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. + * + * @return + * The URL to be used as the client ID sent to the + * SAML IdP. + * + * @throws GuacamoleException + * If guacamole.properties cannot be parsed, or if the + * property is missing. + */ + private URL 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); + } + + /** + * Retrieve 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 URL getIdpUrl() throws GuacamoleException { + return environment.getProperty( + SAML_IDP_URL, + null + ); + } + + /** + * 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 URL 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 URL getLogoutUrl() throws GuacamoleException { + return environment.getProperty( + SAML_LOGOUT_URL, + null + ); Review comment: Same here - no need to specify a default of `null`. That's the default behavior of the single-parameter variant of `getProperty()`. ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/AuthenticationProviderService.java ########## @@ -0,0 +1,208 @@ +/* + * 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.util.Arrays; +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.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 = Util.urlDecoder(request.getParameter("responseHash")); Review comment: Does `getParameter()` not already handle URL encoding transparently? ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java ########## @@ -0,0 +1,236 @@ +/* + * 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 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.FileGuacamoleProperty; +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. + */ + private static final URIGuacamoleProperty SAML_CALLBACK_URL = + new URIGuacamoleProperty() { + + @Override + 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"; } + + }; + + /** + * The Guacamole server environment. + */ + @Inject + 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. + * + * @return + * The URL to be used as the client ID sent to the + * SAML IdP. + * + * @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); + } + + /** + * Retrieve 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, + null + ); + } + + /** + * 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 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, + null + ); + } + + /** + * 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 parameters are missing. + */ + public Saml2Settings getSamlSettings() throws GuacamoleException { + + 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); + } + } + + else { + samlMap = new HashMap<>(); + samlMap.put("onelogin.saml2.sp.entityid", getEntityId().toString()); + samlMap.put("onelogin.saml2.sp.assertion_consumer_service.url", + getCallbackUrl().toString() + "/api/ext/saml/callback"); + samlMap.put("onelogin.saml2.idp.entityid", getIdpUrl().toString()); + samlMap.put("onelogin.saml2.idp.single_sign_on_service.url", + getIdpUrl().toString()); + samlMap.put("onelogin.saml2.idp.single_sign_on_sevice.binding", + "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"); + } + + SettingsBuilder samlBuilder = new SettingsBuilder(); + Saml2Settings samlSettings = samlBuilder.fromValues(samlMap).build(); + samlSettings.setDebug(true); Review comment: Why always debug? ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLAuthenticationProviderResource.java ########## @@ -0,0 +1,156 @@ +/* + * 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 + | NoSuchAlgorithmException + | ParserConfigurationException + | SAXException + | SettingsException + | URISyntaxException + | ValidationError + | XPathExpressionException e) { + throw new GuacamoleServerException(e); + } Review comment: The `GuacamoleServerException` thrown here should have an associated message describing the nature of the failure in context, ideally coupled with the usual logging. I'm guessing that part of the reason there isn't a message here is the breadth of this multicatch, which may need to be slit up a bit to allow useful log messages to be provided in the case of failures. ########## File path: extensions/guacamole-auth-saml/src/main/resources/controllers/samlController.js ########## @@ -0,0 +1,29 @@ +/* + * 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. + */ + +/** + * Controller for the "GUAC_SAML_REDIRECT" field, which redirects + * to the provided URI. + */ +angular.module('guacSAML').controller('guacSAMLController', ['$scope','$window', + function guacSAMLController($scope,$window) { + + $window.location.href = $scope.field.samlRedirect; + +}]); Review comment: I wonder if perhaps it may be worthwhile to begin providing a standard `REDIRECT` field, as it seems commonly leveraged/duplicated when implementing SSO. ########## File path: extensions/guacamole-auth-saml/src/main/java/org/apache/guacamole/auth/saml/SAMLResponseMap.java ########## @@ -0,0 +1,80 @@ +/* + * 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 java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * 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<>(); + + /** + * 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); Review comment: What if `getSamlResponse()` is not invoked for a particular hash, and the size of `samlResponseMap` builds? Is there an overall timeout that should be enforced? ---------------------------------------------------------------- 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]
