Compare commits
13 Commits
2843790cef
...
209c5496d8
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
209c5496d8 | ||
|
|
d49e7ce4f5 | ||
|
|
44892e3301 | ||
|
|
24266872f9 | ||
|
|
cd862c7cd7 | ||
|
|
89bd5f1432 | ||
|
|
57d7d1f691 | ||
|
|
406a79d9eb | ||
|
|
f38ac2ecc8 | ||
|
|
84ed462f40 | ||
|
|
96a657e349 | ||
|
|
8a095e4939 | ||
|
|
703d24e4e4 |
@@ -64,26 +64,16 @@ module Api
|
|||||||
return render_forbidden("No authentication rule configured for this domain")
|
return render_forbidden("No authentication rule configured for this domain")
|
||||||
end
|
end
|
||||||
else
|
else
|
||||||
Rails.logger.info "ForwardAuth: User #{user.email_address} authenticated (no domain specified)"
|
# Fail closed: with no host we cannot resolve an application or evaluate its
|
||||||
end
|
# group policy. Emitting identity headers here would bypass all per-domain
|
||||||
|
# access control, so reject instead.
|
||||||
headers = if app
|
Rails.logger.info "ForwardAuth: Access denied - no host header present"
|
||||||
app.headers_for_user(user)
|
return render_forbidden("No host header present")
|
||||||
else
|
|
||||||
Application::DEFAULT_HEADERS.map { |key, header_name|
|
|
||||||
case key
|
|
||||||
when :user, :email, :name
|
|
||||||
[header_name, user.email_address]
|
|
||||||
when :username
|
|
||||||
[header_name, user.username] if user.username.present?
|
|
||||||
when :groups
|
|
||||||
user.groups.any? ? [header_name, user.groups.map(&:name).join(",")] : nil
|
|
||||||
when :admin
|
|
||||||
[header_name, user.admin? ? "true" : "false"]
|
|
||||||
end
|
|
||||||
}.compact.to_h
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Reaching here implies a matching, active application was resolved above
|
||||||
|
# (every other path returns forbidden), so headers are always scoped to it.
|
||||||
|
headers = app.headers_for_user(user)
|
||||||
headers.each { |key, value| response.headers[key] = value }
|
headers.each { |key, value| response.headers[key] = value }
|
||||||
Rails.logger.debug "ForwardAuth: Headers sent: #{headers.keys.join(", ")}" if headers.any?
|
Rails.logger.debug "ForwardAuth: Headers sent: #{headers.keys.join(", ")}" if headers.any?
|
||||||
|
|
||||||
@@ -148,6 +138,14 @@ module Api
|
|||||||
return render_bearer_error("Application is inactive")
|
return render_bearer_error("Application is inactive")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Re-check group membership at use-time. The ApiKey model only validates
|
||||||
|
# access on creation, so a user removed from the app's allowed groups
|
||||||
|
# afterwards must not keep access via an existing key.
|
||||||
|
unless app.user_allowed?(user)
|
||||||
|
Rails.logger.info "ForwardAuth: API key '#{api_key.name}' denied - user #{user.email_address} lacks group access to #{app.domain_pattern}"
|
||||||
|
return render_bearer_error("Access denied: insufficient group membership")
|
||||||
|
end
|
||||||
|
|
||||||
api_key.touch_last_used!
|
api_key.touch_last_used!
|
||||||
|
|
||||||
headers = app.headers_for_user(user)
|
headers = app.headers_for_user(user)
|
||||||
@@ -198,11 +196,14 @@ module Api
|
|||||||
original_host = request.headers["X-Forwarded-Host"]
|
original_host = request.headers["X-Forwarded-Host"]
|
||||||
original_uri = request.headers["X-Forwarded-Uri"] || request.headers["X-Forwarded-Path"] || "/"
|
original_uri = request.headers["X-Forwarded-Uri"] || request.headers["X-Forwarded-Path"] || "/"
|
||||||
|
|
||||||
original_url = if original_host
|
# X-Forwarded-Host is attacker-influenceable, so only honour the forwarded
|
||||||
"https://#{original_host}#{original_uri}"
|
# URL as a post-login redirect target if it resolves to a known, active
|
||||||
else
|
# forward-auth application. Otherwise this is an open redirect: a spoofed
|
||||||
redirect_url || base_url
|
# host would be stored and reflected into the signin `rd`, then followed
|
||||||
end
|
# (with allow_other_host) after the user authenticates. Fall back to a
|
||||||
|
# validated `rd` or, failing that, the IdP's own base URL.
|
||||||
|
forwarded_url = "https://#{original_host}#{original_uri}" if original_host.present?
|
||||||
|
original_url = validate_redirect_url(forwarded_url) || redirect_url || base_url
|
||||||
|
|
||||||
session[:return_to_after_authenticating] = original_url
|
session[:return_to_after_authenticating] = original_url
|
||||||
|
|
||||||
@@ -242,18 +243,13 @@ module Api
|
|||||||
def determine_base_url(redirect_url)
|
def determine_base_url(redirect_url)
|
||||||
return redirect_url if redirect_url.present?
|
return redirect_url if redirect_url.present?
|
||||||
|
|
||||||
if ENV["CLINCH_HOST"].present?
|
# CLINCH_HOST is the IdP's canonical origin and is mandatory in deployed
|
||||||
host = ENV["CLINCH_HOST"]
|
# environments (enforced at boot in config/initializers/clinch_host.rb).
|
||||||
host.match?(/^https?:\/\//) ? host : "https://#{host}"
|
# We never fall back to the request host: a spoofed X-Forwarded-Host would
|
||||||
else
|
# otherwise redirect the login flow to an attacker-controlled origin. The
|
||||||
request_host = request.host || request.headers["X-Forwarded-Host"]
|
# localhost default only applies to local dev/test.
|
||||||
if request_host.present?
|
host = ENV["CLINCH_HOST"].presence || "http://localhost:3000"
|
||||||
Rails.logger.warn "ForwardAuth: CLINCH_HOST not set, using request host: #{request_host}"
|
host.match?(%r{\Ahttps?://}) ? host : "https://#{host}"
|
||||||
"https://#{request_host}"
|
|
||||||
else
|
|
||||||
raise StandardError, "ForwardAuth: CLINCH_HOST environment variable not set and no request host available."
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ module Authentication
|
|||||||
end
|
end
|
||||||
|
|
||||||
def find_session_by_cookie
|
def find_session_by_cookie
|
||||||
Session.active.find_by(id: cookies.signed[:session_id]) if cookies.signed[:session_id]
|
Session.active.for_active_user.find_by(id: cookies.signed[:session_id]) if cookies.signed[:session_id]
|
||||||
end
|
end
|
||||||
|
|
||||||
def request_authentication
|
def request_authentication
|
||||||
|
|||||||
@@ -4,7 +4,11 @@ class OidcController < ApplicationController
|
|||||||
# Discovery and JWKS endpoints are public
|
# Discovery and JWKS endpoints are public
|
||||||
# authorize is also unauthenticated to handle prompt=none and prompt=login specially
|
# authorize is also unauthenticated to handle prompt=none and prompt=login specially
|
||||||
allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize]
|
allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize]
|
||||||
skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize, :consent]
|
# Machine-to-machine endpoints (token/revoke/userinfo) and pure redirect handlers
|
||||||
|
# (logout/authorize) legitimately skip CSRF. The consent endpoint is browser-facing
|
||||||
|
# and state-changing (it grants OAuth scopes), so it MUST keep CSRF protection — the
|
||||||
|
# consent form already embeds the token via form_with.
|
||||||
|
skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize]
|
||||||
|
|
||||||
# RFC 6749 §4.1.2.1: client_id and redirect_uri must be validated *before* any
|
# RFC 6749 §4.1.2.1: client_id and redirect_uri must be validated *before* any
|
||||||
# other error can be reported via redirect. Failures here render a plain page.
|
# other error can be reported via redirect. Failures here render a plain page.
|
||||||
|
|||||||
@@ -121,6 +121,16 @@ class SessionsController < ApplicationController
|
|||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Re-check account status: active? was verified at the password step, but an
|
||||||
|
# admin may have disabled the account while the user sat on this 2FA screen.
|
||||||
|
# Without this, a disabled account could still mint a valid session here.
|
||||||
|
unless user.active?
|
||||||
|
session.delete(:pending_totp_user_id)
|
||||||
|
session.delete(:pending_remember_me)
|
||||||
|
redirect_to signin_path, alert: "Your account is not active. Please contact an administrator."
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
remember_me = session.delete(:pending_remember_me) || false
|
remember_me = session.delete(:pending_remember_me) || false
|
||||||
|
|
||||||
# Try TOTP verification first (password + TOTP = 2FA)
|
# Try TOTP verification first (password + TOTP = 2FA)
|
||||||
@@ -241,6 +251,14 @@ class SessionsController < ApplicationController
|
|||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Re-check account status: an admin may have disabled the account between the
|
||||||
|
# password step and this passkey verification. Reject before creating a session.
|
||||||
|
unless user.active?
|
||||||
|
session.delete(:pending_webauthn_user_id)
|
||||||
|
render json: {error: "Your account is not active."}, status: :unauthorized
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
# Get the credential and assertion from params
|
# Get the credential and assertion from params
|
||||||
credential_data = params[:credential]
|
credential_data = params[:credential]
|
||||||
if credential_data.blank?
|
if credential_data.blank?
|
||||||
@@ -277,10 +295,14 @@ class SessionsController < ApplicationController
|
|||||||
sign_count: stored_credential.sign_count
|
sign_count: stored_credential.sign_count
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check for suspicious sign count (possible clone)
|
# Clone detection: a non-advancing signature counter signals the credential
|
||||||
|
# may have been copied. Reject the sign-in (do NOT create a session or update
|
||||||
|
# the stored counter) and alert the user, per WebAuthn §6.1.1.
|
||||||
if stored_credential.suspicious_sign_count?(webauthn_credential.sign_count)
|
if stored_credential.suspicious_sign_count?(webauthn_credential.sign_count)
|
||||||
Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id}"
|
Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id} (stored=#{stored_credential.sign_count}, presented=#{webauthn_credential.sign_count})"
|
||||||
# You might want to notify admins or temporarily disable the credential
|
SecurityMailer.suspicious_passkey_used(user, nickname: stored_credential.display_name, **security_event_context).deliver_later
|
||||||
|
render json: {error: "Passkey authentication could not be completed. Please contact support."}, status: :unprocessable_entity
|
||||||
|
return
|
||||||
end
|
end
|
||||||
|
|
||||||
# Update credential usage
|
# Update credential usage
|
||||||
|
|||||||
@@ -28,6 +28,14 @@ class BackchannelLogoutJob < ApplicationJob
|
|||||||
# Send HTTP POST to the application's backchannel logout URI
|
# Send HTTP POST to the application's backchannel logout URI
|
||||||
uri = URI.parse(application.backchannel_logout_uri)
|
uri = URI.parse(application.backchannel_logout_uri)
|
||||||
|
|
||||||
|
# SSRF guard: re-check at request time (with DNS resolution) in case the URI
|
||||||
|
# predates the validation, or a public hostname now resolves to an internal
|
||||||
|
# address. Abort without retrying — retries would not change the outcome.
|
||||||
|
if PrivateAddressCheck.internal_host?(uri.host) || PrivateAddressCheck.resolves_to_internal?(uri.host)
|
||||||
|
Rails.logger.error "BackchannelLogout: Refusing to send logout to #{application.name} - #{uri.host} is or resolves to a non-public address (SSRF guard)"
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
begin
|
begin
|
||||||
response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https", open_timeout: 5, read_timeout: 5) do |http|
|
response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https", open_timeout: 5, read_timeout: 5) do |http|
|
||||||
request = Net::HTTP::Post.new(uri.path.presence || "/")
|
request = Net::HTTP::Post.new(uri.path.presence || "/")
|
||||||
|
|||||||
57
app/lib/private_address_check.rb
Normal file
57
app/lib/private_address_check.rb
Normal file
@@ -0,0 +1,57 @@
|
|||||||
|
require "ipaddr"
|
||||||
|
require "resolv"
|
||||||
|
|
||||||
|
# SSRF guard for outbound requests to admin-configured URLs (currently the OIDC
|
||||||
|
# backchannel logout endpoint). Blocks hosts that are, or resolve to, private,
|
||||||
|
# loopback, link-local (incl. the cloud metadata address 169.254.169.254) or
|
||||||
|
# otherwise non-public address space.
|
||||||
|
module PrivateAddressCheck
|
||||||
|
module_function
|
||||||
|
|
||||||
|
# Hostnames that are internal by definition and must never be dialled.
|
||||||
|
BLOCKED_HOSTNAMES = %w[localhost metadata.google.internal].freeze
|
||||||
|
|
||||||
|
# Fast, DNS-free check: catches IP literals and well-known internal hostnames.
|
||||||
|
# Suitable for model validation (deterministic, immediate admin feedback).
|
||||||
|
def internal_host?(host)
|
||||||
|
host = host.to_s.downcase
|
||||||
|
return true if host.blank?
|
||||||
|
return true if BLOCKED_HOSTNAMES.include?(host)
|
||||||
|
return true if host.end_with?(".localhost")
|
||||||
|
|
||||||
|
ip = parse_ip(host)
|
||||||
|
ip ? internal_ip?(ip) : false
|
||||||
|
end
|
||||||
|
|
||||||
|
# Authoritative check: resolves the hostname and blocks if ANY address is
|
||||||
|
# internal. Suitable for request time — also defeats a public hostname that
|
||||||
|
# has been pointed at an internal IP (DNS rebinding to internal space).
|
||||||
|
def resolves_to_internal?(host)
|
||||||
|
addresses(host).any? { |ip| internal_ip?(ip) }
|
||||||
|
end
|
||||||
|
|
||||||
|
def addresses(host)
|
||||||
|
ip = parse_ip(host)
|
||||||
|
return [ip] if ip
|
||||||
|
|
||||||
|
Resolv.getaddresses(host.to_s).filter_map { |a| parse_ip(a) }
|
||||||
|
rescue StandardError
|
||||||
|
# Resolution failure: surface no addresses. Callers treat "can't resolve" as
|
||||||
|
# not-provably-internal; the dial itself will then fail safely.
|
||||||
|
[]
|
||||||
|
end
|
||||||
|
|
||||||
|
def internal_ip?(ip)
|
||||||
|
ip.loopback? || ip.private? || ip.link_local? || unspecified?(ip)
|
||||||
|
end
|
||||||
|
|
||||||
|
def parse_ip(str)
|
||||||
|
IPAddr.new(str.to_s)
|
||||||
|
rescue IPAddr::Error
|
||||||
|
nil
|
||||||
|
end
|
||||||
|
|
||||||
|
def unspecified?(ip)
|
||||||
|
ip == IPAddr.new("0.0.0.0") || ip == IPAddr.new("::")
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -40,6 +40,12 @@ class SecurityMailer < ApplicationMailer
|
|||||||
mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address
|
mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def suspicious_passkey_used(user, nickname:, ip:, user_agent:, occurred_at:)
|
||||||
|
assign_context(user, ip, user_agent, occurred_at)
|
||||||
|
@nickname = nickname
|
||||||
|
mail subject: "#{SUBJECT_PREFIX}A passkey sign-in was blocked", to: user.email_address
|
||||||
|
end
|
||||||
|
|
||||||
def email_address_changed(user, recipient:, old_email:, new_email:, ip:, user_agent:, occurred_at:)
|
def email_address_changed(user, recipient:, old_email:, new_email:, ip:, user_agent:, occurred_at:)
|
||||||
assign_context(user, ip, user_agent, occurred_at)
|
assign_context(user, ip, user_agent, occurred_at)
|
||||||
@recipient = recipient
|
@recipient = recipient
|
||||||
|
|||||||
@@ -56,6 +56,7 @@ class Application < ApplicationRecord
|
|||||||
message: "must be a valid HTTP or HTTPS URL"
|
message: "must be a valid HTTP or HTTPS URL"
|
||||||
}
|
}
|
||||||
validate :backchannel_logout_uri_must_be_https_in_production, if: -> { backchannel_logout_uri.present? }
|
validate :backchannel_logout_uri_must_be_https_in_production, if: -> { backchannel_logout_uri.present? }
|
||||||
|
validate :backchannel_logout_uri_not_internal, if: -> { backchannel_logout_uri.present? }
|
||||||
|
|
||||||
# Icon validation using ActiveStorage validators
|
# Icon validation using ActiveStorage validators
|
||||||
validate :icon_validation
|
validate :icon_validation
|
||||||
@@ -390,4 +391,17 @@ class Application < ApplicationRecord
|
|||||||
# Let the format validator handle invalid URIs
|
# Let the format validator handle invalid URIs
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# SSRF guard: the backchannel logout URI is dialled server-side on every user
|
||||||
|
# logout, so it must not target internal infrastructure (loopback, private
|
||||||
|
# ranges, or the link-local cloud metadata endpoint). This is the fast,
|
||||||
|
# config-time check; BackchannelLogoutJob re-checks with DNS resolution.
|
||||||
|
def backchannel_logout_uri_not_internal
|
||||||
|
uri = URI.parse(backchannel_logout_uri)
|
||||||
|
if uri.host.present? && PrivateAddressCheck.internal_host?(uri.host)
|
||||||
|
errors.add(:backchannel_logout_uri, "must not point to a private, loopback, or link-local address")
|
||||||
|
end
|
||||||
|
rescue URI::InvalidURIError
|
||||||
|
# Let the format validator handle invalid URIs
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -49,11 +49,21 @@ class OidcRefreshToken < ApplicationRecord
|
|||||||
update!(revoked_at: Time.current)
|
update!(revoked_at: Time.current)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Revoke all refresh tokens in the same family (token rotation security)
|
# Revoke all refresh tokens in the same family (token rotation security).
|
||||||
|
# Also revoke every access token issued within the family: on a detected reuse
|
||||||
|
# attack the stolen chain's access tokens must not remain usable at /userinfo
|
||||||
|
# until they expire.
|
||||||
def revoke_family!
|
def revoke_family!
|
||||||
return unless token_family_id.present?
|
return unless token_family_id.present?
|
||||||
|
|
||||||
OidcRefreshToken.in_family(token_family_id).update_all(revoked_at: Time.current)
|
now = Time.current
|
||||||
|
family = OidcRefreshToken.in_family(token_family_id)
|
||||||
|
access_token_ids = family.pluck(:oidc_access_token_id).compact.uniq
|
||||||
|
|
||||||
|
family.update_all(revoked_at: now)
|
||||||
|
if access_token_ids.any?
|
||||||
|
OidcAccessToken.where(id: access_token_ids, revoked_at: nil).update_all(revoked_at: now)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|||||||
@@ -7,6 +7,9 @@ class Session < ApplicationRecord
|
|||||||
# Scopes
|
# Scopes
|
||||||
scope :active, -> { where("expires_at > ?", Time.current) }
|
scope :active, -> { where("expires_at > ?", Time.current) }
|
||||||
scope :expired, -> { where("expires_at <= ?", Time.current) }
|
scope :expired, -> { where("expires_at <= ?", Time.current) }
|
||||||
|
# Sessions whose owning user is currently active. Used at request time so a
|
||||||
|
# disabled account cannot continue to authenticate with an existing session.
|
||||||
|
scope :for_active_user, -> { joins(:user).where(users: {status: User.statuses[:active]}) }
|
||||||
|
|
||||||
def expired?
|
def expired?
|
||||||
expires_at.present? && expires_at <= Time.current
|
expires_at.present? && expires_at <= Time.current
|
||||||
|
|||||||
@@ -41,6 +41,11 @@ class User < ApplicationRecord
|
|||||||
# Enum - automatically creates scopes (User.active, User.disabled, etc.)
|
# Enum - automatically creates scopes (User.active, User.disabled, etc.)
|
||||||
enum :status, {active: 0, disabled: 1, pending_invitation: 2}
|
enum :status, {active: 0, disabled: 1, pending_invitation: 2}
|
||||||
|
|
||||||
|
# When an account stops being active (e.g. an admin disables it), immediately
|
||||||
|
# terminate its sessions so access is revoked everywhere, not just on expiry.
|
||||||
|
# Defence-in-depth: session lookup also filters by active status at request time.
|
||||||
|
after_update_commit :revoke_sessions_when_deactivated
|
||||||
|
|
||||||
# Scopes
|
# Scopes
|
||||||
scope :admins, -> { joins(:groups).where(groups: {admin: true}).distinct }
|
scope :admins, -> { joins(:groups).where(groups: {admin: true}).distinct }
|
||||||
|
|
||||||
@@ -63,7 +68,10 @@ class User < ApplicationRecord
|
|||||||
def enable_totp!
|
def enable_totp!
|
||||||
require "rotp"
|
require "rotp"
|
||||||
self.totp_secret = ROTP::Base32.random
|
self.totp_secret = ROTP::Base32.random
|
||||||
self.backup_codes = generate_backup_codes
|
# generate_backup_codes assigns the BCrypt hashes to self.backup_codes and
|
||||||
|
# returns the plaintext codes for display. Do NOT reassign backup_codes to the
|
||||||
|
# return value — that would store the plaintext codes and break verification.
|
||||||
|
generate_backup_codes
|
||||||
save!
|
save!
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -86,7 +94,13 @@ class User < ApplicationRecord
|
|||||||
|
|
||||||
require "rotp"
|
require "rotp"
|
||||||
totp = ROTP::TOTP.new(totp_secret)
|
totp = ROTP::TOTP.new(totp_secret)
|
||||||
totp.verify(code, drift_behind: 30, drift_ahead: 30)
|
# Pass `after:` so a code can only be accepted once: ROTP rejects any timestep
|
||||||
|
# at or before the last accepted one, closing the ~90s drift-window replay.
|
||||||
|
verified_at = totp.verify(code, drift_behind: 30, drift_ahead: 30, after: last_otp_at)
|
||||||
|
return false unless verified_at
|
||||||
|
|
||||||
|
update_column(:last_otp_at, verified_at)
|
||||||
|
true
|
||||||
end
|
end
|
||||||
|
|
||||||
# Console/debug helper: get current TOTP code
|
# Console/debug helper: get current TOTP code
|
||||||
@@ -237,6 +251,13 @@ class User < ApplicationRecord
|
|||||||
Group.auto_assign.each { |g| groups << g }
|
Group.auto_assign.each { |g| groups << g }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def revoke_sessions_when_deactivated
|
||||||
|
return unless saved_change_to_status?
|
||||||
|
return if active?
|
||||||
|
|
||||||
|
sessions.destroy_all
|
||||||
|
end
|
||||||
|
|
||||||
def no_reserved_claim_names
|
def no_reserved_claim_names
|
||||||
return if custom_claims.blank?
|
return if custom_claims.blank?
|
||||||
|
|
||||||
|
|||||||
@@ -52,13 +52,17 @@ class WebauthnCredential < ApplicationRecord
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Check if sign count is suspicious (clone detection)
|
# Check if sign count is suspicious (clone detection).
|
||||||
|
#
|
||||||
|
# Per WebAuthn §6.1.1, a signature counter of 0 means the authenticator does
|
||||||
|
# not implement a counter (true of most synced passkeys — Apple/Google report
|
||||||
|
# 0 every time), so it cannot be used for clone detection. Only when BOTH the
|
||||||
|
# stored and presented counts are non-zero does a non-increasing value signal
|
||||||
|
# a possible clone.
|
||||||
def suspicious_sign_count?(new_sign_count)
|
def suspicious_sign_count?(new_sign_count)
|
||||||
return false if sign_count.zero? && new_sign_count > 0 # First use
|
return false if sign_count.zero? || new_sign_count.zero?
|
||||||
return false if new_sign_count > sign_count # Normal increment
|
|
||||||
|
|
||||||
# Sign count didn't increase - possible clone
|
new_sign_count <= sign_count
|
||||||
true
|
|
||||||
end
|
end
|
||||||
|
|
||||||
# Format for display in UI
|
# Format for display in UI
|
||||||
|
|||||||
@@ -9,7 +9,7 @@
|
|||||||
<%= csrf_meta_tags %>
|
<%= csrf_meta_tags %>
|
||||||
<%= csp_meta_tag %>
|
<%= csp_meta_tag %>
|
||||||
|
|
||||||
<script>
|
<script nonce="<%= content_security_policy_nonce %>">
|
||||||
(function() {
|
(function() {
|
||||||
var theme = localStorage.getItem('theme');
|
var theme = localStorage.getItem('theme');
|
||||||
if (theme === 'dark' || (!theme && window.matchMedia('(prefers-color-scheme: dark)').matches)) {
|
if (theme === 'dark' || (!theme && window.matchMedia('(prefers-color-scheme: dark)').matches)) {
|
||||||
|
|||||||
16
app/views/security_mailer/suspicious_passkey_used.html.erb
Normal file
16
app/views/security_mailer/suspicious_passkey_used.html.erb
Normal file
@@ -0,0 +1,16 @@
|
|||||||
|
<p>Hello,</p>
|
||||||
|
|
||||||
|
<p>
|
||||||
|
A sign-in to your Clinch account (<strong><%= @user.email_address %></strong>)
|
||||||
|
using your passkey (<strong><%= @nickname %></strong>) was <strong>blocked</strong>
|
||||||
|
because its security counter did not advance as expected. This can indicate the
|
||||||
|
passkey has been copied (cloned).
|
||||||
|
</p>
|
||||||
|
|
||||||
|
<p>
|
||||||
|
If this was you and you are unable to sign in, remove this passkey and register
|
||||||
|
a new one. If you do not recognise this activity, treat it as a compromise:
|
||||||
|
remove the passkey and review your account security.
|
||||||
|
</p>
|
||||||
|
|
||||||
|
<%= render "event_metadata" %>
|
||||||
11
app/views/security_mailer/suspicious_passkey_used.text.erb
Normal file
11
app/views/security_mailer/suspicious_passkey_used.text.erb
Normal file
@@ -0,0 +1,11 @@
|
|||||||
|
Hello,
|
||||||
|
|
||||||
|
A sign-in to your Clinch account (<%= @user.email_address %>) using your passkey
|
||||||
|
("<%= @nickname %>") was BLOCKED because its security counter did not advance as
|
||||||
|
expected. This can indicate the passkey has been copied (cloned).
|
||||||
|
|
||||||
|
If this was you and you are unable to sign in, remove this passkey and register a
|
||||||
|
new one. If you do not recognise this activity, treat it as a compromise: remove
|
||||||
|
the passkey and review your account security.
|
||||||
|
|
||||||
|
<%= render "event_metadata" %>
|
||||||
@@ -38,7 +38,7 @@
|
|||||||
</svg>
|
</svg>
|
||||||
Continue with Passkey
|
Continue with Passkey
|
||||||
</button>
|
</button>
|
||||||
<div data-webauthn-target="error" class="mt-2 text-sm text-red-600" style="display: none;"></div>
|
<div data-webauthn-target="error" class="mt-2 text-sm text-red-600 hidden"></div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<!-- Password section - shown by default, hidden if WebAuthn is required -->
|
<!-- Password section - shown by default, hidden if WebAuthn is required -->
|
||||||
|
|||||||
@@ -54,7 +54,7 @@
|
|||||||
</svg>
|
</svg>
|
||||||
Use Passkey Instead
|
Use Passkey Instead
|
||||||
</button>
|
</button>
|
||||||
<div data-webauthn-target="error" class="mt-2 text-sm text-red-600" style="display: none;"></div>
|
<div data-webauthn-target="error" class="mt-2 text-sm text-red-600 hidden"></div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|||||||
@@ -118,14 +118,17 @@ Rails.application.configure do
|
|||||||
registrable_domain = domain.domain # Gets "example.com" from "auth.example.com"
|
registrable_domain = domain.domain # Gets "example.com" from "auth.example.com"
|
||||||
|
|
||||||
if registrable_domain.present?
|
if registrable_domain.present?
|
||||||
# Create regex to allow any subdomain of the registrable domain
|
# Allow the registrable domain and any subdomain of it. The pattern is
|
||||||
allowed_hosts << /.*#{Regexp.escape(registrable_domain)}/
|
# anchored (\A...\z) with a mandatory dot before the domain so that
|
||||||
|
# look-alikes such as "evil-example.com" or "example.com.attacker.com"
|
||||||
|
# do NOT match — an unanchored /.*example\.com/ would allow both.
|
||||||
|
allowed_hosts << /\A(.+\.)?#{Regexp.escape(registrable_domain)}\z/i
|
||||||
end
|
end
|
||||||
rescue PublicSuffix::DomainInvalid
|
rescue PublicSuffix::DomainInvalid
|
||||||
# Fallback to simple domain extraction if PublicSuffix fails
|
# Fallback to simple domain extraction if PublicSuffix fails
|
||||||
Rails.logger.warn "Could not parse domain '#{host_domain}' with PublicSuffix, using fallback"
|
Rails.logger.warn "Could not parse domain '#{host_domain}' with PublicSuffix, using fallback"
|
||||||
base_domain = host_domain.split(".").last(2).join(".")
|
base_domain = host_domain.split(".").last(2).join(".")
|
||||||
allowed_hosts << /.*#{Regexp.escape(base_domain)}/
|
allowed_hosts << /\A(.+\.)?#{Regexp.escape(base_domain)}\z/i
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
16
config/initializers/clinch_host.rb
Normal file
16
config/initializers/clinch_host.rb
Normal file
@@ -0,0 +1,16 @@
|
|||||||
|
# CLINCH_HOST is this IdP's canonical external origin, e.g. https://auth.example.com.
|
||||||
|
# It anchors the OIDC issuer, the WebAuthn RP ID, and the forward-auth login
|
||||||
|
# redirect. In deployed (non-local) environments it MUST be set explicitly and
|
||||||
|
# never inferred from request headers — X-Forwarded-Host is attacker-influenceable,
|
||||||
|
# so inferring the origin from it would allow host-header phishing and open
|
||||||
|
# redirects. Fail fast at boot rather than start in an unsafe configuration.
|
||||||
|
#
|
||||||
|
# Skipped during asset precompilation (e.g. the Docker build step, which sets
|
||||||
|
# SECRET_KEY_BASE_DUMMY): no real CLINCH_HOST exists yet and assets don't need it.
|
||||||
|
unless Rails.env.local? || ENV["SECRET_KEY_BASE_DUMMY"].present?
|
||||||
|
if ENV["CLINCH_HOST"].blank?
|
||||||
|
raise "CLINCH_HOST must be set (e.g. https://auth.example.com). It is the " \
|
||||||
|
"canonical origin of this Clinch instance and must not be inferred " \
|
||||||
|
"from request headers."
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -9,13 +9,15 @@ Rails.application.configure do
|
|||||||
# Default to self for everything, plus blob: for file downloads
|
# Default to self for everything, plus blob: for file downloads
|
||||||
policy.default_src :self, "blob:"
|
policy.default_src :self, "blob:"
|
||||||
|
|
||||||
# Scripts: Allow self, importmaps, unsafe-inline for Turbo/StimulusJS, and blob: for downloads
|
# Scripts: self + per-response nonce (see nonce config below) + blob: for
|
||||||
# Note: unsafe_inline is needed for Stimulus controllers and Turbo navigation
|
# downloads. No unsafe-inline — importmap/Turbo/Stimulus inline tags carry the
|
||||||
policy.script_src :self, :unsafe_inline, "blob:"
|
# nonce automatically, and the one hand-written inline script is nonced.
|
||||||
|
policy.script_src :self, "blob:"
|
||||||
|
|
||||||
# Styles: Allow self and unsafe_inline for TailwindCSS dynamic classes
|
# Styles: self + per-response nonce. No unsafe-inline — Tailwind ships as an
|
||||||
# and Stimulus controller style manipulations
|
# external stylesheet, Turbo's injected <style> carries the nonce, and Stimulus
|
||||||
policy.style_src :self, :unsafe_inline
|
# sets styles via the CSSOM (not governed by CSP).
|
||||||
|
policy.style_src :self
|
||||||
|
|
||||||
# Images: Allow self, data URLs, and https for external images
|
# Images: Allow self, data URLs, and https for external images
|
||||||
policy.img_src :self, :data, :https
|
policy.img_src :self, :data, :https
|
||||||
@@ -59,6 +61,13 @@ Rails.application.configure do
|
|||||||
policy.report_uri "/api/csp-violation-report"
|
policy.report_uri "/api/csp-violation-report"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Per-response random nonce applied to script-src and style-src. The app does
|
||||||
|
# not page-cache HTML, so a fresh random nonce per response is the most secure
|
||||||
|
# choice (no reuse across responses). csp_meta_tag (in the layout) and
|
||||||
|
# importmap-rails read this nonce automatically.
|
||||||
|
config.content_security_policy_nonce_generator = ->(_request) { SecureRandom.base64(16) }
|
||||||
|
config.content_security_policy_nonce_directives = %w[script-src style-src]
|
||||||
|
|
||||||
# Start with CSP in report-only mode for testing
|
# Start with CSP in report-only mode for testing
|
||||||
# Set to false after verifying everything works in production
|
# Set to false after verifying everything works in production
|
||||||
config.content_security_policy_report_only = Rails.env.development?
|
config.content_security_policy_report_only = Rails.env.development?
|
||||||
|
|||||||
@@ -4,5 +4,8 @@
|
|||||||
# Use this to limit dissemination of sensitive information.
|
# Use this to limit dissemination of sensitive information.
|
||||||
# See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors.
|
# See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors.
|
||||||
Rails.application.config.filter_parameters += [
|
Rails.application.config.filter_parameters += [
|
||||||
:passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc, :backup
|
:passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc, :backup,
|
||||||
|
# :code partially matches the TOTP/backup `code` param, the OAuth authorization
|
||||||
|
# `code`, and the PKCE `code_verifier`/`code_challenge` — all sensitive in logs.
|
||||||
|
:code
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
module Clinch
|
module Clinch
|
||||||
VERSION = "0.15.0"
|
VERSION = "0.16.0"
|
||||||
end
|
end
|
||||||
|
|||||||
7
db/migrate/20260611000001_add_last_otp_at_to_users.rb
Normal file
7
db/migrate/20260611000001_add_last_otp_at_to_users.rb
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
class AddLastOtpAtToUsers < ActiveRecord::Migration[8.1]
|
||||||
|
def change
|
||||||
|
# Unix timestamp of the most recently accepted TOTP timestep, used to reject
|
||||||
|
# replay of a code within its drift window (passed to ROTP's `after:`).
|
||||||
|
add_column :users, :last_otp_at, :integer
|
||||||
|
end
|
||||||
|
end
|
||||||
3
db/schema.rb
generated
3
db/schema.rb
generated
@@ -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: 2026_06_07_000003) do
|
ActiveRecord::Schema[8.1].define(version: 2026_06_11_000001) 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
|
||||||
@@ -233,6 +233,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_06_07_000003) do
|
|||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
t.json "custom_claims", default: {}, null: false
|
t.json "custom_claims", default: {}, null: false
|
||||||
t.string "email_address", null: false
|
t.string "email_address", null: false
|
||||||
|
t.integer "last_otp_at"
|
||||||
t.datetime "last_sign_in_at"
|
t.datetime "last_sign_in_at"
|
||||||
t.string "name"
|
t.string "name"
|
||||||
t.string "password_digest", null: false
|
t.string "password_digest", null: false
|
||||||
|
|||||||
@@ -113,6 +113,42 @@ module Api
|
|||||||
assert_equal "Application is inactive", json["error"]
|
assert_equal "Application is inactive", json["error"]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "bearer token returns 401 once user is removed from allowed groups" do
|
||||||
|
# App restricted to a specific group; user is a member when the key is made.
|
||||||
|
group = Group.create!(name: "webdav-users")
|
||||||
|
restricted_app = Application.create!(
|
||||||
|
name: "Restricted WebDAV",
|
||||||
|
slug: "restricted-webdav",
|
||||||
|
app_type: "forward_auth",
|
||||||
|
domain_pattern: "restricted.example.com",
|
||||||
|
active: true
|
||||||
|
)
|
||||||
|
restricted_app.allowed_groups << group
|
||||||
|
@user.groups << group
|
||||||
|
|
||||||
|
key = @user.api_keys.create!(name: "Restricted Key", application: restricted_app)
|
||||||
|
token = key.plaintext_token
|
||||||
|
|
||||||
|
# Sanity: access works while membership stands.
|
||||||
|
get "/api/verify", headers: {
|
||||||
|
"Authorization" => "Bearer #{token}",
|
||||||
|
"X-Forwarded-Host" => "restricted.example.com"
|
||||||
|
}
|
||||||
|
assert_response :ok
|
||||||
|
|
||||||
|
# Revoke group membership; the existing key must stop working.
|
||||||
|
@user.groups.destroy(group)
|
||||||
|
|
||||||
|
get "/api/verify", headers: {
|
||||||
|
"Authorization" => "Bearer #{token}",
|
||||||
|
"X-Forwarded-Host" => "restricted.example.com"
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_response :unauthorized
|
||||||
|
json = JSON.parse(response.body)
|
||||||
|
assert_equal "Access denied: insufficient group membership", json["error"]
|
||||||
|
end
|
||||||
|
|
||||||
test "no bearer token falls through to cookie auth" do
|
test "no bearer token falls through to cookie auth" do
|
||||||
# No auth header, no session -> should redirect (cookie flow)
|
# No auth header, no session -> should redirect (cookie flow)
|
||||||
get "/api/verify", headers: {
|
get "/api/verify", headers: {
|
||||||
|
|||||||
@@ -242,6 +242,20 @@ module Api
|
|||||||
assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"]
|
assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Fail closed when no host can be determined: emitting identity headers without
|
||||||
|
# an application would bypass all per-domain group access control.
|
||||||
|
test "should fail closed and emit no identity headers when host is absent" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
|
||||||
|
# Blank both host sources so forwarded_host is not present.
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "", "Host" => ""}
|
||||||
|
|
||||||
|
assert_response 403
|
||||||
|
assert_equal "No host header present", response.headers["x-auth-reason"]
|
||||||
|
assert_nil response.headers["X-Remote-User"]
|
||||||
|
assert_nil response.headers["X-Remote-Groups"]
|
||||||
|
end
|
||||||
|
|
||||||
# Security Tests
|
# Security Tests
|
||||||
test "should handle very long domain names" do
|
test "should handle very long domain names" do
|
||||||
long_domain = "a" * 250 + ".example.com"
|
long_domain = "a" * 250 + ".example.com"
|
||||||
|
|||||||
@@ -34,6 +34,25 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
|
|||||||
# CRITICAL SECURITY TESTS
|
# CRITICAL SECURITY TESTS
|
||||||
# ====================
|
# ====================
|
||||||
|
|
||||||
|
test "consent endpoint rejects cross-site POST without a CSRF token" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
|
||||||
|
# Forgery protection is disabled in the test env by default; enable it so the
|
||||||
|
# before_action actually runs, mirroring production behaviour.
|
||||||
|
original = ActionController::Base.allow_forgery_protection
|
||||||
|
ActionController::Base.allow_forgery_protection = true
|
||||||
|
begin
|
||||||
|
# No authenticity_token param: a forged cross-site submission. Because
|
||||||
|
# :consent is NOT in the verify_authenticity_token skip list, this must be
|
||||||
|
# rejected before the action can grant any OAuth scopes.
|
||||||
|
post "/oauth/authorize/consent", params: {approve: "true"}
|
||||||
|
|
||||||
|
assert_response :unprocessable_entity
|
||||||
|
ensure
|
||||||
|
ActionController::Base.allow_forgery_protection = original
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
test "prevents authorization code reuse - sequential attempts" do
|
test "prevents authorization code reuse - sequential attempts" do
|
||||||
# Create consent
|
# Create consent
|
||||||
OidcUserConsent.create!(
|
OidcUserConsent.create!(
|
||||||
|
|||||||
@@ -213,6 +213,50 @@ class OidcRefreshTokenControllerTest < ActionDispatch::IntegrationTest
|
|||||||
assert_equal family_id, new_refresh_token.token_family_id
|
assert_equal family_id, new_refresh_token.token_family_id
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "reusing a revoked refresh token revokes every access token in the family" do
|
||||||
|
access_token = OidcAccessToken.create!(
|
||||||
|
application: @application,
|
||||||
|
user: @user,
|
||||||
|
scope: "openid profile email"
|
||||||
|
)
|
||||||
|
refresh_token = OidcRefreshToken.create!(
|
||||||
|
application: @application,
|
||||||
|
user: @user,
|
||||||
|
oidc_access_token: access_token,
|
||||||
|
scope: "openid profile email"
|
||||||
|
)
|
||||||
|
family_id = refresh_token.token_family_id
|
||||||
|
old_plaintext = refresh_token.token
|
||||||
|
|
||||||
|
# Rotate once: the old refresh token is revoked; a new access + refresh token
|
||||||
|
# are issued into the same family.
|
||||||
|
post "/oauth/token", params: {
|
||||||
|
grant_type: "refresh_token",
|
||||||
|
refresh_token: old_plaintext,
|
||||||
|
client_id: @application.client_id,
|
||||||
|
client_secret: @client_secret
|
||||||
|
}
|
||||||
|
assert_response :success
|
||||||
|
|
||||||
|
new_refresh = OidcRefreshToken.in_family(family_id).where.not(id: refresh_token.id).first
|
||||||
|
new_access_token = new_refresh.oidc_access_token
|
||||||
|
refute new_access_token.reload.revoked?, "rotated-in access token should start active"
|
||||||
|
|
||||||
|
# Reuse the OLD (now revoked) refresh token -> rotation-attack detection.
|
||||||
|
post "/oauth/token", params: {
|
||||||
|
grant_type: "refresh_token",
|
||||||
|
refresh_token: old_plaintext,
|
||||||
|
client_id: @application.client_id,
|
||||||
|
client_secret: @client_secret
|
||||||
|
}
|
||||||
|
assert_response :bad_request
|
||||||
|
|
||||||
|
# Both the original and the rotated-in access token must now be revoked, so a
|
||||||
|
# stolen access token from anywhere in the chain stops working at /userinfo.
|
||||||
|
assert access_token.reload.revoked?, "original access token should be revoked"
|
||||||
|
assert new_access_token.reload.revoked?, "rotated-in access token should be revoked"
|
||||||
|
end
|
||||||
|
|
||||||
test "userinfo endpoint works with hashed access token" do
|
test "userinfo endpoint works with hashed access token" do
|
||||||
access_token = OidcAccessToken.create!(
|
access_token = OidcAccessToken.create!(
|
||||||
application: @application,
|
application: @application,
|
||||||
|
|||||||
@@ -19,16 +19,21 @@ class TotpSecurityTest < ActionDispatch::IntegrationTest
|
|||||||
|
|
||||||
# First use of the code should succeed
|
# First use of the code should succeed
|
||||||
post totp_verification_path, params: {code: valid_code}
|
post totp_verification_path, params: {code: valid_code}
|
||||||
assert_response :redirect
|
|
||||||
assert_redirected_to root_path
|
assert_redirected_to root_path
|
||||||
|
|
||||||
# Sign out
|
# Sign out
|
||||||
delete session_path
|
delete session_path
|
||||||
assert_response :redirect
|
assert_response :redirect
|
||||||
|
|
||||||
# Note: In the current implementation, TOTP codes CAN be reused within the 60-second time window
|
# Replay the SAME code in a fresh sign-in attempt. Because verify_totp records
|
||||||
# This is standard TOTP behavior. For enhanced security, you could implement used code tracking.
|
# the accepted timestep (ROTP `after:`), the code is now rejected even though
|
||||||
# This test documents the current behavior - codes work within their time window
|
# it is still within its drift window — so we stay on the verification step.
|
||||||
|
post signin_path, params: {email_address: "totp_replay_test@example.com", password: "password123"}
|
||||||
|
assert_redirected_to totp_verification_path
|
||||||
|
|
||||||
|
post totp_verification_path, params: {code: valid_code}
|
||||||
|
assert_redirected_to totp_verification_path
|
||||||
|
assert_equal "Invalid verification code. Please try again.", flash[:alert]
|
||||||
|
|
||||||
user.sessions.delete_all
|
user.sessions.delete_all
|
||||||
user.destroy
|
user.destroy
|
||||||
|
|||||||
40
test/integration/csp_test.rb
Normal file
40
test/integration/csp_test.rb
Normal file
@@ -0,0 +1,40 @@
|
|||||||
|
require "test_helper"
|
||||||
|
|
||||||
|
class CspTest < ActionDispatch::IntegrationTest
|
||||||
|
# In the test env content_security_policy_report_only is false, so the enforcing
|
||||||
|
# Content-Security-Policy header is emitted.
|
||||||
|
test "signin page sends a nonce-based CSP with no unsafe-inline" do
|
||||||
|
get signin_path
|
||||||
|
assert_response :success
|
||||||
|
|
||||||
|
csp = response.headers["Content-Security-Policy"]
|
||||||
|
assert csp.present?, "expected a Content-Security-Policy header"
|
||||||
|
|
||||||
|
script_src = directive(csp, "script-src")
|
||||||
|
style_src = directive(csp, "style-src")
|
||||||
|
|
||||||
|
assert_includes script_src, "'nonce-", "script-src must carry a nonce"
|
||||||
|
assert_includes style_src, "'nonce-", "style-src must carry a nonce"
|
||||||
|
refute_includes script_src, "'unsafe-inline'", "script-src must not allow unsafe-inline"
|
||||||
|
refute_includes style_src, "'unsafe-inline'", "style-src must not allow unsafe-inline"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "the inline theme script carries the script-src nonce" do
|
||||||
|
get signin_path
|
||||||
|
assert_response :success
|
||||||
|
|
||||||
|
header_nonce = response.headers["Content-Security-Policy"][/script-src[^;]*'nonce-([^']+)'/, 1]
|
||||||
|
assert header_nonce.present?, "expected a nonce in the CSP header"
|
||||||
|
|
||||||
|
# The hand-written dark-mode <script> in the layout must use the same nonce,
|
||||||
|
# otherwise it would be blocked under the enforcing policy.
|
||||||
|
assert_match(/<script nonce="#{Regexp.escape(header_nonce)}">/, response.body,
|
||||||
|
"inline theme script must carry the matching CSP nonce")
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def directive(csp, name)
|
||||||
|
csp.split(";").map(&:strip).find { |d| d.start_with?("#{name} ") } || ""
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -153,6 +153,10 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
|
|||||||
|
|
||||||
# Redirect URL Integration Tests
|
# Redirect URL Integration Tests
|
||||||
test "unauthenticated request redirects to signin with parameters" do
|
test "unauthenticated request redirects to signin with parameters" do
|
||||||
|
# grafana.example.com must be a registered forward-auth app for its URL to be
|
||||||
|
# honoured as a redirect target (otherwise it would be an open-redirect vector).
|
||||||
|
grant_everyone_access Application.create!(name: "Grafana", slug: "grafana", app_type: "forward_auth", domain_pattern: "grafana.example.com", active: true)
|
||||||
|
|
||||||
# Test that unauthenticated requests redirect to signin with rd and rm parameters
|
# Test that unauthenticated requests redirect to signin with rd and rm parameters
|
||||||
get "/api/verify", headers: {
|
get "/api/verify", headers: {
|
||||||
"X-Forwarded-Host" => "grafana.example.com"
|
"X-Forwarded-Host" => "grafana.example.com"
|
||||||
@@ -172,7 +176,27 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
|
|||||||
assert_includes location, "grafana.example.com"
|
assert_includes location, "grafana.example.com"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "spoofed X-Forwarded-Host is not reflected as a redirect target" do
|
||||||
|
# No forward-auth app exists for evil.com, and no valid rd is supplied. The
|
||||||
|
# attacker-controlled host must NOT be stored or reflected into the signin URL,
|
||||||
|
# and base_url must come from CLINCH_HOST (or the safe localhost default in
|
||||||
|
# test) rather than the request host.
|
||||||
|
get "/api/verify", headers: {
|
||||||
|
"X-Forwarded-Host" => "evil.com",
|
||||||
|
"X-Forwarded-Uri" => "/steal"
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_response 302
|
||||||
|
assert_match %r{/signin}, response.location
|
||||||
|
refute_includes response.location, "evil.com"
|
||||||
|
refute_match(/evil\.com/, session[:return_to_after_authenticating].to_s)
|
||||||
|
end
|
||||||
|
|
||||||
test "return URL functionality after authentication" do
|
test "return URL functionality after authentication" do
|
||||||
|
# app.example.com must be a registered forward-auth app for its URL to be
|
||||||
|
# honoured as a redirect target.
|
||||||
|
grant_everyone_access Application.create!(name: "App FA", slug: "app-fa", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
|
||||||
|
|
||||||
# Initial request should set return URL
|
# Initial request should set return URL
|
||||||
get "/api/verify", headers: {
|
get "/api/verify", headers: {
|
||||||
"X-Forwarded-Host" => "app.example.com",
|
"X-Forwarded-Host" => "app.example.com",
|
||||||
|
|||||||
@@ -1,6 +1,49 @@
|
|||||||
require "test_helper"
|
require "test_helper"
|
||||||
|
|
||||||
class SessionSecurityTest < ActionDispatch::IntegrationTest
|
class SessionSecurityTest < ActionDispatch::IntegrationTest
|
||||||
|
# ====================
|
||||||
|
# ACCOUNT DEACTIVATION TESTS
|
||||||
|
# ====================
|
||||||
|
|
||||||
|
test "TOTP verification rejects a user disabled mid-flow" do
|
||||||
|
user = User.create!(email_address: "midflow_totp@example.com", password: "password123")
|
||||||
|
user.enable_totp!
|
||||||
|
code = ROTP::TOTP.new(user.totp_secret).now
|
||||||
|
|
||||||
|
# Phase A: password step stashes the pending 2FA user
|
||||||
|
post signin_path, params: {email_address: "midflow_totp@example.com", password: "password123"}
|
||||||
|
assert_redirected_to totp_verification_path
|
||||||
|
|
||||||
|
# Admin disables the account while the user is on the 2FA screen
|
||||||
|
user.update!(status: :disabled)
|
||||||
|
|
||||||
|
# Phase B: completing TOTP must NOT create a session
|
||||||
|
post totp_verification_path, params: {code: code}
|
||||||
|
assert_redirected_to signin_path
|
||||||
|
assert_equal 0, user.reload.sessions.count
|
||||||
|
|
||||||
|
user.destroy
|
||||||
|
end
|
||||||
|
|
||||||
|
test "an existing session stops authenticating once the user is disabled" do
|
||||||
|
user = User.create!(email_address: "disabled_session@example.com", password: "password123")
|
||||||
|
sign_in_as(user)
|
||||||
|
|
||||||
|
get root_path
|
||||||
|
assert_response :success
|
||||||
|
|
||||||
|
# Disable bypassing the destroy callback to isolate the request-time lookup
|
||||||
|
# guard (find_session_by_cookie filtering on active users).
|
||||||
|
user.update_column(:status, User.statuses[:disabled])
|
||||||
|
|
||||||
|
get root_path
|
||||||
|
assert_response :redirect
|
||||||
|
assert_match %r{/signin}, response.location
|
||||||
|
|
||||||
|
user.sessions.delete_all
|
||||||
|
user.destroy
|
||||||
|
end
|
||||||
|
|
||||||
# ====================
|
# ====================
|
||||||
# SESSION TIMEOUT TESTS
|
# SESSION TIMEOUT TESTS
|
||||||
# ====================
|
# ====================
|
||||||
@@ -199,7 +242,7 @@ class SessionSecurityTest < ActionDispatch::IntegrationTest
|
|||||||
slug: "logout-test-app",
|
slug: "logout-test-app",
|
||||||
app_type: "oidc",
|
app_type: "oidc",
|
||||||
redirect_uris: ["http://localhost:4000/callback"].to_json,
|
redirect_uris: ["http://localhost:4000/callback"].to_json,
|
||||||
backchannel_logout_uri: "http://localhost:4000/logout",
|
backchannel_logout_uri: "https://rp.example.com/backchannel-logout",
|
||||||
active: true
|
active: true
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
38
test/lib/private_address_check_test.rb
Normal file
38
test/lib/private_address_check_test.rb
Normal file
@@ -0,0 +1,38 @@
|
|||||||
|
require "test_helper"
|
||||||
|
|
||||||
|
class PrivateAddressCheckTest < ActiveSupport::TestCase
|
||||||
|
# internal_host? — DNS-free checks on IP literals and known hostnames
|
||||||
|
test "flags loopback, private, and link-local IP literals as internal" do
|
||||||
|
%w[
|
||||||
|
127.0.0.1
|
||||||
|
10.0.0.1
|
||||||
|
172.16.5.5
|
||||||
|
192.168.1.1
|
||||||
|
169.254.169.254
|
||||||
|
0.0.0.0
|
||||||
|
::1
|
||||||
|
].each do |host|
|
||||||
|
assert PrivateAddressCheck.internal_host?(host), "expected #{host} to be internal"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "flags localhost-style hostnames as internal" do
|
||||||
|
assert PrivateAddressCheck.internal_host?("localhost")
|
||||||
|
assert PrivateAddressCheck.internal_host?("foo.localhost")
|
||||||
|
assert PrivateAddressCheck.internal_host?("metadata.google.internal")
|
||||||
|
assert PrivateAddressCheck.internal_host?("")
|
||||||
|
end
|
||||||
|
|
||||||
|
test "does not flag public IP literals as internal" do
|
||||||
|
refute PrivateAddressCheck.internal_host?("8.8.8.8")
|
||||||
|
refute PrivateAddressCheck.internal_host?("1.1.1.1")
|
||||||
|
end
|
||||||
|
|
||||||
|
# resolves_to_internal? on IP literals (no DNS needed) exercises the same
|
||||||
|
# address classification used after resolution.
|
||||||
|
test "resolves_to_internal? classifies IP literals" do
|
||||||
|
assert PrivateAddressCheck.resolves_to_internal?("169.254.169.254")
|
||||||
|
assert PrivateAddressCheck.resolves_to_internal?("127.0.0.1")
|
||||||
|
refute PrivateAddressCheck.resolves_to_internal?("8.8.8.8")
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -54,6 +54,15 @@ class SecurityMailerTest < ActionMailer::TestCase
|
|||||||
assert_bodies_contain email, "Old MacBook"
|
assert_bodies_contain email, "Old MacBook"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "suspicious_passkey_used warns about a blocked clone sign-in" do
|
||||||
|
email = SecurityMailer.suspicious_passkey_used(@user, nickname: "Yubikey-5", **CONTEXT)
|
||||||
|
|
||||||
|
assert_equal [@user.email_address], email.to
|
||||||
|
assert_match(/blocked/i, email.subject)
|
||||||
|
assert_bodies_contain email, "Yubikey-5"
|
||||||
|
assert_bodies_match email, /clon/i
|
||||||
|
end
|
||||||
|
|
||||||
test "api_key_created includes the key name" do
|
test "api_key_created includes the key name" do
|
||||||
email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT)
|
email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT)
|
||||||
|
|
||||||
|
|||||||
@@ -56,4 +56,29 @@ class ApplicationTest < ActiveSupport::TestCase
|
|||||||
tempfile&.close
|
tempfile&.close
|
||||||
tempfile&.unlink
|
tempfile&.unlink
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "rejects backchannel_logout_uri pointing at internal addresses (SSRF guard)" do
|
||||||
|
app = applications(:kavita_app)
|
||||||
|
|
||||||
|
internal_uris = [
|
||||||
|
"http://127.0.0.1/logout",
|
||||||
|
"http://localhost/logout",
|
||||||
|
"https://169.254.169.254/latest/meta-data/",
|
||||||
|
"http://10.0.0.5/logout",
|
||||||
|
"http://192.168.1.10/logout"
|
||||||
|
]
|
||||||
|
|
||||||
|
internal_uris.each do |uri|
|
||||||
|
app.backchannel_logout_uri = uri
|
||||||
|
refute app.valid?, "expected #{uri} to be rejected"
|
||||||
|
assert_includes app.errors[:backchannel_logout_uri].join, "private, loopback, or link-local"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "allows backchannel_logout_uri pointing at a public host" do
|
||||||
|
app = applications(:kavita_app)
|
||||||
|
app.backchannel_logout_uri = "https://relying-party.example.com/backchannel-logout"
|
||||||
|
|
||||||
|
assert app.valid?, app.errors.full_messages.to_sentence
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1,6 +1,27 @@
|
|||||||
require "test_helper"
|
require "test_helper"
|
||||||
|
|
||||||
class UserTest < ActiveSupport::TestCase
|
class UserTest < ActiveSupport::TestCase
|
||||||
|
test "disabling a user destroys their active sessions" do
|
||||||
|
user = User.create!(email_address: "disable_sessions@example.com", password: "password123")
|
||||||
|
user.sessions.create!
|
||||||
|
user.sessions.create!
|
||||||
|
assert_equal 2, user.sessions.count
|
||||||
|
|
||||||
|
user.update!(status: :disabled)
|
||||||
|
|
||||||
|
assert_equal 0, user.reload.sessions.count
|
||||||
|
end
|
||||||
|
|
||||||
|
test "reactivating or other updates do not destroy sessions" do
|
||||||
|
user = User.create!(email_address: "keep_sessions@example.com", password: "password123")
|
||||||
|
user.sessions.create!
|
||||||
|
|
||||||
|
# An update that does not change status must leave sessions intact.
|
||||||
|
user.update!(username: "keepsessions")
|
||||||
|
|
||||||
|
assert_equal 1, user.reload.sessions.count
|
||||||
|
end
|
||||||
|
|
||||||
test "downcases and strips email_address" do
|
test "downcases and strips email_address" do
|
||||||
user = User.new(email_address: " DOWNCASED@EXAMPLE.COM ")
|
user = User.new(email_address: " DOWNCASED@EXAMPLE.COM ")
|
||||||
assert_equal("downcased@example.com", user.email_address)
|
assert_equal("downcased@example.com", user.email_address)
|
||||||
|
|||||||
29
test/models/webauthn_credential_test.rb
Normal file
29
test/models/webauthn_credential_test.rb
Normal file
@@ -0,0 +1,29 @@
|
|||||||
|
require "test_helper"
|
||||||
|
|
||||||
|
class WebauthnCredentialTest < ActiveSupport::TestCase
|
||||||
|
# suspicious_sign_count?(new_sign_count) — clone detection per WebAuthn §6.1.1.
|
||||||
|
# Build an in-memory credential with a given stored sign_count; no persistence
|
||||||
|
# needed since the method only reads self.sign_count.
|
||||||
|
def credential(stored:)
|
||||||
|
WebauthnCredential.new(sign_count: stored)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "does not flag when the authenticator reports no counter (synced passkeys)" do
|
||||||
|
# Both 0 -> authenticator doesn't implement a counter; must NOT be suspicious.
|
||||||
|
refute credential(stored: 0).suspicious_sign_count?(0)
|
||||||
|
# Stored 0, first real use.
|
||||||
|
refute credential(stored: 0).suspicious_sign_count?(5)
|
||||||
|
# Stored non-zero but authenticator now reports 0 -> no counter, not a clone.
|
||||||
|
refute credential(stored: 5).suspicious_sign_count?(0)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "does not flag a normal increasing counter" do
|
||||||
|
refute credential(stored: 5).suspicious_sign_count?(6)
|
||||||
|
refute credential(stored: 1).suspicious_sign_count?(1000)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "flags a non-advancing counter as a possible clone" do
|
||||||
|
assert credential(stored: 5).suspicious_sign_count?(5), "equal count is suspicious"
|
||||||
|
assert credential(stored: 5).suspicious_sign_count?(3), "decreasing count is suspicious"
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in New Issue
Block a user