guacamole-client icon indicating copy to clipboard operation
guacamole-client copied to clipboard

GUACAMOLE-1881: Add parameter token containing the domain of LDAP/AD usernames.

Open mike-jumper opened this issue 1 year ago • 1 comments

This change is intended to supersede the changes proposed by poor @josnabattula via PR #931 that I have been very slow to review. It builds off the original commit (27bbd35a3d8d7bfb41ac50dfc145e1314121d289) by:

  • Removing unnecessary use of TokenName.canonicalize() (there's no need to dynamically derive the token name of a static value).
  • Decoupling the concept of extracting the domain from the concept of using that domain to create a token (getDomainToken() renamed and redocumented as getUserDomain()).
  • Removing unnecessary recompilation of the domain extraction Pattern (there's no need to repeatedly recompile the regex each time a user logs in - it never changes).

mike-jumper avatar May 15 '24 18:05 mike-jumper

@mike-jumper Can we get these changes into next release, its been months since I have raise pull request.

josnabattula avatar Jun 10 '24 21:06 josnabattula

@mike-jumper any plans to look into this pull request? it's been more than 6 months.

josnabattula avatar Jul 23 '24 03:07 josnabattula

@josnabattula Are you able to see Nick's comments above?

mike-jumper avatar Jul 27 '24 23:07 mike-jumper

@josnabattula Are you able to see Nick's comments above?

I can see. But those comments are on your branch you have refactored after my commit cheery-picked. I am thinking you are going to resolve it.

josnabattula avatar Jul 29 '24 23:07 josnabattula

Oof, right - that was silly of me. Sorry about that, @josnabattula.

mike-jumper avatar Jul 31 '24 21:07 mike-jumper

@necouchman Could you review again? All review comments are resolved now. thanks.

josnabattula avatar Aug 18 '24 21:08 josnabattula