[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241933797
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
+entries.add(((SearchResultEntry) response).getEntry());
}
-
-// Warn if referrals cannot be followed
-catch (LDAPReferralException e) {
-if (confService.getFollowReferrals()) {
-logger.error("Could not follow referral: {}",
e.getFailedReferral());
-logger.debug("Error encountered trying to follow
referral.", e);
-throw new GuacamoleServerException("Could not
follow LDAP referral.", e);
-}
-else {
-logger.warn("Given a referral, but referrals are
disabled. Error was: {}", e.getMessage());
-logger.debug("Got a referral, but configured to
not follow them.", e);
+else if (response instanceof SearchResultReference &&
+request.isFollowReferrals()) {
+
+Referral referral = ((SearchResultReference)
response).getReferral();
+int referralHop = 0;
+for (String url : referral.getLdapUrls()) {
+LdapConnection referralConnection =
ldapService.referralConnection(
+new LdapUrl(url), ldapConnectionConfig,
referralHop++);
+entries.addAll(search(referralConnection, baseDN,
query));
--- End diff --
Took a stab at this, as well - not entirely sure this is the best/cleanest
way, but let me know what you think. As usual, I'm open to suggestions for how
to approach it.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928473
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapDnGuacamoleProperty.java
---
@@ -0,0 +1,50 @@
+/*
+ * 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.ldap.conf;
+
+import
org.apache.directory.api.ldap.model.exception.LdapInvalidDnException;
+import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty that converts a string to a Dn that can be used
+ * in LDAP connections. An exception is thrown if the provided DN is
invalid
+ * and cannot be parsed.
+ */
+public abstract class LdapDnGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public Dn parseValue(String value) throws GuacamoleException {
+
+if (value == null)
+return null;
+
+try {
+return new Dn(value);
+}
+catch (LdapInvalidDnException e) {
+throw new GuacamoleServerException("Invalid DN specified in
configuration.", e);
--- End diff --
Reworded.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928458
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapFilterGuacamoleProperty.java
---
@@ -0,0 +1,53 @@
+/*
+ * 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.ldap.conf;
+
+import java.text.ParseException;
+import org.apache.directory.api.ldap.model.filter.ExprNode;
+import org.apache.directory.api.ldap.model.filter.FilterParser;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty with a value of an ExprNode query filter. The
string
+ * provided is passed through the FilterParser returning the ExprNode
object,
+ * or an exception is thrown if the filter is invalid and cannot be
correctly
+ * parsed.
+ */
+public abstract class LdapFilterGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public ExprNode parseValue(String value) throws GuacamoleException {
+
+// No value provided, so return null.
+if (value == null)
+return null;
+
+try {
+return FilterParser.parse(value);
+}
+catch (ParseException e) {
+throw new GuacamoleServerException("Error parsing filter", e);
--- End diff --
Reworded.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928420
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java
---
@@ -156,38 +146,84 @@ public LDAPConnection bindAs(String userDN, String
password)
// Bind using provided credentials
try {
-byte[] passwordBytes;
-try {
-
-// Convert password into corresponding byte array
-if (password != null)
-passwordBytes = password.getBytes("UTF-8");
-else
-passwordBytes = null;
-
-}
-catch (UnsupportedEncodingException e) {
-logger.error("Unexpected lack of support for UTF-8: {}",
e.getMessage());
-logger.debug("Support for UTF-8 (as required by Java spec)
not found.", e);
-disconnect(ldapConnection);
-return null;
-}
-
-// Bind as user
-ldapConnection.bind(LDAPConnection.LDAP_V3, userDN,
passwordBytes);
+BindRequest bindRequest = new BindRequestImpl();
+bindRequest.setDn(userDN);
+bindRequest.setCredentials(password);
+ldapConnection.bind(bindRequest);
}
// Disconnect if an error occurs during bind
-catch (LDAPException e) {
-logger.debug("LDAP bind failed.", e);
+catch (LdapException e) {
+logger.debug("Unable to bind to LDAP server.", e);
disconnect(ldapConnection);
return null;
}
return ldapConnection;
}
+
+/**
+ * Generate a new LdapConnection object for following a referral
+ * with the given LdapUrl, and copy the username and password
+ * from the original connection.
+ *
+ * @param referralUrl
+ * The LDAP URL to follow.
+ *
+ * @param ldapConfig
+ * The connection configuration to use to retrieve username and
+ * password.
+ *
+ * @param hop
+ * The current hop number of this referral - once the configured
+ * limit is reached, this method will throw an exception.
+ *
+ * @return
+ * A LdapConnection object that points at the location
+ * specified in the referralUrl.
+ *
+ * @throws GuacamoleException
+ * If an error occurs parsing out the LdapUrl object or the
+ * maximum number of referral hops is reached.
+ */
+public LdapConnection referralConnection(LdapUrl referralUrl,
+LdapConnectionConfig ldapConfig, Integer hop)
--- End diff --
Fixed.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241928440
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
--- End diff --
Reworked to avoid this using the calls you mentioned.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241919019
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -240,17 +244,24 @@ public LDAPAuthenticatedUser
authenticateUser(Credentials credentials)
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
--- End diff --
Unfortunately a different LdapConnection implementation does get used -
LdapReferralConnection. I might be able to clean up some of it, but I think
some of the typecasting will have to stay. We'll see.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916742
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LdapDnGuacamoleProperty.java
---
@@ -0,0 +1,50 @@
+/*
+ * 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.ldap.conf;
+
+import
org.apache.directory.api.ldap.model.exception.LdapInvalidDnException;
+import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.properties.GuacamoleProperty;
+
+/**
+ * A GuacamoleProperty that converts a string to a Dn that can be used
+ * in LDAP connections. An exception is thrown if the provided DN is
invalid
+ * and cannot be parsed.
+ */
+public abstract class LdapDnGuacamoleProperty implements
GuacamoleProperty {
+
+@Override
+public Dn parseValue(String value) throws GuacamoleException {
+
+if (value == null)
+return null;
+
+try {
+return new Dn(value);
+}
+catch (LdapInvalidDnException e) {
+throw new GuacamoleServerException("Invalid DN specified in
configuration.", e);
--- End diff --
Sounds good, I'll rework this error a bit.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916665
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
+entries.add(((SearchResultEntry) response).getEntry());
}
-
-// Warn if referrals cannot be followed
-catch (LDAPReferralException e) {
-if (confService.getFollowReferrals()) {
-logger.error("Could not follow referral: {}",
e.getFailedReferral());
-logger.debug("Error encountered trying to follow
referral.", e);
-throw new GuacamoleServerException("Could not
follow LDAP referral.", e);
-}
-else {
-logger.warn("Given a referral, but referrals are
disabled. Error was: {}", e.getMessage());
-logger.debug("Got a referral, but configured to
not follow them.", e);
+else if (response instanceof SearchResultReference &&
+request.isFollowReferrals()) {
+
+Referral referral = ((SearchResultReference)
response).getReferral();
+int referralHop = 0;
+for (String url : referral.getLdapUrls()) {
+LdapConnection referralConnection =
ldapService.referralConnection(
+new LdapUrl(url), ldapConnectionConfig,
referralHop++);
+entries.addAll(search(referralConnection, baseDN,
query));
--- End diff --
Yeah, you're probably right. The entire recursion of this function was
kind of a wild stab, so I'm not surprised I made a few mistakes along the way.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916587
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
--- End diff --
I'll take a closer look.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241916353
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -240,17 +244,24 @@ public LDAPAuthenticatedUser
authenticateUser(Credentials credentials)
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
--- End diff --
I'll see what makes the most sense - maybe just converting everything to
LdapNetworkConnection is the most straight-forward, since that's really the
only implementation that ever gets used.
---
[GitHub] guacamole-client pull request #345: GUACAMOLE-234: Migrate to Apache Directo...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/345#discussion_r241908453
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java
---
@@ -188,46 +183,50 @@ public String generateQuery(String filter,
* information required to execute the query cannot be read from
* guacamole.properties.
*/
-public List search(LDAPConnection ldapConnection,
-String baseDN, String query) throws GuacamoleException {
+public List search(LdapConnection ldapConnection,
+Dn baseDN, ExprNode query) throws GuacamoleException {
logger.debug("Searching \"{}\" for objects matching \"{}\".",
baseDN, query);
try {
+LdapConnectionConfig ldapConnectionConfig =
+((LdapNetworkConnection) ldapConnection).getConfig();
+
// Search within subtree of given base DN
-LDAPSearchResults results = ldapConnection.search(baseDN,
-LDAPConnection.SCOPE_SUB, query, null, false,
-confService.getLDAPSearchConstraints());
+SearchRequest request = ldapService.getSearchRequest(baseDN,
+query);
+
+SearchCursor results = ldapConnection.search(request);
// Produce list of all entries in the search result,
automatically
// following referrals if configured to do so
-List entries = new ArrayList<>(results.getCount());
-while (results.hasMore()) {
+List entries = new ArrayList<>();
+while (results.next()) {
-try {
-entries.add(results.next());
+Response response = results.get();
+if (response instanceof SearchResultEntry) {
+entries.add(((SearchResultEntry) response).getEntry());
}
-
-// Warn if referrals cannot be followed
-catch (LDAPReferralException e) {
-if (confService.getFollowReferrals()) {
-logger.error("Could not follow referral: {}",
e.getFailedReferral());
-logger.debug("Error encountered trying to follow
referral.", e);
-throw new GuacamoleServerException("Could not
follow LDAP referral.", e);
-}
-else {
-logger.warn("Given a referral, but referrals are
disabled. Error was: {}", e.getMessage());
-logger.debug("Got a referral, but configured to
not follow them.", e);
+else if (response instanceof SearchResultReference &&
+request.isFollowReferrals()) {
+
+Referral referral = ((SearchResultReference)
response).getReferral();
+int referralHop = 0;
+for (String url : referral.getLdapUrls()) {
+LdapConnection referralConnection =
ldapService.referralConnection(
+new LdapUrl(url), ldapConnectionConfig,
referralHop++);
+entries.addAll(search(referralConnection, baseDN,
query));
--- End diff --
I'm not sure this is correct. Each call to `search()` will effectively
reset that `referralHop` counter. Doesn't this need to be tracked recursively
such that it limits the depth of the referral tree?
---
Re: Password management extension
On Fri, Dec 14, 2018 at 2:41 AM geo varghese wrote: > Hi Team, > > Thanks for making such a wonderful software. > > Is there any extension currently available for following customized feature > > 1) When a user clicks on a rdp server, before redirecting to server, it > will show a password interface. > Parameter prompting, including for credentials, is already a requested feature and there has been some work done on it, but there is a long way to go before it's ready: https://issues.apache.org/jira/browse/GUACAMOLE-221 > > 2) When user enter password for server login, it save with connection > details in mysql db. > > 3) Next time user clicks on same server, this password used for login to > server > > That's an interesting notion. It sounds similar to some other changes that have been proposed, particularly some sort of per-user credential management system, where credentials for users can be stored inside the Guacamole database and retrieved for connections. Today no such functionality exists. I encourage you to look through the JIRA system mentioned above at the other open requests and see if any of them fit what you're looking for. Also, you might want to check out the token passing in Guacamole - if your Guacamole login username and password match what is being used on the servers (due to LDAP or AD integration) you can use those in the connections to authenticate seamlessly: http://guacamole.apache.org/doc/gug/configuring-guacamole.html#parameter-tokens -Nick
