-
Notifications
You must be signed in to change notification settings - Fork 928
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: thomask33/06-27-feat_oauth2_implement_rfc_6750_bearer_token_support_for_mcp_compliance
Are you sure you want to change the base?
Conversation
3902793
to
b37d850
Compare
ff83df4
to
3665807
Compare
b37d850
to
caf974c
Compare
3665807
to
56126dd
Compare
caf974c
to
22b8b6d
Compare
56126dd
to
fca6b9a
Compare
14c7196
to
c43b551
Compare
fca6b9a
to
68baa21
Compare
46d50e8
to
61142ba
Compare
3417103
to
66f2908
Compare
77e3e55
to
1e1e046
Compare
66f2908
to
4a0ebd7
Compare
1e1e046
to
17982a7
Compare
17982a7
to
119626d
Compare
4a0ebd7
to
2f305f7
Compare
coderd/oauth2_validation.go
Outdated
// Preserve uniqueness by including a hash of the original name | ||
hash := fmt.Sprintf("%x", sha256.Sum256([]byte(req.ClientName)))[:8] | ||
maxPrefix := 64 - 1 - len(hash) // 1 for separator | ||
return req.ClientName[:maxPrefix] + "-" + hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this? Aren't you dropping the unique constraint up the stack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, but the idea here would be to have the same "compressed" name, even when two different applications come in, so that those can be distinguished in some reproducible way.
Otherwise it would just cut off at 64 chars and any name above 64 would result in the same client name.
coderd/oauth2_validation.go
Outdated
func generateClientName(req codersdk.OAuth2ClientRegistrationRequest) string { | ||
if req.ClientName != "" { | ||
// Ensure client name fits database constraint (varchar(64)) | ||
if len(req.ClientName) > 64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not unreasonable to return invalid_client_metadata
in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily, as that limitation is an arbitrary limit on our side, and the RFC does not dictate a maximum size.
coderd/oauth2_validation.go
Outdated
// determineClientType determines if client is public or confidential | ||
func determineClientType(_ codersdk.OAuth2ClientRegistrationRequest) string { | ||
// For now, default to confidential | ||
// In the future, we might detect based on: | ||
// - token_endpoint_auth_method == "none" -> public | ||
// - application_type == "native" -> might be public | ||
// - Other heuristics | ||
return "confidential" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't plan to do this soon, could we hard-code client type and remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that depends on product.
I saw some request to get the CLI auth to be oauth2 based, and we might end up with a few client apps that could have a different client type.
But I do get the point that this could be a premature optimization/anticipation for something that won't come.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, should these routes be implemented in their own package? Rather than mostly the oauth2
file in coderd
?
2f305f7
to
33190fa
Compare
6487981
to
b612336
Compare
33190fa
to
0b75800
Compare
b612336
to
dd8ddee
Compare
decfff1
to
d40ab47
Compare
dd8ddee
to
54580fa
Compare
d40ab47
to
ac3723a
Compare
54580fa
to
21cdd55
Compare
ac3723a
to
d596e33
Compare
2410fd8
to
7eb343b
Compare
d596e33
to
826b305
Compare
+1 to this |
826b305
to
e119638
Compare
7eb343b
to
3760dd0
Compare
… 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 scenarios Change-Id: I36c711201d598a117f6bfc381fc74e07fc3cc365 Signed-off-by: Thomas Kosiewski <[email protected]>
e119638
to
16c7b15
Compare
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:
Added database schema extensions to support RFC 7591/7592 fields in the
oauth2_provider_apps
tableImplemented
/oauth2/register
endpoint for dynamic client registration (RFC 7591)Added client configuration management endpoints (RFC 7592):
/oauth2/clients/{client_id}
Added comprehensive validation for OAuth2 client metadata:
Enhanced developer documentation with:
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.