Skip to content

Commit 3510bbe

Browse files
author
rodageve
committed
Bug fixes, support Microsoft tenant issuer validation as optional feature, fail oauth connection on jwt validation
1 parent b721615 commit 3510bbe

8 files changed

Lines changed: 192 additions & 119 deletions

File tree

‎src/config/settings.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from config.paths import get_flows_path
1515
from utils.container_utils import determine_docling_host, get_container_host
1616
from utils.embedding_fields import build_knn_vector_field
17-
from utils.env_utils import get_env_float, get_env_int
17+
from utils.env_utils import get_env_float, get_env_int, get_env_set
1818
from utils.logging_config import get_logger
1919

2020
# Import configuration manager
@@ -64,6 +64,10 @@
6464
MICROSOFT_GRAPH_OAUTH_CLIENT_ID = os.getenv("MICROSOFT_GRAPH_OAUTH_CLIENT_ID")
6565
MICROSOFT_GRAPH_OAUTH_CLIENT_SECRET = os.getenv("MICROSOFT_GRAPH_OAUTH_CLIENT_SECRET")
6666
GOOGLE_OAUTH_CLIENT_SECRET = os.getenv("GOOGLE_OAUTH_CLIENT_SECRET")
67+
# Optional comma-separated list of Azure AD tenant UUIDs. When set, tokens from
68+
# tenants not in this list are rejected (business policy, not a standards requirement).
69+
# When unset, any tenant whose token passes signature/audience/expiry checks is accepted.
70+
MICROSOFT_ALLOWED_TENANT_IDS: set[str] | None = get_env_set("MICROSOFT_ALLOWED_TENANT_IDS")
6771

6872
# IBM AMS authentication (Watsonx Data embedded mode)
6973
IBM_AUTH_ENABLED = os.getenv("IBM_AUTH_ENABLED", "false").lower() in ("true", "1", "yes")

‎src/connectors/google_drive/oauth.py‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,30 @@
1313
logger = get_logger(__name__)
1414

1515

16-
def _verify_id_token_if_present(id_token: str | None) -> dict | None:
17-
"""Verify Google ID token and return claims if valid, None otherwise."""
16+
def _verify_id_token(id_token: str | None) -> dict | None:
17+
"""
18+
Verify Google ID token signature, expiry, audience, and issuer.
19+
20+
Returns the verified claims dict on success.
21+
Raises JWTVerificationError (or a subclass) if verification fails — callers
22+
must NOT proceed with credentials that carry an invalid ID token.
23+
Returns None only when no id_token is present or verification is not configured.
24+
"""
1825
if not id_token:
1926
return None
20-
try:
21-
from config.settings import GOOGLE_OAUTH_CLIENT_ID
22-
from utils.jwt_verification import JWTVerificationError, verify_google_id_token
2327

24-
if not GOOGLE_OAUTH_CLIENT_ID:
25-
logger.warning("GOOGLE_OAUTH_CLIENT_ID not configured - skipping ID token verification")
26-
return None
27-
28-
# Verify token with FULL validation
29-
claims = verify_google_id_token(id_token, GOOGLE_OAUTH_CLIENT_ID)
30-
logger.debug("Google ID token verification successful")
31-
return claims
28+
from config.settings import GOOGLE_OAUTH_CLIENT_ID
29+
from utils.jwt_verification import verify_google_id_token
3230

33-
except JWTVerificationError as e:
34-
logger.warning("Google ID token verification failed", error=str(e))
35-
except Exception as e:
36-
logger.error("Unexpected error verifying Google ID token", error=str(e))
31+
if not GOOGLE_OAUTH_CLIENT_ID:
32+
logger.warning("GOOGLE_OAUTH_CLIENT_ID not configured - skipping ID token verification")
33+
return None
3734

38-
return None
35+
# Raises JWTVerificationError on any failure — intentionally not caught here
36+
# so that callers propagate the error and refuse to use invalid credentials.
37+
claims = verify_google_id_token(id_token, GOOGLE_OAUTH_CLIENT_ID)
38+
logger.debug("Google ID token verified, email=%s", claims.get("email"))
39+
return claims
3940

4041

4142
_REFRESH_TIMEOUT_SECONDS = 30
@@ -137,13 +138,9 @@ async def load_credentials(self) -> Credentials | None:
137138
self.creds.expiry = expiry_dt.replace(tzinfo=None)
138139
logger.debug("[GoogleDrive] load_credentials: token expiry=%s", self.creds.expiry)
139140

140-
# Verify ID token if present (security enhancement)
141+
# Verify ID token if present — raises JWTVerificationError on failure
141142
if self.creds and self.creds.id_token:
142-
claims = _verify_id_token_if_present(self.creds.id_token)
143-
if claims:
144-
logger.debug("[GoogleDrive] ID token verified, email=%s", claims.get("email"))
145-
else:
146-
logger.warning("[GoogleDrive] ID token verification failed or skipped")
143+
_verify_id_token(self.creds.id_token)
147144

148145
if needs_upgrade and self.creds:
149146
await self.save_credentials()
@@ -239,6 +236,11 @@ async def handle_authorization_callback(self, authorization_code: str, state: st
239236
self.creds = self._flow.credentials
240237
logger.debug("[GoogleDrive] handle_authorization_callback: token exchange complete")
241238

239+
# Verify the ID token immediately after code exchange. Raises JWTVerificationError
240+
# on failure — the connection is rejected before credentials are persisted.
241+
if self.creds and self.creds.id_token:
242+
_verify_id_token(self.creds.id_token)
243+
242244
await self.save_credentials()
243245
return True
244246

‎src/connectors/onedrive/oauth.py‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,37 +8,42 @@
88
logger = logging.getLogger(__name__)
99

1010

11-
def _verify_access_token_if_present(
11+
def _verify_access_token(
1212
access_token: str | None, tenant_id: str | None = None
1313
) -> dict | None:
14-
"""Verify Microsoft access token and return claims if valid, None otherwise."""
14+
"""
15+
Verify Microsoft access token signature, expiry, audience, and issuer domain.
16+
17+
Returns the verified claims dict on success.
18+
Raises JWTVerificationError (or a subclass) if verification fails — callers
19+
must NOT proceed with a token that fails this check.
20+
Returns None only when no token is present or verification is not configured.
21+
"""
1522
if not access_token:
1623
return None
1724

1825
raw_token = access_token.removeprefix("Bearer ").strip()
19-
try:
20-
from config.settings import MICROSOFT_GRAPH_OAUTH_CLIENT_ID
21-
from utils.jwt_verification import JWTVerificationError, verify_microsoft_access_token
2226

23-
if not MICROSOFT_GRAPH_OAUTH_CLIENT_ID:
24-
logger.warning(
25-
"MICROSOFT_GRAPH_OAUTH_CLIENT_ID not configured - skipping access token verification"
26-
)
27-
return None
27+
from config.settings import MICROSOFT_ALLOWED_TENANT_IDS, MICROSOFT_GRAPH_OAUTH_CLIENT_ID
28+
from utils.jwt_verification import verify_microsoft_access_token
2829

29-
# Verify token with FULL validation
30-
claims = verify_microsoft_access_token(
31-
raw_token, MICROSOFT_GRAPH_OAUTH_CLIENT_ID, tenant_id=tenant_id
30+
if not MICROSOFT_GRAPH_OAUTH_CLIENT_ID:
31+
logger.warning(
32+
"MICROSOFT_GRAPH_OAUTH_CLIENT_ID not configured - skipping access token verification"
3233
)
33-
logger.debug("Microsoft access token verification successful, tenant=%s", claims.get("tid"))
34-
return claims
35-
36-
except JWTVerificationError as e:
37-
logger.warning("Microsoft access token verification failed: %s", str(e))
38-
except Exception as e:
39-
logger.error("Unexpected error verifying Microsoft access token: %s", str(e))
34+
return None
4035

41-
return None
36+
# Raises JWTVerificationError on any failure — intentionally not caught here
37+
# so that callers (get_access_token) propagate the error and refuse to return
38+
# an unverified token.
39+
claims = verify_microsoft_access_token(
40+
raw_token,
41+
MICROSOFT_GRAPH_OAUTH_CLIENT_ID,
42+
tenant_id=tenant_id,
43+
allowed_tenant_ids=MICROSOFT_ALLOWED_TENANT_IDS,
44+
)
45+
logger.debug("OneDrive access token verified, tenant=%s", claims.get("tid"))
46+
return claims
4247

4348

4449
class OneDriveOAuth:
@@ -376,10 +381,7 @@ def get_access_token(self) -> str:
376381
)
377382
if result and "access_token" in result:
378383
access_token = result["access_token"]
379-
# Verify token (security enhancement)
380-
claims = _verify_access_token_if_present(access_token)
381-
if claims:
382-
logger.debug("OneDrive access token verified, tenant=%s", claims.get("tid"))
384+
_verify_access_token(access_token) # raises JWTVerificationError on failure
383385
logger.info("OneDrive get_access_token: Success with current account")
384386
return access_token
385387
else:
@@ -392,10 +394,7 @@ def get_access_token(self) -> str:
392394
result = self.app.acquire_token_silent(self.RESOURCE_SCOPES, account=None)
393395
if result and "access_token" in result:
394396
access_token = result["access_token"]
395-
# Verify token (security enhancement)
396-
claims = _verify_access_token_if_present(access_token)
397-
if claims:
398-
logger.debug("OneDrive access token verified, tenant=%s", claims.get("tid"))
397+
_verify_access_token(access_token) # raises JWTVerificationError on failure
399398
logger.info("OneDrive get_access_token: Fallback success")
400399
return access_token
401400

‎src/connectors/sharepoint/oauth.py‎

Lines changed: 30 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -8,37 +8,42 @@
88
logger = logging.getLogger(__name__)
99

1010

11-
def _verify_access_token_if_present(
11+
def _verify_access_token(
1212
access_token: str | None, tenant_id: str | None = None
1313
) -> dict | None:
14-
"""Verify Microsoft access token and return claims if valid, None otherwise."""
14+
"""
15+
Verify Microsoft access token signature, expiry, audience, and issuer domain.
16+
17+
Returns the verified claims dict on success.
18+
Raises JWTVerificationError (or a subclass) if verification fails — callers
19+
must NOT proceed with a token that fails this check.
20+
Returns None only when no token is present or verification is not configured.
21+
"""
1522
if not access_token:
1623
return None
1724

1825
raw_token = access_token.removeprefix("Bearer ").strip()
19-
try:
20-
from config.settings import MICROSOFT_GRAPH_OAUTH_CLIENT_ID
21-
from utils.jwt_verification import JWTVerificationError, verify_microsoft_access_token
2226

23-
if not MICROSOFT_GRAPH_OAUTH_CLIENT_ID:
24-
logger.warning(
25-
"MICROSOFT_GRAPH_OAUTH_CLIENT_ID not configured - skipping access token verification"
26-
)
27-
return None
27+
from config.settings import MICROSOFT_ALLOWED_TENANT_IDS, MICROSOFT_GRAPH_OAUTH_CLIENT_ID
28+
from utils.jwt_verification import JWTVerificationError, verify_microsoft_access_token
2829

29-
# Verify token with FULL validation
30-
claims = verify_microsoft_access_token(
31-
raw_token, MICROSOFT_GRAPH_OAUTH_CLIENT_ID, tenant_id=tenant_id
30+
if not MICROSOFT_GRAPH_OAUTH_CLIENT_ID:
31+
logger.warning(
32+
"MICROSOFT_GRAPH_OAUTH_CLIENT_ID not configured - skipping access token verification"
3233
)
33-
logger.debug("Microsoft access token verification successful, tenant=%s", claims.get("tid"))
34-
return claims
35-
36-
except JWTVerificationError as e:
37-
logger.warning("Microsoft access token verification failed: %s", str(e))
38-
except Exception as e:
39-
logger.error("Unexpected error verifying Microsoft access token: %s", str(e))
34+
return None
4035

41-
return None
36+
# Raises JWTVerificationError on any failure — intentionally not caught here
37+
# so that callers (get_access_token, get_access_token_for_resource) propagate
38+
# the error and refuse to return an unverified token.
39+
claims = verify_microsoft_access_token(
40+
raw_token,
41+
MICROSOFT_GRAPH_OAUTH_CLIENT_ID,
42+
tenant_id=tenant_id,
43+
allowed_tenant_ids=MICROSOFT_ALLOWED_TENANT_IDS,
44+
)
45+
logger.debug("SharePoint access token verified, tenant=%s", claims.get("tid"))
46+
return claims
4247

4348

4449
class SharePointOAuth:
@@ -396,12 +401,7 @@ def get_access_token(self) -> str:
396401
)
397402
if result and "access_token" in result:
398403
access_token = result["access_token"]
399-
# Verify token (security enhancement)
400-
claims = _verify_access_token_if_present(access_token)
401-
if claims:
402-
logger.debug(
403-
"SharePoint access token verified, tenant=%s", claims.get("tid")
404-
)
404+
_verify_access_token(access_token) # raises JWTVerificationError on failure
405405
logger.info("SharePoint get_access_token: Success with current account")
406406
return access_token
407407
else:
@@ -414,10 +414,7 @@ def get_access_token(self) -> str:
414414
result = self.app.acquire_token_silent(self.RESOURCE_SCOPES, account=None)
415415
if result and "access_token" in result:
416416
access_token = result["access_token"]
417-
# Verify token (security enhancement)
418-
claims = _verify_access_token_if_present(access_token)
419-
if claims:
420-
logger.debug("SharePoint access token verified, tenant=%s", claims.get("tid"))
417+
_verify_access_token(access_token) # raises JWTVerificationError on failure
421418
logger.info("SharePoint get_access_token: Fallback success")
422419
return access_token
423420

@@ -465,12 +462,7 @@ def get_access_token_for_resource(self, resource_url: str) -> str:
465462
)
466463
if result and "access_token" in result:
467464
access_token = result["access_token"]
468-
# Verify token (security enhancement) - note: resource tokens may have different audience
469-
claims = _verify_access_token_if_present(access_token)
470-
if claims:
471-
logger.debug(
472-
"SharePoint resource token verified, tenant=%s", claims.get("tid")
473-
)
465+
_verify_access_token(access_token) # raises JWTVerificationError on failure
474466
logger.info(
475467
"SharePoint get_access_token_for_resource: Success with current account"
476468
)
@@ -492,10 +484,7 @@ def get_access_token_for_resource(self, resource_url: str) -> str:
492484
result = self.app.acquire_token_silent(sharepoint_scopes, account=None)
493485
if result and "access_token" in result:
494486
access_token = result["access_token"]
495-
# Verify token (security enhancement)
496-
claims = _verify_access_token_if_present(access_token)
497-
if claims:
498-
logger.debug("SharePoint resource token verified, tenant=%s", claims.get("tid"))
487+
_verify_access_token(access_token) # raises JWTVerificationError on failure
499488
logger.info("SharePoint get_access_token_for_resource: Fallback success")
500489
return access_token
501490

‎src/tui/config_fields.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,16 @@ class ConfigSection:
189189
"MICROSOFT_GRAPH_OAUTH_CLIENT_SECRET", "Client Secret",
190190
placeholder="", secret=True,
191191
),
192+
ConfigField(
193+
"microsoft_allowed_tenant_ids", "MICROSOFT_ALLOWED_TENANT_IDS",
194+
"Allowed Tenant IDs (optional)",
195+
placeholder="uuid1,uuid2",
196+
helper_text=(
197+
"Comma-separated Azure AD tenant UUIDs. When set, tokens from unlisted tenants "
198+
"are rejected after signature verification (business policy — not required by "
199+
"OAuth standards). Leave empty to allow any tenant."
200+
),
201+
),
192202
], advanced=True, gate_prompt="Configure Microsoft Graph OAuth?"),
193203

194204
# ── AWS ─────────────────────────────────────────────────────

‎src/tui/managers/env_manager.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class EnvConfig:
5656
google_oauth_client_secret: str = ""
5757
microsoft_graph_oauth_client_id: str = ""
5858
microsoft_graph_oauth_client_secret: str = ""
59+
microsoft_allowed_tenant_ids: str = ""
5960

6061
# Optional settings
6162
webhook_base_url: str = ""
@@ -204,6 +205,7 @@ def _env_attr_map(self) -> Dict[str, str]:
204205
"GOOGLE_OAUTH_CLIENT_SECRET": "google_oauth_client_secret", # pragma: allowlist secret
205206
"MICROSOFT_GRAPH_OAUTH_CLIENT_ID": "microsoft_graph_oauth_client_id",
206207
"MICROSOFT_GRAPH_OAUTH_CLIENT_SECRET": "microsoft_graph_oauth_client_secret", # pragma: allowlist secret
208+
"MICROSOFT_ALLOWED_TENANT_IDS": "microsoft_allowed_tenant_ids",
207209
"WEBHOOK_BASE_URL": "webhook_base_url",
208210
"AWS_ACCESS_KEY_ID": "aws_access_key_id",
209211
"AWS_SECRET_ACCESS_KEY": "aws_secret_access_key", # pragma: allowlist secret
@@ -584,6 +586,10 @@ def save_env_file(self) -> bool:
584586
f.write(
585587
f"MICROSOFT_GRAPH_OAUTH_CLIENT_SECRET={self._quote_env_value(self.config.microsoft_graph_oauth_client_secret)}\n"
586588
)
589+
if self.config.microsoft_allowed_tenant_ids:
590+
f.write(
591+
f"MICROSOFT_ALLOWED_TENANT_IDS={self._quote_env_value(self.config.microsoft_allowed_tenant_ids)}\n"
592+
)
587593
f.write("\n")
588594

589595
# Optional settings
@@ -695,6 +701,12 @@ def get_full_setup_fields(self) -> List[tuple[str, str, str, bool]]:
695701
"",
696702
False,
697703
),
704+
(
705+
"microsoft_allowed_tenant_ids",
706+
"Microsoft Allowed Tenant IDs",
707+
"",
708+
False,
709+
),
698710
]
699711

700712
flow_fields = [

‎src/utils/env_utils.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,14 @@ def get_env_int(key: str, default: int) -> int:
2929
def get_env_float(key: str, default: float) -> float:
3030
"""Get an environment variable as a float."""
3131
return safe_float(os.getenv(key), default)
32+
33+
def get_env_set(key: str) -> set[str] | None:
34+
"""Return a set of non-empty strings from a comma-separated env var.
35+
36+
Returns None (not an empty set) when the variable is absent or blank,
37+
so callers can distinguish 'unset = skip check' from 'set-but-empty = block all'.
38+
"""
39+
raw = os.getenv(key, "").strip()
40+
if not raw:
41+
return None
42+
return {v.strip() for v in raw.split(",") if v.strip()}

0 commit comments

Comments
 (0)