Files
clinch/docs/claude-review.md
Dan Milne e36a9a781a
Some checks failed
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled
Add new claims to the discovery endpoint
2025-12-31 17:27:28 +11:00

10 KiB

Clinch - Independent Code Review

Reviewer: Claude (Anthropic) Review Date: December 2024 Codebase Version: Commit 4f31fad Review Type: Security-focused OIDC/OAuth2 correctness review with full application assessment


Executive Summary

Clinch is a self-hosted identity and SSO portal built with Ruby on Rails. This review examined the complete codebase with particular focus on the OIDC/OAuth2 implementation, comparing it against production-grade reference implementations (Rodauth-OAuth, Authelia, Authentik).

Overall Assessment: Production-Ready

The implementation demonstrates solid security practices, proper adherence to OAuth 2.0 and OpenID Connect specifications, and comprehensive test coverage. The codebase is well-structured, readable, and maintainable.


Feature Overview

Authentication Methods

Feature Status Notes
Password Authentication Implemented bcrypt hashing, rate-limited
WebAuthn/Passkeys Implemented FIDO2 compliant, clone detection
TOTP 2FA Implemented With backup codes, admin enforcement
Session Management Implemented Device tracking, revocation

SSO Protocols

Protocol Status Notes
OpenID Connect Implemented Full OIDC Core compliance
OAuth 2.0 Implemented Authorization Code + Refresh Token grants
ForwardAuth Implemented Traefik/Caddy/nginx compatible

User & Access Management

Feature Status Notes
User CRUD Implemented Invitation flow, status management
Group Management Implemented With custom claims
Application Management Implemented OIDC + ForwardAuth types
Group-based Access Control Implemented Per-application restrictions

OIDC/OAuth2 Implementation Review

Specification Compliance

Specification Status Evidence
RFC 6749 (OAuth 2.0) Compliant Proper auth code flow, client authentication
RFC 7636 (PKCE) Compliant S256 and plain methods, enforced for public clients
RFC 7009 (Token Revocation) Compliant Always returns 200 OK, prevents scanning
OpenID Connect Core 1.0 Compliant All required claims, proper JWT structure
OIDC Discovery Compliant .well-known/openid-configuration
OIDC Back-Channel Logout Compliant Logout tokens per spec

ID Token Claims

The implementation includes all required and recommended OIDC claims:

Standard:  iss, sub, aud, exp, iat, nonce
Profile:   email, email_verified, preferred_username, name
Security:  at_hash, auth_time, acr, azp
Custom:    groups, plus arbitrary claims from groups/users/apps

Token Security

Aspect Implementation Assessment
Authorization Codes HMAC-SHA256 hashed, 10-min expiry, single-use Secure
Access Tokens HMAC-SHA256 hashed, configurable TTL, indexed lookup Secure
Refresh Tokens HMAC-SHA256 hashed, rotation with family tracking Secure
ID Tokens RS256 signed JWTs Secure

Security Features

  1. Authorization Code Reuse Prevention

    • Pessimistic database locking prevents race conditions
    • Code reuse triggers revocation of all tokens from that code
    • Location: oidc_controller.rb:342-364
  2. Refresh Token Rotation

    • Old refresh tokens revoked on use
    • Token family tracking detects stolen token reuse
    • Revoked token reuse triggers family-wide revocation
    • Location: oidc_controller.rb:504-513
  3. PKCE Enforcement

    • Required for all public clients
    • Configurable for confidential clients
    • Proper S256 challenge verification
    • Location: oidc_controller.rb:749-814
  4. Pairwise Subject Identifiers

    • Each user gets a unique sub per application
    • Prevents cross-application user tracking
    • Location: oidc_user_consent.rb:59-61

Security Assessment

Strengths

  1. Token Storage Architecture

    • All tokens (auth codes, access, refresh) are HMAC-hashed before storage
    • Database compromise does not reveal usable tokens
    • O(1) indexed lookup via HMAC (not O(n) iteration)
  2. Rate Limiting

    • Sign-in: 20/3min
    • TOTP verification: 10/3min
    • Token endpoint: 60/min
    • Authorization: 30/min
    • WebAuthn enumeration check: 10/min
  3. WebAuthn Implementation

    • Sign count validation (clone detection)
    • Backup eligibility tracking
    • Platform vs roaming authenticator distinction
    • Credential enumeration prevention
  4. TOTP Implementation

    • Encrypted secret storage (ActiveRecord Encryption)
    • Backup codes are bcrypt-hashed and single-use
    • Admin can enforce TOTP requirement per user
  5. Session Security

    • ACR (Authentication Context Class Reference) tracking
    • acr: "1" for password-only, acr: "2" for 2FA/passkey
    • Session activity timestamps
    • Remote session revocation

Attack Mitigations

Attack Vector Mitigation
Credential Stuffing Rate limiting, account lockout via status
Token Theft HMAC storage, short-lived access tokens, rotation
Session Hijacking Secure cookies, session binding
CSRF Rails CSRF protection, state parameter validation
Open Redirect Strict redirect_uri validation against registered URIs
Authorization Code Injection PKCE enforcement, redirect_uri binding
Refresh Token Replay Token rotation, family-based revocation
User Enumeration Constant-time responses, rate limiting

Areas Reviewed (No Issues Found)

  • Redirect URI validation (exact match required)
  • Client authentication (bcrypt for secrets)
  • Error response handling (no sensitive data leakage in production)
  • Logout implementation (backchannel notifications, session cleanup)
  • Custom claims handling (reserved claim protection)

Code Quality Assessment

Architecture

Aspect Assessment
Controller Structure Clean separation, ~900 lines for OIDC (acceptable)
Model Design Well-normalized, proper associations
Service Objects Used appropriately (OidcJwtService, ClaimsMerger)
Concerns TokenPrefixable for shared hashing logic

Code Metrics

Controllers:     ~1,500 lines
Models:          ~1,500 lines
Services:        ~200 lines
Total App Code:  ~3,100 lines
Test Files:      36 files

Readability

  • Clear method naming
  • Inline documentation for complex logic
  • Consistent Ruby style
  • No deeply nested conditionals

Test Coverage

Test Statistics

Total Tests:    341
Assertions:     1,349
Failures:       0
Errors:         0
Run Time:       23.5 seconds (parallel)

Test Categories

Category Files Coverage
OIDC Security 2 Auth code reuse, token rotation, PKCE
Integration 4 WebAuthn, sessions, invitations, forward auth
Controllers 8 All major endpoints
Models 10 Validations, associations, business logic
Jobs 4 Mailers, token cleanup

Security-Specific Tests

The test suite includes dedicated security tests:

  • oidc_authorization_code_security_test.rb - Code reuse, timing attacks, client auth
  • oidc_pkce_controller_test.rb - PKCE flow validation
  • webauthn_credential_enumeration_test.rb - Enumeration prevention
  • session_security_test.rb - Session handling
  • totp_security_test.rb - 2FA bypass prevention
  • input_validation_test.rb - Input sanitization

Comparison with Reference Implementations

vs. Rodauth-OAuth (OpenID Certified)

Aspect Rodauth Clinch
Modularity 46 feature modules Monolithic controller
Token Storage Optional hashing HMAC-SHA256 (always)
PKCE Dedicated feature Integrated
Certification OpenID Certified Not certified

Clinch has comparable security but less modularity.

vs. Authelia (Production-Grade Go)

Aspect Authelia Clinch
PKCE Config always/public/never Per-app toggle
Key Rotation Supported Single key
PAR Support Yes No
DPoP Support Yes No

Clinch lacks some advanced features but covers core use cases.

vs. Authentik (Enterprise Python)

Aspect Authentik Clinch
Scale Enterprise/distributed Single instance
Protocols OAuth, SAML, LDAP, RADIUS OAuth/OIDC, ForwardAuth
Complexity High Low

Clinch is intentionally simpler for self-hosting.


Recommendations

Implemented During Review

The following issues were identified and fixed during this review:

  1. Token lookup performance - Changed from O(n) BCrypt iteration to O(1) HMAC lookup
  2. at_hash claim - Added per OIDC Core spec
  3. auth_time claim - Added for authentication timestamp
  4. acr claim - Added for authentication context class
  5. azp claim - Added for authorized party
  6. Authorization code hashing - Changed from plaintext to HMAC
  7. Consent SID preservation - Fixed to preserve pairwise subject ID
  8. Discovery metadata - Fixed subject_types_supported to ["pairwise"]

Optional Future Enhancements

Enhancement Priority Effort
Key Rotation (multi-key JWKS) Medium Medium
Token Introspection (RFC 7662) Low Low
PAR (RFC 9126) Low Medium
OpenID Certification Low High

Conclusion

Clinch provides a solid, security-conscious OIDC/OAuth2 implementation suitable for self-hosted identity management. The codebase demonstrates:

  • Correct protocol implementation - Follows OAuth 2.0 and OIDC specifications
  • Defense in depth - Multiple layers of security controls
  • Modern authentication - WebAuthn/passkeys, TOTP, proper session management
  • Maintainable code - Clear structure, good test coverage

The implementation is appropriate for its intended use case: a lightweight, self-hosted alternative to complex enterprise identity solutions.


Methodology

This review was conducted by examining:

  1. All OIDC-related controllers, models, and services
  2. Reference implementations (Rodauth-OAuth, Authelia, Authentik) in tmp/
  3. Test files and coverage
  4. Database schema and migrations
  5. Security-critical code paths

Tools used: Static analysis, code reading, test execution, comparison with OpenID-certified implementations.


This review was conducted by Claude (Anthropic) at the request of the project maintainer. The reviewer has no financial interest in the project.