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

Xpra Client not reconnecting after Update to Version 12 #4240

Closed
zvooveDev opened this issue May 28, 2024 · 8 comments
Closed

Xpra Client not reconnecting after Update to Version 12 #4240

zvooveDev opened this issue May 28, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@zvooveDev
Copy link

zvooveDev commented May 28, 2024

Describe the bug
After upgrading Xpra from version 4.4.6-r29-1 to 6.0-r0-1 I have issues reconnecting. When I close the XPra Client Browser Tab, wait a minute and reopen it I get the message "connection failed, invalid address?". Clicking on Connect results in the same message. The only fix in this situation is deleting Website data in Crome.
The problem does not occur if I activate "Disable cache" in the Network Tab of the Chrome development Console.
I can reliably reproduce and fix the problem by upgrading to 6.0-r0-1 (HTML Client Version 12) or downgrading to Version 4.4.6-r29-1 (HTML Client Version 8.1).

To Reproduce
Steps to reproduce the behavior:
Close the XPra Client Browser Tab, wait a minute and reopen it I get the message "connection failed, invalid address?". Clicking on Connect results in the same message. The only fix in this situation is deleting Website data in Crome.

System Information (please complete the following information):
Server OS: ubuntu:22.04
Client OS: Windows 11
Browser: Chrome 124.0.6347.93
Xpra Server Version 6.0-r0-1 (also tested 6.0.1)
Xpra Client Version 12
Application: Delphi Application running in Wine version 8.0.2
We are using an OAuth Proxy, maybe something changed in the local cache which interferes with our tokens?

Any idea what could cause this problem? What exactly is being cached and used accordingly when connecting?

@zvooveDev zvooveDev added the bug Something isn't working label May 28, 2024
@totaam
Copy link
Collaborator

totaam commented May 28, 2024

The message connection failed, invalid address? comes from:
https://github.com/Xpra-org/xpra-html5/blob/aad8e6c116089180eee60f200b11e8301a5cd915/html5/js/Client.js#L2431-L2435
And is called from only two places:

In both cases, there should be a message in your javascript console preceding this - you may need to enable main debug logging to see the one from the disconnect handler.
If the authentication went as far as the server, there should also be a message in your server log (add -d auth there to see more details).

It is possible that the problem comes from the authentication and only shows up as a more general connection failure.

Issues that may be (distantly?) related:
Xpra-org/xpra-html5#278
Xpra-org/xpra-html5#262
Xpra-org/xpra-html5#260
And this one has caused us pain a number of times:
Xpra-org/xpra-html5#292

@totaam
Copy link
Collaborator

totaam commented May 28, 2024

Can you reproduce this with the 5.x LTS branch? (this should narrow things down somewhat)
When I tested with a simple:

xpra start --bind-tcp=0.0.0.0:10000,auth=sys --no-daemon  --start=xterm :6

Closed the tab in Chrome, waited a few seconds and then restored it.
The automatic re-connection failed to prompt me for a password and I was sent back to the connect dialog. But clicking connect from there worked as expected.
I am moving this issue to: Xpra-org/xpra-html5#308

Is your issue the same?
Does the bug occur if you just open the URL again, without restoring the tab?

@zvooveDev
Copy link
Author

Some findings from today:

@totaam
Copy link
Collaborator

totaam commented May 28, 2024

I don't think that it will fix things, but worth a try:
https://github.com/Xpra-org/xpra-html5/releases/tag/v13

@zvooveDev
Copy link
Author

zvooveDev commented May 28, 2024

Updating to Xpra 6.1-r35816-1 and Client Version 13 did not have any effect on the issue. But it's interesting that 5.0.8-r0-1 works fine.

@totaam
Copy link
Collaborator

totaam commented Jun 18, 2024

I'm stuck on this one.
Can you run your server with -d auth and post the log output of both a successful connection and a failed reconnect?

I'm not using Xpra for Authentication but an OAuth2 Proxy

Can you expand on this?
Are you using the keycloak authentication module?
What command line arguments?
Can you share an example proxy config?

Is there anything in the Javascript console? (you may need to enable the "keep logs" option so that it doesn't start clean when it navigates - as even reloading the page may trigger it)


Looking at the changes to the keycloak auth module and its superclass, there were a lot of cosmetic changes and refactoring between v5 and v6 - any issues in these types of changes are normally caught by running the type checks or the unit tests, unfortunately the keycloak module is the only one that does not have proper unit tests that can run automatically..
I don't have an environment to test this at my end.

The only significant change in v6 is "add keycloak validation of authentication groups support": b4ba4ca by @iDmple - but really can't see how it would make any difference if you don't set the claim_field authentication module option or the XPRA_KEYCLOAK_CLAIM_FIELD environment variable


This "groups support" can be undone easily enough by reverting this patch:

--- a/xpra/server/auth/keycloak.py	2024-06-18 22:06:25.184036424 +0700
+++ b/xpra/server/auth/keycloak.py	2024-06-18 22:06:11.348127306 +0700
@@ -16,6 +16,9 @@
 KEYCLOAK_CLIENT_ID = os.environ.get("XPRA_KEYCLOAK_CLIENT_ID", "example_client")
 KEYCLOAK_CLIENT_SECRET_KEY = os.environ.get("XPRA_KEYCLOAK_CLIENT_SECRET_KEY", "secret")
 KEYCLOAK_REDIRECT_URI = os.environ.get("XPRA_KEYCLOAK_REDIRECT_URI", "http://localhost/login/")
+KEYCLOAK_CLAIM_FIELD = os.environ.get("XPRA_KEYCLOAK_CLAIM_FIELD", "")  # field containing groups claim
+KEYCLOAK_AUTH_GROUPS = os.environ.get("XPRA_KEYCLOAK_AUTH_GROUPS", "")  # authorized groups
+KEYCLOAK_AUTH_CONDITION = os.environ.get("XPRA_KEYCLOAK_AUTH_CONDITION", "and")  # authentication condition
 KEYCLOAK_SCOPE = os.environ.get("XPRA_KEYCLOAK_SCOPE", "openid")
 KEYCLOAK_GRANT_TYPE = os.environ.get("XPRA_KEYCLOAK_GRANT_TYPE", "authorization_code")
 
@@ -28,6 +31,9 @@
         self.client_id = kwargs.pop("client_id", KEYCLOAK_CLIENT_ID)
         self.client_secret_key = kwargs.pop("client_secret_key", KEYCLOAK_CLIENT_SECRET_KEY)
         self.redirect_uri = kwargs.pop("redirect_uri", KEYCLOAK_REDIRECT_URI)
+        self.claim_field = kwargs.pop("claim_field", KEYCLOAK_CLAIM_FIELD)
+        self.auth_groups = set(kwargs.pop("auth_groups", KEYCLOAK_AUTH_GROUPS).split())
+        self.auth_condition = kwargs.pop("auth_condition", KEYCLOAK_AUTH_CONDITION)
         self.scope = kwargs.pop("scope", KEYCLOAK_SCOPE)
         self.grant_type = kwargs.pop("grant_type", KEYCLOAK_GRANT_TYPE)
 
@@ -162,8 +168,67 @@
             log("keycloak: token is active")
 
             user_info = keycloak_openid.userinfo(access_token)
-            log("userinfo_info: %r", user_info)
-            log("keycloak authentication succeeded: token is active")
+            log("user_info: %r", user_info)
+
+            log("claim_field: %r", self.claim_field)
+            log("auth_groups: %r", self.auth_groups)
+
+            # if claim_field is defined, check that the auth_groups match the groups_claim condition
+            if self.claim_field:
+                if not self.auth_groups:
+                    log.error("Error: keycloak authentication failed as auth_groups is invalid")
+                    return False
+
+                def extract_groups_claim(data_dict, keys):
+
+                    """
+                    Extracts a nested value from a dictionary using a list of keys.
+
+                    This function is specifically designed to handle nested dictionaries,
+                    such as the ones provided by Keycloak userinfo.
+
+                    For example, userinfo could contain the following structure:
+
+                    "resource_access": {
+                        "roles": {
+                            "group": [
+                                "client-group-1",
+                                "client-group-2"
+                            ]
+                        }
+                    }
+
+                    In this scenario, the function traverses the dictionary using the keys
+                    ['resource_access', 'roles', 'group'] to extract the list of groups
+                    ['client-group-1', 'client-group-2'].
+
+                    Args:
+                        data_dict (dict): The dictionary from which to extract the nested value.
+                        keys (list): A list of keys that represents the path to the desired value
+                                     in the nested dictionary.
+
+                    Returns:
+                        The nested value extracted from the dictionary. If the sequence of keys is
+                        exhausted, it returns the portion of the dictionary that it has navigated to.
+                    """
+                    return extract_groups_claim(data_dict[keys.pop(0)], keys) if len(keys) else data_dict
+
+                groups_claim = set(extract_groups_claim(user_info, self.claim_field.split('.')))
+                log("groups_claim: %r", groups_claim)
+
+                if self.auth_condition == "or":
+                    if not self.auth_groups.intersection(groups_claim):
+                        log.error("Error: keycloak authentication failed as groups claim is not satisfied")
+                        return False
+                elif self.auth_condition == "and":
+                    if not self.auth_groups.issubset(groups_claim):
+                        log.error("Error: keycloak authentication failed as groups claim is not satisfied")
+                        return False
+                else:
+                    log.error("Error: keycloak authentication failed as auth_condition is invalid")
+                    return False
+
+            log("keycloak authentication succeeded")
             return True
         except KeycloakError as e:
             log.error("Error: keycloak authentication failed")

@iDmple
Copy link
Contributor

iDmple commented Jun 18, 2024

For what's it's worth we don't have this issue on our end with the added code. I'd also like to know more about OAuth2 Proxy.

@zvooveDev
Copy link
Author

I just updated to Version 6.1 and Client Version 14 (including other dependencys) and the problem is solved. Thank you for your help.

@totaam totaam closed this as completed Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants