diff --git a/api/controllers/openapi/account.py b/api/controllers/openapi/account.py index 49a5d888e3..97b5fe6ae7 100644 --- a/api/controllers/openapi/account.py +++ b/api/controllers/openapi/account.py @@ -12,14 +12,15 @@ from datetime import UTC, datetime from flask import g from flask_restx import Resource -from sqlalchemy import update -from werkzeug.exceptions import BadRequest +from sqlalchemy import and_, select, update +from werkzeug.exceptions import BadRequest, NotFound from controllers.openapi import openapi_ns from extensions.ext_database import db from extensions.ext_redis import redis_client from libs.oauth_bearer import ( ACCEPT_USER_ANY, + AuthContext, SubjectType, TOKEN_CACHE_KEY_FMT, validate_bearer, @@ -74,42 +75,136 @@ class AccountSessionsSelfApi(Resource): @validate_bearer(accept=ACCEPT_USER_ANY) def delete(self): ctx = g.auth_ctx - - if not ctx.source.startswith("oauth"): - raise BadRequest( - "this endpoint revokes OAuth bearer tokens; " - "use /openapi/v1/personal-access-tokens/self for PATs" - ) - - # Snapshot pre-revoke hash for cache invalidation; UPDATE WHERE - # makes double-revoke idempotent. - row = ( - db.session.query(OAuthAccessToken.token_hash) - .filter( - OAuthAccessToken.id == str(ctx.token_id), - OAuthAccessToken.revoked_at.is_(None), - ) - .one_or_none() - ) - pre_revoke_hash = row[0] if row else None - - stmt = ( - update(OAuthAccessToken) - .where( - OAuthAccessToken.id == str(ctx.token_id), - OAuthAccessToken.revoked_at.is_(None), - ) - .values(revoked_at=datetime.now(UTC), token_hash=None) - ) - db.session.execute(stmt) - db.session.commit() - - if pre_revoke_hash: - redis_client.delete(TOKEN_CACHE_KEY_FMT.format(hash=pre_revoke_hash)) - + _require_oauth_subject(ctx) + _revoke_token_by_id(str(ctx.token_id)) return {"status": "revoked"}, 200 +@openapi_ns.route("/account/sessions") +class AccountSessionsApi(Resource): + @validate_bearer(accept=ACCEPT_USER_ANY) + def get(self): + ctx = g.auth_ctx + now = datetime.now(UTC) + rows = db.session.execute( + select( + OAuthAccessToken.id, + OAuthAccessToken.prefix, + OAuthAccessToken.client_id, + OAuthAccessToken.device_label, + OAuthAccessToken.created_at, + OAuthAccessToken.last_used_at, + OAuthAccessToken.expires_at, + ) + .where( + and_( + *_subject_match(ctx), + OAuthAccessToken.revoked_at.is_(None), + OAuthAccessToken.token_hash.is_not(None), + OAuthAccessToken.expires_at > now, + ) + ) + .order_by(OAuthAccessToken.created_at.desc()) + ).all() + + return { + "sessions": [ + { + "id": str(r.id), + "prefix": r.prefix, + "client_id": r.client_id, + "device_label": r.device_label, + "created_at": _iso(r.created_at), + "last_used_at": _iso(r.last_used_at), + "expires_at": _iso(r.expires_at), + } + for r in rows + ] + }, 200 + + +@openapi_ns.route("/account/sessions/") +class AccountSessionByIdApi(Resource): + @validate_bearer(accept=ACCEPT_USER_ANY) + def delete(self, session_id: str): + ctx = g.auth_ctx + _require_oauth_subject(ctx) + + # Subject-match guard. 404 (not 403) on cross-subject so the + # endpoint doesn't leak token IDs that belong to other subjects. + owns = db.session.execute( + select(OAuthAccessToken.id) + .where( + and_( + OAuthAccessToken.id == session_id, + *_subject_match(ctx), + ) + ) + ).first() + if owns is None: + raise NotFound("session not found") + + _revoke_token_by_id(session_id) + return {"status": "revoked"}, 200 + + +def _subject_match(ctx: AuthContext) -> tuple: + """Where-clauses that scope a query to the bearer's subject. Works + for both account (account_id) and external_sso (email + issuer). + """ + if ctx.subject_type == SubjectType.ACCOUNT: + return (OAuthAccessToken.account_id == str(ctx.account_id),) + return ( + OAuthAccessToken.subject_email == ctx.subject_email, + OAuthAccessToken.subject_issuer == ctx.subject_issuer, + OAuthAccessToken.account_id.is_(None), + ) + + +def _require_oauth_subject(ctx: AuthContext) -> None: + if not ctx.source.startswith("oauth"): + raise BadRequest( + "this endpoint revokes OAuth bearer tokens; " + "use /openapi/v1/personal-access-tokens/self for PATs" + ) + + +def _revoke_token_by_id(token_id: str) -> None: + # Snapshot pre-revoke hash for cache invalidation; UPDATE WHERE + # makes double-revoke idempotent. + row = ( + db.session.query(OAuthAccessToken.token_hash) + .filter( + OAuthAccessToken.id == token_id, + OAuthAccessToken.revoked_at.is_(None), + ) + .one_or_none() + ) + pre_revoke_hash = row[0] if row else None + + stmt = ( + update(OAuthAccessToken) + .where( + OAuthAccessToken.id == token_id, + OAuthAccessToken.revoked_at.is_(None), + ) + .values(revoked_at=datetime.now(UTC), token_hash=None) + ) + db.session.execute(stmt) + db.session.commit() + + if pre_revoke_hash: + redis_client.delete(TOKEN_CACHE_KEY_FMT.format(hash=pre_revoke_hash)) + + +def _iso(dt: datetime | None) -> str | None: + if dt is None: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=UTC) + return dt.isoformat().replace("+00:00", "Z") + + def _load_memberships(account_id): return ( db.session.query(TenantAccountJoin, Tenant) diff --git a/api/tests/unit_tests/controllers/openapi/test_account.py b/api/tests/unit_tests/controllers/openapi/test_account.py index 3fbde1598b..ada626f89b 100644 --- a/api/tests/unit_tests/controllers/openapi/test_account.py +++ b/api/tests/unit_tests/controllers/openapi/test_account.py @@ -9,7 +9,12 @@ from flask import Flask from flask.views import MethodView from controllers.openapi import bp as openapi_bp -from controllers.openapi.account import AccountApi, AccountSessionsSelfApi +from controllers.openapi.account import ( + AccountApi, + AccountSessionByIdApi, + AccountSessionsApi, + AccountSessionsSelfApi, +) from controllers.service_api import bp as service_api_bp if not hasattr(builtins, "MethodView"): @@ -71,3 +76,79 @@ def test_account_methods(dual_app: Flask): def test_sessions_self_methods(dual_app: Flask): rule = _rule(dual_app, "/openapi/v1/account/sessions/self") assert "DELETE" in rule.methods + + +def test_sessions_list_route_registered(dual_app: Flask): + """GET /openapi/v1/account/sessions is new — no /v1/ equivalent.""" + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/openapi/v1/account/sessions" in rules + + +def test_sessions_list_dispatches_to_sessions_api(dual_app: Flask): + rule = _rule(dual_app, "/openapi/v1/account/sessions") + assert dual_app.view_functions[rule.endpoint].view_class is AccountSessionsApi + assert "GET" in rule.methods + + +def test_session_by_id_route_registered(dual_app: Flask): + """DELETE /openapi/v1/account/sessions/ is new — no /v1/ equivalent.""" + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/openapi/v1/account/sessions/" in rules + + +def test_session_by_id_dispatches_to_correct_class(dual_app: Flask): + rule = _rule(dual_app, "/openapi/v1/account/sessions/") + assert dual_app.view_functions[rule.endpoint].view_class is AccountSessionByIdApi + assert "DELETE" in rule.methods + + +def test_subject_match_for_account_filters_by_account_id(): + """Account subject scopes queries via account_id.""" + import uuid as _uuid + + from controllers.openapi.account import _subject_match + from libs.oauth_bearer import AuthContext, SubjectType + + aid = _uuid.uuid4() + ctx = AuthContext( + subject_type=SubjectType.ACCOUNT, + subject_email="user@example.com", + subject_issuer="dify:account", + account_id=aid, + scopes=frozenset({"full"}), + token_id=_uuid.uuid4(), + source="oauth_account", + expires_at=None, + ) + clauses = _subject_match(ctx) + # One predicate, on account_id + assert len(clauses) == 1 + assert "account_id" in str(clauses[0]) + + +def test_subject_match_for_external_sso_filters_by_email_and_issuer(): + """External SSO subject scopes via (subject_email, subject_issuer) + AND account_id IS NULL — so a same-email account row from a + federated tenant cannot be revoked through an SSO bearer. + """ + import uuid as _uuid + + from controllers.openapi.account import _subject_match + from libs.oauth_bearer import AuthContext, SubjectType + + ctx = AuthContext( + subject_type=SubjectType.EXTERNAL_SSO, + subject_email="sso@partner.com", + subject_issuer="https://idp.partner.com", + account_id=None, + scopes=frozenset({"apps:run"}), + token_id=_uuid.uuid4(), + source="oauth_external_sso", + expires_at=None, + ) + clauses = _subject_match(ctx) + assert len(clauses) == 3 + rendered = " ".join(str(c) for c in clauses) + assert "subject_email" in rendered + assert "subject_issuer" in rendered + assert "account_id IS NULL" in rendered