From 772f450b29f39e98d66a8f8ff108c236231b5b1a Mon Sep 17 00:00:00 2001 From: GareArc Date: Sun, 26 Apr 2026 23:57:28 -0700 Subject: [PATCH] feat(api): lift device-flow approve/deny to /openapi/v1 (Phase D.13-14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DeviceApproveApi + DeviceDenyApi (cookie-authed) move to controllers/openapi/oauth_device/{approve,deny}.py. Decorator stack preserved verbatim: setup_required → login_required → account_initialization_required → bearer_feature_required → rate_limit. Audit event names ('oauth.device_flow_approved' / 'oauth.device_flow_denied') unchanged so the OTel exporter registration keeps routing them. The legacy /console/api/oauth/device/{approve,deny} mounts are re-registered on console_ns from the bottom of the new files via a local-import _register_legacy_console_mount() helper. The local import breaks an import cycle that would otherwise form: openapi imports console.wraps for setup_required, console.__init__.py imports console.auth.oauth_device, which would re-import the openapi class mid-load. Deferring console_ns past the class definition resolves it. console/auth/oauth_device.py becomes a 9-line placeholder (the existing console.__init__.py `from .auth import (..., oauth_device, ...)` keeps loading until Phase F prunes the import). Plan: docs/superpowers/plans/2026-04-26-openapi-migration.md (in difyctl repo). --- api/controllers/console/auth/oauth_device.py | 206 +----------------- api/controllers/openapi/__init__.py | 4 + .../openapi/oauth_device/approve.py | 175 +++++++++++++++ api/controllers/openapi/oauth_device/deny.py | 83 +++++++ .../openapi/test_device_approve_deny.py | 71 ++++++ 5 files changed, 339 insertions(+), 200 deletions(-) create mode 100644 api/controllers/openapi/oauth_device/approve.py create mode 100644 api/controllers/openapi/oauth_device/deny.py create mode 100644 api/tests/unit_tests/controllers/openapi/test_device_approve_deny.py diff --git a/api/controllers/console/auth/oauth_device.py b/api/controllers/console/auth/oauth_device.py index 46f32e2df9..896af0a85a 100644 --- a/api/controllers/console/auth/oauth_device.py +++ b/api/controllers/console/auth/oauth_device.py @@ -1,202 +1,8 @@ -"""Console-session-authed device-flow approve/deny. Called by the -/device page after the user signs in. Public lookup is in service_api/oauth.py. +"""Placeholder. Legacy /console/api/oauth/device/{approve,deny} mounts +are registered from the canonical openapi handlers in +controllers/openapi/oauth_device/{approve,deny}.py. This file stays +on disk only so controllers/console/__init__.py's +`from .auth import (... oauth_device, ...)` keeps working until +Phase F retires the legacy paths and prunes that import. """ from __future__ import annotations - -import logging - -from flask_login import login_required -from flask_restx import Resource, reqparse - -from controllers.console import console_ns -from controllers.console.wraps import account_initialization_required, setup_required -from extensions.ext_database import db -from extensions.ext_redis import redis_client -from libs.login import current_account_with_tenant -from libs.oauth_bearer import SubjectType, bearer_feature_required -from libs.rate_limit import LIMIT_APPROVE_CONSOLE, rate_limit -from services.oauth_device_flow import ( - ACCOUNT_ISSUER_SENTINEL, - PREFIX_OAUTH_ACCOUNT, - DeviceFlowRedis, - DeviceFlowStatus, - InvalidTransition, - StateNotFound, - mint_oauth_token, - oauth_ttl_days, -) - -logger = logging.getLogger(__name__) - - -_mutate_parser = reqparse.RequestParser() -_mutate_parser.add_argument("user_code", type=str, required=True, location="json") - - -_APPROVE_GUARD_KEY_FMT = "device_code:{code}:approving" -_APPROVE_GUARD_TTL_SECONDS = 10 - - -@console_ns.route("/oauth/device/approve") -class DeviceApproveApi(Resource): - @setup_required - @login_required - @account_initialization_required - @bearer_feature_required - @rate_limit(LIMIT_APPROVE_CONSOLE) - def post(self): - args = _mutate_parser.parse_args() - user_code = args["user_code"].strip().upper() - - account, tenant = current_account_with_tenant() - store = DeviceFlowRedis(redis_client) - - found = store.load_by_user_code(user_code) - if found is None: - return {"error": "expired_or_unknown"}, 404 - device_code, state = found - if state.status is not DeviceFlowStatus.PENDING: - return {"error": "already_resolved"}, 409 - - # SET NX guard — without it, two in-flight approves both pass - # PENDING, both mint, and the second upsert silently rotates the - # first caller into an already-revoked token. - guard_key = _APPROVE_GUARD_KEY_FMT.format(code=device_code) - if not redis_client.set(guard_key, "1", nx=True, ex=_APPROVE_GUARD_TTL_SECONDS): - return {"error": "approve_in_progress"}, 409 - - try: - ttl_days = oauth_ttl_days(tenant_id=tenant) - mint = mint_oauth_token( - db.session, - redis_client, - subject_email=account.email, - subject_issuer=ACCOUNT_ISSUER_SENTINEL, - account_id=str(account.id), - client_id=state.client_id, - device_label=state.device_label, - prefix=PREFIX_OAUTH_ACCOUNT, - ttl_days=ttl_days, - ) - - poll_payload = _build_account_poll_payload(account, tenant, mint) - try: - store.approve( - device_code, - subject_email=account.email, - account_id=str(account.id), - subject_issuer=ACCOUNT_ISSUER_SENTINEL, - minted_token=mint.token, - token_id=str(mint.token_id), - poll_payload=poll_payload, - ) - except (StateNotFound, InvalidTransition) as e: - # Row minted but state vanished — roll forward; the orphan - # token is revocable via auth devices list / Authorized Apps. - logger.error("device_flow: approve raced on %s: %s", device_code, e) - return {"error": "state_lost"}, 409 - finally: - redis_client.delete(guard_key) - - _emit_approve_audit(state, account, tenant, mint) - return {"status": "approved"}, 200 - - -@console_ns.route("/oauth/device/deny") -class DeviceDenyApi(Resource): - @setup_required - @login_required - @account_initialization_required - @bearer_feature_required - @rate_limit(LIMIT_APPROVE_CONSOLE) - def post(self): - args = _mutate_parser.parse_args() - user_code = args["user_code"].strip().upper() - - store = DeviceFlowRedis(redis_client) - found = store.load_by_user_code(user_code) - if found is None: - return {"error": "expired_or_unknown"}, 404 - device_code, state = found - if state.status is not DeviceFlowStatus.PENDING: - return {"error": "already_resolved"}, 409 - - try: - store.deny(device_code) - except (StateNotFound, InvalidTransition) as e: - logger.error("device_flow: deny raced on %s: %s", device_code, e) - return {"error": "state_lost"}, 409 - - _emit_deny_audit(state) - return {"status": "denied"}, 200 - - -def _build_account_poll_payload(account, tenant, mint) -> dict: - """Pre-render the poll-response body so the unauthenticated poll - handler doesn't re-query accounts/tenants for authz data. - """ - from models import Tenant, TenantAccountJoin - rows = ( - db.session.query(Tenant, TenantAccountJoin) - .join(TenantAccountJoin, TenantAccountJoin.tenant_id == Tenant.id) - .filter(TenantAccountJoin.account_id == account.id) - .all() - ) - workspaces = [ - {"id": str(t.id), "name": t.name, "role": getattr(m, "role", "")} - for t, m in rows - ] - # Prefer active session tenant → DB-flagged current join → first membership. - default_ws_id = None - if tenant and any(w["id"] == str(tenant) for w in workspaces): - default_ws_id = str(tenant) - if default_ws_id is None: - for _t, m in rows: - if getattr(m, "current", False): - default_ws_id = str(m.tenant_id) - break - if default_ws_id is None and workspaces: - default_ws_id = workspaces[0]["id"] - - return { - "token": mint.token, - "expires_at": mint.expires_at.isoformat(), - "subject_type": SubjectType.ACCOUNT, - "account": {"id": str(account.id), "email": account.email, "name": account.name}, - "workspaces": workspaces, - "default_workspace_id": default_ws_id, - "token_id": str(mint.token_id), - } - - -def _emit_approve_audit(state, account, tenant, mint) -> None: - logger.warning( - "audit: oauth.device_flow_approved token_id=%s subject=%s client_id=%s device_label=%s rotated=? expires_at=%s", - mint.token_id, account.email, state.client_id, state.device_label, mint.expires_at, - extra={ - "audit": True, - "event": "oauth.device_flow_approved", - "token_id": str(mint.token_id), - "subject_type": SubjectType.ACCOUNT, - "subject_email": account.email, - "account_id": str(account.id), - "tenant_id": tenant, - "client_id": state.client_id, - "device_label": state.device_label, - "scopes": ["full"], - "expires_at": mint.expires_at.isoformat(), - }, - ) - - -def _emit_deny_audit(state) -> None: - logger.warning( - "audit: oauth.device_flow_denied client_id=%s device_label=%s", - state.client_id, state.device_label, - extra={ - "audit": True, - "event": "oauth.device_flow_denied", - "client_id": state.client_id, - "device_label": state.device_label, - }, - ) diff --git a/api/controllers/openapi/__init__.py b/api/controllers/openapi/__init__.py index 46c5dd9a7e..1b169028fa 100644 --- a/api/controllers/openapi/__init__.py +++ b/api/controllers/openapi/__init__.py @@ -15,14 +15,18 @@ api = ExternalApi( openapi_ns = Namespace("openapi", description="User-scoped operations", path="/") from . import account, index +from .oauth_device import approve as oauth_device_approve from .oauth_device import code as oauth_device_code +from .oauth_device import deny as oauth_device_deny from .oauth_device import lookup as oauth_device_lookup from .oauth_device import token as oauth_device_token __all__ = [ "account", "index", + "oauth_device_approve", "oauth_device_code", + "oauth_device_deny", "oauth_device_lookup", "oauth_device_token", ] diff --git a/api/controllers/openapi/oauth_device/approve.py b/api/controllers/openapi/oauth_device/approve.py new file mode 100644 index 0000000000..681bfd3c53 --- /dev/null +++ b/api/controllers/openapi/oauth_device/approve.py @@ -0,0 +1,175 @@ +"""POST /openapi/v1/oauth/device/approve — user approves a pending +device flow from the /device page. Console-session authed (the user is +signed in via cookie when they hit Approve in the SPA). + +The class is also registered on console_ns at /console/api/oauth/device/approve +from console/auth/oauth_device.py until Phase F retires that mount. +""" +from __future__ import annotations + +import logging + +from flask_login import login_required +from flask_restx import Resource, reqparse + +from controllers.console.wraps import account_initialization_required, setup_required +from controllers.openapi import openapi_ns +from extensions.ext_database import db +from extensions.ext_redis import redis_client +from libs.login import current_account_with_tenant +from libs.oauth_bearer import SubjectType, bearer_feature_required +from libs.rate_limit import LIMIT_APPROVE_CONSOLE, rate_limit +from services.oauth_device_flow import ( + ACCOUNT_ISSUER_SENTINEL, + PREFIX_OAUTH_ACCOUNT, + DeviceFlowRedis, + DeviceFlowStatus, + InvalidTransition, + StateNotFound, + mint_oauth_token, + oauth_ttl_days, +) + +logger = logging.getLogger(__name__) + + +_mutate_parser = reqparse.RequestParser() +_mutate_parser.add_argument("user_code", type=str, required=True, location="json") + + +_APPROVE_GUARD_KEY_FMT = "device_code:{code}:approving" +_APPROVE_GUARD_TTL_SECONDS = 10 + + +@openapi_ns.route("/oauth/device/approve") +class DeviceApproveApi(Resource): + @setup_required + @login_required + @account_initialization_required + @bearer_feature_required + @rate_limit(LIMIT_APPROVE_CONSOLE) + def post(self): + args = _mutate_parser.parse_args() + user_code = args["user_code"].strip().upper() + + account, tenant = current_account_with_tenant() + store = DeviceFlowRedis(redis_client) + + found = store.load_by_user_code(user_code) + if found is None: + return {"error": "expired_or_unknown"}, 404 + device_code, state = found + if state.status is not DeviceFlowStatus.PENDING: + return {"error": "already_resolved"}, 409 + + # SET NX guard — without it, two in-flight approves both pass + # PENDING, both mint, and the second upsert silently rotates the + # first caller into an already-revoked token. + guard_key = _APPROVE_GUARD_KEY_FMT.format(code=device_code) + if not redis_client.set(guard_key, "1", nx=True, ex=_APPROVE_GUARD_TTL_SECONDS): + return {"error": "approve_in_progress"}, 409 + + try: + ttl_days = oauth_ttl_days(tenant_id=tenant) + mint = mint_oauth_token( + db.session, + redis_client, + subject_email=account.email, + subject_issuer=ACCOUNT_ISSUER_SENTINEL, + account_id=str(account.id), + client_id=state.client_id, + device_label=state.device_label, + prefix=PREFIX_OAUTH_ACCOUNT, + ttl_days=ttl_days, + ) + + poll_payload = _build_account_poll_payload(account, tenant, mint) + try: + store.approve( + device_code, + subject_email=account.email, + account_id=str(account.id), + subject_issuer=ACCOUNT_ISSUER_SENTINEL, + minted_token=mint.token, + token_id=str(mint.token_id), + poll_payload=poll_payload, + ) + except (StateNotFound, InvalidTransition) as e: + # Row minted but state vanished — roll forward; the orphan + # token is revocable via auth devices list / Authorized Apps. + logger.error("device_flow: approve raced on %s: %s", device_code, e) + return {"error": "state_lost"}, 409 + finally: + redis_client.delete(guard_key) + + _emit_approve_audit(state, account, tenant, mint) + return {"status": "approved"}, 200 + + +def _build_account_poll_payload(account, tenant, mint) -> dict: + """Pre-render the poll-response body so the unauthenticated poll + handler doesn't re-query accounts/tenants for authz data. + """ + from models import Tenant, TenantAccountJoin + rows = ( + db.session.query(Tenant, TenantAccountJoin) + .join(TenantAccountJoin, TenantAccountJoin.tenant_id == Tenant.id) + .filter(TenantAccountJoin.account_id == account.id) + .all() + ) + workspaces = [ + {"id": str(t.id), "name": t.name, "role": getattr(m, "role", "")} + for t, m in rows + ] + # Prefer active session tenant → DB-flagged current join → first membership. + default_ws_id = None + if tenant and any(w["id"] == str(tenant) for w in workspaces): + default_ws_id = str(tenant) + if default_ws_id is None: + for _t, m in rows: + if getattr(m, "current", False): + default_ws_id = str(m.tenant_id) + break + if default_ws_id is None and workspaces: + default_ws_id = workspaces[0]["id"] + + return { + "token": mint.token, + "expires_at": mint.expires_at.isoformat(), + "subject_type": SubjectType.ACCOUNT, + "account": {"id": str(account.id), "email": account.email, "name": account.name}, + "workspaces": workspaces, + "default_workspace_id": default_ws_id, + "token_id": str(mint.token_id), + } + + +def _emit_approve_audit(state, account, tenant, mint) -> None: + logger.warning( + "audit: oauth.device_flow_approved token_id=%s subject=%s client_id=%s device_label=%s rotated=? expires_at=%s", + mint.token_id, account.email, state.client_id, state.device_label, mint.expires_at, + extra={ + "audit": True, + "event": "oauth.device_flow_approved", + "token_id": str(mint.token_id), + "subject_type": SubjectType.ACCOUNT, + "subject_email": account.email, + "account_id": str(account.id), + "tenant_id": tenant, + "client_id": state.client_id, + "device_label": state.device_label, + "scopes": ["full"], + "expires_at": mint.expires_at.isoformat(), + }, + ) + + +# Legacy /console/api/oauth/device/approve mount — handler defined above. +# Removed in Phase F. The console_ns import is local to defer past +# circular-import resolution between this module and controllers.console. +def _register_legacy_console_mount() -> None: + from controllers.console import console_ns + console_ns.add_resource(DeviceApproveApi, "/oauth/device/approve") + + +_register_legacy_console_mount() diff --git a/api/controllers/openapi/oauth_device/deny.py b/api/controllers/openapi/oauth_device/deny.py new file mode 100644 index 0000000000..598d66b412 --- /dev/null +++ b/api/controllers/openapi/oauth_device/deny.py @@ -0,0 +1,83 @@ +"""POST /openapi/v1/oauth/device/deny — user denies a pending device +flow from the /device page. Console-session authed. + +The class is also registered on console_ns at /console/api/oauth/device/deny +from console/auth/oauth_device.py until Phase F retires that mount. +""" +from __future__ import annotations + +import logging + +from flask_login import login_required +from flask_restx import Resource, reqparse + +from controllers.console.wraps import account_initialization_required, setup_required +from controllers.openapi import openapi_ns +from extensions.ext_redis import redis_client +from libs.oauth_bearer import bearer_feature_required +from libs.rate_limit import LIMIT_APPROVE_CONSOLE, rate_limit +from services.oauth_device_flow import ( + DeviceFlowRedis, + DeviceFlowStatus, + InvalidTransition, + StateNotFound, +) + +logger = logging.getLogger(__name__) + + +_mutate_parser = reqparse.RequestParser() +_mutate_parser.add_argument("user_code", type=str, required=True, location="json") + + +@openapi_ns.route("/oauth/device/deny") +class DeviceDenyApi(Resource): + @setup_required + @login_required + @account_initialization_required + @bearer_feature_required + @rate_limit(LIMIT_APPROVE_CONSOLE) + def post(self): + args = _mutate_parser.parse_args() + user_code = args["user_code"].strip().upper() + + store = DeviceFlowRedis(redis_client) + found = store.load_by_user_code(user_code) + if found is None: + return {"error": "expired_or_unknown"}, 404 + device_code, state = found + if state.status is not DeviceFlowStatus.PENDING: + return {"error": "already_resolved"}, 409 + + try: + store.deny(device_code) + except (StateNotFound, InvalidTransition) as e: + logger.error("device_flow: deny raced on %s: %s", device_code, e) + return {"error": "state_lost"}, 409 + + _emit_deny_audit(state) + return {"status": "denied"}, 200 + + +def _emit_deny_audit(state) -> None: + logger.warning( + "audit: oauth.device_flow_denied client_id=%s device_label=%s", + state.client_id, state.device_label, + extra={ + "audit": True, + "event": "oauth.device_flow_denied", + "client_id": state.client_id, + "device_label": state.device_label, + }, + ) + + +# Legacy /console/api/oauth/device/deny mount — handler defined above. +# Removed in Phase F. The console_ns import is local to defer past +# circular-import resolution between this module and controllers.console. +def _register_legacy_console_mount() -> None: + from controllers.console import console_ns + console_ns.add_resource(DeviceDenyApi, "/oauth/device/deny") + + +_register_legacy_console_mount() diff --git a/api/tests/unit_tests/controllers/openapi/test_device_approve_deny.py b/api/tests/unit_tests/controllers/openapi/test_device_approve_deny.py new file mode 100644 index 0000000000..1718b71a7e --- /dev/null +++ b/api/tests/unit_tests/controllers/openapi/test_device_approve_deny.py @@ -0,0 +1,71 @@ +"""Phase D steps 13-14: device-flow approve/deny lifted to /openapi/v1. +Legacy /console/api/oauth/device/{approve,deny} stays mounted via +re-registration in console/auth/oauth_device.py. +""" +import builtins + +import pytest +from flask import Flask +from flask.views import MethodView + +from controllers.console import bp as console_bp +from controllers.openapi import bp as openapi_bp +from controllers.openapi.oauth_device.approve import DeviceApproveApi +from controllers.openapi.oauth_device.deny import DeviceDenyApi + +if not hasattr(builtins, "MethodView"): + builtins.MethodView = MethodView # type: ignore[attr-defined] + + +@pytest.fixture +def dual_app() -> Flask: + app = Flask(__name__) + app.config["TESTING"] = True + app.register_blueprint(console_bp) + app.register_blueprint(openapi_bp) + return app + + +def _rule(app: Flask, path: str): + return next(r for r in app.url_map.iter_rules() if r.rule == path) + + +def test_openapi_approve_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/openapi/v1/oauth/device/approve" in rules + + +def test_legacy_console_approve_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/console/api/oauth/device/approve" in rules + + +def test_approve_paths_dispatch_to_same_class(dual_app: Flask): + new = _rule(dual_app, "/openapi/v1/oauth/device/approve") + legacy = _rule(dual_app, "/console/api/oauth/device/approve") + assert dual_app.view_functions[new.endpoint].view_class is DeviceApproveApi + assert dual_app.view_functions[legacy.endpoint].view_class is DeviceApproveApi + + +def test_openapi_deny_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/openapi/v1/oauth/device/deny" in rules + + +def test_legacy_console_deny_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/console/api/oauth/device/deny" in rules + + +def test_deny_paths_dispatch_to_same_class(dual_app: Flask): + new = _rule(dual_app, "/openapi/v1/oauth/device/deny") + legacy = _rule(dual_app, "/console/api/oauth/device/deny") + assert dual_app.view_functions[new.endpoint].view_class is DeviceDenyApi + assert dual_app.view_functions[legacy.endpoint].view_class is DeviceDenyApi + + +def test_approve_and_deny_methods(dual_app: Flask): + approve = _rule(dual_app, "/openapi/v1/oauth/device/approve") + deny = _rule(dual_app, "/openapi/v1/oauth/device/deny") + assert "POST" in approve.methods + assert "POST" in deny.methods