You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@guacamole.apache.org by necouchman <gi...@git.apache.org> on 2017/03/18 16:17:32 UTC

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

GitHub user necouchman opened a pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131

    GUACAMOLE-244: Support configuration of alias dereferencing

    This adds the necessary code to allow the user to configure how the LDAP extension behaves with regard to dereferencing aliases.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/necouchman/incubator-guacamole-client GUACAMOLE-244

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-guacamole-client/pull/131.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 #131
    
----
commit 907e0edfcfa23eab3da12c7c3d8ff945b5470830
Author: Nick Couchman <ni...@yahoo.com>
Date:   2017-03-18T16:08:38Z

    GUACAMOLE-244: Support configuration of alias dereferencing

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820463
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    --- End diff --
    
    Perhaps it would be better to implement a `GuacamoleProperty` subclass which actually (and strictly) parses these values? The list of possible values here is begging for an `enum`.
    
    I see `never` is never explicitly handled below, and any mistake in the spelling of the other values will result in the property silently assuming you meant `never`, rather than throwing an explicit exception.
    
    It would be better if incorrect values result in an exception (which will ultimately be logged).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049388
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliases.java ---
    @@ -0,0 +1,71 @@
    +/*
    + * 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;
    +
    +/**
    + * Acceptable values for configuring the dereferencing of aliases in
    + * talking to LDAP servers.
    + */
    +public enum DereferenceAliases {
    --- End diff --
    
    Chose DereferenceAliasesMode.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049478
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +231,51 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.  The default
    +     * behavior if not explicityly defined is to never 
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106883642

--- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
@@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
);
}

+ /**
+ * Returns whether or not LDAP aliases will be dereferenced,
+ * as configured with guacamole.properties.
+ * By default they will never be dereferenced.
+ *
+ * @return
+ * An integer representing the status of of alias
+ * dereferencing, as configured in guacamole.properties.
+ *
+ * @throws GuacamoleException
+ * If guacamole.properties cannot be parsed.
+ */
+ public int getDereferenceAliases() throws GuacamoleException {
+ String derefAliases = environment.getProperty(
+ LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
+ "never"
--- End diff --

Okay, the latest commit takes a stab at an implementation of the new enum property. I referenced the EncryptionMethod and EncryptionMethodProperty classes in doing the implementation. It's slightly different since I'm mapping actual values for the JLDAP library instead of using encryption method to determine default port, but I think it'll do the trick.

Also, I looked up the OpenLDAP values for the DEREF setting in ldap.conf, and it matches JLDAP exactly, so I'm going to stick with the never/searching/finding/always values at this point in time, unless someone would like to suggest other options. Looks like it's more standard than I have experienced - I usually just toggle between never and always.

---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107026057

--- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliases.java ---
@@ -0,0 +1,71 @@
+/*
+ * 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;
+
+/**
+ * Acceptable values for configuring the dereferencing of aliases in
+ * talking to LDAP servers.
+ */
+public enum DereferenceAliases {
+
+ /**
+ * Never dereference aliases. This is the default.
+ */
+ NEVER(0),
+
+ /**
+ * Aliases are dereferenced below the base object, but not to locate
+ * the base object itself. So, if the base object is itself an alias
+ * the search will not complete.
+ */
+ SEARCHING(1),
+
+ /**
+ * Aliases are only dereferenced to locate the base object, but not
+ * after that. So, a search against a base object that is an alias will
+ * find any subordinates of the real object the aliase references, but
+ * further aliases in the search will not be dereferenced.
+ */
+ FINDING(2),
+
+ /**
+ * Aliases will always be dereferenced, both to locate the base object
+ * and when handling results returned by the search.
+ */
+ ALWAYS(3);
+
+ /**
+ * The integer value that the enum represents, which is used in
--- End diff --

"The integer value that the enum represents" is vague, even with the added context that it's used in configuring JLDAP. Ultimately all this says is that it's a number that will be given to JLDAP; it doesn't describe the semantics behind that number.

Should probably reference that this is the integer constant required by `LDAPSearchConstraints` to define alias dereferencing behavior.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824574
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    +        );
    +
    +        if (derefAliases == "always")
    --- End diff --
    
    Doh.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107032658
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +231,51 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.  The default
    +     * behavior if not explicityly defined is to never 
    +     * dereference them.
    +     *
    +     * @return
    +     *     The behavior for handling dereferencing of aliases
    +     *     as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public DereferenceAliases getDereferenceAliases() throws GuacamoleException {
    +        return environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            DereferenceAliases.NEVER
    +        );
    +
    +    }
    +
    +    /**
    +     * Returns a set of LDAPSearchConstraints to apply globally
    +     * to all LDAP searches rather than having various instances
    +     * dispersed throughout the code.  Currently contains the
    --- End diff --
    
    I would recommend, simply:
    
    "Returns a set of LDAPSearchConstraints which should be applied to all LDAP searches."
    
    That a function should be used "rather than [copying the internals of the function everywhere]" applies to all functions by definition, and goes without saying. There's no need to enumerate the bad practices we avoid by having functions.
    
    That said ... if this function effectively replaces the `getDereferenceAliases()` and `getMaxResults()` functions, and it no longer makes sense to call those functions externally, perhaps those functions should be removed (or declared `private`)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107030874

--- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java ---
@@ -153,4 +153,14 @@ private LDAPGuacamoleProperties() {}

};

+ /**
+ * The behavior of alias dereferencing for the LDAP connections.
--- End diff --

But what does this mean?

It's clear to me since I've read what you've written in JIRA, etc., but the comment doesn't tell me what changing this setting does. I would have to dig through other parts of the code to find out that "alias dereferencing" is actually a consideration for queries against LDAP, but the comment as written implies this aliases have something to do with the LDAP connection itself.

In your other PR for the documentation covering this new property (apache/incubator-guacamole-manual#38), you say:

> Controls whether or not the LDAP connection follows (dereferences) aliases as it searches the tree.

Something like that would be perfect. Descriptive, accurate, yet not overly verbose.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106825458
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java ---
    @@ -88,6 +88,7 @@ private void putAllUsers(Map<String, User> users, LDAPConnection ldapConnection,
                 // Set search limits
                 LDAPSearchConstraints constraints = new LDAPSearchConstraints();
                 constraints.setMaxResults(confService.getMaxResults());
    +            constraints.setDereference(confService.getDereferenceAliases());
    --- End diff --
    
    Create getLDAPSearchConstraints in the ConfigurationService class which pulls in getMaxResults() and getDereferenceAliases() and sets up the constraints.  Changed code in UserService and ConnectionService to use this function from confService instead of setting up their own each time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by jmuehlner <gi...@git.apache.org>.

Github user jmuehlner commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107064307
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliasesProperty.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.ldap;
    +
    +import org.apache.guacamole.GuacamoleException;
    +import org.apache.guacamole.GuacamoleServerException;
    +import org.apache.guacamole.properties.GuacamoleProperty;
    +
    +/**
    + * A GuacamoleProperty with a value of DereferenceAliases. The possible strings
    + * "never", "searching", "finding", and "always" are mapped to their values as a
    + * DereferenceAliases enum.  Anything else results in a parse error.
    --- End diff --
    
    Extra space here too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107024925

You mean "... configuring alias dereferencing behavior for queries against LDAP servers." ? Or ... something else more specific? "talking to LDAP servers" is rather broad.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107029332

alias*

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by jmuehlner <gi...@git.apache.org>.

Github user jmuehlner commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107064241
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -216,11 +224,54 @@ public EncryptionMethod getEncryptionMethod() throws GuacamoleException {
          * @throws GuacamoleException
          *     If guacamole.properties cannot be parsed.
          */
    -    public int getMaxResults() throws GuacamoleException {
    +    private int getMaxResults() throws GuacamoleException {
             return environment.getProperty(
                 LDAPGuacamoleProperties.LDAP_MAX_SEARCH_RESULTS,
                 1000 
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.  The default
    --- End diff --
    
    Extra space here too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049357

Hopefully that looks better.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820666
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    +        );
    +
    +        if (derefAliases == "always")
    +            return 3;
    --- End diff --
    
    [`LDAPSearchConstraints`](https://www.novell.com/documentation/developer/jldap/jldapenu/api/com/novell/ldap/LDAPSearchConstraints.html) defines constants for these values (`DEREF_ALWAYS`, `DEREF_FINDING`, ...). We should be using those, rather than hard-coding their low-level integer values.
    
    That said, I'm not sure this is really a good way to achieve this. We're essentially marrying Guacamole's configuration to the internals of JLDAP, which definitely violates separation of concerns. If the internals of JLDAP happened to just make that much sense, then OK, but "always", "finding", "searching", and "never" don't really make intuitive sense.
    
    If `ConfigurationService` will ultimately return the integer that `LDAPSearchConstraints` requires, that's OK - it's internal to the extension, and `ConfigurationService` itself separates that concern from that of the configuration, but you don't want to violate that separation by essentially mapping internals directly to `guacamole.properties`. Ideally, you'd determine a sensible set of values based on the behaviors that the LDAP auth should support, and then translate those into the values required by JLDAP.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049414
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliases.java ---
    @@ -0,0 +1,71 @@
    +/*
    + * 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;
    +
    +/**
    + * Acceptable values for configuring the dereferencing of aliases in
    + * talking to LDAP servers.
    + */
    +public enum DereferenceAliases {
    +
    +    /**
    +     * Never dereference aliases.  This is the default.
    +     */
    +    NEVER(0),
    --- End diff --
    
    Replaced with the LDAPSearchConstraints constants.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820366
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    +        );
    +
    +        if (derefAliases == "always")
    --- End diff --
    
    Java does not do string comparisons in this way. You need to use `.equals()`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049457

Reworded.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820853
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    --- End diff --
    
    "An integer representing the status of alias dereferencing" is not clear in itself. The possible values of that integer, and the semantics of each, are not documented. Just documenting the values is insufficient, though. More on this below, but this would be a perfect spot to use an `enum`, that way all values are very strictly defined and documented.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824918
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    --- End diff --
    
    Beefed up the commentary some, hopefully it's clearer this way, though will probably end up changing with implementation of an enum type.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107023687

Since this is defined based on the integer values required by JLDAP, we should reference the constants provided by JLDAP for this purpose, rather than hard-coding their integer values. See:

https://www.novell.com/documentation/developer/jldap/jldapenu/api/com/novell/ldap/LDAPSearchConstraints.html#DEREF_NEVER

ie: `NEVER(LDAPSearchConstraints.DEREF_NEVER)`.

Same thing for the other values.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107023916
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +231,51 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.  The default
    +     * behavior if not explicityly defined is to never 
    --- End diff --
    
    explicitly*


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824763
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    +        );
    +
    +        if (derefAliases == "always")
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-guacamole-client/pull/131


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107028411
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliases.java ---
    @@ -0,0 +1,71 @@
    +/*
    + * 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;
    +
    +/**
    + * Acceptable values for configuring the dereferencing of aliases in
    + * talking to LDAP servers.
    + */
    +public enum DereferenceAliases {
    --- End diff --
    
    The name of this class kind of throws me at first glance. It sounds like it represents a collection of aliases or all possible legal aliases. The convention that type names are nouns leads to an incorrect interpretation of what this actually is.
    
    Perhaps something like `DereferenceAliasesMode` or `AliasDereferencingMode` or anything-else-that's-a-noun would be more clear?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106829029
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    --- End diff --
    
    The `derefAliases.equals()` comparisons will also throw a `NullPointerException` when `derefAliases` is `null`, which will be the case if that property is not specified.
    
    In any case, this and more can be resolved once things are cleaned up and migrated to an `enum`. There's an example of this being done elsewhere in the LDAP auth, actually. Take a peek at [`EncryptionMethodProperty`](https://github.com/apache/incubator-guacamole-client/blob/1a621886c65d5e99ec44841da41d137000a1c9f4/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/EncryptionMethodProperty.java).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824453
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java ---
    @@ -88,6 +88,7 @@ private void putAllUsers(Map<String, User> users, LDAPConnection ldapConnection,
                 // Set search limits
                 LDAPSearchConstraints constraints = new LDAPSearchConstraints();
                 constraints.setMaxResults(confService.getMaxResults());
    +            constraints.setDereference(confService.getDereferenceAliases());
    --- End diff --
    
    I think that approach probably makes sense.  I think this setMaxResults was a recent-ish change to the code to allow searching on large LDAP trees and avoid whatever built-in default existed in the JLDAP code, but I could be wrong about that.  Anyway, probably makes sense to move the constraints to a more global area and apply them universally.  I'll see what kind of progress I can make on that...though any specific help/suggestions would be welcome.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by jmuehlner <gi...@git.apache.org>.

Github user jmuehlner commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107064203
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/DereferenceAliasesMode.java ---
    @@ -0,0 +1,74 @@
    +/*
    + * 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;
    +
    +import com.novell.ldap.LDAPSearchConstraints;
    +
    +/**
    + * Data type that handles acceptable values for configuring
    + * alias dereferencing behavior when querying LDAP servers.
    + */
    +public enum DereferenceAliasesMode {
    +
    +    /**
    +     * Never dereference aliases.  This is the default.
    --- End diff --
    
    This comment and the 3 following ones all have unnecessary extra spaces.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824798
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    --- End diff --
    
    Yes, probably a good idea...will work on that.
    
    I did fix the checking of the string value, in the meantime, such that it expects one of the four values and throws an error if it doesn't get what it expects.  Still would be more elegant and proper with an enum.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106825017
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java ---
    @@ -153,4 +153,14 @@ private LDAPGuacamoleProperties() {}
     
         };
     
    +    /**
    +     * The behavior of alias dereferncing for the LDAP connections.
    --- End diff --
    
    Trusty VIM spell checker failed me...oh, wait, nevermind.  Even if it was there, it wouldn't like dereferencing, anyway.  Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049425

Fixed.

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820511
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java ---
    @@ -153,4 +153,14 @@ private LDAPGuacamoleProperties() {}
     
         };
     
    +    /**
    +     * The behavior of alias dereferncing for the LDAP connections.
    --- End diff --
    
    dereferencing*


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049565
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +231,51 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.  The default
    +     * behavior if not explicityly defined is to never 
    +     * dereference them.
    +     *
    +     * @return
    +     *     The behavior for handling dereferencing of aliases
    +     *     as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public DereferenceAliases getDereferenceAliases() throws GuacamoleException {
    +        return environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            DereferenceAliases.NEVER
    +        );
    +
    +    }
    +
    +    /**
    +     * Returns a set of LDAPSearchConstraints to apply globally
    +     * to all LDAP searches rather than having various instances
    +     * dispersed throughout the code.  Currently contains the
    --- End diff --
    
    Fixed comments, made getDereferenceAliases and getMaxResults private - I can remove them and just put the logic into the constraints function if that makes more sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106824567
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java ---
    @@ -223,4 +223,36 @@ public int getMaxResults() throws GuacamoleException {
             );
         }
     
    +    /**
    +     * Returns whether or not LDAP aliases will be dereferenced,
    +     * as configured with guacamole.properties.
    +     * By default they will never be dereferenced.
    +     *
    +     * @return
    +     *     An integer representing the status of of alias
    +     *     dereferencing, as configured in guacamole.properties.
    +     *
    +     * @throws GuacamoleException
    +     *     If guacamole.properties cannot be parsed.
    +     */
    +    public int getDereferenceAliases() throws GuacamoleException {
    +        String derefAliases = environment.getProperty(
    +            LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
    +            "never"
    +        );
    +
    +        if (derefAliases == "always")
    +            return 3;
    --- End diff --
    
    I'll look at a couple of other implementations of LDAP clients and see what they do for this type of behavior.  The "finding" and "searching" terms are a little strange to me - I think in the past I've usually run across either two or three objects - usually on, off, and maybe something in between.  Four is a little odd...I'll have to see what the more general consensus is among LDAP clients.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by mike-jumper <gi...@git.apache.org>.

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r106820788
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/user/UserService.java ---
    @@ -88,6 +88,7 @@ private void putAllUsers(Map<String, User> users, LDAPConnection ldapConnection,
                 // Set search limits
                 LDAPSearchConstraints constraints = new LDAPSearchConstraints();
                 constraints.setMaxResults(confService.getMaxResults());
    +            constraints.setDereference(confService.getDereferenceAliases());
    --- End diff --
    
    If we keep building a new `LDAPSearchConstraints` object, would it make more sense to provide a function which does so within `ConfigurationService`, rather than repeatedly calling the same setters on a new `LDAPSearchConstraints` prior to each LDAP query?
    
    I'm also not clear on why `setMaxResults()` is only applied here. If it should apply universally, then I would lean even more strongly in the direction described above. We can abstract away the configuration properties which apply to LDAP constraints within the internals of `ConfigurationService` and just rely on the service to return an appropriate constraints object.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #131: GUACAMOLE-244: Support configu...

Posted by necouchman <gi...@git.apache.org>.

Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107049592
  
    --- Diff: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java ---
    @@ -153,4 +153,14 @@ private LDAPGuacamoleProperties() {}
     
         };
     
    +    /**
    +     * The behavior of alias dereferencing for the LDAP connections.
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---