-
-
Notifications
You must be signed in to change notification settings - Fork 0
Architecture
This page is the coding standard and target architecture of the plugin — the public distillation of the design agreed in issue #318. It exists so that a contributor can find their way around by structure and naming alone, and so that every change can be checked against one written standard. The design itself lives in #318; migration progress is tracked there and on the SSO Roadmap board.
Unlike the rest of this wiki, this page deliberately describes a target: the codebase is being migrated toward it step by step (see Migration status for what has landed).
Four principles govern every design decision. They hold together; where they pull apart, the order is security > minimal code > readability — we never sacrifice a fail-closed guard for fewer lines or a nicer name.
- Self-documenting code. Readable in principle without comments: intention-revealing names, small single-responsibility types, obvious control flow. If a comment is load-bearing for basic comprehension, we refactor the code instead of keeping the comment.
- As little source code as possible. The least code that does the job — no duplication, no dead code, no gold-plating. Removing code beats adding it. Net line count is tracked per PR as a trailing metric, never a gate.
- Fail-closed security. This is a login path. Missing signature, time-bound, audience, or subject → reject, never default-accept. Secrets are never logged and never serialized out. A cleaner or shorter form is acceptable only if it preserves every security invariant.
- One unified object-oriented architecture. A thin controller layer delegating to pure, single-responsibility helpers, with the same pattern across the OIDC, SAML, config, and rate-limit surfaces — no divergent one-off styles. Prefer matching the existing shape over inventing a new one.
Requests flow through three layers, each doing strictly less than the one above it:
HTTP edge controllers: routing, model binding, headers, status mapping — nothing else
│ constructor-injected singletons
flows one linear method per login phase (begin → callback → mint),
holding the security-critical call ORDER as compile-time structure
│ pure, single-responsibility helpers
gates validators, policies, builders, mappers — pure decisions, no I/O
│ typed state boundaries that own their storage
stores state/replay/request caches, the rate limiter
│
config one store: snapshot reads out, one validated write pipeline in
The central idea: today's fail-closed guarantees are largely positional — they hold because long endpoint bodies call the right helper in the right order. The target makes the most dangerous of them constructional: a handful of evidence types (for example a redeemed one-time state, a validated SAML response) whose only way to exist is having passed the gate, so "did the caller check X?" becomes a compile question. Evidence types are deliberately capped at the few places where an ordering mistake is both plausible and security-relevant — the device must never degenerate into ceremony.
Namespaces equal folders equal domains; everything is internal unless
Jellyfin's plugin contract requires public:
| Namespace | Holds |
|---|---|
Web/ |
the auth completion page, its CSP, the views controller |
Http/ |
the login/admin/link controllers, the login outcome and status mapper, the rate-limit filter |
Flows/ |
the OIDC and SAML login flows, session minting, avatar service, HTTP client factory |
Oidc/ |
id-token validation, authorize-state building, PKCE policy/probe, the OIDC state store |
Saml/ |
the SAML response/request core, its validator, replay and request caches, SAML policies |
Linking/ |
the canonical account-link service, resolver, and revoker |
Config/ |
the plugin configuration, the provider config store, validator, and admin service |
Shared/ |
cross-cutting primitives: URL builder, provider-scoped keys, interval gate, canonical base URL, audit |
Public HTTP routes are frozen throughout the migration — every split is internal, and the plugin's API surface stays byte-identical.
These are the rules a change is reviewed against:
-
Default
internal.publiconly where the Jellyfin plugin contract demands it. One file per type; DTOs never live at the bottom of a controller file. -
Helper suffixes are a contract. A type named
*Validator,*Cache,*Builder,*Mapper,*Policy,*Probe,*Store,*Revoker,*Extractor, or*Gateis a pure, single-responsibility helper — and must beinternalandsealed(orstatic). This is not a style preference:ArchitectureConformanceTestsin the test suite enforces it on every PR, in CI. As each migration step establishes a new structural property, it adds the rule that locks it in. -
Verbs carry semantics.
TryRedeemis consuming and at-most-once;PeekCurrentis non-consuming — never a genericGet.Try*returns nullable/bool and never throws for client-caused conditions. Gates are named for what they decide, stores for what they hold. - Closed sums over booleans and nulls wherever a decision has more than two outcomes. No default branch that accepts.
- No premature abstraction. No interface with a single implementation; symmetry between the OIDC and SAML flows is expressed by identical named-step shape, not by inheritance.
- Comments explain why, never what. A comment that narrates mechanics is deleted along with the mechanics it narrated. The test for introducing a new named type: does it delete an explanatory comment or a string convention?
- Load-bearing security comments travel verbatim. The issue-referenced "why" comments guarding fail-closed decisions are part of the invariant surface: any refactor that moves the code moves the comment with it, unchanged, and reviewers treat those comments as acceptance criteria.
-
Greppable invariants are lint rules. Invariants that are textual rather
than structural (no
gh releasemutations, noSystem.Randomfor security values, no hand-rolled throttle cursors) are enforced by the opengrep rules intools/opengrep/rules.yml, which run in CI — one rule per invariant, added as each is discovered.
Deliberate asymmetries stay deliberate: the rate limiter is fail-open by design (mass-lockout protection) and its result type is named so it can never be confused with a fail-closed gate; log sanitization stays inline at every log call and is never extracted into a helper (the CodeQL sanitizer boundary does not survive a method boundary).
The migration is a strangler, never a big-bang: each step is its own issue, branch, and reviewed PR referencing #318, gated on green tests and — for anything touching the login path — an adversarial security review. Steps that move fail-closed logic add pinning tests before the move, and each PR states where every touched invariant lives before and after.
Landed so far:
-
The conformance test net —
ArchitectureConformanceTestsruns the structural rules on every PR (#322, tightened in #323). -
Zero-behavior renames and record extraction —
Response→SamlResponse,AuthRequest→SamlAuthnRequest, the controller's data records extracted into their own files (#324). -
ProviderScopedKey— the provider-scoped one-time-use key, replacing the hand-concatenated key convention (#325). -
IntervalGate— the once-per-interval throttle primitive, extracted (#331) and adopted at all five throttle sites (#335), with an opengrep tripwire against hand-rolling a sixth. -
SsoUrlBuilder— the SSO URL construction sites unified (#339). -
OidcStateStore— the OpenID authorize-state store consolidated behind typed redeem/peek results (#342). -
LoginOutcomeandLoginStatusMapper— the unified login outcome and its status mapping (#347), with client-caused rejections answered as 4xx, never 500 (#349). -
ProviderConfigStore— the config boundary: snapshot reads out, one validated write pipeline in (#348). -
CanonicalLinkService— the login-path account linking extracted into one service (#353).
Still ahead, in order, each its own gated PR: the validated-SAML-response boundary, the provider-admin service, the flow extraction with the controller split, and the auth completion page rewrite. The full ordered plan, the invariant-residence table, and the deletion ledger live in #318.
Before opening a PR, check your change against this list:
- Match the existing shape. Find the nearest pattern (a gate, a store, a flow phase) and follow it — do not invent a parallel style.
-
New helper? Give it one responsibility, a suffix that says what it is,
internal+sealed/static— the conformance tests will hold you to it. - Fail closed. Any new decision point rejects on missing or invalid input; no default branch that accepts.
- Touching the login path, crypto, or config persistence? Keep every load-bearing security comment with its code, verbatim, and expect the change to pass an adversarial security review before merge.
- Write the test first for any fail-closed decision you move or add — a pinning test that fails if the guard disappears.
- Delete as you go. If your change makes a comment, a helper, or a convention redundant, remove it in the same PR.
-
Reference the issue. Every change is issue-driven (
Closes #N); architecture steps reference #318.
- Issue #318 — the full target design and the migration plan.
- The SSO Roadmap board — the live source of truth for what is done and what is next.
- The Security Model page — the invariants this architecture exists to protect.
Repository · Issues · Releases · Security policy — report vulnerabilities privately, never in a public issue. Pages describe what is implemented today; if the wiki disagrees with the code, the code wins.
Getting started
- Installation
- Provider Setup
- Hardening & Options Reference
- Migrating from 9p4
- Troubleshooting
- Rollback
How it works
Security
- Security Model
- Security Conformance (ASVS / RFC 9700)
- Threat Model
- SSO-Only Login — design record
- Single Logout — design record
Standards & process (internal / maintainer)