From c22e576ec525ca8427adb5bb64829c8794e1f408 Mon Sep 17 00:00:00 2001 From: Zhongxin Guo Date: Mon, 8 Sep 2025 02:53:48 -0700 Subject: [PATCH 1/9] implement the rest-server request --- .../src/copilot_agent/utils/authentication.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 44a638a2..5346261e 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -1,6 +1,7 @@ """Authentication Manager.""" import os +import requests from datetime import datetime, timezone @@ -33,8 +34,24 @@ def authenticate(self, username: str, token: str) -> list: if username == "baduser": return ["temp"] else: - # TBD # This function should implement the logic to verify the user's token against the REST server (self.restserver_url). + try: + headers = { + 'Authorization': f'Bearer {token}' + } + response = requests.get(f'{self.restserver_url}/api/v2/users/{username}', headers=headers) + + if response.status_code == 200: + user_data = response.json() + # Extract groups from the response - adjust based on actual API response structure + groups = user_data.get('grouplist', []) + return groups + else: + print(f"Authentication failed for user {username}: {response.status_code}") + return [] + except Exception as e: + print(f"Error during authentication for user {username}: {e}") + return [] return [] def set_authenticate_state(self, username: str, token: str) -> None: From f84f665a3a477c3b52dd5d18acf3d9a5dd074bb1 Mon Sep 17 00:00:00 2001 From: Lei Date: Mon, 8 Sep 2025 18:43:54 +0800 Subject: [PATCH 2/9] improve: using logger instead, move local mode default return to correct location --- .../src/copilot_agent/utils/authentication.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 5346261e..721566a0 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -6,6 +6,7 @@ from datetime import datetime, timezone from ..config import AGENT_MODE_LOCAL +from ..utils.logger import logger class AuthenticationManager: """Manages authentication state, expiration, and revocation for users.""" @@ -33,6 +34,8 @@ def authenticate(self, username: str, token: str) -> list: return ["admin"] if username == "baduser": return ["temp"] + # For any other username in local mode, return empty list + return [] else: # This function should implement the logic to verify the user's token against the REST server (self.restserver_url). try: @@ -47,12 +50,11 @@ def authenticate(self, username: str, token: str) -> list: groups = user_data.get('grouplist', []) return groups else: - print(f"Authentication failed for user {username}: {response.status_code}") + logger.error(f"Authentication failed for user {username}: {response.status_code}") return [] except Exception as e: - print(f"Error during authentication for user {username}: {e}") + logger.error(f"Error during authentication for user {username}: {e}") return [] - return [] def set_authenticate_state(self, username: str, token: str) -> None: """Set the authentication state for a user.""" From 5fe6f20582b334843d7896d69c2a3f46520281f6 Mon Sep 17 00:00:00 2001 From: Lei Qu <59161330+quge009@users.noreply.github.com> Date: Mon, 8 Sep 2025 19:03:27 +0800 Subject: [PATCH 3/9] Update src/copilot-chat/src/copilot_agent/utils/authentication.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/copilot-chat/src/copilot_agent/utils/authentication.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 721566a0..ded3bbec 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -42,7 +42,7 @@ def authenticate(self, username: str, token: str) -> list: headers = { 'Authorization': f'Bearer {token}' } - response = requests.get(f'{self.restserver_url}/api/v2/users/{username}', headers=headers) + response = requests.get(f'{self.restserver_url}/api/v2/users/{username}', headers=headers, timeout=5) if response.status_code == 200: user_data = response.json() From 591d8637f59161c441c802ed1ced6aa0c8b57b90 Mon Sep 17 00:00:00 2001 From: Lei Date: Mon, 8 Sep 2025 19:24:58 +0800 Subject: [PATCH 4/9] fix: sanitize username before being included into the url --- .../src/copilot_agent/utils/authentication.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index ded3bbec..e0d0306a 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -2,6 +2,7 @@ import os import requests +import urllib.parse from datetime import datetime, timezone @@ -17,6 +18,10 @@ def __init__(self, expiration_ms: int = 3600000): valid_groups_env = os.getenv('COPILOT_VALID_GROUPS', 'admin,superuser') self.valid_groups = [g.strip() for g in valid_groups_env.split(',') if g.strip()] + def sanitize_username(self, username: str) -> str: + """Sanitize the username by URL-encoding it to prevent path traversal or injection attacks.""" + return urllib.parse.quote(username, safe='') + def authenticate(self, username: str, token: str) -> list: """ Authenticate a user with token and REST server URL. @@ -42,7 +47,8 @@ def authenticate(self, username: str, token: str) -> list: headers = { 'Authorization': f'Bearer {token}' } - response = requests.get(f'{self.restserver_url}/api/v2/users/{username}', headers=headers, timeout=5) + username_sanitized = self.sanitize_username(username) + response = requests.get(f'{self.restserver_url}/api/v2/users/{username_sanitized}', headers=headers, timeout=5) if response.status_code == 200: user_data = response.json() From c59957e5af6596c5af2f1db3842528ee51e081f6 Mon Sep 17 00:00:00 2001 From: Lei Date: Wed, 10 Sep 2025 16:00:51 +0800 Subject: [PATCH 5/9] update: get is_admin and virtual_cluster from rest server api for authentication purpose --- src/copilot-chat/config/copilot-chat.yaml | 2 +- .../copilot-chat-deployment.yaml.template | 4 +- .../src/copilot_agent/utils/authentication.py | 50 +++++++++++-------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/copilot-chat/config/copilot-chat.yaml b/src/copilot-chat/config/copilot-chat.yaml index 7bae69df..b51dd807 100644 --- a/src/copilot-chat/config/copilot-chat.yaml +++ b/src/copilot-chat/config/copilot-chat.yaml @@ -31,4 +31,4 @@ llm-provider: "azure" llm-endpoint: "https://endpoint.openai.com" llm-model: "gpt-4o" llm-version: "2025-01-01-preview" -valid-groups: "admin,superuser" \ No newline at end of file +valid-vcs: "admin,superuser" \ No newline at end of file diff --git a/src/copilot-chat/deploy/copilot-chat-deployment.yaml.template b/src/copilot-chat/deploy/copilot-chat-deployment.yaml.template index 3e418e74..f7a8c891 100644 --- a/src/copilot-chat/deploy/copilot-chat-deployment.yaml.template +++ b/src/copilot-chat/deploy/copilot-chat-deployment.yaml.template @@ -73,8 +73,8 @@ spec: value: {{ cluster_cfg["copilot-chat"]["agent-host"] }} - name: RESTSERVER_URL value: {{ cluster_cfg["copilot-chat"]["rest-server"]["url"] }} - - name: COPILOT_VALID_GROUPS - value: {{ cluster_cfg["copilot-chat"]["valid-groups"] }} + - name: COPILOT_VALID_VCS + value: {{ cluster_cfg["copilot-chat"]["valid-vcs"] }} ports: - containerPort: {{ cluster_cfg["copilot-chat"]["agent-port"] | default("50000") }} hostPort: {{ cluster_cfg["copilot-chat"]["agent-port"] | default("50000") }} diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index e0d0306a..810707b0 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -15,14 +15,14 @@ def __init__(self, expiration_ms: int = 3600000): self.authenticate_state = {} # username: {token, expires_at, groups} self.expiration_ms = expiration_ms self.restserver_url = os.getenv('RESTSERVER_URL', '') - valid_groups_env = os.getenv('COPILOT_VALID_GROUPS', 'admin,superuser') - self.valid_groups = [g.strip() for g in valid_groups_env.split(',') if g.strip()] + valid_vcs_env = os.getenv('COPILOT_VALID_GROUPS', 'admin,superuser') + self.valid_vcs = [g.strip() for g in valid_vcs_env.split(',') if g.strip()] def sanitize_username(self, username: str) -> str: """Sanitize the username by URL-encoding it to prevent path traversal or injection attacks.""" return urllib.parse.quote(username, safe='') - def authenticate(self, username: str, token: str) -> list: + def authenticate(self, username: str, token: str): """ Authenticate a user with token and REST server URL. @@ -31,16 +31,15 @@ def authenticate(self, username: str, token: str) -> list: token (str): The authentication token provided by the user. Returns: - list: A list of group names the user belongs to if authentication is successful. - Returns an empty list if authentication fails. + tuple: (admin, virtualCluster) """ if AGENT_MODE_LOCAL: if username == "gooduser": - return ["admin"] + return True, ["admin"] if username == "baduser": - return ["temp"] + return False, ["temp"] # For any other username in local mode, return empty list - return [] + return False, [] else: # This function should implement the logic to verify the user's token against the REST server (self.restserver_url). try: @@ -53,24 +52,26 @@ def authenticate(self, username: str, token: str) -> list: if response.status_code == 200: user_data = response.json() # Extract groups from the response - adjust based on actual API response structure - groups = user_data.get('grouplist', []) - return groups + is_admin = user_data.get('admin', False) + virtual_cluster = user_data.get('virtualCluster', []) + return is_admin, virtual_cluster else: logger.error(f"Authentication failed for user {username}: {response.status_code}") - return [] + return False, [] except Exception as e: logger.error(f"Error during authentication for user {username}: {e}") - return [] + return False, [] def set_authenticate_state(self, username: str, token: str) -> None: - """Set the authentication state for a user.""" + """Set the authentication state for a user, storing admin and virtualCluster info.""" expires_at = int(datetime.now(timezone.utc).timestamp() * 1000) + self.expiration_ms - groups = self.authenticate(username, token) - if groups: + is_admin, virtual_cluster = self.authenticate(username, token) + if is_admin is not None and virtual_cluster is not None: self.authenticate_state[username] = { 'token': token, 'expires_at': expires_at, - 'groups': groups + 'is_admin': is_admin, + 'virtual_cluster': virtual_cluster } else: self.revoke(username) @@ -83,14 +84,23 @@ def is_authenticated(self, username: str) -> bool: if state['expires_at'] < now: self.revoke(username) return False - if "groups" not in state: + if "is_admin" not in state: return False - if "groups" in state and not self.get_membership(state["groups"]): + if "virtual_cluster" not in state: return False - return True + if "is_admin" in state and "virtual_cluster" in state: + if state["is_admin"]: + # validate pass condition one: user is an admin + return True + elif not state["is_admin"] and self.get_membership(state["virtual_cluster"]): + # validate pass condition two: user is not an admin, but it belongs to a valid virtualCluster + return True + else: + return False + return False def get_membership(self, groups: list) -> bool: - return any(group in self.valid_groups for group in groups) + return any(group in self.valid_vcs for group in groups) def revoke(self, username: str): if username in self.authenticate_state: From 0b73b2df961336542186b744b5904fddf1dac107 Mon Sep 17 00:00:00 2001 From: Lei Qu <59161330+quge009@users.noreply.github.com> Date: Wed, 10 Sep 2025 16:11:59 +0800 Subject: [PATCH 6/9] Update src/copilot-chat/src/copilot_agent/utils/authentication.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/copilot-chat/src/copilot_agent/utils/authentication.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 80101b50..2c7bc172 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -18,7 +18,7 @@ def __init__(self, expiration_ms: int = 3600000): self.authenticate_state = {} # username: {token, expires_at, groups} self.expiration_ms = expiration_ms self.restserver_url = os.getenv('RESTSERVER_URL', '') - valid_vcs_env = os.getenv('COPILOT_VALID_GROUPS', 'admin,superuser') + valid_vcs_env = os.getenv('COPILOT_VALID_VCS', 'admin,superuser') self.valid_vcs = [g.strip() for g in valid_vcs_env.split(',') if g.strip()] def sanitize_username(self, username: str) -> str: From 5f60a8e721ce200d82758aefcd8aa2cf052a81a0 Mon Sep 17 00:00:00 2001 From: Lei Qu <59161330+quge009@users.noreply.github.com> Date: Wed, 10 Sep 2025 16:12:28 +0800 Subject: [PATCH 7/9] Update src/copilot-chat/src/copilot_agent/utils/authentication.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/copilot-chat/src/copilot_agent/utils/authentication.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 2c7bc172..594f91d9 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -93,10 +93,10 @@ def is_authenticated(self, username: str) -> bool: return False if "is_admin" in state and "virtual_cluster" in state: if state["is_admin"]: - # validate pass condition one: user is an admin + # validate pass condition one: user is an admin return True elif not state["is_admin"] and self.get_membership(state["virtual_cluster"]): - # validate pass condition two: user is not an admin, but it belongs to a valid virtualCluster + # validate pass condition two: user is not an admin, but it belongs to a valid virtualCluster return True else: return False From 1a02b251b71f52b6ac3a1be4279ebf7f68c34727 Mon Sep 17 00:00:00 2001 From: Lei Date: Wed, 10 Sep 2025 17:45:46 +0800 Subject: [PATCH 8/9] minor --- src/copilot-chat/config/copilot-chat.yaml | 2 +- src/copilot-chat/src/copilot_agent/utils/authentication.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/copilot-chat/config/copilot-chat.yaml b/src/copilot-chat/config/copilot-chat.yaml index 78aefda4..0b578ae2 100644 --- a/src/copilot-chat/config/copilot-chat.yaml +++ b/src/copilot-chat/config/copilot-chat.yaml @@ -32,7 +32,7 @@ llm-endpoint: "https://endpoint.openai.com" llm-model: "gpt-4o" llm-version: "2025-01-01-preview" embedding-model: "text-embedding-ada-002" -valid-vcs: "admin,superuser" +valid-vcs: "superuser" environment: "prod" kusto-user-assigned-client-id: "" data-src-kusto-cluster-url: "https://placeholder.kusto.windows.net/" diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index 594f91d9..f32c94b4 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -37,8 +37,10 @@ def authenticate(self, username: str, token: str): tuple: (admin, virtualCluster) """ if AGENT_MODE_LOCAL: + if username == "admin": + return True, [] if username == "gooduser": - return True, ["admin"] + return False, ["superuser"] if username == "baduser": return False, ["temp"] # For any other username in local mode, return empty list From 9f608ce9c1c47ce934c85edda08e1fd4e620fcf9 Mon Sep 17 00:00:00 2001 From: Lei Date: Wed, 10 Sep 2025 18:58:45 +0800 Subject: [PATCH 9/9] minor --- src/copilot-chat/src/copilot_agent/utils/authentication.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copilot-chat/src/copilot_agent/utils/authentication.py b/src/copilot-chat/src/copilot_agent/utils/authentication.py index f32c94b4..784158f9 100644 --- a/src/copilot-chat/src/copilot_agent/utils/authentication.py +++ b/src/copilot-chat/src/copilot_agent/utils/authentication.py @@ -39,7 +39,7 @@ def authenticate(self, username: str, token: str): if AGENT_MODE_LOCAL: if username == "admin": return True, [] - if username == "gooduser": + if username == "gooduser" or username == "dev.ben": return False, ["superuser"] if username == "baduser": return False, ["temp"]