You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/11/26 02:53:42 UTC
[GitHub] [guacamole-client] mike-jumper opened a new pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
mike-jumper opened a new pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657
Similar to the current layout of the database extensions, this change refactors the CAS, OpenID, and SAML extensions beneath a common "guacamole-auth-sso" base such that they can be maintained and distributed together. The result of the build will now include a `guacamole-auth-sso-VERSION.tar.gz` archive containing one subdirectory for each SSO extension.
The primary motivation behind these changes with respect to [GUACAMOLE-1364](https://issues.apache.org/jira/browse/GUACAMOLE-1364) is that the buttons/links on the login screen pointing to the various available SSO providers will be in a single, common area defined by a shared HTML patch. Reorganizing the SSO extensions beneath a common base avoids having to repeat this HTML patch everywhere and allows this shared expectation to make sense structurally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762381595
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/user/SSOAuthenticatedUser.java
##########
@@ -47,58 +47,51 @@
private Credentials credentials;
/**
- * Tokens associated with this authenticated user.
+ * The groups that this user belongs to.
*/
- private Map<String, String> tokens;
-
+ private Set<String> effectiveGroups;
+
/**
- * The unique identifiers of all user groups which this user is a member of.
+ * Parameter tokens to be automatically injected for any connections used
+ * by this user.
*/
- private Set<String> effectiveGroups;
+ private Map<String, String> tokens;
/**
- * Initializes this AuthenticatedUser using the given username and
- * credentials, and an empty map of parameter tokens.
+ * Initializes this SSOAuthenticatedUser, associating it with the given
+ * username, credentials, groups, and parameter tokens. This function must
+ * be invoked for every SSOAuthenticatedUser created.
*
* @param username
* The username of the user that was authenticated.
*
* @param credentials
* The credentials provided when this user was authenticated.
- */
- public void init(String username, Credentials credentials) {
- this.init(username, credentials, Collections.emptyMap(), Collections.emptySet());
- }
-
- /**
- * Initializes this AuthenticatedUser using the given username,
- * credentials, and parameter tokens.
*
- * @param username
- * The username of the user that was authenticated.
+ * @param effectiveGroups
+ * The groups that the authenticated user belongs to.
*
- * @param credentials
- * The credentials provided when this user was authenticated.
- *
* @param tokens
* A map of all the name/value pairs that should be available
- * as tokens when connections are established with this user.
+ * as tokens when connections are established by this user.
*/
public void init(String username, Credentials credentials,
- Map<String, String> tokens, Set<String> effectiveGroups) {
+ Set<String> effectiveGroups, Map<String, String> tokens) {
this.credentials = credentials;
- this.tokens = Collections.unmodifiableMap(tokens);
Review comment:
I think this is mostly just git deciding that this file is the most similar to the CAS version of things, not that these have been truly removed, but I'll double-check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper edited a comment on pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
mike-jumper edited a comment on pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#issuecomment-980488535
I've just now also extracted a set of common base classes (commit cc2e560), avoiding the Guice initialization and parameter token boilerplate shared across most of our SSO implementations. This will come in handy when each SSO implementation needs to provide a REST resource allowing manual redirection to the IdP (part of [GUACAMOLE-1364](https://issues.apache.org/jira/browse/GUACAMOLE-1364)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762384111
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/user/SSOAuthenticatedUser.java
##########
@@ -47,58 +47,51 @@
private Credentials credentials;
/**
- * Tokens associated with this authenticated user.
+ * The groups that this user belongs to.
*/
- private Map<String, String> tokens;
-
+ private Set<String> effectiveGroups;
+
/**
- * The unique identifiers of all user groups which this user is a member of.
+ * Parameter tokens to be automatically injected for any connections used
+ * by this user.
*/
- private Set<String> effectiveGroups;
+ private Map<String, String> tokens;
/**
- * Initializes this AuthenticatedUser using the given username and
- * credentials, and an empty map of parameter tokens.
+ * Initializes this SSOAuthenticatedUser, associating it with the given
+ * username, credentials, groups, and parameter tokens. This function must
+ * be invoked for every SSOAuthenticatedUser created.
*
* @param username
* The username of the user that was authenticated.
*
* @param credentials
* The credentials provided when this user was authenticated.
- */
- public void init(String username, Credentials credentials) {
- this.init(username, credentials, Collections.emptyMap(), Collections.emptySet());
- }
-
- /**
- * Initializes this AuthenticatedUser using the given username,
- * credentials, and parameter tokens.
*
- * @param username
- * The username of the user that was authenticated.
+ * @param effectiveGroups
+ * The groups that the authenticated user belongs to.
*
- * @param credentials
- * The credentials provided when this user was authenticated.
- *
* @param tokens
* A map of all the name/value pairs that should be available
- * as tokens when connections are established with this user.
+ * as tokens when connections are established by this user.
*/
public void init(String username, Credentials credentials,
- Map<String, String> tokens, Set<String> effectiveGroups) {
+ Set<String> effectiveGroups, Map<String, String> tokens) {
this.credentials = credentials;
- this.tokens = Collections.unmodifiableMap(tokens);
Review comment:
OK - I've restored the unmodifiable map and lowercase transformation. The latter was indeed accidentally removed from CAS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#issuecomment-980488535
I've just now also extracted a set of common base classes (commit 3d2e0fb), avoiding the Guice initialization and parameter token boilerplate shared across most of our SSO implementations. This will come in handy when each SSO implementation needs to provide a REST resource allowing manual redirection to the IdP (part of [GUACAMOLE-1364](https://issues.apache.org/jira/browse/GUACAMOLE-1364)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#issuecomment-985965882
Ok, LGTM!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762379472
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationProvider.java
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.sso;
+
+import com.google.common.collect.Iterables;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import java.util.Arrays;
+import java.util.Collections;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.sso.user.SSOAuthenticatedUser;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.net.auth.AbstractAuthenticationProvider;
+import org.apache.guacamole.net.auth.AuthenticatedUser;
+import org.apache.guacamole.net.auth.AuthenticationProvider;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * An AuthenticationProvider which authenticates users against an arbitrary
+ * SSO system. Guice dependency injection is automatically configured via
+ * modules provided by the implementation. Implementations will typically
+ * provide no storage for connections, instead relying on other installed
+ * extensions.
+ */
+public abstract class SSOAuthenticationProvider extends AbstractAuthenticationProvider {
+
+ /**
+ * The Guice injector.
+ */
+ private final Injector injector;
+
+ /**
+ * Creates a new SSOAuthenticationProvider that authenticates users against
+ * an arbitrary SSO system. Guice dependency injection is automatically
+ * configured, with the resulting injector available to implementations via
+ * {@link #getInjector()}. Core authentication functions are provided by
+ * the given SSOAuthenticationProviderService implementation, and
+ * additional may be provided by specifying additional Guice modules.
Review comment:
and additional ... functions? A noun might be nice here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762379472
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationProvider.java
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.sso;
+
+import com.google.common.collect.Iterables;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import java.util.Arrays;
+import java.util.Collections;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.sso.user.SSOAuthenticatedUser;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.net.auth.AbstractAuthenticationProvider;
+import org.apache.guacamole.net.auth.AuthenticatedUser;
+import org.apache.guacamole.net.auth.AuthenticationProvider;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * An AuthenticationProvider which authenticates users against an arbitrary
+ * SSO system. Guice dependency injection is automatically configured via
+ * modules provided by the implementation. Implementations will typically
+ * provide no storage for connections, instead relying on other installed
+ * extensions.
+ */
+public abstract class SSOAuthenticationProvider extends AbstractAuthenticationProvider {
+
+ /**
+ * The Guice injector.
+ */
+ private final Injector injector;
+
+ /**
+ * Creates a new SSOAuthenticationProvider that authenticates users against
+ * an arbitrary SSO system. Guice dependency injection is automatically
+ * configured, with the resulting injector available to implementations via
+ * {@link #getInjector()}. Core authentication functions are provided by
+ * the given SSOAuthenticationProviderService implementation, and
+ * additional may be provided by specifying additional Guice modules.
Review comment:
any additional ... functions? A noun might be nice here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762381086
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/user/SSOAuthenticatedUser.java
##########
@@ -47,58 +47,51 @@
private Credentials credentials;
/**
- * Tokens associated with this authenticated user.
+ * The groups that this user belongs to.
*/
- private Map<String, String> tokens;
-
+ private Set<String> effectiveGroups;
+
/**
- * The unique identifiers of all user groups which this user is a member of.
+ * Parameter tokens to be automatically injected for any connections used
+ * by this user.
*/
- private Set<String> effectiveGroups;
+ private Map<String, String> tokens;
/**
- * Initializes this AuthenticatedUser using the given username and
- * credentials, and an empty map of parameter tokens.
+ * Initializes this SSOAuthenticatedUser, associating it with the given
+ * username, credentials, groups, and parameter tokens. This function must
+ * be invoked for every SSOAuthenticatedUser created.
*
* @param username
* The username of the user that was authenticated.
*
* @param credentials
* The credentials provided when this user was authenticated.
- */
- public void init(String username, Credentials credentials) {
- this.init(username, credentials, Collections.emptyMap(), Collections.emptySet());
- }
-
- /**
- * Initializes this AuthenticatedUser using the given username,
- * credentials, and parameter tokens.
*
- * @param username
- * The username of the user that was authenticated.
+ * @param effectiveGroups
+ * The groups that the authenticated user belongs to.
*
- * @param credentials
- * The credentials provided when this user was authenticated.
- *
* @param tokens
* A map of all the name/value pairs that should be available
- * as tokens when connections are established with this user.
+ * as tokens when connections are established by this user.
*/
public void init(String username, Credentials credentials,
- Map<String, String> tokens, Set<String> effectiveGroups) {
+ Set<String> effectiveGroups, Map<String, String> tokens) {
this.credentials = credentials;
- this.tokens = Collections.unmodifiableMap(tokens);
Review comment:
Why get rid of these conversions here? Like wrapping the tokens in an unmodifable map, or lowercaseing the username?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762384403
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/SSOAuthenticationProvider.java
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.sso;
+
+import com.google.common.collect.Iterables;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import java.util.Arrays;
+import java.util.Collections;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.sso.user.SSOAuthenticatedUser;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.net.auth.AbstractAuthenticationProvider;
+import org.apache.guacamole.net.auth.AuthenticatedUser;
+import org.apache.guacamole.net.auth.AuthenticationProvider;
+import org.apache.guacamole.net.auth.Credentials;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+
+/**
+ * An AuthenticationProvider which authenticates users against an arbitrary
+ * SSO system. Guice dependency injection is automatically configured via
+ * modules provided by the implementation. Implementations will typically
+ * provide no storage for connections, instead relying on other installed
+ * extensions.
+ */
+public abstract class SSOAuthenticationProvider extends AbstractAuthenticationProvider {
+
+ /**
+ * The Guice injector.
+ */
+ private final Injector injector;
+
+ /**
+ * Creates a new SSOAuthenticationProvider that authenticates users against
+ * an arbitrary SSO system. Guice dependency injection is automatically
+ * configured, with the resulting injector available to implementations via
+ * {@link #getInjector()}. Core authentication functions are provided by
+ * the given SSOAuthenticationProviderService implementation, and
+ * additional may be provided by specifying additional Guice modules.
Review comment:
Oops. Reworded via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657#discussion_r762380878
##########
File path: extensions/guacamole-auth-sso/modules/guacamole-auth-sso-base/src/main/java/org/apache/guacamole/auth/sso/user/SSOAuthenticatedUser.java
##########
@@ -113,7 +106,7 @@ public AuthenticationProvider getAuthenticationProvider() {
public Credentials getCredentials() {
return credentials;
}
-
+
Review comment:
You've got some trailing spaces in this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner merged pull request #657: GUACAMOLE-1364: Refactor all SSO extensions beneath common base.
Posted by GitBox <gi...@apache.org>.
jmuehlner merged pull request #657:
URL: https://github.com/apache/guacamole-client/pull/657
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org