From e93821af46eb887e83182b7d892852d69449fdd3 Mon Sep 17 00:00:00 2001 From: GareArc Date: Sun, 26 Apr 2026 23:44:05 -0700 Subject: [PATCH] feat(api): lift GET /oauth/device/lookup to /openapi/v1 (Phase B.8) Same pattern as B.6 / B.7: OAuthDeviceLookupApi moves to controllers/openapi/oauth_device/lookup.py and is re-registered on service_api_ns to keep /v1/oauth/device/lookup serving until Phase F. service_api/oauth.py is now down to /me + /oauth/authorizations/self plus three legacy mounts; remaining handlers move in Phase C. Now-unused imports (LIMIT_LOOKUP_PUBLIC, rate_limit, reqparse, request, DEVICE_FLOW_TTL_SECONDS, DeviceFlowRedis, DeviceFlowStatus) removed. Plan: docs/superpowers/plans/2026-04-26-openapi-migration.md (in difyctl repo). --- api/controllers/openapi/__init__.py | 2 + .../openapi/oauth_device/lookup.py | 49 +++++++++++++++++ api/controllers/service_api/oauth.py | 48 ++--------------- .../controllers/openapi/test_device_lookup.py | 53 +++++++++++++++++++ 4 files changed, 108 insertions(+), 44 deletions(-) create mode 100644 api/controllers/openapi/oauth_device/lookup.py create mode 100644 api/tests/unit_tests/controllers/openapi/test_device_lookup.py diff --git a/api/controllers/openapi/__init__.py b/api/controllers/openapi/__init__.py index 8ca9c668fe..a28faeeae6 100644 --- a/api/controllers/openapi/__init__.py +++ b/api/controllers/openapi/__init__.py @@ -16,11 +16,13 @@ openapi_ns = Namespace("openapi", description="User-scoped operations", path="/" from . import index from .oauth_device import code as oauth_device_code +from .oauth_device import lookup as oauth_device_lookup from .oauth_device import token as oauth_device_token __all__ = [ "index", "oauth_device_code", + "oauth_device_lookup", "oauth_device_token", ] diff --git a/api/controllers/openapi/oauth_device/lookup.py b/api/controllers/openapi/oauth_device/lookup.py new file mode 100644 index 0000000000..7546ba78cc --- /dev/null +++ b/api/controllers/openapi/oauth_device/lookup.py @@ -0,0 +1,49 @@ +"""GET /openapi/v1/oauth/device/lookup — pre-validate user_code from +the /device page before the user signs in. Public; user_code is +high-entropy + short-TTL, per-IP rate limit blocks enumeration. + +The class is also registered on the legacy /v1/ namespace from +service_api/oauth.py until Phase F retires that mount. +""" +from __future__ import annotations + +from flask_restx import Resource, reqparse + +from controllers.openapi import openapi_ns +from extensions.ext_redis import redis_client +from libs.rate_limit import LIMIT_LOOKUP_PUBLIC, rate_limit +from services.oauth_device_flow import ( + DEVICE_FLOW_TTL_SECONDS, + DeviceFlowRedis, + DeviceFlowStatus, +) + +_lookup_parser = reqparse.RequestParser() +_lookup_parser.add_argument("user_code", type=str, required=True, location="args") + + +@openapi_ns.route("/oauth/device/lookup") +class OAuthDeviceLookupApi(Resource): + """Read-only — public for pre-validate before login. user_code is + high-entropy + short-TTL; per-IP rate limit blocks enumeration. + """ + + @rate_limit(LIMIT_LOOKUP_PUBLIC) + def get(self): + args = _lookup_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 {"valid": False, "expires_in_remaining": 0, "client_id": None}, 200 + + _device_code, state = found + if state.status is not DeviceFlowStatus.PENDING: + return {"valid": False, "expires_in_remaining": 0, "client_id": state.client_id}, 200 + + return { + "valid": True, + "expires_in_remaining": DEVICE_FLOW_TTL_SECONDS, + "client_id": state.client_id, + }, 200 diff --git a/api/controllers/service_api/oauth.py b/api/controllers/service_api/oauth.py index 61d984002a..bf6ff04d95 100644 --- a/api/controllers/service_api/oauth.py +++ b/api/controllers/service_api/oauth.py @@ -8,12 +8,13 @@ from __future__ import annotations import logging from datetime import UTC, datetime -from flask import g, request -from flask_restx import Resource, reqparse +from flask import g +from flask_restx import Resource from sqlalchemy import update from werkzeug.exceptions import BadRequest from controllers.openapi.oauth_device.code import OAuthDeviceCodeApi +from controllers.openapi.oauth_device.lookup import OAuthDeviceLookupApi from controllers.openapi.oauth_device.token import OAuthDeviceTokenApi from controllers.service_api import service_api_ns from extensions.ext_database import db @@ -25,18 +26,11 @@ from libs.oauth_bearer import ( validate_bearer, ) from libs.rate_limit import ( - LIMIT_LOOKUP_PUBLIC, LIMIT_ME_PER_ACCOUNT, LIMIT_ME_PER_EMAIL, enforce, - rate_limit, ) from models import Account, OAuthAccessToken, Tenant, TenantAccountJoin -from services.oauth_device_flow import ( - DEVICE_FLOW_TTL_SECONDS, - DeviceFlowRedis, - DeviceFlowStatus, -) logger = logging.getLogger(__name__) @@ -44,6 +38,7 @@ logger = logging.getLogger(__name__) # Removed in Phase F. service_api_ns.add_resource(OAuthDeviceCodeApi, "/oauth/device/code") service_api_ns.add_resource(OAuthDeviceTokenApi, "/oauth/device/token") +service_api_ns.add_resource(OAuthDeviceLookupApi, "/oauth/device/lookup") # ============================================================================ @@ -161,39 +156,4 @@ class OAuthAuthorizationsSelfApi(Resource): return {"status": "revoked"}, 200 -# ============================================================================ -# GET /v1/oauth/device/lookup (unauthenticated — /device page pre-validates) -# ============================================================================ - - -_lookup_parser = reqparse.RequestParser() -_lookup_parser.add_argument("user_code", type=str, required=True, location="args") - - -@service_api_ns.route("/oauth/device/lookup") -class OAuthDeviceLookupApi(Resource): - """Read-only — public for pre-validate before login. user_code is - high-entropy + short-TTL; per-IP rate limit blocks enumeration. - """ - - @rate_limit(LIMIT_LOOKUP_PUBLIC) - def get(self): - args = _lookup_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 {"valid": False, "expires_in_remaining": 0, "client_id": None}, 200 - - _device_code, state = found - if state.status is not DeviceFlowStatus.PENDING: - return {"valid": False, "expires_in_remaining": 0, "client_id": state.client_id}, 200 - - return { - "valid": True, - "expires_in_remaining": DEVICE_FLOW_TTL_SECONDS, - "client_id": state.client_id, - }, 200 - diff --git a/api/tests/unit_tests/controllers/openapi/test_device_lookup.py b/api/tests/unit_tests/controllers/openapi/test_device_lookup.py new file mode 100644 index 0000000000..b28e881d10 --- /dev/null +++ b/api/tests/unit_tests/controllers/openapi/test_device_lookup.py @@ -0,0 +1,53 @@ +"""Phase B step 8: GET /openapi/v1/oauth/device/lookup mounted via the +canonical class. Legacy /v1/oauth/device/lookup re-registered. Both +paths must dispatch to the same class. +""" +import builtins + +import pytest +from flask import Flask +from flask.views import MethodView + +from controllers.openapi import bp as openapi_bp +from controllers.openapi.oauth_device.lookup import OAuthDeviceLookupApi +from controllers.service_api import bp as service_api_bp + +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(service_api_bp) + app.register_blueprint(openapi_bp) + return app + + +def test_openapi_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/openapi/v1/oauth/device/lookup" in rules + + +def test_legacy_v1_route_registered(dual_app: Flask): + rules = {r.rule for r in dual_app.url_map.iter_rules()} + assert "/v1/oauth/device/lookup" in rules + + +def test_both_paths_dispatch_to_same_class(dual_app: Flask): + new = next( + r for r in dual_app.url_map.iter_rules() if r.rule == "/openapi/v1/oauth/device/lookup" + ) + legacy = next( + r for r in dual_app.url_map.iter_rules() if r.rule == "/v1/oauth/device/lookup" + ) + assert dual_app.view_functions[new.endpoint].view_class is OAuthDeviceLookupApi + assert dual_app.view_functions[legacy.endpoint].view_class is OAuthDeviceLookupApi + + +def test_route_accepts_get(dual_app: Flask): + new = next( + r for r in dual_app.url_map.iter_rules() if r.rule == "/openapi/v1/oauth/device/lookup" + ) + assert "GET" in new.methods