Skip to content

Commit 054cf5d

Browse files
authored
improve login loggin (#884)
Added additional login tracking for a more comprehensive picture
1 parent ac190fc commit 054cf5d

6 files changed

Lines changed: 441 additions & 11 deletions

File tree

application/single_app/app.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,23 @@ def _is_idle_timeout_exempt(path):
603603
return True
604604
return any(path.startswith(prefix) for prefix in IDLE_TIMEOUT_EXEMPT_PREFIXES)
605605

606+
607+
def maybe_log_authenticated_browser_request():
608+
"""Record throttled login activity for authenticated browser page requests."""
609+
if request.method != 'GET' or request.path.startswith('/api/'):
610+
return
611+
612+
user_id = session.get('user', {}).get('oid') or session.get('user', {}).get('sub')
613+
if not user_id:
614+
return
615+
616+
maybe_log_authenticated_request_login(
617+
user_id=user_id,
618+
session_state=session,
619+
request_path=request.path,
620+
request_method=request.method
621+
)
622+
606623
@app.before_request
607624
def enforce_idle_session_timeout():
608625
"""
@@ -646,6 +663,7 @@ def enforce_idle_session_timeout():
646663
if should_refresh_last_activity:
647664
session['last_activity_epoch'] = now_epoch
648665
session.modified = True
666+
maybe_log_authenticated_browser_request()
649667
return None
650668

651669
idle_timeout_minutes, _ = get_idle_timeout_settings(request_settings)
@@ -698,6 +716,7 @@ def enforce_idle_session_timeout():
698716

699717
session['last_activity_epoch'] = now_epoch
700718
session.modified = True
719+
maybe_log_authenticated_browser_request()
701720
return None
702721

703722
@app.after_request

application/single_app/config.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@
9494
EXECUTOR_TYPE = 'thread'
9595
EXECUTOR_MAX_WORKERS = 30
9696
SESSION_TYPE = 'filesystem'
97-
VERSION = "0.241.022"
97+
VERSION = "0.241.130"
9898

9999
SECRET_KEY = os.getenv('SECRET_KEY', 'dev-secret-key-change-in-production')
100100

@@ -106,9 +106,9 @@
106106
'Referrer-Policy': 'strict-origin-when-cross-origin',
107107
'Content-Security-Policy': (
108108
"default-src 'self'; "
109-
"script-src 'self' 'unsafe-inline' 'unsafe-eval'; "
109+
"script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net; "
110110
#"script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net https://code.jquery.com https://stackpath.bootstrapcdn.com; "
111-
"style-src 'self' 'unsafe-inline'; "
111+
"style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; "
112112
#"style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://stackpath.bootstrapcdn.com; "
113113
"img-src 'self' data: https: blob:; "
114114
"font-src 'self'; "
@@ -309,6 +309,18 @@ def get_redis_cache_infrastructure_endpoint(redis_hostname: str) -> str:
309309
partition_key=PartitionKey(path="/conversation_id")
310310
)
311311

312+
cosmos_personal_workflows_container_name = "personal_workflows"
313+
cosmos_personal_workflows_container = cosmos_database.create_container_if_not_exists(
314+
id=cosmos_personal_workflows_container_name,
315+
partition_key=PartitionKey(path="/user_id")
316+
)
317+
318+
cosmos_personal_workflow_runs_container_name = "personal_workflow_runs"
319+
cosmos_personal_workflow_runs_container = cosmos_database.create_container_if_not_exists(
320+
id=cosmos_personal_workflow_runs_container_name,
321+
partition_key=PartitionKey(path="/user_id")
322+
)
323+
312324
cosmos_group_conversations_container_name = "group_conversations"
313325
cosmos_group_conversations_container = cosmos_database.create_container_if_not_exists(
314326
id=cosmos_group_conversations_container_name,
@@ -321,6 +333,24 @@ def get_redis_cache_infrastructure_endpoint(redis_hostname: str) -> str:
321333
partition_key=PartitionKey(path="/conversation_id")
322334
)
323335

336+
cosmos_collaboration_conversations_container_name = "collaboration_conversations"
337+
cosmos_collaboration_conversations_container = cosmos_database.create_container_if_not_exists(
338+
id=cosmos_collaboration_conversations_container_name,
339+
partition_key=PartitionKey(path="/id")
340+
)
341+
342+
cosmos_collaboration_messages_container_name = "collaboration_messages"
343+
cosmos_collaboration_messages_container = cosmos_database.create_container_if_not_exists(
344+
id=cosmos_collaboration_messages_container_name,
345+
partition_key=PartitionKey(path="/conversation_id")
346+
)
347+
348+
cosmos_collaboration_user_state_container_name = "collaboration_user_state"
349+
cosmos_collaboration_user_state_container = cosmos_database.create_container_if_not_exists(
350+
id=cosmos_collaboration_user_state_container_name,
351+
partition_key=PartitionKey(path="/user_id")
352+
)
353+
324354
cosmos_settings_container_name = "settings"
325355
cosmos_settings_container = cosmos_database.create_container_if_not_exists(
326356
id=cosmos_settings_container_name,

application/single_app/functions_activity_logging.py

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"""
66

77
import logging
8+
import time
89
import uuid
910
from datetime import datetime
1011
from typing import Optional, Dict, Any
@@ -13,6 +14,83 @@
1314
from config import cosmos_activity_logs_container
1415

1516

17+
USER_LOGIN_ACTIVITY_SESSION_KEY = 'last_user_login_activity_epoch'
18+
USER_LOGIN_ACTIVITY_MIN_INTERVAL_SECONDS = 15 * 60
19+
20+
21+
def _parse_session_epoch(session_state: Optional[dict], session_key: str) -> Optional[int]:
22+
"""Safely parse an epoch value stored in session state."""
23+
if session_state is None:
24+
return None
25+
26+
raw_epoch = session_state.get(session_key)
27+
if raw_epoch is None:
28+
return None
29+
30+
try:
31+
return int(float(raw_epoch))
32+
except (TypeError, ValueError):
33+
return None
34+
35+
36+
def record_user_login_session_activity(
37+
session_state: Optional[dict],
38+
now_epoch: Optional[int] = None
39+
) -> Optional[int]:
40+
"""Persist the last time login activity was recorded for the current session."""
41+
if session_state is None:
42+
return None
43+
44+
resolved_epoch = int(now_epoch if now_epoch is not None else time.time())
45+
session_state[USER_LOGIN_ACTIVITY_SESSION_KEY] = resolved_epoch
46+
47+
if hasattr(session_state, 'modified'):
48+
session_state.modified = True
49+
50+
return resolved_epoch
51+
52+
53+
def maybe_log_authenticated_request_login(
54+
user_id: str,
55+
session_state: Optional[dict],
56+
request_path: str,
57+
request_method: str = 'GET',
58+
now_epoch: Optional[int] = None,
59+
login_method: str = 'authenticated_request',
60+
min_interval_seconds: int = USER_LOGIN_ACTIVITY_MIN_INTERVAL_SECONDS
61+
) -> bool:
62+
"""
63+
Log a throttled login-style activity for authenticated browser requests.
64+
65+
This captures passive SSO/session-based access that never re-enters the
66+
explicit OAuth callback, while preventing per-request log spam.
67+
"""
68+
if not user_id or session_state is None:
69+
return False
70+
71+
normalized_method = (request_method or '').upper()
72+
if normalized_method != 'GET':
73+
return False
74+
75+
resolved_epoch = int(now_epoch if now_epoch is not None else time.time())
76+
last_logged_epoch = _parse_session_epoch(session_state, USER_LOGIN_ACTIVITY_SESSION_KEY)
77+
if last_logged_epoch is not None and (resolved_epoch - last_logged_epoch) < min_interval_seconds:
78+
return False
79+
80+
log_user_login(
81+
user_id,
82+
login_method,
83+
activity_details={
84+
'auth_signal': 'authenticated_request',
85+
'request_path': request_path,
86+
'request_method': normalized_method,
87+
'is_interactive_login': False
88+
}
89+
)
90+
record_user_login_session_activity(session_state, resolved_epoch)
91+
return True
92+
93+
1694
def _get_email_domain(email: str) -> str:
1795
"""Return only the email domain for low-sensitivity audit metadata."""
1896
normalized_email = (email or '').strip()
@@ -1113,7 +1191,8 @@ def log_conversation_archival(
11131191

11141192
def log_user_login(
11151193
user_id: str,
1116-
login_method: str = 'azure_ad'
1194+
login_method: str = 'azure_ad',
1195+
activity_details: Optional[Dict[str, Any]] = None
11171196
) -> None:
11181197
"""
11191198
Log user login activity to the activity_logs container.
@@ -1125,19 +1204,29 @@ def log_user_login(
11251204

11261205
try:
11271206
# Create login activity record
1128-
import uuid
1207+
login_details = {
1208+
'login_method': login_method,
1209+
'success': True
1210+
}
1211+
if activity_details:
1212+
login_details.update({
1213+
key: value for key, value in activity_details.items()
1214+
if value is not None
1215+
})
1216+
11291217
login_activity = {
11301218
'id': str(uuid.uuid4()),
11311219
'user_id': user_id,
11321220
'activity_type': 'user_login',
11331221
'login_method': login_method,
11341222
'timestamp': datetime.utcnow().isoformat(),
11351223
'created_at': datetime.utcnow().isoformat(),
1136-
'details': {
1137-
'login_method': login_method,
1138-
'success': True
1139-
}
1224+
'details': login_details
11401225
}
1226+
1227+
for key, value in login_details.items():
1228+
if key not in {'login_method', 'success'}:
1229+
login_activity[key] = value
11411230

11421231
# Save to activity_logs container
11431232
cosmos_activity_logs_container.create_item(body=login_activity)
@@ -1148,7 +1237,7 @@ def log_user_login(
11481237
extra=login_activity,
11491238
level=logging.INFO
11501239
)
1151-
debug_print(f"✅ User login activity logged for user {user_id}")
1240+
debug_print(f"✅ User login activity logged for user {user_id} via {login_method}")
11521241

11531242
except Exception as e:
11541243
# Log error but don't break the login flow

application/single_app/route_frontend_authentication.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from unittest import result
44
from config import *
5+
from functions_activity_logging import log_user_login, record_user_login_session_activity
56
from functions_authentication import _build_msal_app, _load_cache, _save_cache, clear_requested_oauth_scopes, get_requested_oauth_scopes
67
from functions_debug import debug_print
78
from swagger_wrapper import swagger_route, get_auth_security
@@ -133,10 +134,10 @@ def authorized():
133134

134135
# Log the login activity
135136
try:
136-
from functions_activity_logging import log_user_login
137137
user_id = session['user'].get('oid') or session['user'].get('sub')
138138
if user_id:
139139
log_user_login(user_id, 'azure_ad')
140+
record_user_login_session_activity(session)
140141
except Exception as e:
141142
debug_print(f"Could not log login activity: {e}")
142143

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Authenticated Request Login Activity Fix
2+
3+
Fixed/Implemented in version: **0.241.130**
4+
5+
## Overview
6+
7+
This fix closes the gap between explicit OAuth callback logins and real authenticated usage. Previously, activity tracking only recorded `user_login` when the `/getAToken` callback completed. Users who arrived with an already-authenticated session or seamless SSO reuse could browse the app without creating the login-style activity that the Control Center and profile dashboards rely on.
8+
9+
Version implemented:
10+
`config.py` now reports `VERSION = "0.241.130"` for this fix.
11+
12+
## Issue Description
13+
14+
- Login analytics depended on an explicit callback event rather than the broader fact that the user had an authenticated browser session.
15+
- Passive SSO or session reuse could authenticate the user successfully but leave login metrics empty or understated.
16+
- Simply logging every authenticated request would have created noisy over-counting, especially for API-heavy pages.
17+
18+
## Root Cause
19+
20+
- The application treated `user_login` as a one-time OAuth callback event instead of a reusable authenticated-session signal.
21+
- The authenticated request pipeline had no throttled activity writer for browser page access.
22+
- The explicit callback flow had no session marker to prevent a duplicate login record on the immediate redirect to the landing page.
23+
24+
## Technical Changes
25+
26+
### Throttled Authenticated Request Tracking
27+
28+
Changes implemented:
29+
30+
- Added a shared authenticated-request helper in `functions_activity_logging.py` that records a `user_login` activity with `login_method = authenticated_request`.
31+
- Added a session-scoped throttle window so repeated authenticated page loads do not emit a record on every request.
32+
- Captured request metadata such as `request_path`, `request_method`, and `auth_signal` for later diagnostics.
33+
34+
Files involved:
35+
36+
- `application/single_app/functions_activity_logging.py`
37+
- `application/single_app/app.py`
38+
39+
Behavioral outcome:
40+
41+
Authenticated page visits now show up in the existing login analytics even when the user did not intentionally click a login button during that session.
42+
43+
### OAuth Callback Deduplication
44+
45+
Changes implemented:
46+
47+
- Marked the session immediately after the explicit `/getAToken` callback logs `user_login`.
48+
- Reused that session marker to suppress an immediate second `user_login` on the redirect to `/`.
49+
50+
Files involved:
51+
52+
- `application/single_app/route_frontend_authentication.py`
53+
54+
Behavioral outcome:
55+
56+
Explicit callback logins remain single-counted while passive authenticated navigation still becomes visible later in the session.
57+
58+
## Files Modified
59+
60+
- `application/single_app/functions_activity_logging.py`
61+
- `application/single_app/app.py`
62+
- `application/single_app/route_frontend_authentication.py`
63+
- `application/single_app/config.py`
64+
- `functional_tests/test_authenticated_request_login_activity.py`
65+
66+
## Validation
67+
68+
Testing approach:
69+
70+
- Added a focused functional regression test that loads `functions_activity_logging.py` with stubbed dependencies so the new throttle and dedup behavior can be exercised without a live Cosmos dependency.
71+
- Ran targeted compile checks on the edited Python files.
72+
73+
Validation performed for this implementation:
74+
75+
- `python -m py_compile application/single_app/functions_activity_logging.py`
76+
- `python -m py_compile application/single_app/app.py`
77+
- `python -m py_compile application/single_app/route_frontend_authentication.py`
78+
- `python -m py_compile functional_tests/test_authenticated_request_login_activity.py`
79+
- `python functional_tests/test_authenticated_request_login_activity.py`
80+
81+
## Before And After
82+
83+
Before:
84+
85+
- Login metrics depended almost entirely on explicit OAuth callback completions.
86+
- Passive SSO/session-reuse visits could authenticate successfully without contributing to login analytics.
87+
- A naive request-level fix risked over-counting every authenticated browser/API request.
88+
89+
After:
90+
91+
- Authenticated browser page requests can emit a throttled `user_login` record when no recent login activity has been recorded in the current session.
92+
- Explicit OAuth callback logins still record normally and are not immediately double-counted on redirect.
93+
- Existing dashboards that already query `activity_type = 'user_login'` now see a more representative picture of real authenticated use.
94+
95+
## User Experience Impact
96+
97+
- Admins get more representative login activity in the Control Center and profile trends for users who rely on seamless SSO.
98+
- Users do not see any UI change.
99+
- Login metrics should remain substantially less noisy than per-request tracking because the authenticated-request signal is throttled and limited to browser GET requests.

0 commit comments

Comments
 (0)