From cc7beba9deee2c327099a64073f1d1165e40a638 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Wed, 31 Dec 2025 09:22:18 +1100 Subject: [PATCH] PKCE is now default enabled. You can now create public / no-secret apps OIDC apps --- .../admin/applications_controller.rb | 43 ++-- app/controllers/oidc_controller.rb | 72 +++++-- .../application_form_controller.js | 15 +- app/models/application.rb | 34 ++- app/models/oidc_refresh_token.rb | 1 - app/models/user.rb | 8 + app/views/admin/applications/_form.html.erb | 45 ++++ app/views/admin/applications/show.html.erb | 82 ++++++-- app/views/shared/_flash.html.erb | 2 + config/initializers/version.rb | 2 +- db/schema.rb | 3 +- .../oidc_authorization_code_security_test.rb | 6 +- test/controllers/oidc_pkce_controller_test.rb | 195 +++++++++++++++++- test/fixtures/applications.yml | 3 + .../forward_auth_integration_test.rb | 9 + 15 files changed, 456 insertions(+), 64 deletions(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index be50a90..01453cc 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -26,16 +26,17 @@ module Admin @application.allowed_groups = Group.where(id: group_ids) end - # Get the plain text client secret to show one time + # Get the plain text client secret to show one time (confidential clients only) client_secret = nil - if @application.oidc? + if @application.oidc? && @application.confidential_client? client_secret = @application.generate_new_client_secret! end - if @application.oidc? && client_secret + if @application.oidc? flash[:notice] = "Application created successfully." flash[:client_id] = @application.client_id - flash[:client_secret] = client_secret + flash[:client_secret] = client_secret if client_secret + flash[:public_client] = true if @application.public_client? else flash[:notice] = "Application created successfully." end @@ -74,15 +75,20 @@ module Admin def regenerate_credentials if @application.oidc? - # Generate new client ID and secret + # Generate new client ID (always) new_client_id = SecureRandom.urlsafe_base64(32) - client_secret = @application.generate_new_client_secret! - @application.update!(client_id: new_client_id) flash[:notice] = "Credentials regenerated successfully." flash[:client_id] = @application.client_id - flash[:client_secret] = client_secret + + # Generate new client secret only for confidential clients + if @application.confidential_client? + client_secret = @application.generate_new_client_secret! + flash[:client_secret] = client_secret + else + flash[:public_client] = true + end redirect_to admin_application_path(@application) else @@ -97,15 +103,24 @@ module Admin end def application_params - params.require(:application).permit( + permitted = params.require(:application).permit( :name, :slug, :app_type, :active, :redirect_uris, :description, :metadata, :domain_pattern, :landing_url, :access_token_ttl, :refresh_token_ttl, :id_token_ttl, - :icon, :backchannel_logout_uri, - headers_config: {} - ).tap do |whitelisted| - # Remove client_secret from params if present (shouldn't be updated via form) - whitelisted.delete(:client_secret) + :icon, :backchannel_logout_uri, :is_public_client, :require_pkce + ) + + # Handle headers_config - it comes as a JSON string from the text area + if params[:application][:headers_config].present? + begin + permitted[:headers_config] = JSON.parse(params[:application][:headers_config]) + rescue JSON::ParserError + permitted[:headers_config] = {} + end end + + # Remove client_secret from params if present (shouldn't be updated via form) + permitted.delete(:client_secret) + permitted end end end diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index c53093f..d6aa5d9 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -296,22 +296,33 @@ class OidcController < ApplicationController end def handle_authorization_code_grant - # Get client credentials from Authorization header or params client_id, client_secret = extract_client_credentials - unless client_id && client_secret - render json: { error: "invalid_client" }, status: :unauthorized + unless client_id + render json: { error: "invalid_client", error_description: "client_id is required" }, status: :unauthorized return end - # Find and validate the application + # Find the application application = Application.find_by(client_id: client_id) - unless application && application.authenticate_client_secret(client_secret) - render json: { error: "invalid_client" }, status: :unauthorized + unless application + render json: { error: "invalid_client", error_description: "Unknown client" }, status: :unauthorized return end + # Validate client credentials based on client type + if application.public_client? + # Public clients don't have a secret - they MUST use PKCE (checked later) + Rails.logger.info "OAuth: Public client authentication for #{application.name}" + else + # Confidential clients MUST provide valid client_secret + unless client_secret.present? && application.authenticate_client_secret(client_secret) + render json: { error: "invalid_client", error_description: "Invalid client credentials" }, status: :unauthorized + return + end + end + # Check if application is active unless application.active? Rails.logger.error "OAuth: Token request for inactive application: #{application.name}" @@ -371,8 +382,8 @@ class OidcController < ApplicationController return end - # Validate PKCE if code challenge is present - pkce_result = validate_pkce(auth_code, code_verifier) + # Validate PKCE - required for public clients and optionally for confidential clients + pkce_result = validate_pkce(application, auth_code, code_verifier) unless pkce_result[:valid] render json: { error: pkce_result[:error], @@ -433,18 +444,30 @@ class OidcController < ApplicationController # Get client credentials from Authorization header or params client_id, client_secret = extract_client_credentials - unless client_id && client_secret - render json: { error: "invalid_client" }, status: :unauthorized + unless client_id + render json: { error: "invalid_client", error_description: "client_id is required" }, status: :unauthorized return end - # Find and validate the application + # Find the application application = Application.find_by(client_id: client_id) - unless application && application.authenticate_client_secret(client_secret) - render json: { error: "invalid_client" }, status: :unauthorized + unless application + render json: { error: "invalid_client", error_description: "Unknown client" }, status: :unauthorized return end + # Validate client credentials based on client type + if application.public_client? + # Public clients don't have a secret + Rails.logger.info "OAuth: Public client refresh token request for #{application.name}" + else + # Confidential clients MUST provide valid client_secret + unless client_secret.present? && application.authenticate_client_secret(client_secret) + render json: { error: "invalid_client", error_description: "Invalid client credentials" }, status: :unauthorized + return + end + end + # Check if application is active unless application.active? Rails.logger.error "OAuth: Refresh token request for inactive application: #{application.name}" @@ -716,11 +739,26 @@ class OidcController < ApplicationController private - def validate_pkce(auth_code, code_verifier) - # Skip PKCE validation if no code challenge was stored (legacy clients) - return { valid: true } unless auth_code.code_challenge.present? + def validate_pkce(application, auth_code, code_verifier) + # Check if PKCE is required for this application + pkce_required = application.requires_pkce? + pkce_provided = auth_code.code_challenge.present? - # PKCE is required but no verifier provided + # If PKCE is required but wasn't provided during authorization + if pkce_required && !pkce_provided + client_type = application.public_client? ? "public clients" : "this application" + return { + valid: false, + error: "invalid_request", + error_description: "PKCE is required for #{client_type}. code_challenge must be provided during authorization.", + status: :bad_request + } + end + + # Skip validation if no code challenge was stored (legacy clients without PKCE requirement) + return { valid: true } unless pkce_provided + + # PKCE was provided during authorization but no verifier sent with token request unless code_verifier.present? return { valid: false, diff --git a/app/javascript/controllers/application_form_controller.js b/app/javascript/controllers/application_form_controller.js index 11f64ac..bb38760 100644 --- a/app/javascript/controllers/application_form_controller.js +++ b/app/javascript/controllers/application_form_controller.js @@ -1,7 +1,7 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["appTypeSelect", "oidcFields", "forwardAuthFields"] + static targets = ["appTypeSelect", "oidcFields", "forwardAuthFields", "pkceOptions"] connect() { this.updateFieldVisibility() @@ -21,4 +21,17 @@ export default class extends Controller { this.forwardAuthFieldsTarget.classList.add('hidden') } } + + updatePkceVisibility(event) { + // Show PKCE options for confidential clients, hide for public clients + const isPublicClient = event.target.value === "true" + + if (this.hasPkceOptionsTarget) { + if (isPublicClient) { + this.pkceOptionsTarget.classList.add('hidden') + } else { + this.pkceOptionsTarget.classList.remove('hidden') + } + } + } } \ No newline at end of file diff --git a/app/models/application.rb b/app/models/application.rb index a13ecc5..14c7c21 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -1,6 +1,10 @@ class Application < ApplicationRecord has_secure_password :client_secret, validations: false + # Virtual attribute to control client type during creation + # When true, no client_secret will be generated (public client) + attr_accessor :is_public_client + has_one_attached :icon # Fix SVG content type after attachment @@ -20,7 +24,7 @@ class Application < ApplicationRecord validates :app_type, presence: true, inclusion: { in: %w[oidc forward_auth] } validates :client_id, uniqueness: { allow_nil: true } - validates :client_secret, presence: true, on: :create, if: -> { oidc? } + validates :client_secret, presence: true, on: :create, if: -> { oidc? && confidential_client? } validates :domain_pattern, presence: true, uniqueness: { case_sensitive: false }, if: :forward_auth? validates :landing_url, format: { with: URI::regexp(%w[http https]), allow_nil: true, message: "must be a valid URL" } validates :backchannel_logout_uri, format: { @@ -74,6 +78,24 @@ class Application < ApplicationRecord app_type == "forward_auth" end + # Client type checks (for OIDC) + def public_client? + client_secret_digest.blank? + end + + def confidential_client? + !public_client? + end + + # PKCE requirement check + # Public clients MUST use PKCE (no client secret to protect auth code) + # Confidential clients can optionally require PKCE (OAuth 2.1 recommendation) + def requires_pkce? + return false unless oidc? + return true if public_client? # Always require PKCE for public clients + require_pkce? # Check the flag for confidential clients + end + # Access control def user_allowed?(user) return false unless active? @@ -261,13 +283,19 @@ class Application < ApplicationRecord def generate_client_credentials self.client_id ||= SecureRandom.urlsafe_base64(32) - # Generate and hash the client secret - if new_record? && client_secret.blank? + # Generate client secret only for confidential clients + # Public clients (is_public_client checked) don't get a secret - they use PKCE only + if new_record? && client_secret.blank? && !is_public_client_selected? secret = SecureRandom.urlsafe_base64(48) self.client_secret = secret end end + # Check if the user selected public client option + def is_public_client_selected? + ActiveModel::Type::Boolean.new.cast(is_public_client) + end + def backchannel_logout_uri_must_be_https_in_production return unless Rails.env.production? return unless backchannel_logout_uri.present? diff --git a/app/models/oidc_refresh_token.rb b/app/models/oidc_refresh_token.rb index 10a5147..d60a453 100644 --- a/app/models/oidc_refresh_token.rb +++ b/app/models/oidc_refresh_token.rb @@ -4,7 +4,6 @@ class OidcRefreshToken < ApplicationRecord belongs_to :application belongs_to :user belongs_to :oidc_access_token - has_many :oidc_access_tokens, foreign_key: :oidc_access_token_id, dependent: :nullify before_validation :generate_token_with_prefix, on: :create before_validation :set_expiry, on: :create diff --git a/app/models/user.rb b/app/models/user.rb index 756731e..20280f5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,6 +74,14 @@ class User < ApplicationRecord totp.verify(code, drift_behind: 30, drift_ahead: 30) end + # Console/debug helper: get current TOTP code + def console_totp + return nil unless totp_enabled? + + require "rotp" + ROTP::TOTP.new(totp_secret).now + end + def verify_backup_code(code) return false unless backup_codes.present? diff --git a/app/views/admin/applications/_form.html.erb b/app/views/admin/applications/_form.html.erb index 9d62c8a..3f55c55 100644 --- a/app/views/admin/applications/_form.html.erb +++ b/app/views/admin/applications/_form.html.erb @@ -120,6 +120,51 @@

OIDC Configuration

+ + <% unless application.persisted? %> +
+

Client Type

+
+
+ <%= form.radio_button :is_public_client, "false", checked: !application.is_public_client, class: "mt-1 h-4 w-4 border-gray-300 text-blue-600 focus:ring-blue-500", data: { action: "change->application-form#updatePkceVisibility" } %> +
+ +

Backend server app that can securely store a client secret. Examples: traditional web apps, server-to-server APIs.

+
+
+
+ <%= form.radio_button :is_public_client, "true", checked: application.is_public_client, class: "mt-1 h-4 w-4 border-gray-300 text-blue-600 focus:ring-blue-500", data: { action: "change->application-form#updatePkceVisibility" } %> +
+ +

Frontend-only app that cannot store secrets securely. Examples: SPAs (React/Vue), mobile apps, CLI tools. PKCE is required.

+
+
+
+
+ <% else %> + +
+ Client Type: + <% if application.public_client? %> + Public Client (PKCE Required) + <% else %> + Confidential Client + <% end %> +
+ <% end %> + + +
+
+ <%= form.check_box :require_pkce, class: "h-4 w-4 rounded border-gray-300 text-blue-600 focus:ring-blue-500" %> + <%= form.label :require_pkce, "Require PKCE (Proof Key for Code Exchange)", class: "ml-2 block text-sm font-medium text-gray-900" %> +
+

+ Recommended for enhanced security (OAuth 2.1 best practice). +
Note: Public clients always require PKCE regardless of this setting. +

+
+
<%= form.label :redirect_uris, "Redirect URIs", class: "block text-sm font-medium text-gray-700" %> <%= form.text_area :redirect_uris, rows: 4, class: "mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm font-mono", placeholder: "https://example.com/callback\nhttps://app.example.com/auth/callback" %> diff --git a/app/views/admin/applications/show.html.erb b/app/views/admin/applications/show.html.erb index baafd29..fdee408 100644 --- a/app/views/admin/applications/show.html.erb +++ b/app/views/admin/applications/show.html.erb @@ -1,17 +1,30 @@
- <% if flash[:client_id] && flash[:client_secret] %> + <% if flash[:client_id] %>

🔐 OIDC Client Credentials

-

Copy these credentials now. The client secret will not be shown again.

+ <% if flash[:public_client] %> +

This is a public client. Copy the client ID below.

+ <% else %> +

Copy these credentials now. The client secret will not be shown again.

+ <% end %>
Client ID:
<%= flash[:client_id] %> -
- Client Secret: -
- <%= flash[:client_secret] %> + <% if flash[:client_secret] %> +
+ Client Secret: +
+ <%= flash[:client_secret] %> + <% elsif flash[:public_client] %> +
+ Client Secret: +
+
+ Public clients do not have a client secret. PKCE is required. +
+ <% end %>
<% end %> @@ -93,24 +106,57 @@ <%= button_to "Regenerate Credentials", regenerate_credentials_admin_application_path(@application), method: :post, data: { turbo_confirm: "This will invalidate the current credentials. Continue?" }, class: "text-sm text-red-600 hover:text-red-900" %>
- <% unless flash[:client_id] && flash[:client_secret] %> +
+
+
Client Type
+
+ <% if @application.public_client? %> + Public + <% else %> + Confidential + <% end %> +
+
+
+
PKCE
+
+ <% if @application.requires_pkce? %> + Required + <% else %> + Optional + <% end %> +
+
+
+ <% unless flash[:client_id] %>
Client ID
<%= @application.client_id %>
-
-
Client Secret
-
-
- 🔒 Client secret is stored securely and cannot be displayed -
-

- To get a new client secret, use the "Regenerate Credentials" button above. -

-
-
+ <% if @application.confidential_client? %> +
+
Client Secret
+
+
+ 🔒 Client secret is stored securely and cannot be displayed +
+

+ To get a new client secret, use the "Regenerate Credentials" button above. +

+
+
+ <% else %> +
+
Client Secret
+
+
+ Public clients do not use a client secret. PKCE is required for authorization. +
+
+
+ <% end %> <% end %>
Redirect URIs
diff --git a/app/views/shared/_flash.html.erb b/app/views/shared/_flash.html.erb index 7ef4da8..a97f836 100644 --- a/app/views/shared/_flash.html.erb +++ b/app/views/shared/_flash.html.erb @@ -1,6 +1,8 @@ <%# Enhanced Flash Messages with Support for Multiple Types and Auto-Dismiss %> <% flash.each do |type, message| %> <% next if message.blank? %> + <%# Skip credential-related flash messages - they're displayed in a special credentials box %> + <% next if %w[client_id client_secret public_client].include?(type.to_s) %> <% # Map flash types to styling diff --git a/config/initializers/version.rb b/config/initializers/version.rb index 4fb61fd..913c1ba 100644 --- a/config/initializers/version.rb +++ b/config/initializers/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Clinch - VERSION = "0.7.1" + VERSION = "0.7.2" end diff --git a/db/schema.rb b/db/schema.rb index 208201a..20b0d34 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_12_30_005248) do +ActiveRecord::Schema[8.1].define(version: 2025_12_30_073656) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -77,6 +77,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_12_30_005248) do t.string "name", null: false t.text "redirect_uris" t.integer "refresh_token_ttl", default: 2592000 + t.boolean "require_pkce", default: true, null: false t.string "slug", null: false t.datetime "updated_at", null: false t.index ["active"], name: "index_applications_on_active" diff --git a/test/controllers/oidc_authorization_code_security_test.rb b/test/controllers/oidc_authorization_code_security_test.rb index 0653fc3..c7c2ea2 100644 --- a/test/controllers/oidc_authorization_code_security_test.rb +++ b/test/controllers/oidc_authorization_code_security_test.rb @@ -8,7 +8,8 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest slug: "security-test-app", app_type: "oidc", redirect_uris: ["http://localhost:4000/callback"].to_json, - active: true + active: true, + require_pkce: false ) # Store the plain text client secret for testing @@ -274,7 +275,8 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest slug: "other-app", app_type: "oidc", redirect_uris: ["http://localhost:5000/callback"].to_json, - active: true + active: true, + require_pkce: false ) other_secret = other_app.client_secret diff --git a/test/controllers/oidc_pkce_controller_test.rb b/test/controllers/oidc_pkce_controller_test.rb index 7c9dc09..adf6504 100644 --- a/test/controllers/oidc_pkce_controller_test.rb +++ b/test/controllers/oidc_pkce_controller_test.rb @@ -318,16 +318,199 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest end test "token endpoint works without PKCE (backward compatibility)" do + # Create an application with PKCE not required (legacy behavior) + legacy_app = Application.create!( + name: "Legacy App", + slug: "legacy-app", + app_type: "oidc", + redirect_uris: ["http://localhost:5000/callback"].to_json, + active: true, + require_pkce: false + ) + legacy_app.generate_new_client_secret! + # Create consent for token endpoint OidcUserConsent.create!( user: @user, - application: @application, + application: legacy_app, scopes_granted: "openid profile", granted_at: Time.current, sid: "test-sid-123" ) # Create authorization code without PKCE + auth_code = OidcAuthorizationCode.create!( + application: legacy_app, + user: @user, + code: SecureRandom.urlsafe_base64(32), + redirect_uri: "http://localhost:5000/callback", + scope: "openid profile", + expires_at: 10.minutes.from_now + ) + + token_params = { + grant_type: "authorization_code", + code: auth_code.code, + redirect_uri: "http://localhost:5000/callback" + } + + post "/oauth/token", params: token_params, headers: { + "Authorization" => "Basic " + Base64.strict_encode64("#{legacy_app.client_id}:#{legacy_app.client_secret}") + } + + assert_response :success + tokens = JSON.parse(@response.body) + assert tokens.key?("access_token") + assert tokens.key?("id_token") + assert_equal "Bearer", tokens["token_type"] + + # Cleanup + OidcRefreshToken.where(application: legacy_app).delete_all + OidcAccessToken.where(application: legacy_app).delete_all + OidcAuthorizationCode.where(application: legacy_app).delete_all + OidcUserConsent.where(application: legacy_app).delete_all + legacy_app.destroy + end + + # ==================== + # PUBLIC CLIENT TESTS + # ==================== + + test "public client can authenticate with PKCE" do + # Create a public client (no client_secret) + public_app = Application.create!( + name: "Public App", + slug: "public-app", + app_type: "oidc", + redirect_uris: ["http://localhost:6000/callback"].to_json, + active: true, + is_public_client: true + ) + + assert public_app.public_client? + assert public_app.requires_pkce? + assert_nil public_app.client_secret_digest + + # Create consent + OidcUserConsent.create!( + user: @user, + application: public_app, + scopes_granted: "openid profile", + granted_at: Time.current, + sid: "test-sid-123" + ) + + # PKCE parameters + code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + code_challenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM" + + # Create authorization code with PKCE + auth_code = OidcAuthorizationCode.create!( + application: public_app, + user: @user, + code: SecureRandom.urlsafe_base64(32), + redirect_uri: "http://localhost:6000/callback", + scope: "openid profile", + expires_at: 10.minutes.from_now, + code_challenge: code_challenge, + code_challenge_method: "S256" + ) + + # Token request with PKCE but no client_secret + token_params = { + grant_type: "authorization_code", + code: auth_code.code, + redirect_uri: "http://localhost:6000/callback", + client_id: public_app.client_id, + code_verifier: code_verifier + } + + post "/oauth/token", params: token_params + + assert_response :success + tokens = JSON.parse(@response.body) + assert tokens.key?("access_token") + assert tokens.key?("id_token") + + # Cleanup + OidcRefreshToken.where(application: public_app).delete_all + OidcAccessToken.where(application: public_app).delete_all + OidcAuthorizationCode.where(application: public_app).delete_all + OidcUserConsent.where(application: public_app).delete_all + public_app.destroy + end + + test "public client fails without PKCE" do + # Create a public client (no client_secret) + public_app = Application.create!( + name: "Public App No PKCE", + slug: "public-app-no-pkce", + app_type: "oidc", + redirect_uris: ["http://localhost:7000/callback"].to_json, + active: true, + is_public_client: true + ) + + assert public_app.public_client? + assert public_app.requires_pkce? + + # Create consent + OidcUserConsent.create!( + user: @user, + application: public_app, + scopes_granted: "openid profile", + granted_at: Time.current, + sid: "test-sid-123" + ) + + # Create authorization code WITHOUT PKCE + auth_code = OidcAuthorizationCode.create!( + application: public_app, + user: @user, + code: SecureRandom.urlsafe_base64(32), + redirect_uri: "http://localhost:7000/callback", + scope: "openid profile", + expires_at: 10.minutes.from_now + ) + + # Token request without PKCE should fail + token_params = { + grant_type: "authorization_code", + code: auth_code.code, + redirect_uri: "http://localhost:7000/callback", + client_id: public_app.client_id + } + + post "/oauth/token", params: token_params + + assert_response :bad_request + error = JSON.parse(@response.body) + assert_equal "invalid_request", error["error"] + assert_match /PKCE is required for public clients/, error["error_description"] + + # Cleanup + OidcRefreshToken.where(application: public_app).delete_all + OidcAccessToken.where(application: public_app).delete_all + OidcAuthorizationCode.where(application: public_app).delete_all + OidcUserConsent.where(application: public_app).delete_all + public_app.destroy + end + + test "confidential client with require_pkce fails without PKCE" do + # The default @application has require_pkce: true (default) + assert @application.confidential_client? + assert @application.requires_pkce? + + # Create consent + OidcUserConsent.create!( + user: @user, + application: @application, + scopes_granted: "openid profile", + granted_at: Time.current, + sid: "test-sid-pkce-required" + ) + + # Create authorization code WITHOUT PKCE auth_code = OidcAuthorizationCode.create!( application: @application, user: @user, @@ -337,6 +520,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest expires_at: 10.minutes.from_now ) + # Token request without PKCE should fail token_params = { grant_type: "authorization_code", code: auth_code.code, @@ -347,10 +531,9 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest "Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@application.client_secret}") } - assert_response :success - tokens = JSON.parse(@response.body) - assert tokens.key?("access_token") - assert tokens.key?("id_token") - assert_equal "Bearer", tokens["token_type"] + assert_response :bad_request + error = JSON.parse(@response.body) + assert_equal "invalid_request", error["error"] + assert_match /PKCE is required/, error["error_description"] end end \ No newline at end of file diff --git a/test/fixtures/applications.yml b/test/fixtures/applications.yml index d821ffa..1cf6e19 100644 --- a/test/fixtures/applications.yml +++ b/test/fixtures/applications.yml @@ -13,6 +13,7 @@ kavita_app: https://kavita.example.com/signout-callback-oidc metadata: "{}" active: true + require_pkce: false another_app: name: Another App @@ -24,6 +25,7 @@ another_app: https://app.example.com/auth/callback metadata: "{}" active: true + require_pkce: false audiobookshelf_app: name: Audiobookshelf @@ -35,3 +37,4 @@ audiobookshelf_app: https://abs.example.com/auth/openid/callback metadata: "{}" active: true + require_pkce: false diff --git a/test/integration/forward_auth_integration_test.rb b/test/integration/forward_auth_integration_test.rb index bd23a97..0ae6e9d 100644 --- a/test/integration/forward_auth_integration_test.rb +++ b/test/integration/forward_auth_integration_test.rb @@ -6,6 +6,15 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest @admin_user = users(:two) @group = groups(:one) @group2 = groups(:two) + + # Create a forward_auth application for test.example.com + @test_app = Application.create!( + name: "Test App", + slug: "test-app", + app_type: "forward_auth", + domain_pattern: "test.example.com", + active: true + ) end # Basic Authentication Flow Tests