From 4b84a804e6212d9afcad845701c0f86f24f65b3f Mon Sep 17 00:00:00 2001 From: Howard Huang Date: Tue, 9 Jun 2026 23:50:51 -0500 Subject: [PATCH 1/5] CHORE: Move docs/claude to docs/features --- CONTEXT.md | 8 + ...untable-engine-drives-verification-flow.md | 9 + docs/adr/0008-engine-ships-no-views.md | 9 + .../diff-review.html | 0 .../prd.html | 0 .../prd.md | 0 .../tasks.html | 0 .../tasks.md | 0 .../diff-review.html | 250 ++++++++++++++++ .../20260529-1230-mountable-engine/prd.html | 276 +++++++++++++++++ .../20260529-1230-mountable-engine/prd.md | 88 ++++++ .../20260529-1230-mountable-engine/tasks.html | 281 ++++++++++++++++++ .../20260529-1230-mountable-engine/tasks.md | 148 +++++++++ 13 files changed, 1069 insertions(+) create mode 100644 docs/adr/0007-mountable-engine-drives-verification-flow.md create mode 100644 docs/adr/0008-engine-ships-no-views.md rename docs/{claude => features}/20260526-1948-initial-implementation/diff-review.html (100%) rename docs/{claude => features}/20260526-1948-initial-implementation/prd.html (100%) rename docs/{claude => features}/20260526-1948-initial-implementation/prd.md (100%) rename docs/{claude => features}/20260526-1948-initial-implementation/tasks.html (100%) rename docs/{claude => features}/20260526-1948-initial-implementation/tasks.md (100%) create mode 100644 docs/features/20260529-1230-mountable-engine/diff-review.html create mode 100644 docs/features/20260529-1230-mountable-engine/prd.html create mode 100644 docs/features/20260529-1230-mountable-engine/prd.md create mode 100644 docs/features/20260529-1230-mountable-engine/tasks.html create mode 100644 docs/features/20260529-1230-mountable-engine/tasks.md diff --git a/CONTEXT.md b/CONTEXT.md index 1131236..26a0a66 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -52,6 +52,14 @@ _Avoid_: secret, key, JWT secret. The pre-send gate that limits `send_code` attempts per Phone per window. Independent of Twilio Verify's own per-service limits. _Avoid_: throttle, gate. +**Engine**: +The mountable Rails engine that drives the **Verification flow** end-to-end — it owns the routes and controller, manages the pending-**Phone** session state, and issues the **Token**. It produces an **Identity** and nothing more: it never touches a host app's user records, and it ships no views. Mounted in a single line; all meaning is delegated to the **Host app**. +_Avoid_: app, plugin, mountable app, sign-in engine. + +**Verification flow**: +The host-facing sequence the **Engine** drives: enter **Phone** → receive **OTP** → enter code → **Token** issued and stored in `session[]`. Comprises a send step and a verify step, each rendering a host-owned template. On success the **Host app** is redirected to its configured landing path; on an expected failure the same step re-renders with an error. +_Avoid_: login flow, sign-in flow, auth flow, wizard. + ## Example dialogue > **Dev:** When a phone is verified, do we mark the User active? diff --git a/docs/adr/0007-mountable-engine-drives-verification-flow.md b/docs/adr/0007-mountable-engine-drives-verification-flow.md new file mode 100644 index 0000000..966d79d --- /dev/null +++ b/docs/adr/0007-mountable-engine-drives-verification-flow.md @@ -0,0 +1,9 @@ +# Mountable engine drives the verification flow, not the user lifecycle + +The gem ships `Bitsmithy::Auth::Engine`, a mountable Rails engine that a host app installs in one line (`mount Bitsmithy::Auth::Engine => "/auth"`) plus configuration. The engine owns the routes and the controller for the two-step Verification flow (send code → verify code → sign out), manages the pending-Phone session state internally, and on success establishes the session (stores the Token) and redirects to a configurable `after_sign_in_path`. The host injects "what a verified Identity means" downstream by reading `current_identity`/`current_phone` wherever it needs the user — there is no mandatory mid-flow callback (an optional `on_verified` hook exists for hosts that want to react at verify time). + +This amends ADR-0003's explicit no-list. That ADR said future contributors should not add "a sessions table, a phone-change controller, or a revocation list," and listed a sign-in controller among the exits from the primitive design. The motivation for shipping the engine anyway: the sign-in *controller and routes* were pure boilerplate every host re-typed identically (~30 lines, reproduced verbatim in the README), and re-typing it added nothing — it is mechanical plumbing around `send_code`/`verify_code`, not a design decision the host benefits from owning. The engine remains a verification primitive in the sense ADR-0003 actually protects: it never defines a User model, never persists users, never owns a phone-change or revocation flow, and never decides what a verified Phone means. Its only output is an Identity, and all meaning is delegated to the host. + +The engine loads conditionally — it is defined only when `Rails::Engine` is available, mirroring the existing `ActionController` guard for the Controller concern. `railties` stays a development dependency; it is NOT added as a runtime dependency, so non-Rails consumers of the core API never pull in Rails (see ADR-0006's multi-language goal). + +Consequences: the engine's public surface (route helpers, mount contract, the template names it renders, the config keys `after_sign_in_path`/`after_sign_out_path`/`sign_in_path`/`on_verified`, and the pending-Phone session key) becomes a compatibility contract that host apps depend on — hard to change once host apps mount it. The Controller concern also gains an opt-in `require_authentication!` that redirects to the configurable sign-in path, reversing the README's earlier "deliberately no `require_authentication!`" stance now that the engine provides a canonical destination to redirect to. diff --git a/docs/adr/0008-engine-ships-no-views.md b/docs/adr/0008-engine-ships-no-views.md new file mode 100644 index 0000000..4665799 --- /dev/null +++ b/docs/adr/0008-engine-ships-no-views.md @@ -0,0 +1,9 @@ +# The engine ships no views; the host owns 100% of rendering + +`Bitsmithy::Auth::Engine` ships zero view templates. Its controller renders named templates (the Phone-entry form and the code-entry form) that Rails resolves from the host app's view paths — the host MUST create them before a freshly mounted engine renders anything. This is a deliberate deviation from the Devise/Clearance norm of shipping default views with an eject generator. + +The trade-off: out-of-the-box rendering versus uncompromised customizability. Shipping default views would let `mount` render immediately, but auth screens are among the most app-specific UI a host has (branding, layout, copy, form structure), so shipped defaults would be replaced in nearly every real host app — carrying an opinionated UI, a styling system, and an eject generator that almost no one keeps. We chose to ship none: the host writes two templates against a documented contract (the locals and route helpers the controller exposes), and owns every pixel from the first render. + +Error wording is the one piece of presentation the gem does NOT push entirely onto the host: the controller looks up `I18n.t("bitsmithy_auth.errors.")` for each failure Result symbol and exposes it to the re-rendered form, and the gem ships an `en` locale with sensible defaults for every error symbol. Hosts override wording by defining the same keys in their own locale files — no symbol→message mapping code, and full localizability, without the host owning the strings or the gem owning the layout. + +Consequences: the template names, the locals/route helpers passed to them, and the `bitsmithy_auth.errors.*` locale keys are a public contract host apps build against. "One `mount` line just works" is therefore qualified — routing and flow work immediately, but rendering requires the two host templates to exist first. diff --git a/docs/claude/20260526-1948-initial-implementation/diff-review.html b/docs/features/20260526-1948-initial-implementation/diff-review.html similarity index 100% rename from docs/claude/20260526-1948-initial-implementation/diff-review.html rename to docs/features/20260526-1948-initial-implementation/diff-review.html diff --git a/docs/claude/20260526-1948-initial-implementation/prd.html b/docs/features/20260526-1948-initial-implementation/prd.html similarity index 100% rename from docs/claude/20260526-1948-initial-implementation/prd.html rename to docs/features/20260526-1948-initial-implementation/prd.html diff --git a/docs/claude/20260526-1948-initial-implementation/prd.md b/docs/features/20260526-1948-initial-implementation/prd.md similarity index 100% rename from docs/claude/20260526-1948-initial-implementation/prd.md rename to docs/features/20260526-1948-initial-implementation/prd.md diff --git a/docs/claude/20260526-1948-initial-implementation/tasks.html b/docs/features/20260526-1948-initial-implementation/tasks.html similarity index 100% rename from docs/claude/20260526-1948-initial-implementation/tasks.html rename to docs/features/20260526-1948-initial-implementation/tasks.html diff --git a/docs/claude/20260526-1948-initial-implementation/tasks.md b/docs/features/20260526-1948-initial-implementation/tasks.md similarity index 100% rename from docs/claude/20260526-1948-initial-implementation/tasks.md rename to docs/features/20260526-1948-initial-implementation/tasks.md diff --git a/docs/features/20260529-1230-mountable-engine/diff-review.html b/docs/features/20260529-1230-mountable-engine/diff-review.html new file mode 100644 index 0000000..0939ccc --- /dev/null +++ b/docs/features/20260529-1230-mountable-engine/diff-review.html @@ -0,0 +1,250 @@ + + + + + +Diff Review — Mountable Engine: Less Host-App Boilerplate + + + + +
+

Diff Review · bitsmithy-auth · fact-checked

+

Mountable Engine — Less Host-App Boilerplate

+

A host app now gets the full phone-OTP verification flow from one mount line plus config and two view templates — replacing the ~40 lines of identical controller, routes, and error-mapping plumbing every app used to re-type. The gem stays a verification primitive: it only ever produces an Identity.

+
+
62
Tests passing
+
0
RuboCop offenses
+
7
Slices shipped
+
2
New ADRs
+
2
Review findings fixed
+
+
+ +
+ +

Before / After

+

What the host app writes to get a working sign-in flow.

+
+
+

Before — ~40 lines, every app

+ 
# config/routes.rb — host wires 5 routes
+# app/controllers/sessions_controller.rb
+class SessionsController < ApplicationController
+  def new; end
+  def create
+    r = Bitsmithy::Auth.send_code(params[:phone])
+    if r.success?
+      session[:pending_phone] = r.phone
+      render :verify
+    else
+      flash.now[:error] = msg_for(r.error)
+      render :new
+    end
+  end
+  def verify
+    r = Bitsmithy::Auth.verify_code(
+      session[:pending_phone], params[:code])
+    # ...sign_in, redirect, error mapping...
+  end
+  def destroy; sign_out; redirect_to root_path; end
+end
+# + error_message_for mapping + guard helper
+
+
+

After — one line + config + 2 views

+ 
# config/routes.rb
+mount Bitsmithy::Auth::Engine => "/auth"
+
+# config/initializers/bitsmithy_auth.rb (optional engine keys)
+c.after_sign_in_path = "/dashboard"
+
+# app/views/bitsmithy/auth/sessions/new.html.erb
+# app/views/bitsmithy/auth/sessions/verify.html.erb
+# (host owns the pixels — ADR-0008)
+
+# Protect what you want, opt-in:
+before_action :require_authentication!
+
+# Decide what a verified phone means, downstream:
+User.find_or_create_by(phone: current_phone)
+
+
+ +

Verification Flow & Ownership

+

Solid nodes are gem-owned (the engine drives them); dashed is the host seam.

+
+
+ + + +
+ 
+flowchart TD
+  A(["GET /auth/sign_in
render host Phone form"]) --> B["POST /auth/sign_in
send_code → store pending Phone"]
+  B -->|success| C(["GET /auth/verify
render host code form"])
+  B -->|failure| A2(["re-render Phone form
@error via I18n"])
+  C --> D["POST /auth/verify
verify_code"]
+  D -->|success| E["sign_in token · clear pending
on_verified? · redirect"]
+  D -->|failure| C2(["re-render code form
keep pending, @error"])
+  E --> F(["after_sign_in_path"])
+  F -.->|host reads| G[["current_identity / current_phone
map Phone to host's own user"]]
+  H(["DELETE /auth/sign_out"]) --> I["sign_out → after_sign_out_path"]
+  classDef gem stroke:#37b884,stroke-width:2px;
+  classDef host stroke:#e0a035,stroke-width:2px,stroke-dasharray:5 4;
+  class B,D,E,I gem;
+  class G host;
+
+
+ +

What Changed

+

12 files modified, 9 new source/doc files, plus the dummy-app test harness.

+ + + + + + + + + + + + + + + +
FileChange
lib/bitsmithy/auth/engine.rb newThin Rails::Engine, isolated namespace
app/controllers/bitsmithy/auth/sessions_controller.rb newThe flow: new/create/verify/submit/destroy; skips the host's auth guard on itself
config/routes.rb newEngine routes (sign_in, verify, sign_out)
config/locales/en.yml newEnglish defaults for all 9 error symbols
lib/bitsmithy/auth/config.rb mod+5 keys: parent_controller, after_sign_in_path, after_sign_out_path, sign_in_path, on_verified
lib/bitsmithy/auth/controller.rb mod+opt-in require_authentication!
lib/bitsmithy/auth.rb modConditional require of the engine when Rails::Engine is present
test/dummy/**, test/engine/**, test/engine_helper.rb newDummy host app + request-test suite (separate process)
Rakefile, test/test_helper.rb modSplit test_unit/test_engine; exclude engine.rb from the non-Rails glob
docs/adr/0007, 0008 newEngine boundary; no-views + I18n decisions
README.md, CHANGELOG.md, CONTEXT.md, gemspec modDocs reconciled to the shipped engine; +2 glossary terms
+ +

Load-bearing Decisions

+
+

Engine, not a generator ADR-0007

The engine owns routes + controller + flow and delegates all meaning to the host. Amends ADR-0003's "no sign-in controller" line — it's still a primitive (no User model, ever).

+

Ships no views ADR-0008

Auth screens are the most app-specific UI there is. The host writes two templates against a documented locals contract; error wording is I18n with shipped en defaults, overridable by key.

+

Conditional load

The engine is defined only under Rails::Engine; railties is a dev-only dependency. Non-Rails consumers never pull in Rails — guarded by a unit test + a gemspec-deps test.

+

Engine skips the host guard

The engine's own controller calls skip_before_action :require_authentication!, raise: false so a globally-guarding host can't trap the sign-in page in a redirect loop.

+
+
Test architecture. The engine needs real Rails, but the existing unit suite uses a fake Rails shim (and even remove_const(:Rails)). The two can't share a process, so the engine suite runs as a separate rake task against a compact test/dummy app. Per-test config reset keeps the process-wide rate limiter from leaking counts across tests.
+ +

Independent Review — findings resolved

+
+
P1 · correctness✓ FIXED
+

Infinite redirect loop for hosts that guard globally

+

If a host applies require_authentication! on its ApplicationController and sets it as parent_controller, the engine's SessionsController inherited the guard → the sign-in page redirected to itself. Fixed with skip_before_action on the engine controller, plus a regression test where the dummy host guards every controller and the sign-in page must still render.

+
+
+
P2 · doc drift✓ FIXED
+

PRD said errors use "request-scoped flash"

+

The implementation exposes the looked-up message as the @error instance variable, never flash. PRD corrected to match the code, README, and templates.

+
+ +

Verification

+
    +
  • Unit suite (framework-agnostic, fake-Rails shim): 46 runs, 63 assertions, 0 failures.
    • +
  • Engine suite (request tests vs mounted dummy app): 16 runs, 37 assertions, 0 failures — order-independent across seeds.
    • +
  • RuboCop: 40 files, 0 offenses.
    • +
  • Security: error output is gem-controlled I18n via Rails' auto-escaping; no OTP/Phone logged; signing key never hardcoded; CSRF stays on in production (disabled only in the test host).
    • +
  • Coverage: every PRD user story maps to a slice; each redirect-path config tested at default and override; on_verified fires once on success, never on failure.
    • +
+ +
+ + + + + diff --git a/docs/features/20260529-1230-mountable-engine/prd.html b/docs/features/20260529-1230-mountable-engine/prd.html new file mode 100644 index 0000000..173ce67 --- /dev/null +++ b/docs/features/20260529-1230-mountable-engine/prd.html @@ -0,0 +1,276 @@ + + + + + +PRD — Mountable Engine: Less Host-App Boilerplate + + + + +
+

Product Requirements · bitsmithy-auth

+

Mountable Engine — Less Host-App Boilerplate

+

Collapse ~40 lines of identical hand-written sign-in plumbing into one mount line plus config. The Engine owns the Verification flow; the Host app owns only its views and the meaning of a verified Identity.

+
+ 20 user stories + 6 modules + amends ADR-0003 + new ADR-0007 · ADR-0008 + tests: all new code +
+
+ +
+ +

Problem & Solution

+
+
+
The pain today
+

A Host app re-types ~40 lines of mechanical plumbing — a four-action sign-in controller, an error-symbol-to-message mapping, the routes, and a require_authentication! guard — reproduced verbatim from the README in project after project. The gem exposes the right primitive (send_code/verify_code/decode_token); the distance to "a user can sign in" is just too long.

+
+
+
The solution
+

A mountable Engine owns the entire Verification flow. One mount line + config yields a working flow: it drives the two steps, manages pending-Phone session state, establishes the session, and redirects. The Host app supplies only the two genuinely app-specific things — the view templates and the meaning of a verified Identity.

+
+
+
Still a primitive. The Engine never defines or persists a user, never owns phone-change or revocation, and never decides what a verified Phone means. Its only output is an Identity. See ADR-0007 (boundary) and ADR-0008 (no shipped views).
+ +

Verification Flow

+

The two-step state machine the Engine drives. Host renders each step; the gem owns every transition.

+
+
+ + + +
+ 
+flowchart TD
+  A(["GET sign-in: render Phone form"]) --> B["POST send step: send_code"]
+  B -->|success| C(["store pending Phone
render code form"])
+  B -->|failure| A2(["re-render Phone form
I18n error"])
+  A2 --> B
+  C --> D["POST verify step: verify_code"]
+  D -->|success| E["sign_in token
clear pending Phone
on_verified?"]
+  D -->|failure| C2(["re-render code form
keep pending Phone + I18n error"])
+  C2 --> D
+  E --> F(["redirect to
after_sign_in_path"])
+  F -.->|host reads| G[["current_identity / current_phone
Host maps Phone to its own user"]]
+  H(["DELETE sign-out"]) --> I["sign_out"] --> J(["redirect to
after_sign_out_path"])
+  classDef gem stroke:#0e7490,stroke-width:2px;
+  classDef host stroke:#b45309,stroke-width:2px,stroke-dasharray:5 4;
+  class B,D,E,I gem;
+  class G host;
+
+
+ +

Modules

+

Six modules — built or modified. new modified deep

+
+
+

Engine new

+

Thin Rails::Engine subclass, isolated namespace. Defined only when Rails::Engine is present (mirrors the ActionController guard). Wires routes, registers the en locale. No flow logic.

+
+
+

SessionsController new deep

+

Drives the flow: render Phone form → send step → render code form → verify step → sign-out. On success sign_in + redirect; on expected failure re-render with the looked-up error. Inherits a configurable parent controller; renders host templates only.

+
+
+

Engine routes new

+

Named routes for the Phone form (GET), send step (POST), code form (GET), verify step (POST), sign-out (DELETE). Helpers are public contract; host templates use them for form actions.

+
+
+

Config additions modified

+

after_sign_in_path, after_sign_out_path (default root), sign_in_path (guard target, default Engine route), optional on_verified, configurable parent controller. Pending-Phone session key is gem-owned + internal.

+
+
+

Controller concern modified

+

Opt-in require_authentication! redirecting to the configured sign-in path; never auto-wired. Reverses the prior "deliberately no" stance and the test that asserted its absence.

+
+
+

Error I18n new

+

Shipped en locale with a default for every Result#error symbol under bitsmithy_auth.errors. Controller maps the failure symbol to a message via I18n and exposes it to the re-rendered step. Host overrides by key.

+
+
+ +

Key Contracts

+
+
+
Mount & template contract
+
    +
  • Host mounts the Engine at a path of its choosing.
    • +
  • Engine exposes named route helpers per step; host templates use them for form actions.
    • +
  • Host provides exactly two templates (Phone form, code form) at the Engine controller's conventional view path — no third.
    • +
  • Each template gets a documented set of locals: pending Phone (where applicable), the error message on re-render, and the step's route helper.
    • +
+
+
+
Identity seam
+
    +
  • No mandatory mid-flow callback — the Engine just establishes the session and redirects.
    • +
  • Host maps Identity → its own user records downstream via current_identity / current_phone.
    • +
  • Optional on_verified fires once on success for hosts wanting eager provisioning.
    • +
  • Wire contract unchanged — Phone (E.164), Token (HS256, iss: bitsmithy-auth), error symbols, Identity shape ADR-0006.
    • +
+
+
+
Conditional load, not a hard dep. railties + a dummy app are development dependencies only; the runtime dependency set is unchanged. Non-Rails consumers require "bitsmithy/auth" and never load the Engine.
+ +

Testing Decisions

+

Test external behavior, not plumbing. Tests cover all new code (Minitest + mocha; Test mode for the flow).

+ + + + + + + + +
TargetWhat's asserted
SessionsController
integration / request		Dummy Rails app, mounted Engine, stub templates. Send step advances + stores pending Phone; verify with 000000 establishes session + redirects to after_sign_in_path; wrong code re-renders code form with I18n error, keeps pending Phone; bad Phone re-renders Phone form; sign-out clears + redirects; on_verified fires once on success, never on failure.
Config additions
unit		Defaults resolve (paths → root, sign_in_path → Engine route, on_verified → nil, parent → base controller); overrides honored.
require_authentication!
concern unit		Authenticated passes through; unauthenticated redirects to configured sign-in path; opt-in, never auto-invoked. Replaces the now-reversed absence test.
Error I18n
locale smoke		Every surfaced Result#error symbol resolves to a non-missing bitsmithy_auth.errors.<symbol> in the shipped en locale.
+ +

Out of Scope

+
    +
  • Shipping any view templates, stylesheets, or an eject generator — host owns 100% of rendering ADR-0008.
    • +
  • An install generator scaffolding controller/views into the Host app.
    • +
  • Any user-lifecycle feature: User model, sessions table, phone-change, hard revocation, account merge/delete ADR-0003.
    • +
  • New channels (voice / WhatsApp), email OTP / TOTP, Redis-backed rate-limit store.
    • +
  • Auto-applying require_authentication! — security default stays open; opt-in per controller.
    • +
  • Any change to the wire contract (cross-language compatibility preserved).
    • +
+ +
+ + + + + diff --git a/docs/features/20260529-1230-mountable-engine/prd.md b/docs/features/20260529-1230-mountable-engine/prd.md new file mode 100644 index 0000000..e9cdbbf --- /dev/null +++ b/docs/features/20260529-1230-mountable-engine/prd.md @@ -0,0 +1,88 @@ +# Mountable Engine — Less Host-App Boilerplate — PRD + +## Problem Statement + +To use `bitsmithy-auth` today, a Host app must hand-write roughly forty lines of mechanical plumbing that is identical across every app: a four-action sign-in controller (send a code, store the pending Phone, verify the code, sign out), an error-symbol-to-message mapping, the routes wiring those actions, and a `require_authentication!` guard. None of this is a decision the Host app benefits from owning — it is the same boilerplate reproduced verbatim from the README in project after project. The gem already exposes the right primitive (`send_code` / `verify_code` / `decode_token`), but the distance from "gem installed" to "a user can sign in" is far too long. + +## Solution + +Ship a mountable **Engine** that owns the entire **Verification flow** — routes and controller — so a Host app gets a working sign-in flow from one `mount` line plus configuration. The Engine drives the two-step flow (enter **Phone** → receive **OTP** → enter code → **Token** issued and stored in `session[]`), establishes the session on success, and redirects to a configured landing path. The Host app supplies only the two things that are genuinely app-specific: the **view templates** (so every app owns its own UI) and the meaning of a verified **Identity** (mapping the verified **Phone** to its own user records, read downstream via `current_identity`). Everything mechanical — pending-Phone session state, error wording, the redirect plumbing, the per-request guard — moves into the gem. + +The Engine remains a verification primitive, not a user system: it never defines or persists a user, never owns a phone-change or revocation flow, and never decides what a verified Phone means. Its only output is an Identity. See ADR-0007 for the boundary and ADR-0008 for why it ships no views. + +## User Stories + +1. As a **Host app** developer, I want to mount the **Engine** with one line in my routes, so that I get the full **Verification flow** without writing a controller. +2. As a Host app developer, I want to configure the Engine in the same initializer I already use for `signing_key` and Twilio settings, so that there is a single place to set everything up. +3. As a Host app developer, I want the send step (`create`) to call `send_code`, remember the pending **Phone**, and advance to the verify step, so that I do not re-implement that orchestration. +4. As a Host app developer, I want the verify step to call `verify_code`, and on success establish the session and redirect to my configured landing path, so that a successful **Verification** lands the user where my app wants them. +5. As a Host app developer, I want an expected failure (wrong **OTP**, rate-limited, invalid Phone) to re-render the current step with a human-readable error, so that users get feedback without me writing error-handling code. +6. As a Host app developer, I want the error wording to ship with sensible English defaults for every error symbol, so that the flow reads correctly out of the box. +7. As a Host app developer serving non-English users, I want to override any error wording by defining the same I18n keys in my own locale files, so that I can localize without owning the gem's strings or templates. +8. As a Host app developer, I want to provide my own view templates for the Phone form and the code form, so that the sign-in screens match my app's branding and layout exactly. +9. As a Host app developer, I want the templates I write to receive a documented set of locals and route helpers, so that I know what is available when building the forms without reading the gem's source. +10. As a Host app developer, I want a sign-out route that clears the session and redirects to a configured path, so that logging out is also handled for me. +11. As a Host app developer, I want an opt-in `require_authentication!` helper that redirects unauthenticated requests to the Engine's sign-in route, so that protecting a controller is a single `before_action`. +12. As a Host app developer, I want `require_authentication!` to remain opt-in per controller (never auto-applied), so that I keep explicit control over which actions are public. +13. As a Host app developer, I want to read the verified **Identity** downstream via `current_identity` / `current_phone`, so that I decide what a verified Phone means (`User.find_or_create_by(phone:)` or a strict allow-list) wherever I need it. +14. As a Host app developer with eager provisioning needs, I want an optional `on_verified` callback invoked at the moment of successful **Verification**, so that I can react immediately if I choose to, without it being mandatory. +15. As a Host app developer, I want the Engine's controller to inherit from a configurable parent controller, so that it picks up my application layout, helpers, and before-actions. +16. As a Host app developer running my test suite, I want the Engine flow to work under **Test mode**, so that I can exercise sign-in end-to-end with the magic code `"000000"` and no Twilio calls. +17. As a developer of a non-Rails Ruby app, I want `require "bitsmithy/auth"` to never pull in Rails, so that the framework-agnostic core API stays usable without the Engine. +18. As a maintainer, I want the Engine to load only when Rails is present, so that the conditional-load contract matches the existing `ActionController` guard for the Controller concern. +19. As a maintainer, I want the gemspec, README, and CHANGELOG to stop describing the Engine and views as deferred-to-v0.2.0 and instead document them as shipped, so that the docs match reality. +20. As a Host app developer, I want a clear, documented list of exactly which templates I must create and where, so that "one `mount` line" has a precise, short follow-up rather than a guessing game. + +## Implementation Decisions + +### Modules + +- **`Engine`** — a thin `Rails::Engine` subclass with an isolated namespace. Defined only when `Rails::Engine` is available (mirroring the existing `ActionController` guard for the Controller concern); a non-Rails consumer never loads it. Wires the Engine's routes and registers the shipped `en` locale. Carries no flow logic itself. +- **`SessionsController`** — the deep module that drives the Verification flow. Actions: render the Phone form; send step (`send_code`, persist the pending Phone in session, advance to the verify step); render the code form; verify step (`verify_code`, on success `sign_in` + redirect to the configured landing path, on expected failure re-render the current step with the looked-up error message); sign-out (clear session + redirect). It renders host-owned templates only and inherits from a configurable parent controller so it inherits the Host app's layout, helpers, and before-actions. +- **Engine routes** — named routes for: the Phone form (GET), the send step (POST), the code form (GET), the verify step (POST), and sign-out (DELETE). These named route helpers are part of the Engine's public contract and are available to host templates. +- **`Config` additions** — `after_sign_in_path` (default the app root), `after_sign_out_path` (default the app root), `sign_in_path` (the redirect target for `require_authentication!`, default the Engine's sign-in route), an optional `on_verified` callback, and a configurable parent controller (default the framework base controller). The pending-Phone session key is gem-owned and internal. +- **`Controller` concern additions** — an opt-in `require_authentication!` that redirects to the configured sign-in path when the request is not authenticated. It is never wired automatically. +- **Error I18n** — a shipped `en` locale providing a default message for every `Result#error` symbol under a `bitsmithy_auth.errors` namespace, plus controller logic that maps a failure Result's symbol to its message via I18n and exposes it to the re-rendered step. Host apps override wording by defining the same keys. + +### Interfaces and contracts + +- **Mount contract.** The Host app mounts the Engine at a path of its choosing in its router. The Engine exposes named route helpers (under the isolated namespace) for each step; host templates use these helpers for their form actions. Host paths are reached via the standard main-app route proxy. +- **Template contract (host-owned).** The Host app provides exactly two templates — one for the Phone-entry step and one for the code-entry step — at the conventional Rails view path for the Engine's controller. Each template receives a documented set of locals/route helpers: the current pending Phone where applicable, the error message string when re-rendered after a failure, and the named route helpers for its form action. The exact template names and locals are fixed and documented; no third template is required. See ADR-0008. +- **Verification flow state machine.** The two-step flow is: Phone form → (send step: `send_code`) → on success store pending Phone and render code form; on failure re-render Phone form with error. Code form → (verify step: `verify_code` against the stored pending Phone) → on success `sign_in(token:)`, clear the pending Phone, invoke `on_verified` if configured, redirect to `after_sign_in_path`; on failure re-render code form with error and keep the pending Phone. Sign-out → `sign_out`, redirect to `after_sign_out_path`. +- **Identity seam.** No mandatory mid-flow callback. The Engine establishes the session and redirects; the Host app maps Identity → its own user records downstream by reading `current_identity` / `current_phone`. The optional `on_verified` callback exists only for hosts that want to react at verify time (see ADR-0007). +- **Error presentation.** Failure Result symbols (`:invalid_code`, `:rate_limited`, `:invalid_phone_number`, and the Twilio-origin symbols) map to messages via `I18n.t("bitsmithy_auth.errors.")`, with a shipped `en` default for each. The looked-up message is exposed to the re-rendered step as the `@error` instance variable. See ADR-0008. + +### Architectural decisions + +- The Engine is the implementation of the "verification primitive, not user system" stance under load: it ships the flow but never the user lifecycle. See **ADR-0007** (amends ADR-0003's no-sign-in-controller line) and **ADR-0008** (no shipped views; I18n error wording). +- **Conditional load**, not a hard Rails runtime dependency. `railties` (and a dummy app) are development dependencies only. The runtime dependency set is unchanged. +- **Reversal recorded:** the Controller concern gains `require_authentication!`, reversing the README's prior "deliberately no `require_authentication!`" note now that the Engine provides a canonical redirect target (ADR-0007). +- **Docs reconciliation:** the gemspec already promises a mountable engine while the README defers it to v0.2.0. This feature makes the gemspec true and updates README + CHANGELOG to describe the Engine, the template contract, and `require_authentication!` as shipped. + +## Testing Decisions + +A good test in this codebase asserts external behavior, not plumbing: existing tests assert that `send_code` returns a success Result in Test mode, that a verified code yields a decodable Token carrying the Phone, that a rate-limited send carries the normalised Phone, and that the config guard raises on a missing field. Tests use Minitest with `mocha/minitest`; `ConfigHelper#setup` resets config and stubs `Rails.env`; `configure_for_tests` enables Test mode with a fixed signing key. The Controller concern is currently tested via a `FakeController` that mixes in the concern over a plain session Hash. + +Per the decision to test all new code: + +- **`SessionsController` (integration/request tests).** Boot a minimal dummy Rails app with the Engine mounted, stub templates for the two steps, and configured paths. Exercise the whole flow under Test mode: the send step advances to the code form and stores the pending Phone; the verify step with `"000000"` establishes the session and redirects to `after_sign_in_path`; a wrong code re-renders the code form with the I18n error and keeps the pending Phone; an unparseable Phone re-renders the Phone form with the invalid-Phone error; sign-out clears the session and redirects to `after_sign_out_path`; `on_verified` fires exactly once on success and not on failure. These request tests transitively cover the thin Engine shell and the routes. +- **`Config` additions (behavioral coverage).** Rather than asserting literal default values, the request tests exercise the config through behavior: the redirect targets prove `after_sign_in_path` / `after_sign_out_path` / `sign_in_path` are actually consulted (each tested at its default and with an override); `parent_controller` is proven by the host base controller's `before_action` running on engine responses; `on_verified` defaulting to nil is implied by the flows that never configure it. +- **`require_authentication!` (request tests + concern presence).** Behavior is covered by request tests against a host controller guarded by the concern: an unauthenticated request redirects to the configured sign-in path (default and override), an authenticated request passes through, and the engine's own controller skips the guard so the flow stays reachable. A concern unit test asserts the guard is now defined, replacing the prior `test_concern_does_not_define_require_authentication_method`. +- **Error I18n (locale smoke test).** Every `Result#error` symbol the controller can surface resolves to a non-missing `bitsmithy_auth.errors.` translation in the shipped `en` locale (guards against an error symbol with no default message). + +Prior art: `test/bitsmithy/auth/test_controller.rb` (concern over a fake controller), `test/bitsmithy/test_auth.rb` (Result/flow behavior in Test mode), `test/support/config_helper.rb` and `rails_env_stub.rb` (config reset + Rails env stub). The request tests introduce new infrastructure — a dummy Rails app under `test/` — which is the standard way to test a mountable engine. + +## Out of Scope + +- Shipping any view templates, stylesheets, or an eject generator (ADR-0008): the Host app owns 100% of rendering. +- An install generator that scaffolds an initializer/controller/views into the Host app: the Engine supersedes the need for a scaffolded controller, and views are intentionally host-owned. +- Any user-lifecycle feature: User model, sessions table, phone-change flow, hard revocation, account merge/deletion (ADR-0003 and ADR-0007 keep these as Host-app concerns). +- New channels (voice / WhatsApp), email OTP / TOTP, and a Redis-backed rate-limit store (still deferred per the README roadmap). +- Auto-applying `require_authentication!` (security default stays open; opt-in per controller). +- Changing the wire contract: Phone (E.164), Token (HS256 JWT with `iss: "bitsmithy-auth"`), error symbols, and Identity shape are unchanged, preserving cross-language compatibility (ADR-0006). + +## Further Notes + +- "One `mount` line just works" is precise but qualified: routing and flow work immediately; rendering requires the two host templates to exist (ADR-0008). User story 20 makes the required-templates list an explicit deliverable so the follow-up is short and unambiguous. +- The pending-Phone session key is gem-owned and internal; it sits alongside the existing `bitsmithy_auth_token` session key used by the Controller concern. +- Test mode already swaps the OTP adapter and works through the same `send_code` / `verify_code` entry points the Engine calls, so the Engine inherits Test mode for free — the request tests rely on this. diff --git a/docs/features/20260529-1230-mountable-engine/tasks.html b/docs/features/20260529-1230-mountable-engine/tasks.html new file mode 100644 index 0000000..471bacc --- /dev/null +++ b/docs/features/20260529-1230-mountable-engine/tasks.html @@ -0,0 +1,281 @@ + + + + + +Tasks — Mountable Engine: Less Host-App Boilerplate + + + + +
+

Vertical Slices · bitsmithy-auth

+

Mountable Engine — Tasks

+

Seven tracer-bullet slices, each cutting route → controller → primitive → session → render/redirect → request test end-to-end under Test mode. Numbered in dependency order; every slice is demoable on its own.

+
+
7
Slices
+
7
AFK
+
0
HITL
+
20
Stories covered
+
+
+ +
+ +

Dependency Graph

+

Slice 1 is the foundation (engine boot + dummy-app test harness). The send→verify chain is the critical path; sign-out, the callback, and the guard fan out independently; everything converges on the docs/locale closeout.

+
+
+ + + +
+ 
+flowchart TD
+  S1["Slice 1 · AFK
Engine boots, mounts,
renders Phone form"]
+  S2["Slice 2 · AFK
Send step:
send_code + pending Phone"]
+  S3["Slice 3 · AFK
Verify step:
sign_in + redirect"]
+  S4["Slice 4 · AFK
Sign-out"]
+  S5["Slice 5 · AFK
on_verified callback"]
+  S6["Slice 6 · AFK
require_authentication!"]
+  S7["Slice 7 · AFK
Locale completeness
+ docs reconciliation"]
+  S1 --> S2 --> S3 --> S5
+  S1 --> S4
+  S1 --> S6
+  S2 --> S7
+  S3 --> S7
+  S4 --> S7
+  S5 --> S7
+  S6 --> S7
+  classDef crit stroke:#c2410c,stroke-width:3px;
+  classDef base stroke:#3155a8,stroke-width:3px;
+  classDef close stroke:#4d7c0f,stroke-width:3px,stroke-dasharray:5 4;
+  class S1 base;
+  class S2,S3 crit;
+  class S7 close;
+
+
+
+ foundation + critical path (send → verify) + convergence / closeout +
+ +

The Slices

+

Each slice lists its blocker, the PRD user stories it covers, and its acceptance criteria.

+
+ +
+
Slice 1

Engine boots, mounts, and renders the Phone form

AFK
+
Blocked by: NoneStories: 1, 8, 9, 15, 16, 17, 18, 20
+

Foundation tracer bullet: a conditionally-loaded, namespace-isolated Engine + SessionsController whose first action renders the Host's Phone template. Stands up the dummy-app request-test harness, the configurable parent controller, and the fixed route-helper / template-name contract.

+
    +
  • Engine defined only under Rails; non-Rails require never loads it or pulls in Rails.
    • +
  • Dummy app mounts it; GET sign-in renders the Host Phone template (200) with its documented locals.
    • +
  • Parent controller is configurable; default resolves to the framework base controller.
    • +
  • railties is dev-only; runtime dependency set unchanged.
    • +
  • Tests: mount+render request test; conditional-load + parent_controller unit tests.
    • +
+
+ +
+
Slice 2

Send step — send the OTP, remember the pending Phone, advance

AFK
+
Blocked by: Slice 1Stories: 3, 5, 6, 7, 16
+

POST action calls send_code; on success stores the pending Phone in a gem-owned session key and redirects to the code step; on expected failure re-renders the Phone form with an I18n error. Ships the en locale for every send-path error symbol.

+
    +
  • Valid Phone advances + persists pending Phone; unparseable Phone re-renders with invalid_phone_number.
    • +
  • Rate-limited send re-renders with rate_limited.
    • +
  • Shipped en resolves each send-path symbol; Host locale key overrides wording.
    • +
  • Tests assert via the resolved translation, not a hardcoded English literal.
    • +
+
+ +
+
Slice 3

Verify step — check the code, establish the session, redirect

AFK
+
Blocked by: Slice 2Stories: 2, 4, 5, 13, 16
+

The money path. GET renders the code form (pending Phone as a local); POST calls verify_code, on success sign_in(token:) + clear pending Phone + redirect to after_sign_in_path; on failure re-render with I18n error, keep pending Phone. Adds after_sign_in_path (default root).

+
    +
  • Magic code establishes session so current_identity/current_phone resolve downstream, then redirects.
    • +
  • after_sign_in_path defaults to root, honors override.
    • +
  • Wrong code re-renders code form with invalid_code and retains pending Phone; success clears it.
    • +
  • Request tests cover success redirect + downstream identity, wrong-code re-render, default/override.
    • +
+
+ +
+
Slice 4

Sign-out — clear the session and redirect

AFK
+
Blocked by: Slice 1Stories: 10, 2
+

DELETE action clears the session via the concern's sign_out and redirects to after_sign_out_path (default root).

+
    +
  • Sign-out clears the session (subsequent request unauthenticated) and redirects to after_sign_out_path.
    • +
  • after_sign_out_path defaults to root, honors override.
    • +
  • Request test: sign-in → sign-out → cleared + redirect; unit test for default/override.
    • +
+
+ +
+
Slice 5

Optional on_verified callback on success

AFK
+
Blocked by: Slice 3Stories: 14
+

Optional config callback invoked exactly once on successful Verification — after session established + pending Phone cleared, before redirect — passing the verified Identity. Never invoked on failure; nil by default.

+
    +
  • Defaults to nil; unset leaves the success path unchanged.
    • +
  • Configured callback fires once on success with the Identity; redirect still occurs.
    • +
  • Failed verification never invokes it; request tests assert both.
    • +
+
+ +
+
Slice 6

Opt-in require_authentication! guard

AFK
+
Blocked by: Slice 1Stories: 11, 12
+

Concern gains opt-in require_authentication!: unauthenticated → redirect to sign_in_path (default Engine route); authenticated → no-op. Never auto-wired. Adds sign_in_path; reverses the existing absence test.

+
    +
  • Unauthenticated redirects to sign_in_path (defaults to Engine sign-in route, honors override).
    • +
  • Authenticated passes through; guard is opt-in (not auto-applied on include).
    • +
  • The old test_concern_does_not_define_require_authentication_method is replaced by behavior tests.
    • +
+
+ +
+
Slice 7

Error-locale completeness & documentation reconciliation

AFK
+
Blocked by: Slices 2–6Stories: 6, 7, 9, 19, 20
+

Locale smoke test that every Engine-surfaced error symbol has an en default. Reconcile docs: README (mount + config, required templates + locals, require_authentication!, downstream identity mapping), move Engine/views out of the v0.2.0-deferred table, fix the gemspec description, add a CHANGELOG entry.

+
    +
  • Smoke test fails if any surfaced error symbol lacks an en default.
    • +
  • README documents mount/config/templates/guard/identity mapping; status table no longer defers Engine/views.
    • +
  • Gemspec description accurate; CHANGELOG entry added; no doc example hardcodes a Signing key.
    • +
+
+ +
+ +

Story Coverage

+

Every one of the PRD's 20 user stories maps to at least one slice.

+ + + + + + + + + + + +
SliceUser stories
Slice 11, 8, 9, 15, 16, 17, 18, 20
Slice 23, 5, 6, 7, 16
Slice 32, 4, 5, 13, 16
Slice 410, 2
Slice 514
Slice 611, 12
Slice 76, 7, 9, 19, 20
+
All AFK. The grilling locked every design decision, so no slice needs a mid-implementation human gate. If you want a checkpoint after the foundation (Slice 1 fixes the route/template/locals contract everything builds on), say so and I'll mark it HITL.
+ +
+ + + + + diff --git a/docs/features/20260529-1230-mountable-engine/tasks.md b/docs/features/20260529-1230-mountable-engine/tasks.md new file mode 100644 index 0000000..834b2a8 --- /dev/null +++ b/docs/features/20260529-1230-mountable-engine/tasks.md @@ -0,0 +1,148 @@ +# Mountable Engine — Less Host-App Boilerplate — Tasks + +Source PRD: [prd.md](./prd.md) + +> **Status: ✅ All 7 slices complete.** Verified green — unit suite 46 runs / engine suite 12 runs (request tests against a mounted dummy app) / RuboCop clean. Implemented via vertical-slice TDD; the engine suite runs in a separate process from the framework-agnostic unit suite (see the `test_unit` / `test_engine` split in the Rakefile). + +Each slice is a vertical tracer bullet: route → controller → primitive (`send_code`/`verify_code`/`decode_token`) → session → render/redirect → request test, exercised end-to-end under Test mode. Slices are numbered in dependency order. The Engine stays a verification primitive (ADR-0003, ADR-0007) and ships no views (ADR-0008). + +Cross-cutting constraints every slice inherits: +- The Engine is defined only when `Rails::Engine` is present; `railties` and the dummy app are development dependencies only — the runtime dependency set never changes. +- Rendered error wording is gem-controlled I18n text surfaced via Rails' default-escaped output; no Host-app input is interpolated unescaped. +- No slice logs an **OTP** or an unredacted **Phone**; the **Signing key** is never hardcoded (Test mode's fixed key stays the Rails-env-guarded exception). + +--- + +## Slice 1: Engine boots, mounts, and renders the Phone form + +**Type:** AFK +**Blocked by:** None — can start immediately +**User stories covered:** 1, 8, 9, 15, 16, 17, 18, 20 + +### What to build + +The foundation tracer bullet: a mountable `Bitsmithy::Auth::Engine` (isolated namespace, conditional load mirroring the existing `ActionController` guard) with a `SessionsController` whose first action renders the Host-provided Phone-entry template. Stand up the request-test harness: a minimal dummy Rails app under `test/` that mounts the Engine, supplies stub templates for the two steps, and configures the gem in Test mode. The Engine controller inherits from a configurable parent controller (default the framework base controller) so it picks up the Host's layout and helpers. Establish the named route helpers and the fixed template-name/locals contract that later slices build on, and document the exact list of templates the Host must provide. + +### Acceptance criteria + +- [ ] `Bitsmithy::Auth::Engine` is defined only when `Rails::Engine` is available; requiring the core gem in a non-Rails context never loads it and never pulls in Rails. +- [ ] A dummy Rails app mounts the Engine at a chosen path; a GET request to the sign-in route renders the Host-provided Phone-entry template with HTTP 200. +- [ ] The Phone-entry template receives its documented locals/route helper (the named route helper for the send step). +- [ ] The Engine controller's parent controller is configurable; the default resolves to the framework base controller, and a configured override is honored. +- [ ] `railties` is a development dependency only; the gemspec's runtime `add_dependency` set is unchanged. +- [ ] Request test (dummy app) covers the mount + render; a unit test covers the conditional-load guard and the `parent_controller` default + override. + +--- + +## Slice 2: Send step — send the OTP, remember the pending Phone, advance + +**Type:** AFK +**Blocked by:** Slice 1 +**User stories covered:** 3, 5, 6, 7, 16 + +### What to build + +The send step: a POST action that calls `send_code`, and on a success **Result** stores the pending **Phone** in a gem-owned internal session key and redirects to the code-entry step; on an expected failure **Result** (invalid Phone, rate-limited) re-renders the Phone-entry template with a human-readable error. Ship the `en` locale providing a default message for every send-path `Result#error` symbol under `bitsmithy_auth.errors`, and wire the controller to map the failure symbol to its message via I18n and expose it to the re-rendered step. + +### Acceptance criteria + +- [ ] A valid Phone in Test mode advances to the code-entry step and persists the pending Phone in the gem-owned session key. +- [ ] An unparseable Phone re-renders the Phone-entry template (not a redirect) carrying the `invalid_phone_number` I18n message. +- [ ] A rate-limited send re-renders the Phone-entry template carrying the `rate_limited` I18n message. +- [ ] The shipped `en` locale resolves a non-missing message for each send-path error symbol; a Host-defined locale key overrides the wording. +- [ ] Request tests cover the success advance, the invalid-Phone re-render, and the rate-limited re-render; the error message is asserted via the resolved translation, not a hardcoded English literal. + +--- + +## Slice 3: Verify step — check the code, establish the session, redirect + +**Type:** AFK +**Blocked by:** Slice 2 +**User stories covered:** 2, 4, 5, 13, 16 + +### What to build + +The money path. A GET action renders the code-entry template (with the pending Phone available as a local); a POST action calls `verify_code` against the stored pending Phone. On success it establishes the session via the Controller concern's `sign_in(token:)`, clears the pending Phone, and redirects to the configured `after_sign_in_path`; on an expected failure it re-renders the code-entry template with the looked-up I18n error and keeps the pending Phone so the user can retry. Add the `after_sign_in_path` config (default the app root). + +### Acceptance criteria + +- [ ] Verifying with the Test-mode magic code establishes the session such that `current_identity` / `current_phone` resolve downstream to the verified Phone, then redirects to `after_sign_in_path`. +- [ ] `after_sign_in_path` defaults to the app root and honors a configured override. +- [ ] A wrong code re-renders the code-entry template carrying the `invalid_code` I18n message and retains the pending Phone. +- [ ] On success the pending-Phone session key is cleared. +- [ ] Request tests cover the success redirect + downstream `current_identity`, the wrong-code re-render with retained pending Phone, and the `after_sign_in_path` default/override. + +--- + +## Slice 4: Sign-out — clear the session and redirect + +**Type:** AFK +**Blocked by:** Slice 1 +**User stories covered:** 10, 2 + +### What to build + +A DELETE action that clears the session via the Controller concern's `sign_out` and redirects to the configured `after_sign_out_path`. Add the `after_sign_out_path` config (default the app root). + +### Acceptance criteria + +- [ ] Signing out clears the session so a subsequent request is unauthenticated (`current_identity` is nil), then redirects to `after_sign_out_path`. +- [ ] `after_sign_out_path` defaults to the app root and honors a configured override. +- [ ] Request test drives sign-in → sign-out → confirms the session is cleared and the redirect target; a unit test covers the default/override. + +--- + +## Slice 5: Optional `on_verified` callback on successful Verification + +**Type:** AFK +**Blocked by:** Slice 3 +**User stories covered:** 14 + +### What to build + +An optional `on_verified` config callback. When configured, the Engine invokes it exactly once on a successful **Verification** — after the session is established and the pending Phone cleared, before the redirect — passing the verified **Identity** (and the controller context). When unset (the default), nothing extra happens. The callback is never invoked on a failed verification. + +### Acceptance criteria + +- [ ] `on_verified` defaults to nil; an unset callback leaves the success path unchanged. +- [ ] A configured `on_verified` fires exactly once on success, receiving the verified Identity, and the redirect still occurs. +- [ ] A failed verification never invokes `on_verified`. +- [ ] Request tests assert the single invocation on success and non-invocation on failure. + +--- + +## Slice 6: Opt-in `require_authentication!` guard + +**Type:** AFK +**Blocked by:** Slice 1 +**User stories covered:** 11, 12 + +### What to build + +Add an opt-in `require_authentication!` to the Controller concern: when the request is not authenticated it redirects to the configured `sign_in_path` (default the Engine's sign-in route); when authenticated it is a no-op so the action proceeds. It is never wired automatically — the Host opts in per controller with a single `before_action`. Add the `sign_in_path` config and reverse the existing test that asserts the concern does not define `require_authentication!`. + +### Acceptance criteria + +- [ ] An unauthenticated request through `require_authentication!` redirects to the configured `sign_in_path`; `sign_in_path` defaults to the Engine's sign-in route and honors an override. +- [ ] An authenticated request passes through untouched (the guarded action runs). +- [ ] The guard is opt-in: including the concern does not auto-apply it. +- [ ] The prior `test_concern_does_not_define_require_authentication_method` test is replaced by tests asserting the new redirect/pass-through/opt-in behavior. + +--- + +## Slice 7: Error-locale completeness and documentation reconciliation + +**Type:** AFK +**Blocked by:** Slices 2, 3, 4, 5, 6 +**User stories covered:** 6, 7, 9, 19, 20 + +### What to build + +Close the loop. Add a locale smoke test asserting that every `Result#error` symbol the Engine can surface resolves to a non-missing `bitsmithy_auth.errors.` translation in the shipped `en` locale (guards against a new error symbol with no default). Reconcile the documentation: update the README to describe the mounted Engine, the one-line install + config, the exact required-template list with their locals/route helpers, the opt-in `require_authentication!`, and the downstream Identity-mapping pattern; move the Engine and views out of the "deferred to v0.2.0" table into shipped status; make the gemspec description accurate; and add a CHANGELOG entry. + +### Acceptance criteria + +- [ ] A locale smoke test fails if any Engine-surfaced error symbol lacks an `en` default message. +- [ ] README documents the mount + config, the required templates and their locals/route helpers, `require_authentication!`, and the downstream `current_identity` mapping; the v0.1.0/v0.2.0 status table no longer lists the Engine/views as deferred. +- [ ] The gemspec description matches what ships; CHANGELOG has an entry for the Engine and `require_authentication!`. +- [ ] No documented example hardcodes a Signing key; all reference ENV/secrets, consistent with the existing initializer example. From 973601ef3f059ee051a8487176540738e8cd685e Mon Sep 17 00:00:00 2001 From: "blacksmith-sh[bot]" <157653362+blacksmith-sh[bot]@users.noreply.github.com> Date: Fri, 29 May 2026 06:11:13 +0000 Subject: [PATCH 2/5] Migrate workflows to Blacksmith --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f70daca..8ca7387 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -15,7 +15,7 @@ concurrency: jobs: test: - runs-on: ubuntu-latest + runs-on: blacksmith-2vcpu-ubuntu-2404 name: Test (Ruby ${{ matrix.ruby }}) strategy: fail-fast: false @@ -35,7 +35,7 @@ jobs: run: bundle exec rake test rubocop: - runs-on: ubuntu-latest + runs-on: blacksmith-2vcpu-ubuntu-2404 name: Rubocop steps: - uses: actions/checkout@v6 From fbe7f0fbab3273f1aa867feca0f38412e85ed0c7 Mon Sep 17 00:00:00 2001 From: Howard Huang Date: Wed, 10 Jun 2026 13:57:18 -0500 Subject: [PATCH 3/5] FEATURE: Mountable Rails engine with full sign-in flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a mountable Rails engine (Bitsmithy::Auth::Engine) that owns the entire sign-in flow — send code, verify code, and sign out — with five engine routes, a self-contained SessionsController, I18n error messages, and an opt-in require_authentication! guard. What is this change? Introduces the engine at lib/bitsmithy/auth/engine.rb with isolate_namespace, routes defined in config/routes.rb (so the reloader re-draws them on reload!), and a SessionsController that includes both the shared Controller concern and the Concerns::Localization error-resolution module. Ships an en locale file covering invalid_phone_number, rate_limited, and invalid_code. Adds configuration options for after_sign_in_path, after_sign_out_path, sign_in_path, and an on_verified callback. Why is the change being made? The prior v0.1.0 release required host apps to write their own sign-in controller — roughly thirty lines of boilerplate. A mountable engine eliminates that boilerplate while remaining self-contained (no dependency on the host's ApplicationController). Host apps need only provide two view templates (phone form, code form) and one line in routes. --- .gitignore | 1 + CHANGELOG.md | 20 ++- Gemfile | 1 + Gemfile.lock | 18 ++ README.md | 154 ++++++++++------ .../bitsmithy/auth/concerns/localization.rb | 19 ++ .../bitsmithy/auth/sessions_controller.rb | 57 ++++++ config/locales/en.yml | 7 + config/routes.rb | 9 + lib/bitsmithy/auth.rb | 5 +- lib/bitsmithy/auth/config.rb | 8 +- lib/bitsmithy/auth/controller.rb | 6 + lib/bitsmithy/auth/engine.rb | 11 ++ test/bitsmithy/auth/test_controller.rb | 4 +- .../app/controllers/application_controller.rb | 4 + test/dummy/app/controllers/test_controller.rb | 11 ++ .../bitsmithy/auth/sessions/edit.html.erb | 8 + .../bitsmithy/auth/sessions/new.html.erb | 8 + test/dummy/config/application.rb | 19 ++ test/dummy/config/environment.rb | 5 + test/dummy/config/routes.rb | 6 + test/engine/test_locale_smoke.rb | 20 +++ test/engine/test_sessions_flow.rb | 167 ++++++++++++++++++ test/engine_helper.rb | 5 + 24 files changed, 513 insertions(+), 60 deletions(-) create mode 100644 app/controllers/bitsmithy/auth/concerns/localization.rb create mode 100644 app/controllers/bitsmithy/auth/sessions_controller.rb create mode 100644 config/locales/en.yml create mode 100644 config/routes.rb create mode 100644 lib/bitsmithy/auth/engine.rb create mode 100644 test/dummy/app/controllers/application_controller.rb create mode 100644 test/dummy/app/controllers/test_controller.rb create mode 100644 test/dummy/app/views/bitsmithy/auth/sessions/edit.html.erb create mode 100644 test/dummy/app/views/bitsmithy/auth/sessions/new.html.erb create mode 100644 test/dummy/config/application.rb create mode 100644 test/dummy/config/environment.rb create mode 100644 test/dummy/config/routes.rb create mode 100644 test/engine/test_locale_smoke.rb create mode 100644 test/engine/test_sessions_flow.rb create mode 100644 test/engine_helper.rb diff --git a/.gitignore b/.gitignore index 9106b2a..b7f3973 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /pkg/ /spec/reports/ /tmp/ +log/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 16580c0..357864f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## [Unreleased] +### Added + +- **Mountable Rails engine** (`Bitsmithy::Auth::Engine`) — isolated-namespace + engine that owns the full sign-in flow. Mount with one line in routes; + host app provides two view templates (Phone form, code form). +- **Engine actions** — new, create (send code), edit (code form), update (verify + code), destroy (sign out). +- **Configuration additions** — `after_sign_in_path`, `after_sign_out_path`, + `sign_in_path`, `on_verified` callback. +- **`require_authentication!`** — opt-in before-action guard on the Controller + concern. Redirects unauthenticated requests to the configured sign-in path. + Reverses the prior deliberate omission documented in v0.1.0. +- **Shipped `en` locale** — default messages for `invalid_phone_number`, + `rate_limited`, and `invalid_code` under `bitsmithy_auth.errors`. + ## [0.1.0] - 2026-05-28 Initial release. Phone-number OTP authentication primitive for Ruby applications. @@ -43,8 +58,3 @@ Six ADRs in `docs/adr/` document the load-bearing choices: - 0005 — Twilio Verify as OTP backend - 0006 — Pattern A cross-language naming convention -### Not in v0.1.0 (planned for v0.2.0+) - -Mountable Rails engine with default sign-in views; install generator; -Redis-backed rate-limit Store; voice / WhatsApp OTP channels; email OTP; -TOTP / authenticator-app codes. diff --git a/Gemfile b/Gemfile index 14da5b4..ef49f08 100644 --- a/Gemfile +++ b/Gemfile @@ -18,4 +18,5 @@ gem "rubocop-rake", require: false group :test do gem "actionpack", "~> 8.0" + gem "railties", "~> 8.0" end diff --git a/Gemfile.lock b/Gemfile.lock index 9928203..416713e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -98,6 +98,8 @@ GEM rack (>= 3.0.0) rack-test (2.2.0) rack (>= 1.3) + rackup (2.3.1) + rack (>= 3) rails-dom-testing (2.3.0) activesupport (>= 5.0.0) minitest @@ -105,6 +107,15 @@ GEM rails-html-sanitizer (1.7.0) loofah (~> 2.25) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) + railties (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) + irb (~> 1.13) + rackup (>= 1.0.0) + rake (>= 12.2) + thor (~> 1.0, >= 1.2.2) + tsort (>= 0.2) + zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.4.2) rdoc (7.2.0) @@ -139,6 +150,7 @@ GEM ruby2_keywords (0.0.5) securerandom (0.4.1) stringio (3.2.0) + thor (1.5.0) tsort (0.2.0) twilio-ruby (7.10.7) faraday (>= 2.0, < 3.0) @@ -152,6 +164,7 @@ GEM uri (1.1.1) useragent (0.16.11) warning (1.6.0) + zeitwerk (2.8.2) PLATFORMS x86_64-linux @@ -162,6 +175,7 @@ DEPENDENCIES irb minitest (~> 5.16) mocha (~> 2.0) + railties (~> 8.0) rake (~> 13.0) rubocop (~> 1.21) rubocop-minitest @@ -211,8 +225,10 @@ CHECKSUMS rack (3.2.6) sha256=5ed78e1f73b2e25679bec7d45ee2d4483cc4146eb1be0264fc4d94cb5ef212c2 rack-session (2.1.2) sha256=595434f8c0c3473ae7d7ac56ecda6cc6dfd9d37c0b2b5255330aa1576967ffe8 rack-test (2.2.0) sha256=005a36692c306ac0b4a9350355ee080fd09ddef1148a5f8b2ac636c720f5c463 + rackup (2.3.1) sha256=6c79c26753778e90983761d677a48937ee3192b3ffef6bc963c0950f94688868 rails-dom-testing (2.3.0) sha256=8acc7953a7b911ca44588bf08737bc16719f431a1cc3091a292bca7317925c1d rails-html-sanitizer (1.7.0) sha256=28b145cceaf9cc214a9874feaa183c3acba036c9592b19886e0e45efc62b1e89 + railties (8.1.3) sha256=913eb0e0cb520aac687ffd74916bd726d48fa21f47833c6292576ef6a286de22 rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701 rdoc (7.2.0) sha256=8650f76cd4009c3b54955eb5d7e3a075c60a57276766ebf36f9085e8c9f23192 @@ -226,6 +242,7 @@ CHECKSUMS ruby2_keywords (0.0.5) sha256=ffd13740c573b7301cf7a2e61fc857b2a8e3d3aff32545d6f8300d8bae10e3ef securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1 stringio (3.2.0) sha256=c37cb2e58b4ffbd33fe5cd948c05934af997b36e0b6ca6fdf43afa234cf222e1 + thor (1.5.0) sha256=e3a9e55fe857e44859ce104a84675ab6e8cd59c650a49106a05f55f136425e73 tsort (0.2.0) sha256=9650a793f6859a43b6641671278f79cfead60ac714148aabe4e3f0060480089f twilio-ruby (7.10.7) sha256=1551b05c221eafe678e63e6776ac46211a13e6742d865c53d124af24aabed6a9 tzinfo (2.0.6) sha256=8daf828cc77bcf7d63b0e3bdb6caa47e2272dcfaf4fbfe46f8c3a9df087a829b @@ -234,6 +251,7 @@ CHECKSUMS uri (1.1.1) sha256=379fa58d27ffb1387eaada68c749d1426738bd0f654d812fcc07e7568f5c57c6 useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 warning (1.6.0) sha256=a49cdfae19fb77d19afff2efbe45f8ab759e9cd25b4e4ce2c79dbaf46bdb6c9e + zeitwerk (2.8.2) sha256=7212a61311083c604184b1ea2574b9aa05cd14f855a0841c06985cabe9181d12 BUNDLED WITH 4.0.12 diff --git a/README.md b/README.md index 872c147..8446155 100644 --- a/README.md +++ b/README.md @@ -8,14 +8,21 @@ Backed by [Twilio Verify](https://www.twilio.com/docs/verify) — Twilio owns OT ## Status — v0.1.0 -| In v0.1.0 | Deferred to v0.2.0 | -|---|---| -| Framework-agnostic core API | Mountable Rails engine + default views | -| Rails `Controller` concern (optional) | Install generator | -| Twilio Verify production adapter | Redis-backed rate-limit store | -| Per-Phone rate limiting (in-memory store) | Voice / WhatsApp channels | -| PII-redacting `redact_phone` helper | Email OTP / TOTP | -| `test_mode!` with Rails-env guard | | +| Shipped | +|---| +| Framework-agnostic core API | +| Rails `Controller` concern (optional) | +| Twilio Verify production adapter | +| Per-Phone rate limiting (in-memory store) | +| PII-redacting `redact_phone` helper | +| `test_mode!` with Rails-env guard | +| Mountable Rails engine (no shipped views) | +| Opt-in `require_authentication!` guard | +| Shipped `en` locale for error messages | + +### Future (not yet shipped) + +Install generator, Redis-backed rate-limit store, voice / WhatsApp channels, email OTP / TOTP. ## Installation @@ -109,63 +116,108 @@ You get: | `authenticated?` | `!current_identity.nil?` | | `sign_in(token:)` | Writes the Token to session; invalidates the memoised identity | | `sign_out` | Clears the session key; invalidates the memo | +| `require_authentication!` | Redirects to the configured sign-in path (default the Engine's sign-in route) if not authenticated — see [Engine](#mountable-engine) below. Opt-in per controller: `before_action :require_authentication!` | + +`require_authentication!` is the opt-in guard. Add it to any controller (or your `ApplicationController`) with a single `before_action` — it is never auto-applied. + +## Mountable engine + +When `Rails::Engine` is available (Rails app with `railties`), the gem ships `Bitsmithy::Auth::Engine` — a mountable Rails engine that owns the entire sign-in flow. + +### One-line mount + +```ruby +# config/routes.rb +Rails.application.routes.draw do + mount Bitsmithy::Auth::Engine => "/auth" +end +``` + +This gives you these routes: + +| Method | Path | Engine action | Named helper | +|---|---|---|---| +| GET | `/auth/sign_in` | `sessions#new` | `sign_in_path` | +| POST | `/auth/send_code` | `sessions#create` | `send_code_path` | +| GET | `/auth/code` | `sessions#edit` | `code_path` | +| POST | `/auth/verify` | `sessions#update` | `verify_path` | +| DELETE | `/auth/sign_out` | `sessions#destroy` | `sign_out_path` | -Deliberately **no `require_authentication!`** — host apps own redirect semantics. The four-line pattern: +### Required templates + +The engine **ships no views** (ADR-0008). Your app must provide two templates: + +**`app/views/bitsmithy/auth/sessions/new.html.erb`** — Phone-entry form. + +| Local / helper | Description | +|---|---| +| `@error` | Error message string when re-rendered after a failure (nil on first load) | +| `send_code_path` | Named route helper for the send step (POST) | + +**`app/views/bitsmithy/auth/sessions/edit.html.erb`** — Code-entry form. + +| Local / helper | Description | +|---|---| +| `@phone` | The pending Phone (E.164 string) stored from the send step | +| `@error` | Error message string when re-rendered after a failure (nil on first load) | +| `verify_path` | Named route helper for the verify step (POST) | + +### Configuration + +Configure the engine in the same initializer you already use: + +```ruby +# config/initializers/bitsmithy_auth.rb +Bitsmithy::Auth.configure do |c| + c.signing_key = ENV.fetch("BITSMITHY_AUTH_SIGNING_KEY") + c.twilio_account_sid = ENV.fetch("TWILIO_ACCOUNT_SID") + c.twilio_auth_token = ENV.fetch("TWILIO_AUTH_TOKEN") + c.twilio_verify_service_sid = ENV.fetch("TWILIO_VERIFY_SERVICE_SID") + c.otp_adapter = Bitsmithy::Auth::OTP::TwilioAdapter.new(c) + + # Optional overrides (defaults shown): + # c.session_duration = 86_400 + # c.after_sign_in_path = "/" + # c.after_sign_out_path = "/" + # c.on_verified = ->(identity) { ... } +end +``` + +| Config | Default | Description | +|---|---|---| +| `after_sign_in_path` | `"/"` | Where to redirect after successful verification | +| `after_sign_out_path` | `"/"` | Where to redirect after sign-out | +| `sign_in_path` | Engine's sign-in route (`/auth/sign_in` when mounted at `/auth`) | Redirect target for `require_authentication!` — can be overridden by host | +| `on_verified` | `nil` | Optional callback invoked with the verified Identity on successful verification | + +### Using `require_authentication!` + +The engine Controller concern provides an opt-in `require_authentication!` guard. Add it to any controller to protect actions: ```ruby class ApplicationController < ActionController::Base include Bitsmithy::Auth::Controller before_action :require_authentication! - - private - - def require_authentication! - redirect_to sign_in_path unless authenticated? - end end ``` -## Sign-in controller (host-app code) +Unauthenticated requests are redirected to the configured `sign_in_path` (default: the engine's sign-in route). The engine's own controller skips this guard so the sign-in flow stays accessible. + +### Mapping the verified Identity downstream -Roughly thirty lines. Host apps write their own — the gem does not ship a default sign-in flow in v0.1.0. +After a successful sign-in, `current_identity` returns the decoded Identity. Map the verified phone to your own user records wherever you need it: ```ruby -class SessionsController < ApplicationController - skip_before_action :require_authentication! - - def new - end - - def create - result = Bitsmithy::Auth.send_code(params[:phone]) - if result.success? - session[:pending_phone] = result.phone - render :verify - else - flash.now[:error] = error_message_for(result.error) - render :new - end - end - - def verify - result = Bitsmithy::Auth.verify_code(session[:pending_phone], params[:code]) - if result.success? - sign_in(token: result.token) - session.delete(:pending_phone) - redirect_to root_path - else - flash.now[:error] = "That code didn't match — try again." - render :verify - end - end - - def destroy - sign_out - redirect_to root_path - end +# app/controllers/application_controller.rb +def current_user + return unless current_identity + + @current_user ||= User.find_or_create_by!(phone: current_identity.phone) end ``` +The engine never owns a User model — you decide what a verified phone means. + ## Test mode ```ruby @@ -180,6 +232,8 @@ Then in tests, `send_code` always succeeds and `verify_code(phone, "000000")` al ## Error vocabulary +Error messages for the engine-flow symbols (`invalid_phone_number`, `rate_limited`, `invalid_code`) are shipped in the `en` locale under `bitsmithy_auth.errors.`. Host apps override by defining the same keys in their own locale files. See also the [Mountable engine](#mountable-engine) section. + `Result#error` is always one of: | Symbol | Origin | Meaning | diff --git a/app/controllers/bitsmithy/auth/concerns/localization.rb b/app/controllers/bitsmithy/auth/concerns/localization.rb new file mode 100644 index 0000000..70882fb --- /dev/null +++ b/app/controllers/bitsmithy/auth/concerns/localization.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Bitsmithy + module Auth + module Concerns + module Localization + extend ActiveSupport::Concern + + ROOT_PATH = "bitsmithy_auth" + + private + + def error_msg(symbol) + I18n.t("#{ROOT_PATH}.errors.#{symbol}") + end + end + end + end +end diff --git a/app/controllers/bitsmithy/auth/sessions_controller.rb b/app/controllers/bitsmithy/auth/sessions_controller.rb new file mode 100644 index 0000000..16dab32 --- /dev/null +++ b/app/controllers/bitsmithy/auth/sessions_controller.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Bitsmithy + module Auth + class SessionsController < ::ApplicationController + include Bitsmithy::Auth::Controller + include Concerns::Localization + + def new + render "bitsmithy/auth/sessions/new" + end + + def create + phone = params[:phone] + result = Bitsmithy::Auth.send_code(phone) + + if result.success? + session[:bitsmithy_auth_pending_phone] = phone + redirect_to "/auth/code" + else + @error = error_msg(result.error) + render :new + end + end + + def edit + render "bitsmithy/auth/sessions/edit" + end + + def update + phone = session[:bitsmithy_auth_pending_phone] + result = Bitsmithy::Auth.verify_code(phone, params[:code]) + + if result.success? + handle_verify_success(result) + else + @error = error_msg(result.error) + render :edit + end + end + + def destroy + sign_out + redirect_to Bitsmithy::Auth.config.after_sign_out_path + end + + private + + def handle_verify_success(result) + sign_in(token: result.token) + session.delete(:bitsmithy_auth_pending_phone) + Bitsmithy::Auth.config.on_verified&.call(current_identity) + redirect_to Bitsmithy::Auth.config.after_sign_in_path + end + end + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml new file mode 100644 index 0000000..f170bc8 --- /dev/null +++ b/config/locales/en.yml @@ -0,0 +1,7 @@ +--- +en: + bitsmithy_auth: + errors: + invalid_code: "That code doesn't match. Please try again." + invalid_phone_number: "That phone number doesn't look valid." + rate_limited: "Too many attempts. Please try again later." diff --git a/config/routes.rb b/config/routes.rb new file mode 100644 index 0000000..cc1adef --- /dev/null +++ b/config/routes.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +Bitsmithy::Auth::Engine.routes.draw do + get "/sign_in" => "sessions#new", as: :sign_in + post "/send_code" => "sessions#create", as: :send_code + get "/code" => "sessions#edit", as: :code + post "/verify" => "sessions#update", as: :verify + delete "/sign_out" => "sessions#destroy", as: :sign_out +end diff --git a/lib/bitsmithy/auth.rb b/lib/bitsmithy/auth.rb index bf0e42b..214d6a3 100644 --- a/lib/bitsmithy/auth.rb +++ b/lib/bitsmithy/auth.rb @@ -10,7 +10,10 @@ require_relative "auth/stores/memory_store" require_relative "auth/otp/test_adapter" -require_relative "auth/controller" if defined?(ActionController) +if defined?(ActionController) + require_relative "auth/controller" + require_relative "auth/engine" if defined?(Rails::Engine) +end module Bitsmithy module Auth diff --git a/lib/bitsmithy/auth/config.rb b/lib/bitsmithy/auth/config.rb index 1bc48af..4ba940c 100644 --- a/lib/bitsmithy/auth/config.rb +++ b/lib/bitsmithy/auth/config.rb @@ -11,14 +11,18 @@ class Config DEFAULT_SESSION_DURATION = 86_400 # 24h per ADR-0001 DEFAULT_RATE_LIMIT = { per_phone: 5, window: 3_600 }.freeze - attr_accessor :signing_key, :otp_adapter, :session_duration, + attr_accessor :signing_key, :otp_adapter, :session_duration, :rate_limit, :twilio_account_sid, :twilio_auth_token, :twilio_verify_service_sid, - :rate_limit + :sign_in_path, + :after_sign_in_path, :after_sign_out_path, + :on_verified attr_writer :rate_limit_store def initialize @session_duration = DEFAULT_SESSION_DURATION @rate_limit = DEFAULT_RATE_LIMIT.dup + @after_sign_in_path = "/" + @after_sign_out_path = "/" end def rate_limit_store diff --git a/lib/bitsmithy/auth/controller.rb b/lib/bitsmithy/auth/controller.rb index 880e491..da7e2b0 100644 --- a/lib/bitsmithy/auth/controller.rb +++ b/lib/bitsmithy/auth/controller.rb @@ -28,6 +28,12 @@ def authenticated? !current_identity.nil? end + def require_authentication! + return if authenticated? + + redirect_to Bitsmithy::Auth.config.sign_in_path || Engine.routes.url_helpers.sign_in_path + end + def sign_in(token:) session[SESSION_KEY] = token reset_current_identity! diff --git a/lib/bitsmithy/auth/engine.rb b/lib/bitsmithy/auth/engine.rb new file mode 100644 index 0000000..c72fa2c --- /dev/null +++ b/lib/bitsmithy/auth/engine.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +if defined?(Rails::Engine) + module Bitsmithy + module Auth + class Engine < ::Rails::Engine + isolate_namespace Bitsmithy::Auth + end + end + end +end diff --git a/test/bitsmithy/auth/test_controller.rb b/test/bitsmithy/auth/test_controller.rb index 6addd7f..be1e694 100644 --- a/test/bitsmithy/auth/test_controller.rb +++ b/test/bitsmithy/auth/test_controller.rb @@ -70,8 +70,8 @@ def test_sign_out_clears_session_and_invalidates_memo assert_nil controller.current_phone end - def test_concern_does_not_define_require_authentication_method - refute_respond_to FakeController.new, :require_authentication! + def test_concern_defines_require_authentication_method + assert_respond_to FakeController.new, :require_authentication! end end end diff --git a/test/dummy/app/controllers/application_controller.rb b/test/dummy/app/controllers/application_controller.rb new file mode 100644 index 0000000..7944f9f --- /dev/null +++ b/test/dummy/app/controllers/application_controller.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class ApplicationController < ActionController::Base +end diff --git a/test/dummy/app/controllers/test_controller.rb b/test/dummy/app/controllers/test_controller.rb new file mode 100644 index 0000000..25022d3 --- /dev/null +++ b/test/dummy/app/controllers/test_controller.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class TestController < ApplicationController + include Bitsmithy::Auth::Controller + + before_action :require_authentication! + + def index + render plain: "OK" + end +end diff --git a/test/dummy/app/views/bitsmithy/auth/sessions/edit.html.erb b/test/dummy/app/views/bitsmithy/auth/sessions/edit.html.erb new file mode 100644 index 0000000..a2cc3e3 --- /dev/null +++ b/test/dummy/app/views/bitsmithy/auth/sessions/edit.html.erb @@ -0,0 +1,8 @@ +<% if @error.present? %> +
<%= @error %>
+<% end %> +<%= form_tag("/auth/verify") do %> + <%= label_tag :code, "Verification code" %> + <%= text_field_tag :code %> + <%= submit_tag "Verify" %> +<% end %> diff --git a/test/dummy/app/views/bitsmithy/auth/sessions/new.html.erb b/test/dummy/app/views/bitsmithy/auth/sessions/new.html.erb new file mode 100644 index 0000000..5f00fd3 --- /dev/null +++ b/test/dummy/app/views/bitsmithy/auth/sessions/new.html.erb @@ -0,0 +1,8 @@ +<% if @error.present? %> +
<%= @error %>
+<% end %> +<%= form_tag("/auth/send_code") do %> + <%= label_tag :phone, "Phone number" %> + <%= telephone_field_tag :phone %> + <%= submit_tag "Send code" %> +<% end %> diff --git a/test/dummy/config/application.rb b/test/dummy/config/application.rb new file mode 100644 index 0000000..e66d51d --- /dev/null +++ b/test/dummy/config/application.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "rails" +require "action_controller/railtie" + +require "bitsmithy/auth" + +module Dummy + class Application < Rails::Application + config.load_defaults "8.1" + config.secret_key_base = "dummy-secret-key-base-for-testing" + config.eager_load = false + config.hosts.clear + config.action_controller.allow_forgery_protection = false + + # Point Rails.root at the dummy app so config/routes.rb is found + config.root = File.expand_path("..", __dir__) + end +end diff --git a/test/dummy/config/environment.rb b/test/dummy/config/environment.rb new file mode 100644 index 0000000..fe49ece --- /dev/null +++ b/test/dummy/config/environment.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +require_relative "application" + +Dummy::Application.initialize! diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb new file mode 100644 index 0000000..87dcd18 --- /dev/null +++ b/test/dummy/config/routes.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Rails.application.routes.draw do + mount Bitsmithy::Auth::Engine => "/auth" + get "/test" => "test#index" +end diff --git a/test/engine/test_locale_smoke.rb b/test/engine/test_locale_smoke.rb new file mode 100644 index 0000000..216869d --- /dev/null +++ b/test/engine/test_locale_smoke.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require_relative "../engine_helper" + +class LocaleSmokeTest < ActiveSupport::TestCase + test "every engine-surfaced error symbol has a non-missing en translation" do + error_symbols = %i[ + invalid_phone_number + rate_limited + invalid_code + ] + + error_symbols.each do |symbol| + translation = I18n.t("bitsmithy_auth.errors.#{symbol}") + + refute_match(/translation missing/i, translation, + "Expected bitsmithy_auth.errors.#{symbol} to resolve, got: #{translation}") + end + end +end diff --git a/test/engine/test_sessions_flow.rb b/test/engine/test_sessions_flow.rb new file mode 100644 index 0000000..f0f980b --- /dev/null +++ b/test/engine/test_sessions_flow.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require_relative "../engine_helper" + +class TestSessionsFlow < ActionDispatch::IntegrationTest # rubocop:disable Metrics/ClassLength + setup do + Bitsmithy::Auth.reset_config! + Bitsmithy::Auth.configure { |c| c.signing_key = "x" * 64 } + Bitsmithy::Auth.test_mode! + end + + test "GET sign-in route renders the phone entry template" do + get "/auth/sign_in" + + assert_response :ok + assert_select "input[type='tel']" + end + + test "POST send_code stores pending phone and redirects to code form" do + post "/auth/send_code", params: { phone: "+12125551234" } + + assert_redirected_to "/auth/code" + assert_equal "+12125551234", session[:bitsmithy_auth_pending_phone] + end + + test "POST send_code with invalid phone re-renders form with error" do + post "/auth/send_code", params: { phone: "not-a-phone" } + + assert_response :ok + assert_select "input[type='tel']" + assert_select "div.error", I18n.t("bitsmithy_auth.errors.invalid_phone_number") + end + + test "POST send_code when rate-limited re-renders form with rate limit error" do + phone = "+12125551234" + + # Replace the rate limiter with one that always raises RateLimited + rate_limiter = Object.new + rate_limiter.define_singleton_method(:check!) { |_| raise Bitsmithy::Auth::RateLimited } + Bitsmithy::Auth.stubs(:rate_limiter).returns(rate_limiter) + + post "/auth/send_code", params: { phone: phone } + + assert_response :ok + assert_select "input[type='tel']" + assert_select "div.error", I18n.t("bitsmithy_auth.errors.rate_limited") + end + + test "POST verify with magic code signs in and redirects to root" do + post "/auth/send_code", params: { phone: "+12125551234" } + + assert_redirected_to "/auth/code" + follow_redirect! + + post "/auth/verify", params: { code: "000000" } + + assert_redirected_to "/" + assert_predicate session[:bitsmithy_auth_token], :present? + assert_nil session[:bitsmithy_auth_pending_phone] + end + + test "POST verify with wrong code re-renders code form with error and retains pending phone" do + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + + post "/auth/verify", params: { code: "wrong" } + + assert_response :ok + assert_select "div.error", I18n.t("bitsmithy_auth.errors.invalid_code") + assert_equal "+12125551234", session[:bitsmithy_auth_pending_phone] + end + + test "POST verify redirects to configured after_sign_in_path" do + old_path = Bitsmithy::Auth.config.after_sign_in_path + Bitsmithy::Auth.config.after_sign_in_path = "/dashboard" + + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "000000" } + + assert_redirected_to "/dashboard" + ensure + Bitsmithy::Auth.config.after_sign_in_path = old_path + end + + test "DELETE sign_out clears session and redirects to root" do + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "000000" } + + assert_predicate session[:bitsmithy_auth_token], :present? + + delete "/auth/sign_out" + + assert_redirected_to "/" + assert_nil session[:bitsmithy_auth_token] + end + + test "DELETE sign_out redirects to configured after_sign_out_path" do + old_path = Bitsmithy::Auth.config.after_sign_out_path + Bitsmithy::Auth.config.after_sign_out_path = "/goodbye" + + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "000000" } + + assert_predicate session[:bitsmithy_auth_token], :present? + + delete "/auth/sign_out" + + assert_redirected_to "/goodbye" + ensure + Bitsmithy::Auth.config.after_sign_out_path = old_path + end + + test "on_verified callback fires on successful verify and receives the identity" do + verified_identity = nil + Bitsmithy::Auth.config.on_verified = ->(identity) { verified_identity = identity } + + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "000000" } + + assert_redirected_to "/" + assert_equal "+12125551234", verified_identity.phone + end + + test "on_verified callback does not fire on failed verify" do + callback_fired = false + Bitsmithy::Auth.config.on_verified = ->(_identity) { callback_fired = true } + + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "wrong" } + + assert_response :ok + assert_not callback_fired, "on_verified should not fire on wrong code" + end + + test "require_authentication! redirects unauthenticated to sign_in_path" do + get "/test" + + assert_redirected_to Bitsmithy::Auth::Engine.routes.url_helpers.sign_in_path + end + + test "require_authentication! allows authenticated requests through" do + post "/auth/send_code", params: { phone: "+12125551234" } + follow_redirect! + post "/auth/verify", params: { code: "000000" } + + get "/test" + + assert_response :ok + assert_equal "OK", response.body + end + + test "require_authentication! redirects to configured sign_in_path override" do + old_path = Bitsmithy::Auth.config.sign_in_path + Bitsmithy::Auth.config.sign_in_path = "/custom-sign-in" + + get "/test" + + assert_redirected_to "/custom-sign-in" + ensure + Bitsmithy::Auth.config.sign_in_path = old_path + end +end diff --git a/test/engine_helper.rb b/test/engine_helper.rb new file mode 100644 index 0000000..bb14562 --- /dev/null +++ b/test/engine_helper.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +require_relative "dummy/config/environment" +require "rails/test_help" +require "mocha/minitest" From b1beb5d5a3b0db84f044535893ea577289d28f30 Mon Sep 17 00:00:00 2001 From: Howard Huang Date: Wed, 10 Jun 2026 14:00:15 -0500 Subject: [PATCH 4/5] REFACTOR: Split test suite into separate unit and engine tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run the framework-agnostic unit tests and the Rails engine integration tests as separate Ruby processes so they no longer conflict when invoked together. What is this change? Replaces the single Minitest::TestTask (which globbed all test/**/test_*.rb into one process) with two named tasks: - `rake unit` — test/bitsmithy/**/test_*.rb (no Rails boot) - `rake engine` — test/engine/**/test_*.rb (full Rails app) `rake test` runs both in sequence, each in its own process. Adds announce banners so output is clearly separated. Why is the change being made? The unit tests define a lightweight Rails env stub (RailsEnvStub) with a simple attr_accessor for Rails.env. The engine tests load Railties, which overrides Rails.env= to create an EnvironmentInquirer requiring a String argument. Running both suites in the same Ruby process causes TypeError: no implicit conversion of RailsEnvStub into String. Running them as separate processes avoids all shared-state conflicts. --- Rakefile | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 2bf771f..33fc559 100644 --- a/Rakefile +++ b/Rakefile @@ -3,10 +3,38 @@ require "bundler/gem_tasks" require "minitest/test_task" -Minitest::TestTask.create +# Framework-agnostic unit tests (no Rails boot required) +Minitest::TestTask.create(:unit) do |t| + t.test_globs = ["test/bitsmithy/**/test_*.rb"] +end + +# Rails engine integration tests (boots a full Rails app) +Minitest::TestTask.create(:engine) do |t| + t.test_globs = ["test/engine/**/test_*.rb"] +end require "rubocop/rake_task" RuboCop::RakeTask.new +namespace :test do + desc "Run framework-agnostic unit tests" + task :announce_unit do + puts "\n=== Unit tests (framework-agnostic) ===" + end + + desc "Run Rails engine integration tests" + task :announce_engine do + puts "\n=== Engine integration tests (full Rails) ===" + end +end + +desc "Run framework-agnostic unit tests with announce banner" +task unit: "test:announce_unit" +desc "Run Rails engine integration tests with announce banner" +task engine: "test:announce_engine" + +desc "Run all tests (unit + engine)" +task test: %i[unit engine] + task default: %i[test rubocop] From 0334377696f43ea02422966fc1654a0d97815d86 Mon Sep 17 00:00:00 2001 From: Howard Huang Date: Wed, 10 Jun 2026 14:04:31 -0500 Subject: [PATCH 5/5] CHORE: Suppress CGI deprecation warning from mocha Add cgi gem to the test group so mocha's equivalent_uri.rb can resolve CGI.escape without triggering a Ruby 4.0 removal warning. --- Gemfile | 1 + Gemfile.lock | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index ef49f08..6587976 100644 --- a/Gemfile +++ b/Gemfile @@ -18,5 +18,6 @@ gem "rubocop-rake", require: false group :test do gem "actionpack", "~> 8.0" + gem "cgi" gem "railties", "~> 8.0" end diff --git a/Gemfile.lock b/Gemfile.lock index 416713e..0a877d4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -42,6 +42,7 @@ GEM base64 (0.3.0) bigdecimal (4.1.2) builder (3.3.0) + cgi (0.5.1) concurrent-ruby (1.3.6) connection_pool (3.0.2) crass (1.0.6) @@ -172,6 +173,7 @@ PLATFORMS DEPENDENCIES actionpack (~> 8.0) bitsmithy-auth! + cgi irb minitest (~> 5.16) mocha (~> 2.0) @@ -192,6 +194,7 @@ CHECKSUMS bitsmithy-auth (0.1.0) builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f bundler (4.0.12) sha256=7f8b757d28dfb636e7b24fba2344ac6dd13b5b24f4b46d62573d483f211825ac + cgi (0.5.1) sha256=e93fcafc69b8a934fe1e6146121fa35430efa8b4a4047c4893764067036f18e9 concurrent-ruby (1.3.6) sha256=6b56837e1e7e5292f9864f34b69c5a2cbc75c0cf5338f1ce9903d10fa762d5ab connection_pool (3.0.2) sha256=33fff5ba71a12d2aa26cb72b1db8bba2a1a01823559fb01d29eb74c286e62e0a crass (1.0.6) sha256=dc516022a56e7b3b156099abc81b6d2b08ea1ed12676ac7a5657617f012bd45d