Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUACAMOLE-577: Support for configuring the Guacamole Proxy in LDAP Connections #353

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

necouchman
Copy link
Contributor

This pull request implements the changes necessary to support the GuacamoleProxyConfiguration in LDAP connections (and any future extensions that make use of the SimpleConnection class. Not entirely sure this is the right/best way to go, but took a stab at it.

@necouchman
Copy link
Contributor Author

Okay, fixed a few things up and made some of the requested changes. Let me know how it looks, now.

@necouchman
Copy link
Contributor Author

I may end up closing this PR and just redoing it - I tried to resolve the conflicts between current master and this PR, and it was feeling a little like the kitty-in-a-bathtub gif...

@mike-jumper
Copy link
Contributor

Ah, yes. That gif captures the feeling of refactoring so perfectly.

@necouchman necouchman force-pushed the jira/577 branch 4 times, most recently from 6786e23 to 1eb479d Compare May 14, 2021 14:22
@necouchman
Copy link
Contributor Author

Okay, @mike-jumper, I've reworked this on top of the existing master, so I think it's good, now...

@necouchman necouchman requested a review from mike-jumper May 14, 2021 14:25
@necouchman necouchman force-pushed the jira/577 branch 4 times, most recently from 865e2c6 to e6989d9 Compare May 31, 2021 01:35
@necouchman
Copy link
Contributor Author

necouchman commented May 31, 2021

I reworked the proxy attributes from LDAP as you suggested, but came across a couple of things:

  • I used the INTEGER syntax for the guacProxyPort attribute, but it's unclear to me if there are any implications to this - that is, will the Apache Directory API's Attribute getString() method actually return a String for this, or is it going to throw an exception? I think it'll return a String, because this isn't a binary value, but I've no easy way to verify that.
  • It ended up being easier to go ahead and grab the default proxy configuration in the LDAP ConnectionService code and use that as a base for then trying to pull values from LDAP. I know this is a bit redundant, but it seemed like the cleanest way to me to handle 1) initialization of those variables in ConnectionService (vs. assigning null and then checking to see if anything had been pulled from LDAP), and 2) cleanly handling situations where one value has been specified in LDAP but the others haven't (e.g. user assigns guacProxyHostname in LDAP, but leaves out guacProxyPort and guacProxyEncryption, and then GuacamolepProxyConfiguration is initialized with one real value and two unknown/other values). If there's a better way, don't hesitate to send that along.

@necouchman necouchman force-pushed the jira/577 branch 2 times, most recently from 4b9c977 to bbe957d Compare August 21, 2021 14:14
@mike-jumper mike-jumper changed the base branch from main to staging/1.6.0 July 31, 2024 21:35
Copy link
Contributor

@mike-jumper mike-jumper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflict, but otherwise LGTM.

@necouchman
Copy link
Contributor Author

Merge conflict, but otherwise LGTM.

Merge conflict is resolved.

@mike-jumper mike-jumper merged commit 5f8afbd into apache:staging/1.6.0 Aug 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants