From 7796c38c086aa18b6e59135edc86bbce26ec956d Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sun, 23 Nov 2025 11:16:06 +1100 Subject: [PATCH] Add pairwise SID with a UUIDv4, a significatant upgrade over User.id.to_s. Complete allowing admin to enforce TOTP per user --- README.md | 5 +- app/controllers/admin/users_controller.rb | 18 +++-- app/controllers/oidc_controller.rb | 98 ++++++++++++++++++++--- app/controllers/sessions_controller.rb | 39 +++++++-- app/controllers/totp_controller.rb | 42 +++++++++- app/models/oidc_user_consent.rb | 10 +++ app/models/user.rb | 4 +- app/services/oidc_jwt_service.rb | 46 ++++++++--- app/views/admin/users/_form.html.erb | 19 +++++ app/views/admin/users/index.html.erb | 23 +++--- app/views/profiles/show.html.erb | 57 +++++++++---- app/views/totp/backup_codes.html.erb | 9 ++- config/routes.rb | 1 + db/schema.rb | 4 +- test/services/oidc_jwt_service_test.rb | 92 ++++++++++++++++++++- 15 files changed, 398 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index cb53f4d..a29594f 100644 --- a/README.md +++ b/README.md @@ -76,11 +76,11 @@ Clinch sits in a sweet spot between two excellent open-source identity solutions - **User statuses** - Active, disabled, or pending invitation ### Authentication Methods +- **WebAuthn/Passkeys** - Modern passwordless authentication using FIDO2 standards - **Password authentication** - Secure bcrypt-based password storage -- **Magic login links** - Passwordless login via email (15-minute expiry) - **TOTP 2FA** - Optional time-based one-time passwords with QR code setup - **Backup codes** - 10 single-use recovery codes per user -- **Configurable 2FA enforcement** - Admins can require TOTP for specific users/groups +- **Configurable 2FA enforcement** - Admins can require TOTP for specific users ### SSO Protocols @@ -96,6 +96,7 @@ Features: - **Refresh tokens** - Long-lived tokens (30 days default) with automatic rotation and revocation - **Configurable token expiry** - Set access token (5min-24hr), refresh token (1-90 days), and ID token TTL per application - **Token security** - BCrypt-hashed tokens, automatic cleanup of expired tokens +- **Pairwise subject identifiers** - Each user gets a unique, stable `sub` claim per application for enhanced privacy Client apps (Audiobookshelf, Kavita, Grafana, etc.) redirect to Clinch for login and receive ID tokens, access tokens, and refresh tokens. diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 2df6bc5..a74d4a9 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -30,13 +30,7 @@ module Admin end def update - # Prevent changing params for the current user's email and admin status - # to avoid locking themselves out - update_params = user_params.dup - - if @user == Current.session.user - update_params.delete(:admin) - end + update_params = user_params # Only update password if provided update_params.delete(:password) if update_params[:password].blank? @@ -76,7 +70,15 @@ module Admin end def user_params - params.require(:user).permit(:email_address, :name, :password, :admin, :status, custom_claims: {}) + # Base attributes that all admins can modify + base_params = params.require(:user).permit(:email_address, :name, :password, :status, :totp_required, custom_claims: {}) + + # Only allow modifying admin status when editing other users (prevent self-demotion) + if params[:id] != Current.session.user.id.to_s + base_params[:admin] = params[:user][:admin] if params[:user][:admin].present? + end + + base_params end end end diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 829dbb6..7b424b7 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -365,8 +365,17 @@ class OidcController < ApplicationController scope: auth_code.scope ) - # Generate ID token (JWT) - id_token = OidcJwtService.generate_id_token(user, application, nonce: auth_code.nonce) + # Find user consent for this application + consent = OidcUserConsent.find_by(user: user, application: application) + + unless consent + Rails.logger.error "OIDC Security: Token requested without consent record (user: #{user.id}, app: #{application.id})" + render json: { error: "invalid_grant", error_description: "Authorization consent not found" }, status: :bad_request + return + end + + # Generate ID token (JWT) with pairwise SID + id_token = OidcJwtService.generate_id_token(user, application, consent: consent, nonce: auth_code.nonce) # Return tokens render json: { @@ -457,8 +466,17 @@ class OidcController < ApplicationController token_family_id: refresh_token_record.token_family_id # Keep same family for rotation tracking ) - # Generate new ID token (JWT, no nonce for refresh grants) - id_token = OidcJwtService.generate_id_token(user, application) + # Find user consent for this application + consent = OidcUserConsent.find_by(user: user, application: application) + + unless consent + Rails.logger.error "OIDC Security: Refresh token used without consent record (user: #{user.id}, app: #{application.id})" + render json: { error: "invalid_grant", error_description: "Authorization consent not found" }, status: :bad_request + return + end + + # Generate new ID token (JWT with pairwise SID, no nonce for refresh grants) + id_token = OidcJwtService.generate_id_token(user, application, consent: consent) # Return new tokens render json: { @@ -498,9 +516,13 @@ class OidcController < ApplicationController return end + # Find user consent for this application to get pairwise SID + consent = OidcUserConsent.find_by(user: user, application: access_token.application) + subject = consent&.sid || user.id.to_s + # Return user claims claims = { - sub: user.id.to_s, + sub: subject, email: user.email_address, email_verified: true, preferred_username: user.email_address, @@ -609,11 +631,19 @@ class OidcController < ApplicationController reset_session end - # If post_logout_redirect_uri is provided, redirect there + # If post_logout_redirect_uri is provided, validate and redirect if post_logout_redirect_uri.present? - redirect_uri = post_logout_redirect_uri - redirect_uri += "?state=#{state}" if state.present? - redirect_to redirect_uri, allow_other_host: true + validated_uri = validate_logout_redirect_uri(post_logout_redirect_uri) + + if validated_uri + redirect_uri = validated_uri + redirect_uri += "?state=#{state}" if state.present? + redirect_to redirect_uri, allow_other_host: true + else + # Invalid redirect URI - log warning and go to default + Rails.logger.warn "OIDC Logout: Invalid post_logout_redirect_uri attempted: #{post_logout_redirect_uri}" + redirect_to root_path + end else # Default redirect to home page redirect_to root_path @@ -685,4 +715,54 @@ class OidcController < ApplicationController [params[:client_id], params[:client_secret]] end end + + def validate_logout_redirect_uri(uri) + return nil unless uri.present? + + begin + parsed_uri = URI.parse(uri) + + # Only allow HTTP/HTTPS schemes (prevent javascript:, data:, etc.) + return nil unless parsed_uri.is_a?(URI::HTTP) || parsed_uri.is_a?(URI::HTTPS) + + # Only allow HTTPS in production + return nil if Rails.env.production? && parsed_uri.scheme != 'https' + + # Check if URI matches any registered OIDC application's redirect URIs + # According to OIDC spec, post_logout_redirect_uri should be pre-registered + Application.oidc.active.find_each do |app| + # Check if this URI matches any of the app's registered redirect URIs + if app.parsed_redirect_uris.any? { |registered_uri| logout_uri_matches?(uri, registered_uri) } + return uri + end + end + + # No matching application found + nil + rescue URI::InvalidURIError + nil + end + end + + # Check if logout URI matches a registered redirect URI + # More lenient than exact match - allows same host/path with different query params + def logout_uri_matches?(provided, registered) + # Exact match is always valid + return true if provided == registered + + # Parse both URIs to compare components + begin + provided_parsed = URI.parse(provided) + registered_parsed = URI.parse(registered) + + # Match if scheme, host, port, and path are the same + # (allows different query params which is common for logout redirects) + provided_parsed.scheme == registered_parsed.scheme && + provided_parsed.host == registered_parsed.host && + provided_parsed.port == registered_parsed.port && + provided_parsed.path == registered_parsed.path + rescue URI::InvalidURIError + false + end + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 126cadc..29bb35e 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -6,7 +6,18 @@ class SessionsController < ApplicationController def new # Redirect to signup if this is first run - redirect_to signup_path if User.count.zero? + if User.count.zero? + respond_to do |format| + format.html { redirect_to signup_path } + format.json { render json: { error: "No users exist. Please complete initial setup." }, status: :service_unavailable } + end + return + end + + respond_to do |format| + format.html # render HTML login page + format.json { render json: { error: "Authentication required" }, status: :unauthorized } + end end def create @@ -33,8 +44,22 @@ class SessionsController < ApplicationController return end - # Check if TOTP is required - if user.totp_enabled? + # Check if TOTP is required or enabled + if user.totp_required? || user.totp_enabled? + # If TOTP is required but not yet set up, redirect to setup + if user.totp_required? && !user.totp_enabled? + # Store user ID in session for TOTP setup + session[:pending_totp_setup_user_id] = user.id + # Preserve the redirect URL through TOTP setup + if params[:rd].present? + validated_url = validate_redirect_url(params[:rd]) + session[:totp_redirect_url] = validated_url if validated_url + end + redirect_to new_totp_path, alert: "Your administrator requires two-factor authentication. Please set it up now to continue." + return + end + + # TOTP is enabled, proceed to verification # Store user ID in session temporarily for TOTP verification session[:pending_totp_user_id] = user.id # Preserve the redirect URL through TOTP verification (after validation) @@ -275,12 +300,12 @@ class SessionsController < ApplicationController redirect_domain = uri.host.downcase return nil unless redirect_domain.present? - # Check against our ForwardAuthRules - matching_rule = ForwardAuthRule.active.find do |rule| - rule.matches_domain?(redirect_domain) + # Check against our forward auth applications + matching_app = Application.forward_auth.active.find do |app| + app.matches_domain?(redirect_domain) end - matching_rule ? url : nil + matching_app ? url : nil rescue URI::InvalidURIError nil diff --git a/app/controllers/totp_controller.rb b/app/controllers/totp_controller.rb index 37d2553..75d1513 100644 --- a/app/controllers/totp_controller.rb +++ b/app/controllers/totp_controller.rb @@ -5,6 +5,9 @@ class TotpController < ApplicationController # GET /totp/new - Show QR code to set up TOTP def new + # Check if user is being forced to set up TOTP by admin + @totp_setup_required = session[:pending_totp_setup_user_id].present? + # Generate TOTP secret but don't save yet @totp_secret = ROTP::Base32.random @provisioning_uri = ROTP::TOTP.new(@totp_secret, issuer: "Clinch").provisioning_uri(@user.email_address) @@ -30,8 +33,16 @@ class TotpController < ApplicationController # Store plain codes temporarily in session for display after redirect session[:temp_backup_codes] = plain_codes - # Redirect to backup codes page with success message - redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes now." + # Check if this was a required setup from login + if session[:pending_totp_setup_user_id].present? + session.delete(:pending_totp_setup_user_id) + # Mark that user should be auto-signed in after viewing backup codes + session[:auto_signin_after_forced_totp] = true + redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes, then you'll be signed in." + else + # Regular setup from profile + redirect_to backup_codes_totp_path, notice: "Two-factor authentication has been enabled successfully! Save these backup codes now." + end else redirect_to new_totp_path, alert: "Invalid verification code. Please try again." end @@ -43,6 +54,12 @@ class TotpController < ApplicationController if session[:temp_backup_codes].present? @backup_codes = session[:temp_backup_codes] session.delete(:temp_backup_codes) # Clear after use + + # Check if this was a forced TOTP setup during login + @auto_signin_pending = session[:auto_signin_after_forced_totp].present? + if @auto_signin_pending + session.delete(:auto_signin_after_forced_totp) + end else # This will be shown after password verification for existing users # Since we can't display BCrypt hashes, redirect to regenerate @@ -81,6 +98,18 @@ class TotpController < ApplicationController redirect_to backup_codes_totp_path, notice: "New backup codes have been generated. Save them now!" end + # POST /totp/complete_setup - Complete forced TOTP setup and sign in + def complete_setup + # Sign in the user after they've saved their backup codes + # This is only used when admin requires TOTP and user just set it up during login + if session[:totp_redirect_url].present? + session[:return_to_after_authenticating] = session.delete(:totp_redirect_url) + end + + start_new_session_for @user + redirect_to after_authentication_url, notice: "Two-factor authentication enabled. Signed in successfully.", allow_other_host: true + end + # DELETE /totp - Disable TOTP (requires password) def destroy unless @user.authenticate(params[:password]) @@ -88,6 +117,12 @@ class TotpController < ApplicationController return end + # Prevent disabling if admin requires TOTP + if @user.totp_required? + redirect_to profile_path, alert: "Two-factor authentication is required by your administrator and cannot be disabled." + return + end + @user.disable_totp! redirect_to profile_path, notice: "Two-factor authentication has been disabled." end @@ -99,7 +134,8 @@ class TotpController < ApplicationController end def redirect_if_totp_enabled - if @user.totp_enabled? + # Allow setup if admin requires it, even if already enabled (for regeneration) + if @user.totp_enabled? && !session[:pending_totp_setup_user_id].present? redirect_to profile_path, alert: "Two-factor authentication is already enabled." end end diff --git a/app/models/oidc_user_consent.rb b/app/models/oidc_user_consent.rb index 07ced38..09dcf2e 100644 --- a/app/models/oidc_user_consent.rb +++ b/app/models/oidc_user_consent.rb @@ -6,6 +6,7 @@ class OidcUserConsent < ApplicationRecord validates :user_id, uniqueness: { scope: :application_id } before_validation :set_granted_at, on: :create + before_validation :set_sid, on: :create # Parse scopes_granted into an array def scopes @@ -44,9 +45,18 @@ class OidcUserConsent < ApplicationRecord end.join(', ') end + # Find consent by SID + def self.find_by_sid(sid) + find_by(sid: sid) + end + private def set_granted_at self.granted_at ||= Time.current end + + def set_sid + self.sid ||= SecureRandom.uuid + end end diff --git a/app/models/user.rb b/app/models/user.rb index 8cbbabd..fd3753f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,7 +44,9 @@ class User < ApplicationRecord end def disable_totp! - update!(totp_secret: nil, totp_required: false, backup_codes: nil) + # Note: This does NOT clear totp_required flag + # Admins control that flag via admin panel, users cannot remove admin-required 2FA + update!(totp_secret: nil, backup_codes: nil) end def totp_provisioning_uri(issuer: "Clinch") diff --git a/app/services/oidc_jwt_service.rb b/app/services/oidc_jwt_service.rb index 03978c4..712cf4a 100644 --- a/app/services/oidc_jwt_service.rb +++ b/app/services/oidc_jwt_service.rb @@ -1,14 +1,17 @@ class OidcJwtService class << self # Generate an ID token (JWT) for the user - def generate_id_token(user, application, nonce: nil) + def generate_id_token(user, application, consent: nil, nonce: nil) now = Time.current.to_i # Use application's configured ID token TTL (defaults to 1 hour) ttl = application.id_token_expiry_seconds + # Use pairwise SID from consent if available, fallback to user ID + subject = consent&.sid || user.id.to_s + payload = { iss: issuer_url, - sub: user.id.to_s, + sub: subject, aud: application.client_id, exp: now + ttl, iat: now, @@ -66,8 +69,13 @@ class OidcJwtService # In production, this should come from ENV or config # For now, we'll use a placeholder that can be overridden host = ENV.fetch("CLINCH_HOST", "localhost:3000") - # Ensure URL has https:// protocol - host.match?(/^https?:\/\//) ? host : "https://#{host}" + # Ensure URL has protocol - use https:// in production, http:// in development + if host.match?(/^https?:\/\//) + host + else + protocol = Rails.env.production? ? "https" : "http" + "#{protocol}://#{host}" + end end private @@ -75,17 +83,37 @@ class OidcJwtService # Get or generate RSA private key def private_key @private_key ||= begin + key_source = nil + # Try ENV variable first (best for Docker/Kamal) if ENV["OIDC_PRIVATE_KEY"].present? - OpenSSL::PKey::RSA.new(ENV["OIDC_PRIVATE_KEY"]) + key_source = ENV["OIDC_PRIVATE_KEY"] # Then try Rails credentials elsif Rails.application.credentials.oidc_private_key.present? - OpenSSL::PKey::RSA.new(Rails.application.credentials.oidc_private_key) + key_source = Rails.application.credentials.oidc_private_key + end + + if key_source.present? + begin + # Handle both actual newlines and escaped \n sequences + # Some .env loaders may escape newlines, so we need to convert them back + key_data = key_source.gsub("\\n", "\n") + OpenSSL::PKey::RSA.new(key_data) + rescue OpenSSL::PKey::RSAError => e + Rails.logger.error "OIDC: Failed to load private key: #{e.message}" + Rails.logger.error "OIDC: Key source length: #{key_source.length}, starts with: #{key_source[0..50]}" + raise "Invalid OIDC private key format. Please ensure the key is in PEM format with proper newlines." + end else - # Generate a new key for development - # In production, you MUST set OIDC_PRIVATE_KEY env var or add to credentials + # In production, we should never generate a key on the fly + # because it would be different across servers/deployments + if Rails.env.production? + raise "OIDC private key not configured. Set OIDC_PRIVATE_KEY environment variable or add to Rails credentials." + end + + # Generate a new key for development/test only Rails.logger.warn "OIDC: No private key found in ENV or credentials, generating new key (development only)" - Rails.logger.warn "OIDC: Set OIDC_PRIVATE_KEY environment variable in production!" + Rails.logger.warn "OIDC: Set OIDC_PRIVATE_KEY environment variable for consistency across restarts" OpenSSL::PKey::RSA.new(2048) end end diff --git a/app/views/admin/users/_form.html.erb b/app/views/admin/users/_form.html.erb index 68bc915..9811e0b 100644 --- a/app/views/admin/users/_form.html.erb +++ b/app/views/admin/users/_form.html.erb @@ -35,6 +35,25 @@ <% end %> +
+
+ <%= form.check_box :totp_required, class: "h-4 w-4 rounded border-gray-300 text-blue-600 focus:ring-blue-500" %> + <%= form.label :totp_required, "Require Two-Factor Authentication", class: "ml-2 block text-sm text-gray-900" %> + <% if user.totp_required? && !user.totp_enabled? %> + (User has not set up 2FA yet) + <% end %> +
+ <% if user.totp_required? && !user.totp_enabled? %> +

+ + + + Warning: This user will be prompted to set up 2FA on their next login. +

+ <% end %> +

When enabled, this user must use two-factor authentication to sign in.

+
+
<%= form.label :custom_claims, "Custom Claims (JSON)", class: "block text-sm font-medium text-gray-700" %> <%= form.text_area :custom_claims, value: (user.custom_claims.present? ? JSON.pretty_generate(user.custom_claims) : ""), rows: 8, diff --git a/app/views/admin/users/index.html.erb b/app/views/admin/users/index.html.erb index 66a020a..2c1f6d5 100644 --- a/app/views/admin/users/index.html.erb +++ b/app/views/admin/users/index.html.erb @@ -85,15 +85,20 @@ <% end %> - <% if user.totp_enabled? %> - - - - <% else %> - - - - <% end %> +
+ <% if user.totp_enabled? %> + + + + <% else %> + + + + <% end %> + <% if user.totp_required? %> + Required + <% end %> +
<%= user.groups.count %> diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index 6a1f8fb..e909c6d 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -98,23 +98,52 @@

Two-factor authentication is enabled

+ <% if @user.totp_required? %> +

+ + + + Required by administrator +

+ <% end %>
-
- - -
+ <% if @user.totp_required? %> +
+
+ + + +

+ Your administrator requires two-factor authentication. You cannot disable it. +

+
+
+
+ +
+ <% else %> +
+ + +
+ <% end %> <% else %> <%= link_to new_totp_path, class: "inline-flex items-center rounded-md border border-transparent bg-blue-600 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" do %> Enable 2FA diff --git a/app/views/totp/backup_codes.html.erb b/app/views/totp/backup_codes.html.erb index b68fae2..24be041 100644 --- a/app/views/totp/backup_codes.html.erb +++ b/app/views/totp/backup_codes.html.erb @@ -45,8 +45,13 @@
- <%= link_to "Done", profile_path, - class: "inline-flex justify-center rounded-md border border-transparent bg-blue-600 py-2 px-4 text-sm font-medium text-white shadow-sm hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" %> + <% if @auto_signin_pending %> + <%= button_to "Continue to Sign In", complete_totp_setup_path, method: :post, + class: "inline-flex justify-center rounded-md border border-transparent bg-blue-600 py-2 px-4 text-sm font-medium text-white shadow-sm hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" %> + <% else %> + <%= link_to "Done", profile_path, + class: "inline-flex justify-center rounded-md border border-transparent bg-blue-600 py-2 px-4 text-sm font-medium text-white shadow-sm hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" %> + <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 152f801..19b45e7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,6 +67,7 @@ Rails.application.routes.draw do post '/totp/verify_password', to: 'totp#verify_password', as: :verify_password_totp get '/totp/regenerate_backup_codes', to: 'totp#regenerate_backup_codes', as: :regenerate_backup_codes_totp post '/totp/regenerate_backup_codes', to: 'totp#create_new_backup_codes', as: :create_new_backup_codes_totp + post '/totp/complete_setup', to: 'totp#complete_setup', as: :complete_totp_setup # WebAuthn (Passkeys) routes get '/webauthn/new', to: 'webauthn#new', as: :new_webauthn diff --git a/db/schema.rb b/db/schema.rb index a4378c1..79ed96a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2025_11_12_120314) do +ActiveRecord::Schema[8.1].define(version: 2025_11_22_235519) do create_table "application_groups", force: :cascade do |t| t.integer "application_id", null: false t.datetime "created_at", null: false @@ -120,10 +120,12 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_12_120314) do t.datetime "created_at", null: false t.datetime "granted_at", null: false t.text "scopes_granted", null: false + t.string "sid" t.datetime "updated_at", null: false t.integer "user_id", null: false t.index ["application_id"], name: "index_oidc_user_consents_on_application_id" t.index ["granted_at"], name: "index_oidc_user_consents_on_granted_at" + t.index ["sid"], name: "index_oidc_user_consents_on_sid" t.index ["user_id", "application_id"], name: "index_oidc_user_consents_on_user_id_and_application_id", unique: true t.index ["user_id"], name: "index_oidc_user_consents_on_user_id" end diff --git a/test/services/oidc_jwt_service_test.rb b/test/services/oidc_jwt_service_test.rb index 9a5621f..14db9b8 100644 --- a/test/services/oidc_jwt_service_test.rb +++ b/test/services/oidc_jwt_service_test.rb @@ -111,11 +111,95 @@ class OidcJwtServiceTest < ActiveSupport::TestCase refute_includes decoded, 'roles', "Should not have roles when not configured" end - test "should use RSA private key from environment" do - ENV.stub(:fetch, "OIDC_PRIVATE_KEY") { "test-private-key" } + test "should load RSA private key from environment with escaped newlines" do + # Simulate how direnv exports multi-line strings with \n escape sequences + key_with_escaped_newlines = "-----BEGIN PRIVATE KEY-----\\nMIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDg3SfOR4UW6wV2\\nyKnE/pN5/tvUC7Fpol5/NjJQHm24F8+r6iipdLWJrJ3T2oEzaKw/RTGYPBQvjj6c\\nz3+tc7QkJLOESJCA0WqgawE1WdKSx5ug3sP0Y7woTPipt+afGaV58YvV/sqFD1ft\\nU+2w8olBHqWphUCd/LakfvqHbwrmF58IASk4IbGceqQ7f98d/8C8TrR6k3SKQAto\\n0OWo+xuyJg0RoSS8S220/qyIukXxtHS89NQj3dgJI06fGCSATCu8uVdsKwBDNw3F\\nBSQEX3xhk8E/JXXZfwRFR1K3zUIVQu8haQ3YA52b0jkzE2xI6TaHVbuGdifmGAmX\\nb5jsJ/eNAgMBAAECggEAAWJb3PwlOUANWTe630Pp1OegV5M1Tn2vi+oQPosPl1iX\\nFlbymrj80EfaRPWo84oKnq0t1/RnogrbDa3txgdpSVCsEWk9N2SyoJXy8+MZu6Er\\nQHka8qfBVfe4PbHyRj3FSeQKvZOEvvOgNJkYpIFeb5zkHa1ISyloEWvAxr0njJbQ\\n0F2jML4sUeduYulCWI9dSJdB+yp8BsmOPu8VzUFthW/GPPuw4a4ngzoGtPV6f/kp\\ncjPa2YT8L8z6zXE0IiDU8bc5abC++QBNLJrMy55tM+zfgGyShandITbcpuWptIqT\\n2yhMulifOMw0hdV0cYRqetkWkevz07nrwnh/1FGjYQKBgQD9C/Ls720tULS7SIdh\\nuDWnrtMG4sidSbxWJTOqPUNZ9a0vaHnx/FwlmvURyCojn5leLByY8ZNN08DxKBVq\\nwH6ZJe7KGOik5wMtFV1zrhyHNpa/H/RrLaYAZqCVlGYyOVqNa7mA7oOIeqtbv9x+\\nOaEz3BnoXHOJOwM10h20Nos6bQKBgQDjfQCSQXcrkV8hKf+F65N7Kcf7JMlZQAA3\\n9dvJxxek683bhYTLZhubY/tegfhxlZGkgP3eHKI1XyUYBCNBnztn3t1zD0ovcqRX\\no21m5TaJ0fGW4X3iyi1IWioMBPXffR8tXk5+LnWVZ26RgmaBG1rgOJEQ5bHYMtHj\\n+jo9JLV9oQKBgQDt1nNHm2qEcxzMAsmsYVWc+8bA7BsfKxTn6yN6WQaa4T0cGBi2\\nBzoc5l59jiN9RB8E0nU2k6ieN+9bOw+WPMNA8tRUA8F2bOMhVrl1ZyrNM9PQZBp5\\nOniSW+OHc+nyPtILpjq/Im9isdmp7NUzlrsbYT7AlVTKoTrNNWZR4gpOqQKBgQC3\\nIWwSUS00H4TrV7nh/zDsl0fr/0Mv2/vRENTsbJ+2HjXMIII0k3Bp+WTkQdDU70kd\\nmtHDul1CheOAn+QZ8auLBLhU5dwcsjdmbaOmj6MF88J+aexDY+psMlli76NXVIyC\\no0ahAZmaunciIE2QZYsUsbTmW2J93vtkgY3cpu6LwQKBgDigl7dCQl38Vt7FhxjJ\\naC6wmmM8YX6y5f5t3caVVBizVhx8xOXQla96zB0nW6ibTpaIKCSdORxMGAoajTZ9\\n8Ww2gOfZpZeojU2YHTV/KFd7wHGYE8QaBKqP6DuibLnP5farjuwPeGvbjZW6e9cy\\nntHkSPI0VmhqsUQEMgPnYuCg\\n-----END PRIVATE KEY-----" - private_key = @service.private_key - assert_equal "test-private-key", private_key.to_s, "Should use private key from environment" + # Clear any cached keys + OidcJwtService.instance_variable_set(:@private_key, nil) + + # Stub ENV to return the test key + original_value = ENV["OIDC_PRIVATE_KEY"] + ENV["OIDC_PRIVATE_KEY"] = key_with_escaped_newlines + + # The service should convert \n to actual newlines and load successfully + private_key = OidcJwtService.send(:private_key) + + assert_not_nil private_key + assert_kind_of OpenSSL::PKey::RSA, private_key + assert_equal 2048, private_key.n.num_bits + ensure + # Restore original value and clear cached key + ENV["OIDC_PRIVATE_KEY"] = original_value + OidcJwtService.instance_variable_set(:@private_key, nil) + end + + test "should handle key with actual newlines" do + # Generate a real test key + test_key = OpenSSL::PKey::RSA.new(2048) + key_pem = test_key.to_pem + + # Clear any cached keys + OidcJwtService.instance_variable_set(:@private_key, nil) + + # Stub ENV to return the test key + original_value = ENV["OIDC_PRIVATE_KEY"] + ENV["OIDC_PRIVATE_KEY"] = key_pem + + private_key = OidcJwtService.send(:private_key) + + assert_not_nil private_key + assert_kind_of OpenSSL::PKey::RSA, private_key + assert_equal 2048, private_key.n.num_bits + ensure + # Restore original value and clear cached key + ENV["OIDC_PRIVATE_KEY"] = original_value + OidcJwtService.instance_variable_set(:@private_key, nil) + end + + test "should raise error for invalid key format" do + # Clear any cached keys + OidcJwtService.instance_variable_set(:@private_key, nil) + + # Stub ENV to return invalid key + original_value = ENV["OIDC_PRIVATE_KEY"] + ENV["OIDC_PRIVATE_KEY"] = "invalid-key-data" + + error = assert_raises RuntimeError do + OidcJwtService.send(:private_key) + end + + assert_match /Invalid OIDC private key format/, error.message + ensure + # Restore original value and clear cached key + ENV["OIDC_PRIVATE_KEY"] = original_value + OidcJwtService.instance_variable_set(:@private_key, nil) + end + + test "should raise error in production when no key configured" do + # Skip this test if we can't properly stub Rails.env + skip "Skipping production env test" unless Rails.env.development? || Rails.env.test? + + # Clear any cached keys + OidcJwtService.instance_variable_set(:@private_key, nil) + + # Temporarily remove the key + original_value = ENV["OIDC_PRIVATE_KEY"] + ENV.delete("OIDC_PRIVATE_KEY") + + # Stub Rails.env to be production + Rails.env = ActiveSupport::StringInquirer.new("production") + + error = assert_raises RuntimeError do + OidcJwtService.send(:private_key) + end + + assert_match /OIDC private key not configured/, error.message + ensure + # Restore original environment and clear cached key + ENV["OIDC_PRIVATE_KEY"] = original_value if original_value + Rails.env = ActiveSupport::StringInquirer.new(ENV.fetch("RAILS_ENV", "test")) + OidcJwtService.instance_variable_set(:@private_key, nil) end test "should generate RSA private key when missing" do