[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r241549475
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
--- End diff --
SLF4J formerly recommended that instance variables be used (non-static),
but no longer takes either stance:
https://www.slf4j.org/faq.html#declared_static
If we have to pick something to be the standard going forward, I'd say
let's stick with the accepted idiom of `private final static` loggers, with the
exception being where it's actually necessary to not be `static` (dependency
injection).
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r241526587
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
+
+/**
+ * The name of the token which will be replaced with the username of
the
+ * current user if specified within the name of a secret. This token
+ * applies to both connections and connection groups.
+ */
+private static final String USERNAME_TOKEN = "GUAC_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_NAME_TOKEN =
"CONNECTION_GROUP_NAME";
+
+/**
+ * The name of the token which will be replaced with the identifier of
the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_IDENTIFIER_TOKEN =
"CONNECTION_GROUP_ID";
+
+/**
+ * The name of the token which will be replaced with the \"hostname\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_HOSTNAME_TOKEN =
"CONNECTION_HOSTNAME";
+
+/**
+ * The name of the token which will be replaced with the \"username\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_USERNAME_TOKEN =
"CONNECTION_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection if specified within the name of a secret. This
token
+ * only applies only to connections.
+ */
+private static final String CONNECTION_NAME_TOKEN = "CONNECTION_NAME";
+
+/**
+ * The name of the token which will be replaced with the identifier of
the
+ * current connection if specified within the name of a secret. This
token
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r241526412
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/secret/AzureKeyVaultSecretService.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.vault.azure.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.microsoft.azure.keyvault.KeyVaultClient;
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import com.microsoft.azure.keyvault.models.SecretBundle;
+import com.microsoft.rest.ServiceCallback;
+import java.util.concurrent.CompletableFuture;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.guacamole.GuacamoleException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultAuthenticationException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.CachedVaultSecretService;
+
+/**
+ * Service which retrieves secrets from Azure Key Vault.
+ */
+@Singleton
+public class AzureKeyVaultSecretService extends CachedVaultSecretService {
+
+/**
+ * Pattern which matches contiguous groups of characters which are not
+ * allowed within Azure Key Vault secret names.
+ */
+private static final Pattern DISALLOWED_CHARACTERS =
Pattern.compile("[^a-zA-Z0-9-]+");
+
+/**
+ * Service for retrieving configuration information.
+ */
+@Inject
+private AzureKeyVaultConfigurationService confService;
+
+/**
+ * Provider for Azure Key Vault credentials.
+ */
+@Inject
+private Provider credentialProvider;
+
+/**
+ * {@inheritDoc}
+ *
+ * Azure Key Vault allows strictly a-z, A-Z, 0-9, and "-". This
--- End diff --
Yep, exactly right.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r241525041
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/conf/AzureKeyVaultAuthenticationException.java
---
@@ -0,0 +1,57 @@
+/*
+ * 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.vault.azure.conf;
+
+/**
+ * Unchecked exception thrown by AzureKeyVaultCredentials if an error
occurs
+ * during the authentication process. Note that the base
KeyVaultCredentials
+ * base class does not provide for checked exceptions within the
authentication
+ * process.
+ *
+ * @see AzureKeyVaultCredentials#doAuthenticate(java.lang.String,
java.lang.String, java.lang.String)
+ */
+public class AzureKeyVaultAuthenticationException extends RuntimeException
{
--- End diff --
Yep. From the top of the class:
https://github.com/apache/guacamole-client/blob/4d90b34732d81efd1fbdeab8df9d9edb939f6266/extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/conf/AzureKeyVaultAuthenticationException.java#L22-L30
The API provided for Azure does not allow for checked exceptions like
`GuacamoleException` to be thrown. Only unchecked exceptions can be used for
things which must be thrown within callbacks, etc. in their API. We catch this
specific exception and translate into a `GuacamoleException` before things make
their way back out to Guacamole.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r241524434
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/AzureKeyVaultAuthenticationProviderModule.java
---
@@ -0,0 +1,61 @@
+/*
+ * 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.vault.azure;
+
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.vault.VaultAuthenticationProviderModule;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultCredentials;
+import
org.apache.guacamole.auth.vault.azure.secret.AzureKeyVaultSecretService;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+
+/**
+ * Guice module which configures injections specific to Azure Key Vault
+ * support.
+ */
+public class AzureKeyVaultAuthenticationProviderModule
+extends VaultAuthenticationProviderModule {
+
+/**
+ * Creates a new AzureKeyVaultAuthenticationiProviderModule which
--- End diff --
OK - will fix.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request: https://github.com/apache/guacamole-client/pull/336#discussion_r238684733 --- Diff: extensions/guacamole-auth-vault/.gitignore --- @@ -0,0 +1,2 @@ +target/ --- End diff -- Looks like these .gitignore files are not present in most of the other modules, at least at the base. Even the jdbc module doesn't have it at the root? ---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240048911
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
+
+/**
+ * The name of the token which will be replaced with the username of
the
+ * current user if specified within the name of a secret. This token
+ * applies to both connections and connection groups.
+ */
+private static final String USERNAME_TOKEN = "GUAC_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_NAME_TOKEN =
"CONNECTION_GROUP_NAME";
+
+/**
+ * The name of the token which will be replaced with the identifier of
the
+ * current connection group if specified within the name of a secret.
This
+ * token only applies only to connection groups.
+ */
+private static final String CONNECTION_GROUP_IDENTIFIER_TOKEN =
"CONNECTION_GROUP_ID";
+
+/**
+ * The name of the token which will be replaced with the \"hostname\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_HOSTNAME_TOKEN =
"CONNECTION_HOSTNAME";
+
+/**
+ * The name of the token which will be replaced with the \"username\"
+ * connection parameter of the current connection if specified within
the
+ * name of a secret. This token only applies only to connections.
+ */
+private static final String CONNECTION_USERNAME_TOKEN =
"CONNECTION_USERNAME";
+
+/**
+ * The name of the token which will be replaced with the name of the
+ * current connection if specified within the name of a secret. This
token
+ * only applies only to connections.
+ */
+private static final String CONNECTION_NAME_TOKEN = "CONNECTION_NAME";
+
+/**
+ * The name of the token which will be replaced with the identifier of
the
+ * current connection if specified within the name of a secret. This
token
+
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r238686408
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/AzureKeyVaultAuthenticationProviderModule.java
---
@@ -0,0 +1,61 @@
+/*
+ * 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.vault.azure;
+
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.auth.vault.VaultAuthenticationProviderModule;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultCredentials;
+import
org.apache.guacamole.auth.vault.azure.secret.AzureKeyVaultSecretService;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+
+/**
+ * Guice module which configures injections specific to Azure Key Vault
+ * support.
+ */
+public class AzureKeyVaultAuthenticationProviderModule
+extends VaultAuthenticationProviderModule {
+
+/**
+ * Creates a new AzureKeyVaultAuthenticationiProviderModule which
--- End diff --
AzureKeyVaultAuthenticationiProviderModule ->
AzureKeyVaultAuthenticationProviderModule
(extra i)
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240043908
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/secret/AzureKeyVaultSecretService.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.vault.azure.secret;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.microsoft.azure.keyvault.KeyVaultClient;
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import com.microsoft.azure.keyvault.models.SecretBundle;
+import com.microsoft.rest.ServiceCallback;
+import java.util.concurrent.CompletableFuture;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.guacamole.GuacamoleException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultAuthenticationException;
+import
org.apache.guacamole.auth.vault.azure.conf.AzureKeyVaultConfigurationService;
+import org.apache.guacamole.auth.vault.secret.CachedVaultSecretService;
+
+/**
+ * Service which retrieves secrets from Azure Key Vault.
+ */
+@Singleton
+public class AzureKeyVaultSecretService extends CachedVaultSecretService {
+
+/**
+ * Pattern which matches contiguous groups of characters which are not
+ * allowed within Azure Key Vault secret names.
+ */
+private static final Pattern DISALLOWED_CHARACTERS =
Pattern.compile("[^a-zA-Z0-9-]+");
+
+/**
+ * Service for retrieving configuration information.
+ */
+@Inject
+private AzureKeyVaultConfigurationService confService;
+
+/**
+ * Provider for Azure Key Vault credentials.
+ */
+@Inject
+private Provider credentialProvider;
+
+/**
+ * {@inheritDoc}
+ *
+ * Azure Key Vault allows strictly a-z, A-Z, 0-9, and "-". This
--- End diff --
Is this `` here because of the `{@inheritDoc}` tag above? I know we've
discussed this before, but it's been a while...
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240044367
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-base/src/main/java/org/apache/guacamole/auth/vault/user/VaultUserContext.java
---
@@ -0,0 +1,325 @@
+/*
+ * 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.vault.user;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.auth.vault.conf.VaultConfigurationService;
+import org.apache.guacamole.net.auth.Connection;
+import org.apache.guacamole.net.auth.ConnectionGroup;
+import org.apache.guacamole.net.auth.TokenInjectingUserContext;
+import org.apache.guacamole.net.auth.UserContext;
+import org.apache.guacamole.auth.vault.secret.VaultSecretService;
+import org.apache.guacamole.protocol.GuacamoleConfiguration;
+import org.apache.guacamole.token.GuacamoleTokenUndefinedException;
+import org.apache.guacamole.token.TokenFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * UserContext implementation which automatically injects tokens
containing the
+ * values of secrets retrieved from a vault.
+ */
+public class VaultUserContext extends TokenInjectingUserContext {
+
+/**
+ * Logger for this class.
+ */
+private final Logger logger =
LoggerFactory.getLogger(VaultUserContext.class);
--- End diff --
Just curious, in the past I think the `logger` object has generally been
declared `private static final` - is there a reason why it's better to/not to
use `static` for those? I think a lot of the recent changes have not included
the `static` keyword, so not sure if there's a reason things have trended in
that direction. It'd be good to be consistent about it.
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/336#discussion_r240043831
--- Diff:
extensions/guacamole-auth-vault/modules/guacamole-auth-vault-azure/src/main/java/org/apache/guacamole/auth/vault/azure/conf/AzureKeyVaultCredentials.java
---
@@ -0,0 +1,115 @@
+/*
+ * 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.vault.azure.conf;
+
+import com.google.inject.Inject;
+import com.microsoft.aad.adal4j.AuthenticationContext;
+import com.microsoft.aad.adal4j.AuthenticationResult;
+import com.microsoft.aad.adal4j.ClientCredential;
+import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials;
+import java.net.MalformedURLException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import org.apache.guacamole.GuacamoleException;
+
+/**
+ * KeyVaultCredentials implementation which retrieves the required client
ID
+ * and key from guacamole.properties. Note that KeyVaultCredentials as
+ * implemented in the Azure Java SDK is NOT THREADSAFE; it leverages a
+ * non-concurrent HashMap for authentication result caching and does not
+ * perform any synchronization.
+ */
+public class AzureKeyVaultCredentials extends KeyVaultCredentials {
+
+/**
+ * Service for retrieving configuration information.
+ */
+@Inject
+private AzureKeyVaultConfigurationService confService;
+
+/**
+ * {@inheritDoc}
+ *
+ * @throws AzureKeyVaultAuthenticationException
+ * If an error occurs preventing successful authentication. Note
that
+ * this exception is unchecked. Uses of this class which need to be
+ * aware of errors in the authentication process must manually
catch
+ * this exception.
+ */
+@Override
+public String doAuthenticate(String authorization, String resource,
+String scope) throws AzureKeyVaultAuthenticationException {
+
+// Read Azure credentials from guacamole.properties
+ClientCredential credentials;
+try {
+credentials = confService.getClientCredentials();
+}
+catch (GuacamoleException e) {
--- End diff --
What's the rationale for not passing this through as a `GuacamoleException`?
---
[GitHub] guacamole-client pull request #336: GUACAMOLE-641: Add support for populatin...
GitHub user mike-jumper opened a pull request: https://github.com/apache/guacamole-client/pull/336 GUACAMOLE-641: Add support for populating arbitrary parameter tokens from key vaults. These changes add a new family of extensions, similar to "guacamole-auth-jdbc", which provide support for retrieval of secrets from key vaults: "guacamole-auth-vault". Initial support for Azure Key Vault is present through the "guacamole-auth-vault-azure" module, with the necessary structure in place to allow other implementations to be provided in the future. The general support works as follows: 1. A JSON file is included within `GUACAMOLE_HOME` which defines a token/secret mapping. Besides defining tokens, the names of each secret may also contain tokens which allow the secret name to vary by connection ID, hostname, username, etc. There is a specific set of tokens available for use within secret names. 2. When a user attempts to connect to a connection or connection group, the key vault implementation is queried to retrieve each defined secret. Tokens are then defined for only those secrets whose names are fully defined within the current context and which have values stored within the vault. 3. The extension defining the connection is given these tokens. If the extension supports substitution of parameter tokens, as all current extensions do, those tokens will be substituted within the configuration of the connection. Guacamole extensions are not required to do this, however. 4. To avoid excessive retrieval requests, the result of retrieving a particular secret is cached (by default for 10 seconds). This thus allows secret values like passwords and private keys to be stored off-site within the vault and retrieved dynamically based on context. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mike-jumper/guacamole-client key-vault Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-client/pull/336.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #336 commit a1d6eeac5f4fce22291a6b9f2c826915441b1329 Author: Michael Jumper Date: 2018-10-07T05:32:26Z GUACAMOLE-641: Allow token retrieval/generation to fail with an error. commit fa7878fc22f5c4f012b38e517e37dbfc9a2f31ee Author: Michael Jumper Date: 2018-10-07T05:36:30Z GUACAMOLE-641: Provide strict filtering mode for TokenFilter which disallows undefined tokens. commit 34d14262c092878ccdf8bec1e73ab6292f7ba72a Author: Michael Jumper Date: 2018-10-07T20:28:12Z GUACAMOLE-641: Add generic vault support with an initial Azure Key Vault implementation. commit 0bd1343d016adee73834c31791162feba4c65400 Author: Michael Jumper Date: 2018-10-08T01:21:20Z GUACAMOLE-641: Automatically cache requests for secrets from the vault. commit 35ee93de8a8ef58241e7e3c6bb145447857d6d19 Author: Michael Jumper Date: 2018-10-16T17:29:05Z GUACAMOLE-641: Retrieve secrets from Azure Key Vault. commit 0f3ac8161915a07d1476f2b09f998ac040c5488c Author: Michael Jumper Date: 2018-10-16T21:16:14Z GUACAMOLE-641: Allow tokens to be easily injected on-demand. commit 4d90b34732d81efd1fbdeab8df9d9edb939f6266 Author: Michael Jumper Date: 2018-10-16T21:51:24Z GUACAMOLE-641: Retrieve tokens asynchronously and in parallel. ---
