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 @@
Backend server app that can securely store a client secret. Examples: traditional web apps, server-to-server APIs.
+Frontend-only app that cannot store secrets securely. Examples: SPAs (React/Vue), mobile apps, CLI tools. PKCE is required.
+
+ Recommended for enhanced security (OAuth 2.1 best practice).
+
Note: Public clients always require PKCE regardless of this setting.
+
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 %><%= flash[:client_id] %>
- <%= flash[:client_secret] %>
+ <% if flash[:client_secret] %>
+ <%= flash[:client_secret] %>
+ <% elsif flash[:public_client] %>
+ <%= @application.client_id %>
- To get a new client secret, use the "Regenerate Credentials" button above. -
-+ To get a new client secret, use the "Regenerate Credentials" button above. +
+