Untrack SECURITY_REVIEW_TODO.md and gitignore it

Keep the findings tracker local-only; it should not be published.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

.github/workflows/build.yml

@@ -0,0 +1,133 @@
+name: Build and publish image
+
+# Publishes the multi-arch image (amd64 + arm64) to GitHub Packages
+# (ghcr.io/dkam/clinch) whenever config/initializers/version.rb changes on
+# main — a version bump IS the release. Each arch builds natively (no QEMU); a
+# merge job stitches them into one manifest tagged :vX.Y.Z (+ :latest for
+# non-pre-releases).
+#
+# To cut a release: edit Clinch::VERSION in config/initializers/version.rb,
+# commit, push. For a dev build: set a pre-release version (e.g. "1.1.0-dev") —
+# it publishes :v1.1.0-dev but does not move :latest. Or run this workflow
+# manually from the Actions tab.
+
+on:
+  push:
+    branches: [ main ]
+    paths:
+      - config/initializers/version.rb
+  workflow_dispatch:
+
+env:
+  IMAGE: ghcr.io/${{ github.repository }}
+
+jobs:
+  # Read the SemVer constant; decide whether this release moves :latest.
+  prepare:
+    runs-on: ubuntu-latest
+    outputs:
+      version: ${{ steps.version.outputs.version }}
+      latest: ${{ steps.version.outputs.latest }}
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v5
+
+      - name: Read version from config/initializers/version.rb
+        id: version
+        run: |
+          V=$(ruby -e "require './config/initializers/version'; puts Clinch::VERSION")
+          echo "version=$V" >> "$GITHUB_OUTPUT"
+          # A pre-release (e.g. 1.1.0-dev) publishes its own tag but not :latest.
+          if [[ "$V" == *-* ]]; then latest=false; else latest=true; fi
+          echo "latest=$latest" >> "$GITHUB_OUTPUT"
+          echo "Building v$V (move :latest = $latest)"
+
+  build:
+    needs: prepare
+    runs-on: ${{ matrix.runner }}
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - platform: linux/amd64
+            arch: amd64
+            runner: ubuntu-latest
+          - platform: linux/arm64
+            arch: arm64
+            runner: ubuntu-24.04-arm
+    permissions:
+      contents: read
+      packages: write
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v5
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+
+      - name: Log in to GitHub Container Registry
+        uses: docker/login-action@v3
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Build and push by digest
+        id: build
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          platforms: ${{ matrix.platform }}
+          cache-from: type=gha,scope=${{ matrix.arch }}
+          cache-to: type=gha,mode=max,scope=${{ matrix.arch }}
+          outputs: type=image,name=${{ env.IMAGE }},push-by-digest=true,name-canonical=true,push=true
+
+      - name: Export digest
+        run: |
+          mkdir -p /tmp/digests
+          digest="${{ steps.build.outputs.digest }}"
+          touch "/tmp/digests/${digest#sha256:}"
+
+      - name: Upload digest
+        uses: actions/upload-artifact@v4
+        with:
+          name: digests-${{ matrix.arch }}
+          path: /tmp/digests/*
+          if-no-files-found: error
+          retention-days: 1
+
+  merge:
+    needs: [prepare, build]
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      packages: write
+    steps:
+      - name: Download digests
+        uses: actions/download-artifact@v4
+        with:
+          path: /tmp/digests
+          pattern: digests-*
+          merge-multiple: true
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+
+      - name: Log in to GitHub Container Registry
+        uses: docker/login-action@v3
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Create and push the multi-arch manifest
+        working-directory: /tmp/digests
+        run: |
+          tags="-t ${{ env.IMAGE }}:v${{ needs.prepare.outputs.version }}"
+          if [ "${{ needs.prepare.outputs.latest }}" = "true" ]; then
+            tags="$tags -t ${{ env.IMAGE }}:latest"
+          fi
+          docker buildx imagetools create $tags $(printf '${{ env.IMAGE }}@sha256:%s ' *)
+
+      - name: Inspect result
+        run: docker buildx imagetools inspect ${{ env.IMAGE }}:latest

.gitignore

@@ -70,3 +70,6 @@ yarn-debug.log*
 
 # Ignore bootsnap cache
 /tmp/cache/bootsnap*
+
+# Local-only: do not publish the security findings tracker
+SECURITY_REVIEW_TODO.md

.ruby-version

@@ -1 +1 @@
-4.0.3
+4.0.5

Dockerfile

@@ -8,7 +8,7 @@
 # For a containerized dev environment, see Dev Containers: https://guides.rubyonrails.org/getting_started_with_devcontainer.html
 
 # Make sure RUBY_VERSION matches the Ruby version in .ruby-version
-ARG RUBY_VERSION=4.0.3
+ARG RUBY_VERSION=4.0.5
 FROM docker.io/library/ruby:$RUBY_VERSION-slim AS base
 
 LABEL org.opencontainers.image.source=https://github.com/dkam/clinch

Gemfile.lock

@@ -1,7 +1,7 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    action_text-trix (2.1.18)
+    action_text-trix (2.1.19)
       railties
     actioncable (8.1.3)
       actionpack (= 8.1.3)
@@ -85,9 +85,9 @@ GEM
     bigdecimal (4.1.2)
     bindata (2.5.1)
     bindex (0.8.1)
-    bootsnap (1.24.1)
+    bootsnap (1.24.6)
       msgpack (~> 1.2)
-    brakeman (8.0.4)
+    brakeman (8.0.5)
       racc
     builder (3.3.0)
     bundler-audit (0.9.3)
@@ -102,11 +102,11 @@ GEM
       rack-test (>= 0.6.3)
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
-    cbor (0.5.10.2)
+    cbor (0.5.10.3)
     childprocess (5.1.0)
       logger (~> 1.5)
     chunky_png (1.4.0)
-    concurrent-ruby (1.3.6)
+    concurrent-ruby (1.3.7)
     connection_pool (3.0.2)
     cose (1.3.1)
       cbor (~> 0.5.9)
@@ -131,12 +131,12 @@ GEM
     ffi (1.17.4-arm64-darwin)
     ffi (1.17.4-x86_64-linux-gnu)
     ffi (1.17.4-x86_64-linux-musl)
-    fugit (1.12.1)
+    fugit (1.12.2)
       et-orbi (~> 1.4)
       raabro (~> 1.4)
     globalid (1.3.0)
       activesupport (>= 6.1)
-    i18n (1.14.8)
+    i18n (1.15.2)
       concurrent-ruby (~> 1.0)
     image_processing (1.14.0)
       mini_magick (>= 4.9.5, < 6)
@@ -151,13 +151,13 @@ GEM
       prism (>= 1.3.0)
       rdoc (>= 4.0.0)
       reline (>= 0.4.2)
-    jbuilder (2.14.1)
+    jbuilder (2.15.1)
       actionview (>= 7.0.0)
       activesupport (>= 7.0.0)
-    json (2.19.4)
+    json (2.19.9)
-    jwt (3.1.2)
+    jwt (3.2.0)
       base64
-    kamal (2.11.0)
+    kamal (2.12.0)
       activesupport (>= 7.0)
       base64 (~> 0.2)
       bcrypt_pbkdf (~> 1.0)
@@ -186,14 +186,14 @@ GEM
       net-imap
       net-pop
       net-smtp
-    marcel (1.1.0)
+    marcel (1.2.1)
     matrix (0.4.3)
     mini_magick (5.3.1)
       logger
     mini_mime (1.1.5)
     minitest (5.27.0)
-    msgpack (1.8.0)
+    msgpack (1.8.3)
-    net-imap (0.6.4)
+    net-imap (0.6.4.1)
       date
       net-protocol
     net-pop (0.1.2)
@@ -208,25 +208,25 @@ GEM
       net-protocol
     net-ssh (7.3.2)
     nio4r (2.7.5)
-    nokogiri (1.19.3-aarch64-linux-gnu)
+    nokogiri (1.19.4-aarch64-linux-gnu)
       racc (~> 1.4)
-    nokogiri (1.19.3-aarch64-linux-musl)
+    nokogiri (1.19.4-aarch64-linux-musl)
       racc (~> 1.4)
-    nokogiri (1.19.3-arm-linux-gnu)
+    nokogiri (1.19.4-arm-linux-gnu)
       racc (~> 1.4)
-    nokogiri (1.19.3-arm-linux-musl)
+    nokogiri (1.19.4-arm-linux-musl)
       racc (~> 1.4)
-    nokogiri (1.19.3-arm64-darwin)
+    nokogiri (1.19.4-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.19.3-x86_64-linux-gnu)
+    nokogiri (1.19.4-x86_64-linux-gnu)
       racc (~> 1.4)
-    nokogiri (1.19.3-x86_64-linux-musl)
+    nokogiri (1.19.4-x86_64-linux-musl)
       racc (~> 1.4)
-    openssl (4.0.1)
+    openssl (4.0.2)
     openssl-signature_algorithm (1.3.0)
       openssl (> 2.0)
     ostruct (0.6.3)
-    parallel (1.28.0)
+    parallel (2.1.0)
     parser (3.3.11.1)
       ast (~> 2.4.1)
       racc
@@ -238,11 +238,11 @@ GEM
       actionpack (>= 7.0.0)
       activesupport (>= 7.0.0)
       rack
-    psych (5.3.1)
+    psych (5.4.0)
       date
       stringio
     public_suffix (7.0.5)
-    puma (8.0.1)
+    puma (8.0.2)
       nio4r (~> 2.0)
     raabro (1.4.0)
     racc (1.8.1)
@@ -299,11 +299,11 @@ GEM
       chunky_png (~> 1.0)
       rqrcode_core (~> 2.0)
     rqrcode_core (2.1.0)
-    rubocop (1.84.2)
+    rubocop (1.87.0)
       json (~> 2.3)
       language_server-protocol (~> 3.17.0.2)
       lint_roller (~> 1.1.0)
-      parallel (~> 1.10)
+      parallel (>= 1.10)
       parser (>= 3.3.0.2)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 2.9.3, < 3.0)
@@ -321,20 +321,20 @@ GEM
     ruby-vips (2.3.0)
       ffi (~> 1.12)
       logger
-    rubyzip (3.2.2)
+    rubyzip (3.4.0)
     safety_net_attestation (0.5.0)
       jwt (>= 2.0, < 4.0)
     securerandom (0.4.1)
-    selenium-webdriver (4.43.0)
+    selenium-webdriver (4.45.0)
       base64 (~> 0.2)
       logger (~> 1.4)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 4.0)
       websocket (~> 1.0)
-    sentry-rails (6.5.0)
+    sentry-rails (6.6.2)
       railties (>= 5.2.0)
-      sentry-ruby (~> 6.5.0)
+      sentry-ruby (~> 6.6.2)
-    sentry-ruby (6.5.0)
+    sentry-ruby (6.6.2)
       bigdecimal
       concurrent-ruby (~> 1.0, >= 1.0.2)
       logger
@@ -344,7 +344,7 @@ GEM
       simplecov_json_formatter (~> 0.1)
     simplecov-html (0.13.2)
     simplecov_json_formatter (0.1.4)
-    solid_cable (3.0.12)
+    solid_cable (4.0.0)
       actioncable (>= 7.2)
       activejob (>= 7.2)
       activerecord (>= 7.2)
@@ -360,13 +360,13 @@ GEM
       fugit (~> 1.11)
       railties (>= 7.1)
       thor (>= 1.3.1)
-    sqlite3 (2.9.3-aarch64-linux-gnu)
+    sqlite3 (2.9.5-aarch64-linux-gnu)
-    sqlite3 (2.9.3-aarch64-linux-musl)
+    sqlite3 (2.9.5-aarch64-linux-musl)
-    sqlite3 (2.9.3-arm-linux-gnu)
+    sqlite3 (2.9.5-arm-linux-gnu)
-    sqlite3 (2.9.3-arm-linux-musl)
+    sqlite3 (2.9.5-arm-linux-musl)
-    sqlite3 (2.9.3-arm64-darwin)
+    sqlite3 (2.9.5-arm64-darwin)
-    sqlite3 (2.9.3-x86_64-linux-gnu)
+    sqlite3 (2.9.5-x86_64-linux-gnu)
-    sqlite3 (2.9.3-x86_64-linux-musl)
+    sqlite3 (2.9.5-x86_64-linux-musl)
     sshkit (1.25.0)
       base64
       logger
@@ -374,10 +374,10 @@ GEM
       net-sftp (>= 2.1.2)
       net-ssh (>= 2.8.0)
       ostruct
-    standard (1.54.0)
+    standard (1.55.0)
       language_server-protocol (~> 3.17.0.2)
       lint_roller (~> 1.0)
-      rubocop (~> 1.84.0)
+      rubocop (~> 1.87.0)
       standard-custom (~> 1.0.0)
       standard-performance (~> 1.8)
     standard-custom (1.0.2)
@@ -389,20 +389,20 @@ GEM
     stimulus-rails (1.3.4)
       railties (>= 6.0.0)
     stringio (3.2.0)
-    tailwindcss-rails (4.4.0)
+    tailwindcss-rails (4.6.0)
       railties (>= 7.0.0)
       tailwindcss-ruby (~> 4.0)
-    tailwindcss-ruby (4.2.4)
+    tailwindcss-ruby (4.3.1)
-    tailwindcss-ruby (4.2.4-aarch64-linux-gnu)
+    tailwindcss-ruby (4.3.1-aarch64-linux-gnu)
-    tailwindcss-ruby (4.2.4-aarch64-linux-musl)
+    tailwindcss-ruby (4.3.1-aarch64-linux-musl)
-    tailwindcss-ruby (4.2.4-arm64-darwin)
+    tailwindcss-ruby (4.3.1-arm64-darwin)
-    tailwindcss-ruby (4.2.4-x86_64-linux-gnu)
+    tailwindcss-ruby (4.3.1-x86_64-linux-gnu)
-    tailwindcss-ruby (4.2.4-x86_64-linux-musl)
+    tailwindcss-ruby (4.3.1-x86_64-linux-musl)
     thor (1.5.0)
-    thruster (0.1.20)
+    thruster (0.1.21)
-    thruster (0.1.20-aarch64-linux)
+    thruster (0.1.21-aarch64-linux)
-    thruster (0.1.20-arm64-darwin)
+    thruster (0.1.21-arm64-darwin)
-    thruster (0.1.20-x86_64-linux)
+    thruster (0.1.21-x86_64-linux)
     timeout (0.6.1)
     tpm-key_attestation (0.14.1)
       bindata (~> 2.4)
@@ -432,13 +432,13 @@ GEM
       safety_net_attestation (~> 0.5.0)
       tpm-key_attestation (~> 0.14.0)
     websocket (1.2.11)
-    websocket-driver (0.8.0)
+    websocket-driver (0.8.1)
       base64
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.7.5)
+    zeitwerk (2.8.2)
 
 PLATFORMS
   aarch64-linux

app/controllers/admin/access_checks_controller.rb

@@ -0,0 +1,20 @@
+module Admin
+  class AccessChecksController < BaseController
+    def new
+      load_options
+      @user = User.find_by(id: params[:user_id])
+      @application = Application.find_by(id: params[:application_id])
+      return unless @user && @application
+
+      @allowed = @application.user_allowed?(@user)
+      @via = @user.groups & @application.allowed_groups
+    end
+
+    private
+
+    def load_options
+      @users = User.order(:email_address)
+      @applications = Application.order(:name)
+    end
+  end
+end

app/controllers/admin/applications_controller.rb

@@ -3,7 +3,21 @@ module Admin
     before_action :set_application, only: [:show, :edit, :update, :destroy, :regenerate_credentials]
 
     def index
-      @applications = Application.order(created_at: :desc)
+      @applications = Application.order(created_at: :desc).includes(:allowed_groups)
+
+      # Distinct active users that have access to each app, preloaded to avoid N+1.
+      @user_count_by_app = User.where(status: User.statuses[:active])
+        .joins(groups: :applications)
+        .group("applications.id")
+        .distinct
+        .count("users.id")
+
+      # Top-of-page summary
+      @total_users_with_access = User.where(status: User.statuses[:active])
+        .joins(groups: :applications)
+        .distinct
+        .count("users.id")
+      @total_groups_granting_access = Group.joins(:applications).distinct.count
     end
 
     def show

app/controllers/api/forward_auth_controller.rb

@@ -64,26 +64,16 @@ module Api
           return render_forbidden("No authentication rule configured for this domain")
         end
       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
 
+      # 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 }
       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")
       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!
 
       headers = app.headers_for_user(user)
@@ -158,7 +156,7 @@ module Api
     end
 
     def render_bearer_error(message)
-      render json: { error: message }, status: :unauthorized
+      render json: {error: message}, status: :unauthorized
     end
 
     def check_forward_auth_token
@@ -198,15 +196,18 @@ module Api
       original_host = request.headers["X-Forwarded-Host"]
       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
 
-      login_params = { rd: original_url, rm: request.method }
+      login_params = {rd: original_url, rm: request.method}
       login_url = "#{base_url}/signin?#{login_params.to_query}"
 
       redirect_to login_url, allow_other_host: true, status: :found
@@ -242,18 +243,13 @@ module Api
     def determine_base_url(redirect_url)
       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

app/controllers/concerns/authentication.rb

@@ -31,7 +31,7 @@ module Authentication
   end
 
   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
 
   def request_authentication
@@ -62,9 +62,14 @@ module Authentication
     return if redirect_host.blank?
 
     csp = request.content_security_policy
-    return unless csp&.respond_to?(:form_action) && csp.form_action.respond_to?(:<<)
+    return unless csp
 
-    csp.form_action << "https://#{redirect_host}"
+    # NOTE: `csp.form_action` (no args) is destructive — it deletes the directive
+    # and returns its old value, so reading it twice yields nil. Mutate the
+    # underlying `directives` hash (a public reader of the real values) instead.
+    form_action = (csp.directives["form-action"] ||= ["'self'"])
+    host = "https://#{redirect_host}"
+    form_action << host unless form_action.include?(host)
   rescue URI::InvalidURIError
     nil
   end
@@ -186,7 +191,7 @@ module Authentication
     token = SecureRandom.urlsafe_base64(32)
     Rails.cache.write(
       "forward_auth_token:#{token}",
-      { session_id: session_obj.id, host: bound_host },
+      {session_id: session_obj.id, host: bound_host},
       expires_in: 60.seconds
     )
 

app/controllers/oidc_controller.rb

@@ -4,7 +4,11 @@ class OidcController < ApplicationController
   # Discovery and JWKS endpoints are public
   # authorize is also unauthenticated to handle prompt=none and prompt=login specially
   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
   # other error can be reported via redirect. Failures here render a plain page.

app/controllers/sessions_controller.rb

@@ -121,6 +121,16 @@ class SessionsController < ApplicationController
         return
       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
 
       # Try TOTP verification first (password + TOTP = 2FA)
@@ -241,6 +251,14 @@ class SessionsController < ApplicationController
       return
     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
     credential_data = params[:credential]
     if credential_data.blank?
@@ -277,10 +295,14 @@ class SessionsController < ApplicationController
         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)
-        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
 
       # Update credential usage

app/helpers/application_helper.rb

@@ -31,7 +31,7 @@ module ApplicationHelper
     end
     lines << "OIDC_DISCOVERY_URL=#{OidcJwtService.issuer_url}"
     lines << "OIDC_PROVIDER_NAME='Clinch'"
-    lines << "OIDC_REQUIRE_PKCE=#{application.requires_pkce? ? 'true' : 'false'}"
+    lines << "OIDC_REQUIRE_PKCE=#{application.requires_pkce? ? "true" : "false"}"
     lines
   end
 
@@ -70,19 +70,23 @@ module ApplicationHelper
     MONOGRAM_PALETTE[index]
   end
 
-  # Renders an application icon as a <picture> that swaps based on the user's
+  # Renders an application icon with optional dark-mode variant. If
-  # color-scheme preference. If only `icon` is attached, the same image is used
+  # `icon_dark` is attached, we render both <img> tags and Tailwind's class-
-  # in both modes. Caller is responsible for ensuring at least app.icon is
+  # based `dark:` modifier hides the inactive one — so it follows the in-app
-  # attached; the monogram fallback handles the no-icon case separately.
+  # theme toggle (.dark on <html>), not the OS preference. If only `icon` is
+  # attached, the same image is used in both modes. Caller must ensure at
+  # least app.icon is attached; the monogram fallback handles no-icon.
   def app_icon_picture(app, class:, alt: nil)
     img_class = binding.local_variable_get(:class)
     alt ||= "#{app.name} icon"
-    light = url_for(app.icon)
+
-    dark = app.icon_dark.attached? ? url_for(app.icon_dark) : nil
+    if app.icon_dark.attached?
-    tag.picture do
+      safe_join([
-      sources = []
+        image_tag(app.icon, class: "#{img_class} dark:hidden", alt: alt),
-      sources << tag.source(media: "(prefers-color-scheme: dark)", srcset: dark) if dark
+        image_tag(app.icon_dark, class: "#{img_class} hidden dark:block", alt: alt)
-      safe_join(sources + [image_tag(app.icon, class: img_class, alt: alt)])
+      ])
+    else
+      image_tag(app.icon, class: img_class, alt: alt)
     end
   end
 end

app/jobs/backchannel_logout_job.rb

@@ -28,6 +28,14 @@ class BackchannelLogoutJob < ApplicationJob
     # Send HTTP POST to the application's 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
       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 || "/")

app/lib/private_address_check.rb

@@ -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
+    # 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

app/mailers/security_mailer.rb

@@ -40,6 +40,12 @@ class SecurityMailer < ApplicationMailer
     mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address
   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:)
     assign_context(user, ip, user_agent, occurred_at)
     @recipient = recipient

app/mailers/totp_mailer.rb

@@ -2,6 +2,6 @@ class TotpMailer < ApplicationMailer
   def enabled(user)
     @user = user
     mail subject: "Two-factor authentication enabled on your account",
-         to: user.email_address
+      to: user.email_address
   end
 end

app/models/application.rb

@@ -56,6 +56,7 @@ class Application < ApplicationRecord
     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_not_internal, if: -> { backchannel_logout_uri.present? }
 
   # Icon validation using ActiveStorage validators
   validate :icon_validation
@@ -390,4 +391,17 @@ class Application < ApplicationRecord
       # Let the format validator handle invalid URIs
     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

app/models/oidc_refresh_token.rb

@@ -49,11 +49,21 @@ class OidcRefreshToken < ApplicationRecord
     update!(revoked_at: Time.current)
   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!
     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
 
   private

app/models/session.rb

@@ -7,6 +7,9 @@ class Session < ApplicationRecord
   # Scopes
   scope :active, -> { 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?
     expires_at.present? && expires_at <= Time.current

app/models/user.rb

@@ -41,6 +41,11 @@ class User < ApplicationRecord
   # Enum - automatically creates scopes (User.active, User.disabled, etc.)
   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
   scope :admins, -> { joins(:groups).where(groups: {admin: true}).distinct }
 
@@ -63,7 +68,10 @@ class User < ApplicationRecord
   def enable_totp!
     require "rotp"
     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!
   end
 
@@ -86,7 +94,13 @@ class User < ApplicationRecord
 
     require "rotp"
     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
 
   # Console/debug helper: get current TOTP code
@@ -237,6 +251,13 @@ class User < ApplicationRecord
     Group.auto_assign.each { |g| groups << g }
   end
 
+  def revoke_sessions_when_deactivated
+    return unless saved_change_to_status?
+    return if active?
+
+    sessions.destroy_all
+  end
+
   def no_reserved_claim_names
     return if custom_claims.blank?
 

app/models/webauthn_credential.rb

@@ -52,13 +52,17 @@ class WebauthnCredential < ApplicationRecord
     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)
-    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
 
   # Format for display in UI

app/views/admin/access_checks/new.html.erb

@@ -0,0 +1,77 @@
+<div class="mb-6">
+  <h1 class="text-2xl font-semibold text-gray-900 dark:text-gray-100">Access check</h1>
+  <p class="mt-2 text-sm text-gray-700 dark:text-gray-300">Pick a user and an application to see whether the user can access it and, if so, which group(s) grant that access.</p>
+</div>
+
+<div class="bg-white dark:bg-gray-800 shadow sm:rounded-lg">
+  <div class="px-4 py-5 sm:p-6">
+    <%= form_with url: admin_access_path, method: :get, class: "space-y-4" do |form| %>
+      <div class="grid grid-cols-1 gap-4 sm:grid-cols-2">
+        <div>
+          <%= form.label :user_id, "User", class: "block text-sm font-medium text-gray-700 dark:text-gray-300" %>
+          <%= form.select :user_id,
+                @users.map { |u| [u.email_address, u.id] },
+                { include_blank: "Select a user…", selected: @user&.id },
+                class: "mt-1 block w-full rounded-md border-gray-300 dark:border-gray-600 dark:bg-gray-800 dark:text-gray-100 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm" %>
+        </div>
+        <div>
+          <%= form.label :application_id, "Application", class: "block text-sm font-medium text-gray-700 dark:text-gray-300" %>
+          <%= form.select :application_id,
+                @applications.map { |a| [a.name, a.id] },
+                { include_blank: "Select an application…", selected: @application&.id },
+                class: "mt-1 block w-full rounded-md border-gray-300 dark:border-gray-600 dark:bg-gray-800 dark:text-gray-100 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm" %>
+        </div>
+      </div>
+      <div>
+        <%= form.submit "Check access", class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-semibold text-white shadow-sm hover:bg-blue-500 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-600" %>
+      </div>
+    <% end %>
+
+    <% if @user && @application %>
+      <div class="mt-6 rounded-md border <%= @allowed ? "border-green-200 dark:border-green-700 bg-green-50 dark:bg-green-900/30" : "border-red-200 dark:border-red-700 bg-red-50 dark:bg-red-900/30" %> p-4">
+        <div class="flex items-start gap-3">
+          <% if @allowed %>
+            <svg class="h-6 w-6 text-green-600 dark:text-green-400 shrink-0" fill="none" viewBox="0 0 24 24" stroke="currentColor">
+              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 13l4 4L19 7"/>
+            </svg>
+          <% else %>
+            <svg class="h-6 w-6 text-red-600 dark:text-red-400 shrink-0" fill="none" viewBox="0 0 24 24" stroke="currentColor">
+              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M6 18L18 6M6 6l12 12"/>
+            </svg>
+          <% end %>
+          <div class="flex-1">
+            <p class="text-sm font-medium <%= @allowed ? "text-green-800 dark:text-green-200" : "text-red-800 dark:text-red-200" %>">
+              <%= @user.email_address %> <%= @allowed ? "can access" : "cannot access" %> <%= @application.name %>.
+            </p>
+            <% if @allowed %>
+              <p class="mt-1 text-xs text-green-700 dark:text-green-300">
+                Granted via:
+                <% @via.each_with_index do |g, i| %>
+                  <%= link_to g.name, admin_group_path(g), class: "underline" %><%= "," unless i == @via.size - 1 %>
+                <% end %>
+              </p>
+            <% else %>
+              <p class="mt-1 text-xs text-red-700 dark:text-red-300">
+                <% reasons = [] %>
+                <% reasons << "the application is inactive" unless @application.active? %>
+                <% reasons << "the user is #{@user.status.humanize.downcase}" unless @user.active? %>
+                <% if @application.active? && @user.active? %>
+                  <% if @application.allowed_groups.empty? %>
+                    <% reasons << "the application has no allowed groups (default deny)" %>
+                  <% else %>
+                    <% reasons << "the user shares no group with the application's allowed groups" %>
+                  <% end %>
+                <% end %>
+                Reason: <%= reasons.join("; ") %>.
+              </p>
+            <% end %>
+            <p class="mt-2 text-xs text-gray-600 dark:text-gray-400">
+              <%= link_to "View user", admin_user_path(@user), class: "underline" %> ·
+              <%= link_to "View application", admin_application_path(@application), class: "underline" %>
+            </p>
+          </div>
+        </div>
+      </div>
+    <% end %>
+  </div>
+</div>

app/views/admin/applications/index.html.erb

@@ -8,6 +8,21 @@
   </div>
 </div>
 
+<dl class="mt-4 grid grid-cols-3 gap-4">
+  <div class="rounded-lg bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 px-4 py-3">
+    <dt class="text-xs text-gray-500 dark:text-gray-400">Applications</dt>
+    <dd class="mt-1 text-2xl font-semibold text-gray-900 dark:text-gray-100"><%= @applications.size %></dd>
+  </div>
+  <div class="rounded-lg bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 px-4 py-3">
+    <dt class="text-xs text-gray-500 dark:text-gray-400">Users with access</dt>
+    <dd class="mt-1 text-2xl font-semibold text-gray-900 dark:text-gray-100"><%= @total_users_with_access %></dd>
+  </div>
+  <div class="rounded-lg bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 px-4 py-3">
+    <dt class="text-xs text-gray-500 dark:text-gray-400">Groups granting access</dt>
+    <dd class="mt-1 text-2xl font-semibold text-gray-900 dark:text-gray-100"><%= @total_groups_granting_access %></dd>
+  </div>
+</dl>
+
 <div class="mt-8 flow-root">
   <div class="-mx-4 -my-2 overflow-x-auto sm:-mx-6 lg:-mx-8">
     <div class="inline-block min-w-full py-2 align-middle sm:px-6 lg:px-8">
@@ -18,7 +33,7 @@
             <th scope="col" class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-gray-100">Slug</th>
             <th scope="col" class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-gray-100">Type</th>
             <th scope="col" class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-gray-100">Status</th>
-            <th scope="col" class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-gray-100">Groups</th>
+            <th scope="col" class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-gray-100">Access</th>
             <th scope="col" class="relative py-3.5 pl-3 pr-4 sm:pr-0">
               <span class="sr-only">Actions</span>
             </th>
@@ -58,10 +73,13 @@
                 <% end %>
               </td>
               <td class="whitespace-nowrap px-3 py-4 text-sm text-gray-500 dark:text-gray-400">
-                <% if application.allowed_groups.empty? %>
+                <% groups_count = application.allowed_groups.size %>
-                  <span class="text-gray-400 dark:text-gray-500">All users</span>
+                <% users_count = @user_count_by_app[application.id] || 0 %>
+                <% if groups_count.zero? %>
+                  <span class="inline-flex items-center rounded-full bg-amber-100 dark:bg-amber-900/40 px-2 py-0.5 text-xs font-medium text-amber-700 dark:text-amber-300">No one</span>
                 <% else %>
-                  <%= application.allowed_groups.count %>
+                  <span class="text-gray-700 dark:text-gray-200"><%= pluralize(users_count, "user") %></span>
+                  <span class="text-gray-400 dark:text-gray-500"> · <%= pluralize(groups_count, "group") %></span>
                 <% end %>
               </td>
               <td class="relative whitespace-nowrap py-4 pl-3 pr-4 text-right text-sm font-medium sm:pr-0">

app/views/admin/users/index.html.erb

@@ -61,7 +61,7 @@
           <% @users.each do |user| %>
             <tr>
               <td class="whitespace-nowrap py-4 pl-4 pr-3 text-sm font-medium text-gray-900 dark:text-gray-100 sm:pl-0">
-                <%= user.email_address %>
+                <%= link_to user.email_address, admin_user_path(user), class: "text-blue-600 hover:text-blue-900" %>
               </td>
               <td class="whitespace-nowrap px-3 py-4 text-sm text-gray-500 dark:text-gray-400">
                 <% if user.status.present? %>
@@ -110,6 +110,7 @@
                         data: { turbo_method: :post },
                         class: "text-yellow-600 hover:text-yellow-900" %>
                   <% end %>
+                  <%= link_to "View", admin_user_path(user), class: "text-blue-600 hover:text-blue-900" %>
                   <%= link_to "Edit", edit_admin_user_path(user), class: "text-blue-600 hover:text-blue-900" %>
                   <%= link_to "Delete", admin_user_path(user),
                       data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to delete this user?" },

app/views/layouts/application.html.erb

@@ -9,7 +9,7 @@
     <%= csrf_meta_tags %>
     <%= csp_meta_tag %>
 
-    <script>
+    <script nonce="<%= content_security_policy_nonce %>">
       (function() {
         var theme = localStorage.getItem('theme');
         if (theme === 'dark' || (!theme && window.matchMedia('(prefers-color-scheme: dark)').matches)) {

app/views/security_mailer/suspicious_passkey_used.html.erb

@@ -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" %>

app/views/security_mailer/suspicious_passkey_used.text.erb

@@ -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" %>

app/views/sessions/new.html.erb

@@ -38,7 +38,7 @@
         </svg>
         Continue with Passkey
       </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>
 
     <!-- Password section - shown by default, hidden if WebAuthn is required -->

app/views/sessions/verify_totp.html.erb

@@ -54,7 +54,7 @@
             </svg>
             Use Passkey Instead
           </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>
     <% end %>

app/views/shared/_sidebar.html.erb

@@ -66,6 +66,16 @@
                     Groups
                   <% end %>
                 </li>
+
+                <!-- Admin: Access check -->
+                <li>
+                  <%= link_to admin_access_path, class: "group flex gap-x-3 rounded-md p-2 text-sm font-semibold leading-6 #{ current_path.start_with?('/admin/access') ? 'bg-gray-50 dark:bg-gray-800 text-blue-600 dark:text-blue-400' : 'text-gray-700 dark:text-gray-300 hover:text-blue-600 dark:hover:text-blue-400 hover:bg-gray-50 dark:hover:bg-gray-800' }" do %>
+                    <svg class="h-6 w-6 shrink-0" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor">
+                      <path stroke-linecap="round" stroke-linejoin="round" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z" />
+                    </svg>
+                    Access check
+                  <% end %>
+                </li>
               <% end %>
 
               <!-- Profile -->
@@ -196,6 +206,14 @@
                     Groups
                   <% end %>
                 </li>
+                <li>
+                  <%= link_to admin_access_path, class: "group flex gap-x-3 rounded-md p-2 text-sm font-semibold leading-6 #{ current_path.start_with?('/admin/access') ? 'bg-gray-50 dark:bg-gray-800 text-blue-600 dark:text-blue-400' : 'text-gray-700 dark:text-gray-300 hover:text-blue-600 dark:hover:text-blue-400 hover:bg-gray-50 dark:hover:bg-gray-800' }", data: { action: "click->mobile-sidebar#closeSidebar" } do %>
+                    <svg class="h-6 w-6 shrink-0" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor">
+                      <path stroke-linecap="round" stroke-linejoin="round" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z" />
+                    </svg>
+                    Access check
+                  <% end %>
+                </li>
               <% end %>
               <li>
                 <%= link_to profile_path, class: "group flex gap-x-3 rounded-md p-2 text-sm font-semibold leading-6 #{ current_path == '/profile' ? 'bg-gray-50 dark:bg-gray-800 text-blue-600 dark:text-blue-400' : 'text-gray-700 dark:text-gray-300 hover:text-blue-600 dark:hover:text-blue-400 hover:bg-gray-50 dark:hover:bg-gray-800' }", data: { action: "click->mobile-sidebar#closeSidebar" } do %>

config/environments/production.rb

@@ -118,14 +118,17 @@ Rails.application.configure do
       registrable_domain = domain.domain  # Gets "example.com" from "auth.example.com"
 
       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
     rescue PublicSuffix::DomainInvalid
       # Fallback to simple domain extraction if PublicSuffix fails
       Rails.logger.warn "Could not parse domain '#{host_domain}' with PublicSuffix, using fallback"
       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
 
@@ -136,9 +139,6 @@ Rails.application.configure do
 
   # Allow internal IP access for cross-compose or host networking
   if ENV["CLINCH_ALLOW_INTERNAL_IPS"] == "true"
-    # Specific host IP
-    allowed_hosts << "192.168.2.246"
-
     # Private IP ranges for internal network access
     allowed_hosts += [
       /192\.168\.\d+\.\d+/,    # 192.168.0.0/16 private network
@@ -157,17 +157,5 @@ Rails.application.configure do
   # Skip DNS rebinding protection for the default health check endpoint.
   config.host_authorization = {exclude: ->(request) { request.path == "/up" }}
 
-  # Sentry configuration for production
+  # Sentry is configured in config/initializers/sentry.rb, gated on SENTRY_DSN.
-  # Only enabled if SENTRY_DSN environment variable is set
-  if ENV["SENTRY_DSN"].present?
-    config.sentry.enabled = true
-
-    # Performance monitoring: sample 20% of transactions for traces
-    # Adjust based on your traffic volume and Sentry plan limits
-    config.sentry.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.2).to_f
-
-    # Continuous profiling: disabled by default in production due to cost
-    # Enable temporarily for performance investigations if needed
-    config.sentry.profiles_sample_rate = ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.0).to_f
-  end
 end

config/initializers/clinch_host.rb

@@ -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

config/initializers/content_security_policy.rb

@@ -9,13 +9,15 @@ Rails.application.configure do
     # Default to self for everything, plus blob: for file downloads
     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
     policy.img_src :self, :data, :https
@@ -51,14 +53,22 @@ Rails.application.configure do
     # Child sources: Allow self for any future iframes
     policy.child_src :self
 
-    # Additional security headers for WebAuthn
+    # Do not enforce Trusted Types. The only valid value for
-    # Required for WebAuthn to work properly
+    # require-trusted-types-for is 'script'; there is no 'none' token, so
-    policy.require_trusted_types_for :none
+    # emitting it produces an invalid directive that browsers reject. To leave
+    # Trusted Types unenforced (needed for WebAuthn), omit the directive entirely.
 
     # CSP reporting using report_uri (supported method)
     policy.report_uri "/api/csp-violation-report"
   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
   # Set to false after verifying everything works in production
   config.content_security_policy_report_only = Rails.env.development?

config/initializers/filter_parameter_logging.rb

@@ -4,5 +4,8 @@
 # Use this to limit dissemination of sensitive information.
 # See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors.
 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
 ]

config/initializers/sentry.rb

@@ -1,62 +1,44 @@
-# Sentry configuration for error tracking and performance monitoring
+# Sentry configuration for error tracking and performance monitoring.
-# Only initializes if SENTRY_DSN environment variable is set
+# Only initializes if the SENTRY_DSN environment variable is set.
 
 return unless ENV["SENTRY_DSN"].present?
 
-Rails.application.configure do
+Sentry.init do |config|
-  config.sentry.dsn = ENV["SENTRY_DSN"]
+  config.dsn = ENV["SENTRY_DSN"]
 
-  # Set environment (defaults to Rails.env)
+  # Environment label (defaults to Rails.env)
-  config.sentry.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env
+  config.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env
 
-  # Set release version from Git or environment variable
+  # Release version from an env var or the current Git SHA
-  config.sentry.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence || nil
+  config.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence
 
-  # Sample rate for performance monitoring (0.0 to 1.0)
+  # Only report from production unless explicitly enabled elsewhere.
-  config.sentry.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.1).to_f
+  config.enabled_environments =
-
+    if ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
-  # Enable profiling in development/staging, disable in production unless explicitly enabled
+      %w[production development]
-  config.sentry.profiles_sample_rate = if Rails.env.production?
+    else
-    ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.0).to_f
+      %w[production]
-  else
-    ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.5).to_f
-  end
-
-  # Include additional context
-  config.sentry.before_send = lambda do |event, hint|
-    # Filter out sensitive information
-    if event.context[:extra]
-      event.context[:extra].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key/i) || value.to_s.match?(/password|secret/i)
-      }
     end
 
-    # Filter sensitive parameters
+  # Don't send cookies, request bodies, or user IPs by default.
-    if event.context[:request]
+  config.send_default_pii = false
-      event.context[:request].reject! { |key, value|
+
-        key.to_s.match?(/password|secret|token|key|authorization/i)
+  # Breadcrumbs for debugging
-      }
+  config.breadcrumbs_logger = [:active_support_logger, :http_logger]
+
+  # Performance monitoring sample rate (0.0 to 1.0)
+  config.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.1).to_f
+
+  # Profiling: disabled in production by default due to cost.
+  config.profiles_sample_rate =
+    if Rails.env.production?
+      ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.0).to_f
+    else
+      ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.5).to_f
     end
 
-    event
-  end
-
-  # Include breadcrumbs for debugging
-  config.sentry.breadcrumbs_logger = [:active_support_logger, :http_logger]
-
-  # Send session data for user context
-  config.sentry.user_context = lambda do
-    if Current.user.present?
-      {
-        id: Current.user.id,
-        email: Current.user.email_address,
-        admin: Current.user.admin?
-      }
-    end
-  end
-
   # Ignore common non-critical exceptions
-  config.sentry.excluded_exceptions += [
+  config.excluded_exceptions += [
     "ActionController::RoutingError",
     "ActionController::InvalidAuthenticityToken",
     "ActionController::UnknownFormat",
@@ -66,75 +48,38 @@ Rails.application.configure do
     "ActiveRecord::RecordNotFound"
   ]
 
-  # Add CSP-specific tags for security events
+  # Attach application/user context and scrub anything sensitive before sending.
-  config.sentry.tags = lambda do
+  config.before_send = lambda do |event, _hint|
-    {
+    event.tags = (event.tags || {}).merge(
-      # Add application context
       app_name: "clinch",
-      app_environment: Rails.env,
+      app_environment: Rails.env
-      # Add CSP policy status
+    )
-      csp_enabled: defined?(Rails.application.config.content_security_policy) &&
-        Rails.application.config.content_security_policy.present?
-    }
-  end
 
-  # Enhance before_send to handle CSP events properly
+    if defined?(Current) && Current.respond_to?(:user) && Current.user
-  config.sentry.before_send = lambda do |event, hint|
+      event.user = (event.user || {}).merge(
-    # Filter out sensitive information
+        id: Current.user.id,
-    if event.context[:extra]
+        email: Current.user.email_address,
-      event.context[:extra].reject! { |key, value|
+        admin: Current.user.admin?
+      )
+    end
+
+    if event.extra.is_a?(Hash)
+      event.extra.reject! do |key, value|
         key.to_s.match?(/password|secret|token|key/i) || value.to_s.match?(/password|secret/i)
-      }
+      end
-    end
-
-    # Filter sensitive parameters
-    if event.context[:request]
-      event.context[:request].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key|authorization/i)
-      }
-    end
-
-    # Special handling for CSP violations
-    if event.tags&.dig(:csp_violation)
-      # Ensure CSP violations have proper security context
-      event.context[:server] = event.context[:server] || {}
-      event.context[:server][:name] = "clinch-auth-service"
-      event.context[:server][:environment] = Rails.env
-
-      # Add additional security context
-      event.context[:extra] ||= {}
-      event.context[:extra][:security_context] = {
-        csp_reporting: true,
-        user_authenticated: event.context[:user].present?,
-        request_origin: event.context[:request]&.dig(:headers, "Origin"),
-        request_referer: event.context[:request]&.dig(:headers, "Referer")
-      }
     end
 
     event
   end
 
-  # Add CSP-specific breadcrumbs for security events
+  # Scrub sensitive data out of breadcrumbs.
-  config.sentry.before_breadcrumb = lambda do |breadcrumb, hint|
+  config.before_breadcrumb = lambda do |breadcrumb, _hint|
-    # Filter out sensitive breadcrumb data
+    if breadcrumb.data.is_a?(Hash)
-    if breadcrumb[:data]
+      breadcrumb.data.reject! do |key, value|
-      breadcrumb[:data].reject! { |key, value|
+        key.to_s.match?(/password|secret|token|key|authorization/i) || value.to_s.match?(/password|secret/i)
-        key.to_s.match?(/password|secret|token|key|authorization/i) ||
+      end
-          value.to_s.match?(/password|secret/i)
-      }
-    end
-
-    # Mark CSP-related events
-    if breadcrumb[:message]&.include?("CSP Violation") ||
-        breadcrumb[:category]&.include?("csp")
-      breadcrumb[:data] ||= {}
-      breadcrumb[:data][:security_event] = true
-      breadcrumb[:data][:csp_violation] = true
     end
 
     breadcrumb
   end
-
-  # Only send errors in production unless explicitly enabled
-  config.sentry.enabled = Rails.env.production? || ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
 end

config/initializers/version.rb

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

config/routes.rb

@@ -95,6 +95,7 @@ Rails.application.routes.draw do
       end
     end
     resources :groups
+    get "access", to: "access_checks#new"
   end
 
   # Render dynamic PWA files from app/views/pwa/* (remember to link manifest in application.html.erb)

db/migrate/20260420073319_add_oidc_authorization_code_id_to_tokens.rb

@@ -1,8 +1,8 @@
 class AddOidcAuthorizationCodeIdToTokens < ActiveRecord::Migration[8.1]
   def change
     add_reference :oidc_access_tokens, :oidc_authorization_code,
-                  null: true, foreign_key: true, index: true
+      null: true, foreign_key: true, index: true
     add_reference :oidc_refresh_tokens, :oidc_authorization_code,
-                  null: true, foreign_key: true, index: true
+      null: true, foreign_key: true, index: true
   end
 end

db/migrate/20260611000001_add_last_otp_at_to_users.rb

@@ -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

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 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|
     t.bigint "blob_id", 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.json "custom_claims", default: {}, null: false
     t.string "email_address", null: false
+    t.integer "last_otp_at"
     t.datetime "last_sign_in_at"
     t.string "name"
     t.string "password_digest", null: false

test/controllers/admin/access_checks_controller_test.rb

@@ -0,0 +1,47 @@
+require "test_helper"
+
+module Admin
+  class AccessChecksControllerTest < ActionDispatch::IntegrationTest
+    setup do
+      @admin = users(:two)
+      sign_in_as(@admin)
+      @kavita = applications(:kavita_app)
+    end
+
+    test "new renders the form with users and applications" do
+      get admin_access_path
+      assert_response :success
+      assert_match @kavita.name, response.body
+      assert_match "alice@example.com", response.body
+    end
+
+    test "returns 'can access' with via group when user is in an allowed group" do
+      get admin_access_path, params: {
+        user_id: users(:alice).id,
+        application_id: @kavita.id
+      }
+      assert_response :success
+      assert_match "can access", response.body
+      assert_match "Administrators", response.body # alice is in admin_group; kavita has admin_group
+    end
+
+    test "returns 'cannot access' with reason when user shares no group with the app" do
+      lonely = User.create!(email_address: "lonely@example.com", password: "password123", skip_auto_assign: true)
+      get admin_access_path, params: {
+        user_id: lonely.id,
+        application_id: @kavita.id
+      }
+      assert_response :success
+      assert_match "cannot access", response.body
+      assert_match "shares no group", response.body
+    end
+
+    test "renders form unchanged when ids are missing" do
+      get admin_access_path, params: {user_id: "", application_id: ""}
+      assert_response :success
+      # No result panel should render. The panel-only phrases:
+      refute_match "Granted via", response.body
+      refute_match "Reason:", response.body
+    end
+  end
+end

test/controllers/admin/groups_controller_test.rb

@@ -27,7 +27,7 @@ module Admin
       @group.applications = [applications(:kavita_app)]
 
       patch admin_group_path(@group), params: {
-        group: { name: @group.name }
+        group: {name: @group.name}
       }
 
       assert_redirected_to admin_group_path(@group)

test/controllers/api/forward_auth_bearer_test.rb

@@ -113,6 +113,42 @@ module Api
       assert_equal "Application is inactive", json["error"]
     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
       # No auth header, no session -> should redirect (cookie flow)
       get "/api/verify", headers: {

test/controllers/api/forward_auth_controller_test.rb

@@ -186,7 +186,7 @@ module Api
       # Under default-deny the user must be in at least one group to access the app.
       # This rewritten test verifies that when an app's headers_config disables the
       # groups header, no x-remote-groups is sent regardless of memberships.
-      app = grant_everyone_access Application.create!(
+      grant_everyone_access Application.create!(
         name: "Headers Hidden", slug: "headers-hidden", app_type: "forward_auth",
         domain_pattern: "hidden.example.com",
         active: true,
@@ -242,6 +242,20 @@ module Api
       assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"]
     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
     test "should handle very long domain names" do
       long_domain = "a" * 250 + ".example.com"
@@ -545,7 +559,7 @@ module Api
     end
 
     test "should track failed attempts and eventually rate limit" do
-      cache = Rails.application.config.forward_auth_cache
+      Rails.application.config.forward_auth_cache
 
       # Make 50 failed requests (no session = unauthorized)
       50.times do

test/controllers/oidc_authorization_code_security_test.rb

@@ -34,6 +34,25 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
   # 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
     # Create consent
     OidcUserConsent.create!(

test/controllers/oidc_refresh_token_controller_test.rb

@@ -213,6 +213,50 @@ class OidcRefreshTokenControllerTest < ActionDispatch::IntegrationTest
     assert_equal family_id, new_refresh_token.token_family_id
   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
     access_token = OidcAccessToken.create!(
       application: @application,

test/controllers/totp_security_test.rb

@@ -19,16 +19,21 @@ class TotpSecurityTest < ActionDispatch::IntegrationTest
 
     # First use of the code should succeed
     post totp_verification_path, params: {code: valid_code}
-    assert_response :redirect
     assert_redirected_to root_path
 
     # Sign out
     delete session_path
     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.destroy

test/helpers/application_helper_test.rb

@@ -31,4 +31,23 @@ class ApplicationHelperTest < ActionView::TestCase
     # not a guarantee for all pairs, but should hold for at least one pair
     assert_not_equal monogram_color("Kavita"), monogram_color("Navidrome")
   end
+
+  test "app_icon_picture renders both icons with Tailwind dark: toggles when icon_dark is attached" do
+    app = applications(:kavita_app)
+    app.icon.attach(io: StringIO.new("light"), filename: "light.png", content_type: "image/png")
+    app.icon_dark.attach(io: StringIO.new("dark"), filename: "dark.png", content_type: "image/png")
+
+    html = app_icon_picture(app, class: "h-10 w-10 rounded-lg")
+    assert_match(/dark:hidden/, html)
+    assert_match(/hidden dark:block/, html)
+  end
+
+  test "app_icon_picture renders one img when no icon_dark is attached" do
+    app = applications(:kavita_app)
+    app.icon.attach(io: StringIO.new("light"), filename: "light.png", content_type: "image/png")
+
+    html = app_icon_picture(app, class: "h-10 w-10")
+    refute_match(/dark:hidden/, html)
+    refute_match(/hidden dark:block/, html)
+  end
 end

test/integration/csp_test.rb

@@ -0,0 +1,76 @@
+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
+
+  test "signin page adds the OAuth redirect_uri host to form-action without 500ing" do
+    # A user must exist, otherwise /signin redirects to signup before the CSP
+    # branch runs.
+    User.create!(email_address: "csp_oauth@example.com", password: "password123")
+
+    app = Application.create!(
+      name: "CSP OAuth App",
+      slug: "csp-oauth-app",
+      app_type: "oidc",
+      redirect_uris: ["https://app.example.com/callback"].to_json,
+      active: true,
+      require_pkce: false
+    )
+
+    # An unauthenticated authorize request stores the full /oauth/authorize URL
+    # in the session and redirects to signin (oidc_controller.rb:202).
+    get "/oauth/authorize", params: {
+      client_id: app.client_id,
+      redirect_uri: app.parsed_redirect_uris.first,
+      response_type: "code",
+      scope: "openid"
+    }
+    assert_redirected_to signin_path
+
+    # Following to signin must reach allow_oauth_redirect_in_csp without raising.
+    # Regression: csp.form_action is a destructive getter, so reading it twice
+    # returned nil and `nil << host` raised NoMethodError -> 500.
+    follow_redirect!
+    assert_response :success
+
+    form_action = directive(response.headers["Content-Security-Policy"], "form-action")
+    assert_includes form_action, "'self'", "form-action must keep its default 'self'"
+    assert_includes form_action, "https://app.example.com",
+      "form-action must include the OAuth client's redirect_uri host"
+  end
+
+  private
+
+  def directive(csp, name)
+    csp.split(";").map(&:strip).find { |d| d.start_with?("#{name} ") } || ""
+  end
+end

test/integration/forward_auth_integration_test.rb

@@ -153,6 +153,10 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
 
   # Redirect URL Integration Tests
   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
     get "/api/verify", headers: {
       "X-Forwarded-Host" => "grafana.example.com"
@@ -172,7 +176,27 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
     assert_includes location, "grafana.example.com"
   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
+    # 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
     get "/api/verify", headers: {
       "X-Forwarded-Host" => "app.example.com",

test/integration/session_security_test.rb

@@ -1,6 +1,49 @@
 require "test_helper"
 
 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
   # ====================
@@ -199,7 +242,7 @@ class SessionSecurityTest < ActionDispatch::IntegrationTest
       slug: "logout-test-app",
       app_type: "oidc",
       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
     )
 

test/lib/private_address_check_test.rb

@@ -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

test/mailers/security_mailer_test.rb

@@ -54,6 +54,15 @@ class SecurityMailerTest < ActionMailer::TestCase
     assert_bodies_contain email, "Old MacBook"
   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
     email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT)
 

test/models/application_test.rb

@@ -56,4 +56,29 @@ class ApplicationTest < ActiveSupport::TestCase
     tempfile&.close
     tempfile&.unlink
   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

test/models/user_test.rb

@@ -1,6 +1,27 @@
 require "test_helper"
 
 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
     user = User.new(email_address: " DOWNCASED@EXAMPLE.COM ")
     assert_equal("downcased@example.com", user.email_address)

test/models/webauthn_credential_test.rb

@@ -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

test/test_helpers/session_test_helper.rb

@@ -17,7 +17,11 @@ module SessionTestHelper
   # written under the old "empty allowed_groups = public" rule keep working.
   # New tests should attach groups explicitly to model real access intent.
   def grant_everyone_access(app)
-    everyone = (groups(:everyone) rescue Group.find_by(auto_assign: true))
+    everyone = begin
+      groups(:everyone)
+    rescue
+      Group.find_by(auto_assign: true)
+    end
     app.allowed_groups << everyone unless app.allowed_groups.include?(everyone)
     app
   end