8000 feat(oauth2): implement RFC 7591/7592 Dynamic Client Registration for… · coder/coder@caf974c · GitHub
[go: up one dir, main page]

Skip to content

Commit caf974c

Browse files
committed
feat(oauth2): implement RFC 7591/7592 Dynamic Client Registration for 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 <tk@coder.com>
1 parent 56126dd commit caf974c

28 files changed

+5385
-74
lines changed

CLAUDE.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,32 @@ The frontend is contained in the site folder.
196196

197197
For building Frontend refer to [this document](docs/about/contributing/frontend.md)
198198

199+
## RFC Compliance Development
200+
201+
### Implementing Standard Protocols
202+
203+
When implementing standard protocols (OAuth2, OpenID Connect, etc.):
204+
205+
1. **Fetch and Analyze Official RFCs**:
206+
- Always read the actual RFC specifications before implementation
207+
- Use WebFetch tool to get current RFC content for compliance verification
208+
- Document RFC requirements in code comments
209+
210+
2. **Default Values Matter**:
211+
- Pay close attention to RFC-specified default values
212+
- Example: RFC 7591 specifies `client_secret_basic` as default, not `client_secret_post`
213+
- Ensure consistency between database migrations and application code
214+
215+
3. **Security Requirements**:
216+
- Follow RFC security considerations precisely
217+
- Example: RFC 7592 prohibits returning registration access tokens in GET responses
218+
- Implement proper error responses per protocol specifications
219+
220+
4. **Validation Compliance**:
221+
- Implement comprehensive validation per RFC requirements
222+
- Support protocol-specific features (e.g., custom schemes for native OAuth2 apps)
223+
- Test edge cases defined in specifications
224+
199225
## Common Patterns
200226

201227
### OAuth2/Authentication Work
@@ -270,6 +296,32 @@ if errors.Is(err, errInvalidPKCE) {
270296
- Test both positive and negative cases
271297
- Use `testutil.WaitLong` for timeouts in tests
272298

299+
## Testing Best Practices
300+
301+
### Avoiding Race Conditions
302+
303+
1. **Unique Test Identifiers**:
304+
- Never use hardcoded names in concurrent tests
305+
- Use `time.Now().UnixNano()` or similar for unique identifiers
306+
- Example: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())`
307+
308+
2. **Database Constraint Awareness**:
309+
- Understand unique constraints that can cause test conflicts
310+
- Generate unique values for all constrained fields
311+
- Test name isolation prevents cross-test interference
312+
313+
### RFC Protocol Testing
314+
315+
1. **Compliance Test Coverage**:
316+
- Test all RFC-defined error codes and responses
317+
- Validate proper HTTP status codes for different scenarios
318+
- Test protocol-specific edge cases (URI formats, token formats, etc.)
319+
320+
2. **Security Boundary Testing**:
321+
- Test client isolation and privilege separation
322+
- Verify information disclosure protections
323+
- Test token security and proper invalidation
324+
273325
## Code Navigation and Investigation
274326

275327
### Using Go LSP Tools (STRONGLY RECOMMENDED)
@@ -409,3 +461,67 @@ Always run the full test suite after OAuth2 changes:
409461
7. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go`
410462
8. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly
411463
9. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
464+
10. **Race conditions in tests** - Use unique identifiers instead of hardcoded names
465+
11. **RFC compliance failures** - Verify against actual RFC specifications, not assumptions
466+
12. **Authorization context errors in public endpoints** - Use `dbauthz.AsSystemRestricted(ctx)` pattern
467+
13. **Default value mismatches** - Ensure database migrations match application code defaults
468+
14. **Bearer token authentication issues** - Check token extraction precedence and format validation
469+
15. **URI validation failures** - Support both standard schemes and custom schemes per protocol requirements
470+
16. **Log message formatting errors** - Use lowercase, descriptive messages without special characters
471+
472+
## Systematic Debugging Approach
473+
474+
### Multi-Issue Problem Solving
475+
476+
When facing multiple failing tests or complex integration issues:
477+
478+
1. **Identify Root Causes**:
479+
- Run failing tests individually to isolate issues
480+
- Use LSP tools to trace through call chains
481+
- Check both compilation and runtime errors
482+
483+
2. **Fix in Logical Order**:
484+
- Address compilation issues first (imports, syntax)
485+
- Fix authorization and RBAC issues next
486+
- Resolve business logic and validation issues
487+
- Handle edge cases and race conditions last
488+
489+
3. **Verification Strategy**:
490+
- Test each fix individually before moving to next issue
491+
- Use `make lint` and `make gen` after database changes
492+
- Verify RFC compliance with actual specifications
493+
- Run comprehensive test suites before considering complete
494+
495+
### Authorization Context Patterns
496+
497+
Common patterns for different endpoint types:
498+
499+
```go
500+
// Public endpoints needing system access (OAuth2 registration)
501+
app, err := api.Database.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID)
502+
503+
// Authenticated endpoints with user context
504+
app, err := api.Database.GetOAuth2ProviderAppByClientID(ctx, clientID)
505+
506+
// System operations in middleware
507+
roles, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), userID)
508+
```
509+
510+
## Protocol Implementation Checklist
511+
512+
### OAuth2/Authentication Protocol Implementation
513+
514+
Before completing OAuth2 or authentication feature work:
515+
516+
- [ ] Verify RFC compliance by reading actual specifications
517+
- [ ] Implement proper error response formats per protocol
518+
- [ ] Add comprehensive validation for all protocol fields
519+
- [ ] Test security boundaries and token handling
520+
- [ ] Update RBAC permissions for new resources
521+
- [ ] Add audit logging support if applicable
522+
- [ ] Create database migrations with proper defaults
523+
- [ ] Update in-memory database implementations
524+
- [ ] Add comprehensive test coverage including edge cases
525+
- [ ] Verify linting and formatting compliance
526+
- [ ] Test both positive and negative scenarios
527+
- [ ] Document protocol-specific patterns and requirements

0 commit comments

Comments
 (0)
0