PKCE is now default enabled. You can now create public / no-secret apps OIDC apps

This commit is contained in:
Dan Milne
2025-12-31 09:22:18 +11:00
parent 00eca6d8b2
commit cc7beba9de
15 changed files with 456 additions and 64 deletions

View File

@@ -26,16 +26,17 @@ module Admin
@application.allowed_groups = Group.where(id: group_ids) @application.allowed_groups = Group.where(id: group_ids)
end 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 client_secret = nil
if @application.oidc? if @application.oidc? && @application.confidential_client?
client_secret = @application.generate_new_client_secret! client_secret = @application.generate_new_client_secret!
end end
if @application.oidc? && client_secret if @application.oidc?
flash[:notice] = "Application created successfully." flash[:notice] = "Application created successfully."
flash[:client_id] = @application.client_id 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 else
flash[:notice] = "Application created successfully." flash[:notice] = "Application created successfully."
end end
@@ -74,15 +75,20 @@ module Admin
def regenerate_credentials def regenerate_credentials
if @application.oidc? if @application.oidc?
# Generate new client ID and secret # Generate new client ID (always)
new_client_id = SecureRandom.urlsafe_base64(32) new_client_id = SecureRandom.urlsafe_base64(32)
client_secret = @application.generate_new_client_secret!
@application.update!(client_id: new_client_id) @application.update!(client_id: new_client_id)
flash[:notice] = "Credentials regenerated successfully." flash[:notice] = "Credentials regenerated successfully."
flash[:client_id] = @application.client_id flash[:client_id] = @application.client_id
# Generate new client secret only for confidential clients
if @application.confidential_client?
client_secret = @application.generate_new_client_secret!
flash[:client_secret] = client_secret flash[:client_secret] = client_secret
else
flash[:public_client] = true
end
redirect_to admin_application_path(@application) redirect_to admin_application_path(@application)
else else
@@ -97,15 +103,24 @@ module Admin
end end
def application_params def application_params
params.require(:application).permit( permitted = params.require(:application).permit(
:name, :slug, :app_type, :active, :redirect_uris, :description, :metadata, :name, :slug, :app_type, :active, :redirect_uris, :description, :metadata,
:domain_pattern, :landing_url, :access_token_ttl, :refresh_token_ttl, :id_token_ttl, :domain_pattern, :landing_url, :access_token_ttl, :refresh_token_ttl, :id_token_ttl,
:icon, :backchannel_logout_uri, :icon, :backchannel_logout_uri, :is_public_client, :require_pkce
headers_config: {} )
).tap do |whitelisted|
# 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) # Remove client_secret from params if present (shouldn't be updated via form)
whitelisted.delete(:client_secret) permitted.delete(:client_secret)
end permitted
end end
end end
end end

View File

@@ -296,22 +296,33 @@ class OidcController < ApplicationController
end end
def handle_authorization_code_grant def handle_authorization_code_grant
# Get client credentials from Authorization header or params # Get client credentials from Authorization header or params
client_id, client_secret = extract_client_credentials client_id, client_secret = extract_client_credentials
unless client_id && client_secret unless client_id
render json: { error: "invalid_client" }, status: :unauthorized render json: { error: "invalid_client", error_description: "client_id is required" }, status: :unauthorized
return return
end end
# Find and validate the application # Find the application
application = Application.find_by(client_id: client_id) application = Application.find_by(client_id: client_id)
unless application && application.authenticate_client_secret(client_secret) unless application
render json: { error: "invalid_client" }, status: :unauthorized render json: { error: "invalid_client", error_description: "Unknown client" }, status: :unauthorized
return return
end 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 # Check if application is active
unless application.active? unless application.active?
Rails.logger.error "OAuth: Token request for inactive application: #{application.name}" Rails.logger.error "OAuth: Token request for inactive application: #{application.name}"
@@ -371,8 +382,8 @@ class OidcController < ApplicationController
return return
end end
# Validate PKCE if code challenge is present # Validate PKCE - required for public clients and optionally for confidential clients
pkce_result = validate_pkce(auth_code, code_verifier) pkce_result = validate_pkce(application, auth_code, code_verifier)
unless pkce_result[:valid] unless pkce_result[:valid]
render json: { render json: {
error: pkce_result[:error], error: pkce_result[:error],
@@ -433,18 +444,30 @@ class OidcController < ApplicationController
# Get client credentials from Authorization header or params # Get client credentials from Authorization header or params
client_id, client_secret = extract_client_credentials client_id, client_secret = extract_client_credentials
unless client_id && client_secret unless client_id
render json: { error: "invalid_client" }, status: :unauthorized render json: { error: "invalid_client", error_description: "client_id is required" }, status: :unauthorized
return return
end end
# Find and validate the application # Find the application
application = Application.find_by(client_id: client_id) application = Application.find_by(client_id: client_id)
unless application && application.authenticate_client_secret(client_secret) unless application
render json: { error: "invalid_client" }, status: :unauthorized render json: { error: "invalid_client", error_description: "Unknown client" }, status: :unauthorized
return return
end 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 # Check if application is active
unless application.active? unless application.active?
Rails.logger.error "OAuth: Refresh token request for inactive application: #{application.name}" Rails.logger.error "OAuth: Refresh token request for inactive application: #{application.name}"
@@ -716,11 +739,26 @@ class OidcController < ApplicationController
private private
def validate_pkce(auth_code, code_verifier) def validate_pkce(application, auth_code, code_verifier)
# Skip PKCE validation if no code challenge was stored (legacy clients) # Check if PKCE is required for this application
return { valid: true } unless auth_code.code_challenge.present? 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? unless code_verifier.present?
return { return {
valid: false, valid: false,

View File

@@ -1,7 +1,7 @@
import { Controller } from "@hotwired/stimulus" import { Controller } from "@hotwired/stimulus"
export default class extends Controller { export default class extends Controller {
static targets = ["appTypeSelect", "oidcFields", "forwardAuthFields"] static targets = ["appTypeSelect", "oidcFields", "forwardAuthFields", "pkceOptions"]
connect() { connect() {
this.updateFieldVisibility() this.updateFieldVisibility()
@@ -21,4 +21,17 @@ export default class extends Controller {
this.forwardAuthFieldsTarget.classList.add('hidden') 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')
}
}
}
} }

View File

@@ -1,6 +1,10 @@
class Application < ApplicationRecord class Application < ApplicationRecord
has_secure_password :client_secret, validations: false 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 has_one_attached :icon
# Fix SVG content type after attachment # Fix SVG content type after attachment
@@ -20,7 +24,7 @@ class Application < ApplicationRecord
validates :app_type, presence: true, validates :app_type, presence: true,
inclusion: { in: %w[oidc forward_auth] } inclusion: { in: %w[oidc forward_auth] }
validates :client_id, uniqueness: { allow_nil: true } 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 :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 :landing_url, format: { with: URI::regexp(%w[http https]), allow_nil: true, message: "must be a valid URL" }
validates :backchannel_logout_uri, format: { validates :backchannel_logout_uri, format: {
@@ -74,6 +78,24 @@ class Application < ApplicationRecord
app_type == "forward_auth" app_type == "forward_auth"
end 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 # Access control
def user_allowed?(user) def user_allowed?(user)
return false unless active? return false unless active?
@@ -261,13 +283,19 @@ class Application < ApplicationRecord
def generate_client_credentials def generate_client_credentials
self.client_id ||= SecureRandom.urlsafe_base64(32) self.client_id ||= SecureRandom.urlsafe_base64(32)
# Generate and hash the client secret # Generate client secret only for confidential clients
if new_record? && client_secret.blank? # 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) secret = SecureRandom.urlsafe_base64(48)
self.client_secret = secret self.client_secret = secret
end end
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 def backchannel_logout_uri_must_be_https_in_production
return unless Rails.env.production? return unless Rails.env.production?
return unless backchannel_logout_uri.present? return unless backchannel_logout_uri.present?

View File

@@ -4,7 +4,6 @@ class OidcRefreshToken < ApplicationRecord
belongs_to :application belongs_to :application
belongs_to :user belongs_to :user
belongs_to :oidc_access_token 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 :generate_token_with_prefix, on: :create
before_validation :set_expiry, on: :create before_validation :set_expiry, on: :create

View File

@@ -74,6 +74,14 @@ class User < ApplicationRecord
totp.verify(code, drift_behind: 30, drift_ahead: 30) totp.verify(code, drift_behind: 30, drift_ahead: 30)
end 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) def verify_backup_code(code)
return false unless backup_codes.present? return false unless backup_codes.present?

View File

@@ -120,6 +120,51 @@
<div id="oidc-fields" class="space-y-6 border-t border-gray-200 pt-6 <%= 'hidden' unless application.oidc? || !application.persisted? %>" data-application-form-target="oidcFields"> <div id="oidc-fields" class="space-y-6 border-t border-gray-200 pt-6 <%= 'hidden' unless application.oidc? || !application.persisted? %>" data-application-form-target="oidcFields">
<h3 class="text-base font-semibold text-gray-900">OIDC Configuration</h3> <h3 class="text-base font-semibold text-gray-900">OIDC Configuration</h3>
<!-- Client Type Selection (only for new applications) -->
<% unless application.persisted? %>
<div class="border border-gray-200 rounded-lg p-4 bg-gray-50">
<h4 class="text-sm font-semibold text-gray-900 mb-3">Client Type</h4>
<div class="space-y-3">
<div class="flex items-start">
<%= 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" } %>
<div class="ml-3">
<label for="application_is_public_client_false" class="block text-sm font-medium text-gray-900">Confidential Client (Recommended)</label>
<p class="text-sm text-gray-500">Backend server app that can securely store a client secret. Examples: traditional web apps, server-to-server APIs.</p>
</div>
</div>
<div class="flex items-start">
<%= 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" } %>
<div class="ml-3">
<label for="application_is_public_client_true" class="block text-sm font-medium text-gray-900">Public Client</label>
<p class="text-sm text-gray-500">Frontend-only app that cannot store secrets securely. Examples: SPAs (React/Vue), mobile apps, CLI tools. <strong class="text-amber-600">PKCE is required.</strong></p>
</div>
</div>
</div>
</div>
<% else %>
<!-- Show client type for existing applications (read-only) -->
<div class="flex items-center gap-2 text-sm">
<span class="font-medium text-gray-700">Client Type:</span>
<% if application.public_client? %>
<span class="inline-flex items-center rounded-md bg-amber-50 px-2 py-1 text-xs font-medium text-amber-700 ring-1 ring-inset ring-amber-600/20">Public Client (PKCE Required)</span>
<% else %>
<span class="inline-flex items-center rounded-md bg-green-50 px-2 py-1 text-xs font-medium text-green-700 ring-1 ring-inset ring-green-600/20">Confidential Client</span>
<% end %>
</div>
<% end %>
<!-- PKCE Requirement (only for confidential clients) -->
<div id="pkce-options" data-application-form-target="pkceOptions" class="<%= 'hidden' if application.persisted? && application.public_client? %>">
<div class="flex items-center">
<%= 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" %>
</div>
<p class="ml-6 text-sm text-gray-500">
Recommended for enhanced security (OAuth 2.1 best practice).
<br><span class="text-xs text-gray-400">Note: Public clients always require PKCE regardless of this setting.</span>
</p>
</div>
<div> <div>
<%= form.label :redirect_uris, "Redirect URIs", class: "block text-sm font-medium text-gray-700" %> <%= 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" %> <%= 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" %>

View File

@@ -1,17 +1,30 @@
<div class="mb-6"> <div class="mb-6">
<% if flash[:client_id] && flash[:client_secret] %> <% if flash[:client_id] %>
<div class="bg-yellow-50 border border-yellow-200 rounded-md p-4 mb-6"> <div class="bg-yellow-50 border border-yellow-200 rounded-md p-4 mb-6">
<h4 class="text-sm font-medium text-yellow-800 mb-2">🔐 OIDC Client Credentials</h4> <h4 class="text-sm font-medium text-yellow-800 mb-2">🔐 OIDC Client Credentials</h4>
<% if flash[:public_client] %>
<p class="text-xs text-yellow-700 mb-3">This is a public client. Copy the client ID below.</p>
<% else %>
<p class="text-xs text-yellow-700 mb-3">Copy these credentials now. The client secret will not be shown again.</p> <p class="text-xs text-yellow-700 mb-3">Copy these credentials now. The client secret will not be shown again.</p>
<% end %>
<div class="space-y-2"> <div class="space-y-2">
<div> <div>
<span class="text-xs font-medium text-yellow-700">Client ID:</span> <span class="text-xs font-medium text-yellow-700">Client ID:</span>
</div> </div>
<code class="block bg-yellow-100 px-3 py-2 rounded font-mono text-xs break-all"><%= flash[:client_id] %></code> <code class="block bg-yellow-100 px-3 py-2 rounded font-mono text-xs break-all"><%= flash[:client_id] %></code>
<% if flash[:client_secret] %>
<div class="mt-3"> <div class="mt-3">
<span class="text-xs font-medium text-yellow-700">Client Secret:</span> <span class="text-xs font-medium text-yellow-700">Client Secret:</span>
</div> </div>
<code class="block bg-yellow-100 px-3 py-2 rounded font-mono text-xs break-all"><%= flash[:client_secret] %></code> <code class="block bg-yellow-100 px-3 py-2 rounded font-mono text-xs break-all"><%= flash[:client_secret] %></code>
<% elsif flash[:public_client] %>
<div class="mt-3">
<span class="text-xs font-medium text-yellow-700">Client Secret:</span>
</div>
<div class="bg-yellow-100 px-3 py-2 rounded text-xs text-yellow-600">
Public clients do not have a client secret. PKCE is required.
</div>
<% end %>
</div> </div>
</div> </div>
<% end %> <% end %>
@@ -93,13 +106,36 @@
<%= 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" %> <%= 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" %>
</div> </div>
<dl class="space-y-4"> <dl class="space-y-4">
<% unless flash[:client_id] && flash[:client_secret] %> <div class="grid grid-cols-2 gap-4">
<div>
<dt class="text-sm font-medium text-gray-500">Client Type</dt>
<dd class="mt-1 text-sm text-gray-900">
<% if @application.public_client? %>
<span class="inline-flex items-center rounded-full bg-blue-100 px-2 py-1 text-xs font-medium text-blue-700">Public</span>
<% else %>
<span class="inline-flex items-center rounded-full bg-gray-100 px-2 py-1 text-xs font-medium text-gray-700">Confidential</span>
<% end %>
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500">PKCE</dt>
<dd class="mt-1 text-sm text-gray-900">
<% if @application.requires_pkce? %>
<span class="inline-flex items-center rounded-full bg-green-100 px-2 py-1 text-xs font-medium text-green-700">Required</span>
<% else %>
<span class="inline-flex items-center rounded-full bg-gray-100 px-2 py-1 text-xs font-medium text-gray-700">Optional</span>
<% end %>
</dd>
</div>
</div>
<% unless flash[:client_id] %>
<div> <div>
<dt class="text-sm font-medium text-gray-500">Client ID</dt> <dt class="text-sm font-medium text-gray-500">Client ID</dt>
<dd class="mt-1 text-sm text-gray-900"> <dd class="mt-1 text-sm text-gray-900">
<code class="block bg-gray-100 px-3 py-2 rounded font-mono text-xs break-all"><%= @application.client_id %></code> <code class="block bg-gray-100 px-3 py-2 rounded font-mono text-xs break-all"><%= @application.client_id %></code>
</dd> </dd>
</div> </div>
<% if @application.confidential_client? %>
<div> <div>
<dt class="text-sm font-medium text-gray-500">Client Secret</dt> <dt class="text-sm font-medium text-gray-500">Client Secret</dt>
<dd class="mt-1 text-sm text-gray-900"> <dd class="mt-1 text-sm text-gray-900">
@@ -111,6 +147,16 @@
</p> </p>
</dd> </dd>
</div> </div>
<% else %>
<div>
<dt class="text-sm font-medium text-gray-500">Client Secret</dt>
<dd class="mt-1 text-sm text-gray-900">
<div class="bg-blue-50 px-3 py-2 rounded text-xs text-blue-600">
Public clients do not use a client secret. PKCE is required for authorization.
</div>
</dd>
</div>
<% end %>
<% end %> <% end %>
<div> <div>
<dt class="text-sm font-medium text-gray-500">Redirect URIs</dt> <dt class="text-sm font-medium text-gray-500">Redirect URIs</dt>

View File

@@ -1,6 +1,8 @@
<%# Enhanced Flash Messages with Support for Multiple Types and Auto-Dismiss %> <%# Enhanced Flash Messages with Support for Multiple Types and Auto-Dismiss %>
<% flash.each do |type, message| %> <% flash.each do |type, message| %>
<% next if message.blank? %> <% 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 # Map flash types to styling

View File

@@ -1,5 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
module Clinch module Clinch
VERSION = "0.7.1" VERSION = "0.7.2"
end end

3
db/schema.rb generated
View File

@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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| create_table "active_storage_attachments", force: :cascade do |t|
t.bigint "blob_id", null: false t.bigint "blob_id", null: false
t.datetime "created_at", 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.string "name", null: false
t.text "redirect_uris" t.text "redirect_uris"
t.integer "refresh_token_ttl", default: 2592000 t.integer "refresh_token_ttl", default: 2592000
t.boolean "require_pkce", default: true, null: false
t.string "slug", null: false t.string "slug", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.index ["active"], name: "index_applications_on_active" t.index ["active"], name: "index_applications_on_active"

View File

@@ -8,7 +8,8 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
slug: "security-test-app", slug: "security-test-app",
app_type: "oidc", app_type: "oidc",
redirect_uris: ["http://localhost:4000/callback"].to_json, redirect_uris: ["http://localhost:4000/callback"].to_json,
active: true active: true,
require_pkce: false
) )
# Store the plain text client secret for testing # Store the plain text client secret for testing
@@ -274,7 +275,8 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
slug: "other-app", slug: "other-app",
app_type: "oidc", app_type: "oidc",
redirect_uris: ["http://localhost:5000/callback"].to_json, redirect_uris: ["http://localhost:5000/callback"].to_json,
active: true active: true,
require_pkce: false
) )
other_secret = other_app.client_secret other_secret = other_app.client_secret

View File

@@ -318,16 +318,199 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
end end
test "token endpoint works without PKCE (backward compatibility)" do 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 # Create consent for token endpoint
OidcUserConsent.create!( OidcUserConsent.create!(
user: @user, user: @user,
application: @application, application: legacy_app,
scopes_granted: "openid profile", scopes_granted: "openid profile",
granted_at: Time.current, granted_at: Time.current,
sid: "test-sid-123" sid: "test-sid-123"
) )
# Create authorization code without PKCE # 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!( auth_code = OidcAuthorizationCode.create!(
application: @application, application: @application,
user: @user, user: @user,
@@ -337,6 +520,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
expires_at: 10.minutes.from_now expires_at: 10.minutes.from_now
) )
# Token request without PKCE should fail
token_params = { token_params = {
grant_type: "authorization_code", grant_type: "authorization_code",
code: auth_code.code, code: auth_code.code,
@@ -347,10 +531,9 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
"Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@application.client_secret}") "Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@application.client_secret}")
} }
assert_response :success assert_response :bad_request
tokens = JSON.parse(@response.body) error = JSON.parse(@response.body)
assert tokens.key?("access_token") assert_equal "invalid_request", error["error"]
assert tokens.key?("id_token") assert_match /PKCE is required/, error["error_description"]
assert_equal "Bearer", tokens["token_type"]
end end
end end

View File

@@ -13,6 +13,7 @@ kavita_app:
https://kavita.example.com/signout-callback-oidc https://kavita.example.com/signout-callback-oidc
metadata: "{}" metadata: "{}"
active: true active: true
require_pkce: false
another_app: another_app:
name: Another App name: Another App
@@ -24,6 +25,7 @@ another_app:
https://app.example.com/auth/callback https://app.example.com/auth/callback
metadata: "{}" metadata: "{}"
active: true active: true
require_pkce: false
audiobookshelf_app: audiobookshelf_app:
name: Audiobookshelf name: Audiobookshelf
@@ -35,3 +37,4 @@ audiobookshelf_app:
https://abs.example.com/auth/openid/callback https://abs.example.com/auth/openid/callback
metadata: "{}" metadata: "{}"
active: true active: true
require_pkce: false

View File

@@ -6,6 +6,15 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
@admin_user = users(:two) @admin_user = users(:two)
@group = groups(:one) @group = groups(:one)
@group2 = groups(:two) @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 end
# Basic Authentication Flow Tests # Basic Authentication Flow Tests