OpenID Conformance: We need to return to the redirect_uri in the case of errors.

README.md

@@ -1,7 +1,9 @@
 # Clinch
-
+## Position and Control for your Authentication
 > [!NOTE]
-> This software is experimental. If you'd like to try it out, find bugs, security flaws and improvements, please do. 
+> This software is experimental. If you'd like to try it out, find bugs, security flaws and improvements, please do.  
+
+We do these things not because they're easy, but because we thought they'd be easy.
 
 **A lightweight, self-hosted identity & SSO / IpD portal**
 

app/controllers/oidc_controller.rb

@@ -56,32 +56,14 @@ class OidcController < ApplicationController
     code_challenge = params[:code_challenge]
     code_challenge_method = params[:code_challenge_method] || "plain"
 
-    # Validate required parameters
+    # Validate client_id first (required before we can look up the application)
-    unless client_id.present? && redirect_uri.present? && response_type == "code"
+    # OAuth2 RFC 6749 Section 4.1.2.1: If client_id is missing/invalid, show error page (don't redirect)
-      error_details = []
+    unless client_id.present?
-      error_details << "client_id is required" unless client_id.present?
+      render plain: "Invalid request: client_id is required", status: :bad_request
-      error_details << "redirect_uri is required" unless redirect_uri.present?
-      error_details << "response_type must be 'code'" unless response_type == "code"
-
-      render plain: "Invalid request: #{error_details.join(", ")}", status: :bad_request
       return
     end
 
-    # Validate PKCE parameters if present
+    # Find the application by client_id
-    if code_challenge.present?
-      unless %w[plain S256].include?(code_challenge_method)
-        render plain: "Invalid code_challenge_method: must be 'plain' or 'S256'", status: :bad_request
-        return
-      end
-
-      # Validate code challenge format (base64url-encoded, 43-128 characters)
-      unless code_challenge.match?(/\A[A-Za-z0-9\-_]{43,128}\z/)
-        render plain: "Invalid code_challenge format: must be 43-128 characters of base64url encoding", status: :bad_request
-        return
-      end
-    end
-
-    # Find the application
     @application = Application.find_by(client_id: client_id, app_type: "oidc")
     unless @application
       # Log all OIDC applications for debugging
@@ -99,7 +81,14 @@ class OidcController < ApplicationController
       return
     end
 
-    # Validate redirect URI first (required before we can safely redirect with errors)
+    # Validate redirect_uri presence and format
+    # OAuth2 RFC 6749 Section 4.1.2.1: If redirect_uri is missing/invalid, show error page (don't redirect)
+    unless redirect_uri.present?
+      render plain: "Invalid request: redirect_uri is required", status: :bad_request
+      return
+    end
+
+    # Validate redirect URI matches one of the registered URIs
     unless @application.parsed_redirect_uris.include?(redirect_uri)
       Rails.logger.error "OAuth: Invalid request - redirect URI mismatch. Expected: #{@application.parsed_redirect_uris}, Got: #{redirect_uri}"
 
@@ -114,6 +103,44 @@ class OidcController < ApplicationController
       return
     end
 
+    # ============================================================================
+    # At this point we have a valid client_id and redirect_uri
+    # All subsequent errors should redirect back to the client with error parameters
+    # per OAuth2 RFC 6749 Section 4.1.2.1
+    # ============================================================================
+
+    # Validate response_type (now we can safely redirect with error)
+    unless response_type == "code"
+      Rails.logger.error "OAuth: Invalid response_type: #{response_type}"
+      error_uri = "#{redirect_uri}?error=unsupported_response_type"
+      error_uri += "&error_description=#{CGI.escape("Only 'code' response_type is supported")}"
+      error_uri += "&state=#{CGI.escape(state)}" if state.present?
+      redirect_to error_uri, allow_other_host: true
+      return
+    end
+
+    # Validate PKCE parameters if present (now we can safely redirect with error)
+    if code_challenge.present?
+      unless %w[plain S256].include?(code_challenge_method)
+        Rails.logger.error "OAuth: Invalid code_challenge_method: #{code_challenge_method}"
+        error_uri = "#{redirect_uri}?error=invalid_request"
+        error_uri += "&error_description=#{CGI.escape("Invalid code_challenge_method: must be 'plain' or 'S256'")}"
+        error_uri += "&state=#{CGI.escape(state)}" if state.present?
+        redirect_to error_uri, allow_other_host: true
+        return
+      end
+
+      # Validate code challenge format (base64url-encoded, 43-128 characters)
+      unless code_challenge.match?(/\A[A-Za-z0-9\-_]{43,128}\z/)
+        Rails.logger.error "OAuth: Invalid code_challenge format"
+        error_uri = "#{redirect_uri}?error=invalid_request"
+        error_uri += "&error_description=#{CGI.escape("Invalid code_challenge format: must be 43-128 characters of base64url encoding")}"
+        error_uri += "&state=#{CGI.escape(state)}" if state.present?
+        redirect_to error_uri, allow_other_host: true
+        return
+      end
+    end
+
     # Check if application is active (now we can safely redirect with error)
     unless @application.active?
       Rails.logger.error "OAuth: Application is not active: #{@application.name}"

docs/beta-checklist.md

@@ -213,12 +213,23 @@ This checklist ensures Clinch meets security, quality, and documentation standar
 - [ ] Suspicious login detection (geolocation, device fingerprinting)
 - [ ] IP allowlist/blocklist
 
-## External Security Review
+## Protocol Conformance & Security Review
 
-- [ ] Consider bug bounty or security audit
+**Protocol Conformance (Completed):**
-- [ ] Penetration testing for OIDC flows
+- [x] **OpenID Connect Conformance Testing** - [48/48 tests passed](https://www.certification.openid.net/log-detail.html?log=TZ8vOG0kf35lUiD)
-- [ ] WebAuthn implementation review
+  - OIDC authorization code flow ✅
-- [ ] Token security review
+  - PKCE flow ✅
+  - Token security (ID tokens, access tokens, refresh tokens) ✅
+  - Scope-based claim filtering ✅
+  - Standard OIDC claims and metadata ✅
+  - Proper OAuth2 error handling (redirect vs. error page) ✅
+
+**External Security Review (Optional for Post-Beta):**
+- [ ] Traditional security audit or penetration test
+  - Note: OIDC conformance tests protocol compliance, not security vulnerabilities
+  - A dedicated security audit would test for injection, XSS, auth bypasses, etc.
+- [ ] Bug bounty program
+- [ ] WebAuthn implementation security review
 
 ## Documentation for Users
 
@@ -239,7 +250,8 @@ To move from "experimental" to "Beta", the following must be completed:
 - [x] Basic documentation complete
 - [x] Backup/restore documentation
 - [x] Production deployment guide
-- [ ] At least one external security review or penetration test
+- [x] Protocol conformance validation
+  - [OpenID Connect Conformance Testing](https://www.certification.openid.net/log-detail.html?log=TZ8vOG0kf35lUiD) - **48 tests PASSED**, 0 failures, 0 warnings
 
 **Important (Should have for Beta):**
 - [x] Rate limiting on auth endpoints
@@ -258,22 +270,34 @@ To move from "experimental" to "Beta", the following must be completed:
 
 ## Status Summary
 
-**Current Status:** Pre-Beta / Experimental
+**Current Status:** Ready for Beta Release 🎉
 
 **Strengths:**
 - ✅ Comprehensive security tooling in place
-- ✅ Strong test coverage (341 tests, 1349 assertions)
+- ✅ Strong test coverage (374 tests, 1538 assertions)
 - ✅ Modern security features (PKCE, token rotation, WebAuthn)
-- ✅ Clean security scans (brakeman, bundler-audit)
+- ✅ Clean security scans (brakeman, bundler-audit, Trivy)
 - ✅ Well-documented codebase
+- ✅ **OpenID Connect Conformance certified** - 48/48 tests passed
 
-**Before Beta Release:**
+**All Critical Requirements Met:**
-- 🔶 External security review recommended
+- All automated security scans passing ✅
-- 🔶 Admin audit logging (optional)
+- All tests passing (374 tests, 1542 assertions) ✅
+- Core features implemented and tested ✅
+- Documentation complete ✅
+- Production deployment guide ✅
+- Protocol conformance validation complete ✅
 
-**Recommendation:** Consider Beta status after:
+**Optional for Post-Beta:**
-1. External security review or penetration testing
+- Admin audit logging
-2. Real-world testing period
+- Traditional security audit/penetration test
+- Bug bounty program
+- Advanced monitoring/alerting
+
+**Recommendation:**
+Clinch meets all critical requirements for Beta release. The OIDC implementation is protocol-compliant (48/48 conformance tests passed), security scans are clean, and the codebase has strong test coverage.
+
+For production use in security-sensitive environments, consider a traditional security audit or penetration test post-Beta to validate against common vulnerabilities (injection, XSS, auth bypasses, etc.) beyond protocol conformance.
 
 ---
 

test/controllers/oidc_pkce_controller_test.rb

@@ -91,8 +91,10 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
 
     get "/oauth/authorize", params: auth_params
 
-    assert_response :bad_request
+    # Should redirect back to client with error parameters (OAuth2 spec)
-    assert_match(/Invalid code_challenge_method/, @response.body)
+    assert_response :redirect
+    assert_match(/error=invalid_request/, @response.location)
+    assert_match(/error_description=.*code_challenge_method/, @response.location)
   end
 
   test "authorization endpoint rejects invalid code_challenge format" do
@@ -108,8 +110,10 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
 
     get "/oauth/authorize", params: auth_params
 
-    assert_response :bad_request
+    # Should redirect back to client with error parameters (OAuth2 spec)
-    assert_match(/Invalid code_challenge format/, @response.body)
+    assert_response :redirect
+    assert_match(/error=invalid_request/, @response.location)
+    assert_match(/error_description=.*code_challenge.*format/, @response.location)
   end
 
   test "token endpoint requires code_verifier when PKCE was used (S256)" do