Compare commits
9 Commits
49068aa344
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c85d25c4b9 | ||
|
|
1b0d323572 | ||
|
|
d1d626c540 | ||
|
|
782e197d91 | ||
|
|
020759bfb3 | ||
|
|
85f50bfc96 | ||
|
|
b55139eb1c | ||
|
|
8f578ed3f4 | ||
|
|
aa5736ddab |
133
.github/workflows/build.yml
vendored
Normal file
133
.github/workflows/build.yml
vendored
Normal file
@@ -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
|
||||
3
.gitignore
vendored
3
.gitignore
vendored
@@ -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
|
||||
|
||||
@@ -1 +1 @@
|
||||
4.0.3
|
||||
4.0.5
|
||||
|
||||
@@ -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
|
||||
|
||||
110
Gemfile.lock
110
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)
|
||||
jwt (3.1.2)
|
||||
json (2.19.9)
|
||||
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)
|
||||
net-imap (0.6.4)
|
||||
msgpack (1.8.3)
|
||||
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.5.0)
|
||||
sentry-ruby (~> 6.6.2)
|
||||
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.3-aarch64-linux-musl)
|
||||
sqlite3 (2.9.3-arm-linux-gnu)
|
||||
sqlite3 (2.9.3-arm-linux-musl)
|
||||
sqlite3 (2.9.3-arm64-darwin)
|
||||
sqlite3 (2.9.3-x86_64-linux-gnu)
|
||||
sqlite3 (2.9.3-x86_64-linux-musl)
|
||||
sqlite3 (2.9.5-aarch64-linux-gnu)
|
||||
sqlite3 (2.9.5-aarch64-linux-musl)
|
||||
sqlite3 (2.9.5-arm-linux-gnu)
|
||||
sqlite3 (2.9.5-arm-linux-musl)
|
||||
sqlite3 (2.9.5-arm64-darwin)
|
||||
sqlite3 (2.9.5-x86_64-linux-gnu)
|
||||
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.2.4-aarch64-linux-gnu)
|
||||
tailwindcss-ruby (4.2.4-aarch64-linux-musl)
|
||||
tailwindcss-ruby (4.2.4-arm64-darwin)
|
||||
tailwindcss-ruby (4.2.4-x86_64-linux-gnu)
|
||||
tailwindcss-ruby (4.2.4-x86_64-linux-musl)
|
||||
tailwindcss-ruby (4.3.1)
|
||||
tailwindcss-ruby (4.3.1-aarch64-linux-gnu)
|
||||
tailwindcss-ruby (4.3.1-aarch64-linux-musl)
|
||||
tailwindcss-ruby (4.3.1-arm64-darwin)
|
||||
tailwindcss-ruby (4.3.1-x86_64-linux-gnu)
|
||||
tailwindcss-ruby (4.3.1-x86_64-linux-musl)
|
||||
thor (1.5.0)
|
||||
thruster (0.1.20)
|
||||
thruster (0.1.20-aarch64-linux)
|
||||
thruster (0.1.20-arm64-darwin)
|
||||
thruster (0.1.20-x86_64-linux)
|
||||
thruster (0.1.21)
|
||||
thruster (0.1.21-aarch64-linux)
|
||||
thruster (0.1.21-arm64-darwin)
|
||||
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
|
||||
|
||||
@@ -1,110 +0,0 @@
|
||||
# Security Review — Tracking
|
||||
|
||||
Status of findings from the multi-surface security review (OIDC/OAuth2, ForwardAuth,
|
||||
WebAuthn/TOTP, sessions, admin/config). Work landed on branch
|
||||
`security/forward-auth-and-consent-csrf`.
|
||||
|
||||
## ✅ Done (branch `security/forward-auth-and-consent-csrf`)
|
||||
|
||||
All HIGH findings are closed. Each fix has tests; suite is green.
|
||||
|
||||
| Commit | Fix | Sev |
|
||||
|--------|-----|-----|
|
||||
| `703d24e` | ForwardAuth fail-open when no host header; consent endpoint CSRF | HIGH ×2 |
|
||||
| `8a095e4` | Bearer API-key skipped group check at use-time | HIGH |
|
||||
| `96a657e` | Open redirect via unvalidated `X-Forwarded-Host` in login redirect | HIGH |
|
||||
| `84ed462` | `CLINCH_HOST` made mandatory in deployed envs; dropped request-host fallback | MEDIUM |
|
||||
| `f38ac2e` | TOTP code replay within drift window (+ latent plaintext backup-code bug) | HIGH |
|
||||
| `406a79d` | SSRF via `backchannel_logout_uri` (metadata/loopback/RFC1918) | HIGH |
|
||||
| `57d7d1f` | Host-auth regex unanchored (`evil-example.com` matched) | HIGH |
|
||||
| `89bd5f1` | Disabled user could complete 2FA mid-flow / keep session; enforce active status | HIGH |
|
||||
| `cd862c7` | TOTP/backup/OAuth/PKCE `code` params not filtered from logs | MEDIUM |
|
||||
| `2426687` | `revoke_family!` didn't revoke access tokens on refresh-token reuse | HIGH |
|
||||
| `44892e3` | WebAuthn clone detection logged but didn't block; false-positive on synced passkeys | HIGH |
|
||||
| `d49e7ce` | CSP `unsafe-inline` removed (script-src + style-src → nonces) | HIGH |
|
||||
|
||||
**Verified false positive (no change):** PKCE *is* required by default —
|
||||
`require_pkce` column defaults to `true` (`db/schema.rb`), token endpoint enforces
|
||||
it, admin UI exposes the opt-out. Operational check: confirm no legacy confidential
|
||||
apps sit on `require_pkce = false`.
|
||||
|
||||
**Follow-up before relying on CSP change:** do one manual browser pass (DevTools
|
||||
console) on `/signin`, OAuth consent, a Turbo navigation, dark-mode toggle, and a
|
||||
WebAuthn sign-in — expect zero CSP violations. Dev is report-only so violations
|
||||
surface as warnings without breaking. Fallback if style-src surprises: keep
|
||||
`style-src 'unsafe-inline'`, ship script-src only.
|
||||
|
||||
## ☐ Remaining — MEDIUM
|
||||
|
||||
- [ ] **`id_token_hint` ignored at OIDC logout** — any client can redirect logout to
|
||||
any other registered client's post-logout URI. Validate the hint's `aud` and
|
||||
scope the redirect to that app. `app/controllers/oidc_controller.rb` (logout).
|
||||
- [ ] **`offline_access` doesn't gate refresh-token issuance** — refresh tokens are
|
||||
minted unconditionally; gate on the granted scope.
|
||||
`app/controllers/oidc_controller.rb` (authorization_code grant, ~line 564).
|
||||
- [ ] **CSP-report endpoint hardening** — unauthenticated, no rate limit / body-size
|
||||
cap, logs raw CRLF (log injection). Sanitize values, cap size, rate-limit.
|
||||
`app/controllers/api/csp_controller.rb`.
|
||||
- [ ] **Port not stripped from `X-Forwarded-Host`** in main verify + bearer paths →
|
||||
403 outages on non-standard ports (also a correctness bug). Reuse the
|
||||
port-stripping done in `check_forward_auth_token`.
|
||||
`app/controllers/api/forward_auth_controller.rb`.
|
||||
- [ ] **WebAuthn `acr:"2"` without enforced user verification** — `user_verification:
|
||||
"preferred"` lets a PIN-less key authenticate yet reports verified 2FA. Use
|
||||
`"required"`, or downgrade `acr` to `"1"` when the UV flag is absent.
|
||||
`app/controllers/sessions_controller.rb` (webauthn_challenge/verify),
|
||||
`app/controllers/webauthn_controller.rb`.
|
||||
- [ ] **`RESERVED_CLAIMS` incomplete** — missing `at_hash`/`auth_time`/`acr`; and
|
||||
`ApplicationUserClaims` has no reserved-name validation (User/Group do). Could
|
||||
let a custom claim overwrite a security claim. `app/services/oidc_jwt_service.rb`,
|
||||
`app/models/application_user_claim.rb`.
|
||||
- [ ] **`reset_session` not called on login** — defensive best practice for an IdP;
|
||||
clears pre-auth session state. `app/controllers/concerns/authentication.rb`
|
||||
(`start_new_session_for`).
|
||||
- [x] **Hardcoded private IP `192.168.2.246`** in `config/environments/production.rb`
|
||||
— removed; it was redundant with the `192.168.0.0/16` regex already in the
|
||||
`CLINCH_ALLOW_INTERNAL_IPS` block.
|
||||
- [ ] **CSP `form-action` widened by unvalidated `redirect_uri`** before auth — only
|
||||
add to `form-action` if the client_id+redirect_uri is a registered pair.
|
||||
`app/controllers/concerns/authentication.rb` (`allow_oauth_redirect_in_csp`).
|
||||
- [ ] **SVG `style` attribute permits `url()`/`expression()`** — mitigated today by
|
||||
`Content-Disposition: attachment`, but fragile. Sanitize CSS values or drop
|
||||
`style` from the allowlist. `app/models/svg_scrubber.rb`.
|
||||
- [ ] **WebAuthn error messages leak internals** — return generic errors to client,
|
||||
log detail server-side. `app/controllers/sessions_controller.rb`,
|
||||
`app/controllers/webauthn_controller.rb`.
|
||||
- [ ] **Account enumeration via webauthn challenge** — distinguishes "user not found"
|
||||
vs "no passkey". Return a uniform message. `app/controllers/sessions_controller.rb`
|
||||
(`webauthn_challenge`).
|
||||
- [ ] **`token_family_id` only 31 bits** (`SecureRandom.random_number(2**31)`) —
|
||||
birthday collision ~46k; use a UUID/string. `app/models/oidc_refresh_token.rb`.
|
||||
- [ ] **Session cookie uses sequential integer DB id** — HMAC-signed so not forgeable,
|
||||
but consider a random `token` column (Rails 8 generator default).
|
||||
`app/models/session.rb`, `app/controllers/concerns/authentication.rb`.
|
||||
- [ ] **Login rate-limit is IP-only** — no account lockout (distributed brute force /
|
||||
credential stuffing). Add failed-count + `locked_until` on users.
|
||||
- [ ] **Backup-code rate limit not reset on success** and is cache-based (resets on
|
||||
cache flush). Reset on success; consider DB-backed counter. `app/models/user.rb`.
|
||||
|
||||
## ☐ Remaining — LOW / INFO
|
||||
|
||||
- [ ] Public clients can't revoke their own tokens (revoke endpoint requires secret).
|
||||
- [ ] Basic-auth client creds not URL-decoded per RFC 6749 §2.3.1.
|
||||
- [ ] `token_hmac` columns nullable at DB level despite model `presence: true`.
|
||||
- [ ] Group names allow commas → injection into `X-Remote-Groups` (false memberships
|
||||
downstream). Add a format validator. `app/models/group.rb`.
|
||||
- [ ] `fa_token` leaks in redirect URL / Referer / history (60s TTL, host-bound).
|
||||
- [ ] Admin `domain_pattern` allows ReDoS — add a format validator.
|
||||
`app/models/application.rb`.
|
||||
- [ ] Forced-TOTP-setup login path can redirect-loop (`totp_required` + no TOTP).
|
||||
- [ ] `complete_setup` creates an unprompted session for any authenticated user.
|
||||
- [ ] Password min length only 8 — consider 12 + a max (bcrypt 72-byte truncation).
|
||||
- [ ] `support_unencrypted_data: true` left enabled (TOTP secret encryption migration).
|
||||
`config/initializers/active_record_encryption.rb`.
|
||||
- [ ] All crypto keys derived from a single `SECRET_KEY_BASE` root — document setting
|
||||
independent `ACTIVE_RECORD_ENCRYPTION_*` keys in production.
|
||||
- [ ] Log injection via user `email_address` in ForwardAuth logs (strip CRLF / use
|
||||
structured logging). `app/controllers/api/forward_auth_controller.rb`.
|
||||
- [ ] WebAuthn RP ID is the registrable domain (cross-subdomain credential roaming) —
|
||||
set `CLINCH_RP_ID` to the exact host unless roaming is intended.
|
||||
`config/initializers/webauthn.rb`.
|
||||
@@ -2,17 +2,12 @@ module Admin
|
||||
class AccessChecksController < BaseController
|
||||
def new
|
||||
load_options
|
||||
end
|
||||
|
||||
def create
|
||||
load_options
|
||||
@user = User.find_by(id: params[:user_id])
|
||||
@application = Application.find_by(id: params[:application_id])
|
||||
return render :new unless @user && @application
|
||||
return unless @user && @application
|
||||
|
||||
@allowed = @application.user_allowed?(@user)
|
||||
@via = @user.groups & @application.allowed_groups
|
||||
render :new
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -156,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
|
||||
@@ -207,7 +207,7 @@ module Api
|
||||
|
||||
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
|
||||
|
||||
@@ -191,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
|
||||
)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -35,7 +35,7 @@ module PrivateAddressCheck
|
||||
return [ip] if ip
|
||||
|
||||
Resolv.getaddresses(host.to_s).filter_map { |a| parse_ip(a) }
|
||||
rescue StandardError
|
||||
rescue
|
||||
# Resolution failure: surface no addresses. Callers treat "can't resolve" as
|
||||
# not-provably-internal; the dial itself will then fail safely.
|
||||
[]
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
|
||||
<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: :post, class: "space-y-4" do |form| %>
|
||||
<%= 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" %>
|
||||
|
||||
@@ -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
|
||||
# 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
|
||||
# Sentry is configured in config/initializers/sentry.rb, gated on SENTRY_DSN.
|
||||
end
|
||||
|
||||
@@ -53,9 +53,10 @@ Rails.application.configure do
|
||||
# Child sources: Allow self for any future iframes
|
||||
policy.child_src :self
|
||||
|
||||
# Additional security headers for WebAuthn
|
||||
# Required for WebAuthn to work properly
|
||||
policy.require_trusted_types_for :none
|
||||
# Do not enforce Trusted Types. The only valid value for
|
||||
# require-trusted-types-for is 'script'; there is no 'none' token, so
|
||||
# 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"
|
||||
|
||||
@@ -1,62 +1,44 @@
|
||||
# Sentry configuration for error tracking and performance monitoring
|
||||
# Only initializes if SENTRY_DSN environment variable is set
|
||||
# Sentry configuration for error tracking and performance monitoring.
|
||||
# Only initializes if the SENTRY_DSN environment variable is set.
|
||||
|
||||
return unless ENV["SENTRY_DSN"].present?
|
||||
|
||||
Rails.application.configure do
|
||||
config.sentry.dsn = ENV["SENTRY_DSN"]
|
||||
Sentry.init do |config|
|
||||
config.dsn = ENV["SENTRY_DSN"]
|
||||
|
||||
# Set environment (defaults to Rails.env)
|
||||
config.sentry.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env
|
||||
# Environment label (defaults to Rails.env)
|
||||
config.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env
|
||||
|
||||
# Set release version from Git or environment variable
|
||||
config.sentry.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence || nil
|
||||
# Release version from an env var or the current Git SHA
|
||||
config.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence
|
||||
|
||||
# Sample rate for performance monitoring (0.0 to 1.0)
|
||||
config.sentry.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.1).to_f
|
||||
|
||||
# Enable profiling in development/staging, disable in production unless explicitly enabled
|
||||
config.sentry.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
|
||||
|
||||
# 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)
|
||||
}
|
||||
# Only report from production unless explicitly enabled elsewhere.
|
||||
config.enabled_environments =
|
||||
if ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
|
||||
%w[production development]
|
||||
else
|
||||
%w[production]
|
||||
end
|
||||
|
||||
# Filter sensitive parameters
|
||||
if event.context[:request]
|
||||
event.context[:request].reject! { |key, value|
|
||||
key.to_s.match?(/password|secret|token|key|authorization/i)
|
||||
}
|
||||
# Don't send cookies, request bodies, or user IPs by default.
|
||||
config.send_default_pii = false
|
||||
|
||||
# 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
|
||||
config.sentry.tags = lambda do
|
||||
{
|
||||
# Add application context
|
||||
# Attach application/user context and scrub anything sensitive before sending.
|
||||
config.before_send = lambda do |event, _hint|
|
||||
event.tags = (event.tags || {}).merge(
|
||||
app_name: "clinch",
|
||||
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
|
||||
app_environment: Rails.env
|
||||
)
|
||||
|
||||
# Enhance before_send to handle CSP events properly
|
||||
config.sentry.before_send = lambda do |event, hint|
|
||||
# Filter out sensitive information
|
||||
if event.context[:extra]
|
||||
event.context[:extra].reject! { |key, value|
|
||||
if defined?(Current) && Current.respond_to?(:user) && Current.user
|
||||
event.user = (event.user || {}).merge(
|
||||
id: Current.user.id,
|
||||
email: Current.user.email_address,
|
||||
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
|
||||
|
||||
# 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
|
||||
end
|
||||
|
||||
event
|
||||
end
|
||||
|
||||
# Add CSP-specific breadcrumbs for security events
|
||||
config.sentry.before_breadcrumb = lambda do |breadcrumb, hint|
|
||||
# Filter out sensitive breadcrumb data
|
||||
if breadcrumb[:data]
|
||||
breadcrumb[:data].reject! { |key, value|
|
||||
key.to_s.match?(/password|secret|token|key|authorization/i) ||
|
||||
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
|
||||
# Scrub sensitive data out of breadcrumbs.
|
||||
config.before_breadcrumb = lambda do |breadcrumb, _hint|
|
||||
if breadcrumb.data.is_a?(Hash)
|
||||
breadcrumb.data.reject! do |key, value|
|
||||
key.to_s.match?(/password|secret|token|key|authorization/i) || value.to_s.match?(/password|secret/i)
|
||||
end
|
||||
end
|
||||
|
||||
breadcrumb
|
||||
end
|
||||
|
||||
# Only send errors in production unless explicitly enabled
|
||||
config.sentry.enabled = Rails.env.production? || ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
|
||||
end
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
module Clinch
|
||||
VERSION = "0.16.0"
|
||||
VERSION = "0.16.3"
|
||||
end
|
||||
|
||||
@@ -96,7 +96,6 @@ Rails.application.routes.draw do
|
||||
end
|
||||
resources :groups
|
||||
get "access", to: "access_checks#new"
|
||||
post "access", to: "access_checks#create"
|
||||
end
|
||||
|
||||
# Render dynamic PWA files from app/views/pwa/* (remember to link manifest in application.html.erb)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -15,8 +15,8 @@ module Admin
|
||||
assert_match "alice@example.com", response.body
|
||||
end
|
||||
|
||||
test "create returns 'can access' with via group when user is in an allowed group" do
|
||||
post admin_access_path, params: {
|
||||
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
|
||||
}
|
||||
@@ -25,9 +25,9 @@ module Admin
|
||||
assert_match "Administrators", response.body # alice is in admin_group; kavita has admin_group
|
||||
end
|
||||
|
||||
test "create returns 'cannot access' with reason when user shares no group with the app" do
|
||||
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)
|
||||
post admin_access_path, params: {
|
||||
get admin_access_path, params: {
|
||||
user_id: lonely.id,
|
||||
application_id: @kavita.id
|
||||
}
|
||||
@@ -36,8 +36,8 @@ module Admin
|
||||
assert_match "shares no group", response.body
|
||||
end
|
||||
|
||||
test "create renders form unchanged when ids are missing" do
|
||||
post admin_access_path, params: {user_id: "", application_id: ""}
|
||||
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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
@@ -559,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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user