Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: implement OAuth2 dynamic client registration (RFC 7591/7592)#18645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

ThomasK33
Copy link
Member

Implement OAuth2 Dynamic Client Registration (RFC 7591/7592)

This PR implements OAuth2 Dynamic Client Registration according to RFC 7591 and Client Configuration Management according to RFC 7592. These standards allow OAuth2 clients to register themselves programmatically with Coder as an authorization server.

Key changes include:

  1. Added database schema extensions to support RFC 7591/7592 fields in theoauth2_provider_apps table

  2. Implemented/oauth2/register endpoint for dynamic client registration (RFC 7591)

  3. Added client configuration management endpoints (RFC 7592):

    • GET/PUT/DELETE/oauth2/clients/{client_id}
    • Registration access token validation middleware
  4. Added comprehensive validation for OAuth2 client metadata:

    • URI validation with support for custom schemes for native apps
    • Grant type and response type validation
    • Token endpoint authentication method validation
  5. Enhanced developer documentation with:

    • RFC compliance guidelines
    • Testing best practices to avoid race conditions
    • Systematic debugging approaches for OAuth2 implementations

The implementation follows security best practices from the RFCs, including proper token handling, secure defaults, and appropriate error responses. This enables third-party applications to integrate with Coder's OAuth2 provider capabilities programmatically.

@ThomasK33Graphite App
Copy link
MemberAuthor

ThomasK33 commentedJun 27, 2025
edited
Loading

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch from3902793 tob37d850CompareJune 27, 2025 17:02
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch fromff83df4 to3665807CompareJune 27, 2025 17:02
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch fromb37d850 tocaf974cCompareJune 27, 2025 17:11
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch from3665807 to56126ddCompareJune 27, 2025 17:11
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch fromcaf974c to22b8b6dCompareJune 27, 2025 17:29
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch from56126dd tofca6b9aCompareJune 27, 2025 17:29
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch 2 times, most recently from14c7196 toc43b551CompareJune 27, 2025 17:54
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch fromfca6b9a to68baa21CompareJune 27, 2025 17:54
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch 12 times, most recently from46d50e8 to61142baCompareJune 30, 2025 08:58
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch 3 times, most recently froma8a29d4 tof9e693dCompareJuly 2, 2025 14:06
@ThomasK33Graphite App
Copy link
MemberAuthor

In general, should these routes be implemented in their own package? Rather than mostly theoauth2 file incoderd?

Looking at the router/chi.Mux incoderd.go, I see that all the HTTP handlers are implemented on the API struct.

Are you suggesting moving those handlers into a different package or moving the logic behind them into their own package, which then gets called from those API handlers? (I'm assuming the latter, but I want to be sure.)

@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch from3760dd0 to2a41a65CompareJuly 2, 2025 15:50
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch 2 times, most recently from9629df1 todf790c7CompareJuly 2, 2025 16:35
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch 2 times, most recently from52c88e0 to4799b4bCompareJuly 2, 2025 16:44
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch fromdf790c7 to17af791CompareJuly 2, 2025 16:45
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance branch 2 times, most recently froma07ba99 to5c1b9f6CompareJuly 2, 2025 16:59
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch from17af791 to3e15358CompareJuly 2, 2025 16:59
@ThomasK33ThomasK33 changed the base branch fromthomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance tographite-base/18645July 2, 2025 17:14
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch from3e15358 to6bea7a6CompareJuly 2, 2025 17:15
@graphite-appgraphite-appbot changed the base branch fromgraphite-base/18645 tomainJuly 2, 2025 17:15
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch from6bea7a6 toe33d3d6CompareJuly 2, 2025 17:15
… MCP compliance- Add comprehensive OAuth2 dynamic client registration support- Implement RFC 7591 client registration endpoint with proper validation- Implement RFC 7592 client management protocol (GET/PUT/DELETE)- Add RFC 6750 Bearer token authentication support- Fix authorization context issues with AsSystemRestricted- Add proper RBAC permissions for OAuth2 resources- Implement registration access token security per RFC 7592- Add comprehensive validation for redirect URIs, grant types, response types- Support custom schemes for native applications- Add database migration with all RFC-required fields- Add audit logging support for OAuth2 operations- Ensure full RFC compliance with proper error handling- Add comprehensive test coverage for all scenariosChange-Id: I36c711201d598a117f6bfc381fc74e07fc3cc365Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33ThomasK33force-pushed thethomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branch frome33d3d6 to4f4e4f1CompareJuly 3, 2025 15:00
@ThomasK33Graphite App
Copy link
MemberAuthor

Alright, so I did the refactoring in two PRs:

  • this PR refactors the handlers into aoauth2provider package (merging in the functions from theidentityprovider package)
  • this PR refactors the tests from the coderd package into theoauth2provider package.

The main reason for this two PR approach was to make sure that the refactorings didn't break the tests, and that the test refactoring did not break the code.

Copy link
Member

@johnstcnjohnstcn left a comment

Choose a reason for hiding this comment

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

I don't have any further blocking comments, but would appreciate a second pair of eyes before merge in case I missed something.

Copy link
Member

Choose a reason for hiding this comment

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

nit: the validation tests could probably live in codersdk alongside the func itself but open to it staying here either

@ThomasK33ThomasK33 merged commit74e1d5c intomainJul 3, 2025
40 checks passed
@ThomasK33Graphite App
Copy link
MemberAuthor

Merge activity

@ThomasK33ThomasK33 deleted the thomask33/06-27-feat_oauth2_implement_rfc_7591_7592_dynamic_client_registration_for_mcp_compliance branchJuly 3, 2025 16:33
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJul 3, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

@EmyrkEmyrkAwaiting requested review from Emyrk

Assignees

@ThomasK33ThomasK33

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@ThomasK33@johnstcn@Emyrk

[8]ページ先頭

©2009-2025 Movatter.jp