Skip to content

feat(framework): Implement OAuth connector Control API#7597

Draft
charlesbvll wants to merge 28 commits into
mainfrom
add-oauth-connector-control-api
Draft

feat(framework): Implement OAuth connector Control API#7597
charlesbvll wants to merge 28 commits into
mainfrom
add-oauth-connector-control-api

Conversation

@charlesbvll

@charlesbvll charlesbvll commented Jul 14, 2026

Copy link
Copy Markdown
Member

Follow up to #7591

Implements ListConnectors, DisconnectConnector, BeginConnectorOAuth, and CompleteConnectorOAuth

  • Adds a minimal injectable OAuth provider interface
  • Enforces account isolation, state/expiry validation, PKCE, and single-use sessions
  • Adds sanitized provider and persistence error handling
  • Covers success, validation, isolation, failure, and disconnect flows with tests

…orestate-methods

# Conflicts:
#	framework/proto/flwr/proto/control.proto
#	framework/py/flwr/proto/control_pb2.py
#	framework/py/flwr/proto/control_pb2.pyi
#	framework/py/flwr/supercore/corestate/corestate_test.py
#	framework/py/flwr/supercore/corestate/sql_corestate.py
#	framework/py/flwr/supercore/state/alembic/versions/rev_2026_07_09_add_oauth_connector_tables.py
@github-actions github-actions Bot added the Maintainer Used to determine what PRs (mainly) come from Flower maintainers. label Jul 14, 2026
Base automatically changed from add-oauth-connector-corestate-methods to main July 15, 2026 12:20
Comment thread framework/py/flwr/superlink/servicer/control/connectors/__init__.py Outdated
Comment thread framework/py/flwr/superlink/servicer/control/connectors/handlers.py Outdated
Comment thread framework/py/flwr/supercore/error/base.py Outdated
Copilot AI review requested due to automatic review settings July 15, 2026 12:25
Comment thread framework/py/flwr/superlink/servicer/control/connectors/provider.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the Control API surface for OAuth-based connectors (list, disconnect, begin OAuth, complete OAuth) by wiring new connector handler logic into the Control servicer, adding a minimal provider interface, and extending error-code/catalog support and tests.

Changes:

  • Wire ListConnectors, DisconnectConnector, BeginConnectorOAuth, and CompleteConnectorOAuth to new connector handler functions and add injectable OAuth provider registration.
  • Add connector OAuth provider protocol/types plus handler implementations (PKCE, expiry/state validation, single-use sessions, account isolation) and comprehensive tests.
  • Add new connector-related API error codes/specs (but see critical issue below).

Critical issues

  • ApiErrorCode and ApiErrorCatalog removed ACCOUNT_AUTHENTICATION_* entries that are still referenced elsewhere in the repo, and the enum reuses their previous integer values for new connector codes. This will break runtime lookups and can break backwards compatibility for clients relying on stable error-code integers. (Comments added with concrete inline fixes.)
  • complete_connector_oauth validates request.code.strip() but passes the unstripped code to the provider exchange, allowing whitespace-padded codes to pass validation but fail during exchange. (Comment added with inline fix.)

Simplicity/readability suggestions

  • None blocking beyond the targeted fixes above.

Consistency concerns

  • corestate_test.py adds unused UTC/timezone imports (likely lint failure). (Comment added with inline fix.)

Whether the PR should be split

  • No; the changes are cohesive around the OAuth connector Control API.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
framework/py/flwr/superlink/servicer/control/control_servicer.py Wire connector RPCs to new connector handlers and inject provider map.
framework/py/flwr/superlink/servicer/control/control_grpc.py Thread connector OAuth providers into gRPC server construction.
framework/py/flwr/superlink/servicer/control/connectors/provider.py Add provider protocol and provider-map construction helpers.
framework/py/flwr/superlink/servicer/control/connectors/handlers.py Implement connector OAuth business logic (PKCE, session validation/claiming, persistence).
framework/py/flwr/superlink/servicer/control/connectors/handlers_test.py Add end-to-end tests for connector OAuth flows and servicer wiring.
framework/py/flwr/superlink/servicer/control/connectors/init.py Export connector OAuth public types/helpers.
framework/py/flwr/supercore/error/base.py Add connector error codes (currently breaks existing auth codes; needs adjustment).
framework/py/flwr/supercore/error/catalog.py Add connector error specs (currently drops auth error specs; needs restoration).
framework/py/flwr/supercore/corestate/corestate_test.py Update datetime imports (currently includes unused imports).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread framework/py/flwr/supercore/error/base.py Outdated
Comment thread framework/py/flwr/supercore/error/catalog.py
Comment thread framework/py/flwr/superlink/servicer/control/connectors/handlers.py Outdated
Comment thread framework/py/flwr/supercore/corestate/corestate_test.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintainer Used to determine what PRs (mainly) come from Flower maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants