fix(auth): address PR #758 round-7 important review
- Coerce refresh_expires_in to int before arithmetic in both callback paths so IdPs that serialize the field as a JSON string (e.g. AWS Cognito) don't trigger an unhandled TypeError 500. - Drop the orphaned oauth_session row written by _check_logged_in. The canonical Flow 2 row is created by generate_oauth_url_for_flow2 keyed by `state`, which is what the unified callback looks up; the flow2_<hex> session_id was never matched and just churned the table for 10 minutes per call. - Match delete_cookie attributes (httponly, secure, samesite) to the set_cookie call on logout so browsers reliably evict the cookie even on implementations that consider security flags during deletion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
ec9b9b2a75
commit
27fcf05d3a
@@ -432,21 +432,12 @@ async def _check_logged_in(ctx: Context, user_id: str) -> str:
|
||||
# Store state in session for validation on callback
|
||||
storage = await get_shared_storage()
|
||||
|
||||
# Create OAuth session for Flow 2. Identifier is purely random
|
||||
# so audit-log entries / DB rows don't carry the user_id in the
|
||||
# session_id field (PR #758 round-4 review medium 3).
|
||||
session_id = f"flow2_{secrets.token_hex(16)}"
|
||||
# The canonical Flow 2 oauth_session row is written inside
|
||||
# generate_oauth_url_for_flow2 (keyed by `state`, with the PKCE
|
||||
# verifier and nonce); the unified callback looks it up by `state`.
|
||||
# No additional row is needed here.
|
||||
redirect_uri = f"{os.getenv('NEXTCLOUD_MCP_SERVER_URL', 'http://localhost:8000')}/oauth/callback"
|
||||
|
||||
await storage.store_oauth_session(
|
||||
session_id=session_id,
|
||||
client_redirect_uri="", # No client redirect for Flow 2
|
||||
state=state,
|
||||
flow_type="flow2",
|
||||
is_provisioning=True,
|
||||
ttl_seconds=600, # 10 minute TTL
|
||||
)
|
||||
|
||||
# Define scopes for Nextcloud access
|
||||
# Note: offline_access is only included when enabled in settings.
|
||||
# The actual scope sent to the IdP is determined by
|
||||
|
||||
Reference in New Issue
Block a user