Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/68#discussion_r77446177
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/AuthenticationProviderService.java
---
@@ -205,38 +214,80 @@ private LDAPConnection bindAs(Credentials credentials)
* denied.
*/
public AuthenticatedUser authenticateUser(Credentials credentials)
- throws GuacamoleException {
-
- // Attempt bind
- LDAPConnection ldapConnection;
- try {
- ldapConnection = bindAs(credentials);
- }
- catch (GuacamoleException e) {
- logger.error("Cannot bind with LDAP server: {}",
e.getMessage());
- logger.debug("Error binding with LDAP server.", e);
- ldapConnection = null;
- }
-
- // If bind fails, permission to login is denied
- if (ldapConnection == null)
- throw new GuacamoleInvalidCredentialsException("Permission
denied.", CredentialsInfo.USERNAME_PASSWORD);
-
- try {
-
- // Return AuthenticatedUser if bind succeeds
- AuthenticatedUser authenticatedUser =
authenticatedUserProvider.get();
- authenticatedUser.init(credentials);
- return authenticatedUser;
-
- }
-
- // Always disconnect
- finally {
- ldapService.disconnect(ldapConnection);
- }
-
+ throws GuacamoleException {
+
+ // Attempt bind
+ LDAPConnection ldapConnection;
+ try {
+ ldapConnection = bindAs(credentials);
+ }
+ catch (GuacamoleException e) {
+ logger.error("Cannot bind with LDAP server: {}",
e.getMessage());
+ logger.debug("Error binding with LDAP server.", e);
+ ldapConnection = null;
+ }
+
+ // If bind fails, permission to login is denied
+ if (ldapConnection == null)
+ throw new
GuacamoleInvalidCredentialsException("Permission denied.",
CredentialsInfo.USERNAME_PASSWORD);
+
+ boolean authenticated=true;
--- End diff --
> only way will be ...
I don't think that *has* to be the only other possibility. There really
must be a better way.
> however that will mean we have to repeat following code ...
If only there were a programming construct which allowed us to reuse pieces
of code ... ;)
Anyway, it's clear the logic here needs some cleaning, but it's hard to
read through given the other comments already made.
Perhaps we should revisit this after the stylistic issues are addressed?
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
