8000 feat: implement OAuth2 dynamic client registration (RFC 7591/7592) (#… · coder/coder@74e1d5c · GitHub
[go: up one dir, main page]
Skip to content

Commit 74e1d5c

Browse files
ThomasK33ThomasK33
authored
feat: implement OAuth2 dynamic client registration (RFC 7591/7592) (#18645)
# 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 the `oauth2_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.
1 parent 699dd8e commit 74e1d5c

30 files changed

+5798
-129
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< 8000 /code>+
- 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