Phase 1: auth middleware strategy-dispatcher refactor#79
Merged
Conversation
…spatcher Adds the strategy-pattern scaffolding described in design doc section 3.5, as dead code (not wired into validateRequest yet — that lands in commit 2). - src/Core/Auth/AuthStrategy.php: two-phase interface (matches, preAuth, verify, name). Two phases preserve current validateRequest ordering where parseIdentity runs before URL parsing and verifySignature runs after response hydration. - src/Core/Auth/HmacSessionStrategy.php: wraps the existing HMAC-SHA256 flow (Api::parseIdentityString + Api::verifySignature) with identical logic. Handles both IS_PRIVATE=true (sig required) and IS_PRIVATE=false (identity-only) flavors. matches() returns false when IS_PRIVATE and no signature header, deferring to the /sign helper path. - src/Core/Auth/AuthDispatcher.php: picks the first matching strategy. buildDefault() returns Phase-1 stack (Hmac only). Returning null means "no strategy claimed this request" — caller (commit 2) will interpret as generateSignature fallthrough. - src/Core/Api.php: $key, $signature, $utcDate bumped from private to public so strategies can populate. No behavior change; characterization tests still use reflection and remain green. 18/18 Phase 0 unit tests still passing. Smoke-tested dispatcher selection via CLI: null when no auth headers (IS_PRIVATE mode), hmac_session when both present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds per-install constant AUTH_STRATEGY_DISPATCHER (default 'off'). When 'off', the legacy inline auth path runs — bit-for-bit identical behavior to v4.2.0, and the Phase 0 characterization tests remain the contract. When 'on', validateRequest selects an AuthStrategy via AuthDispatcher::buildDefault() and calls its preAuth() / verify() at the same two points where the legacy code called parseIdentityString / verifySignature, preserving response-body ordering on failure. Minor field additions on Api: - $authStrategy (public, null on legacy path): the selected strategy. - AUTH_STRATEGY_DISPATCHER entry in $defaultEnvironmentConstants. Flag mechanism (per-install constant) deviates from design doc O2's original AppConfig plan. Reason: AppConfig is keyed by kyte_account, which the auth flow itself is responsible for resolving — chicken-and-egg. Per-install constants match the pattern used for IS_PRIVATE, SESSION_TIMEOUT, SIGNATURE_TIMEOUT, etc. Design doc O2 updated to reflect the revision. 18/18 Phase 0 unit tests green with flag=off. Integration-smoke with flag=on (CLI script using the same DB fixtures as SignatureTest) exercises matches → preAuth → verify and confirms: account + key loaded, signature verified, anonymous session state set correctly in $api->response. Commit 3 (shadow-mode harness) is the next step before any flag=on rollout on real traffic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the AUTH_STRATEGY_DISPATCHER='shadow' mode required by the
Phase 1 exit criteria. When enabled, validateRequest:
1. Snapshots $api->response before any auth work.
2. Runs the legacy inline auth path normally — this is what serves the
client response. Shadow never affects serving.
3. After legacy's verifySignature completes, invokes
AuthShadowHarness::runAndCompare, which resets Api state to pre-auth,
runs AuthDispatcher->select()->preAuth()->verify(), compares the new
strategy's fingerprint against the legacy fingerprint, logs any
divergence via ActivityLogger (action types AUTH_SHADOW_DIFF and
AUTH_SHADOW_EXCEPTION), and restores legacy state in a finally block.
Fingerprint compared: key_id, account_id, user_id, response[session],
response[token], response[uid], response[name], response[email],
signature, utc_epoch.
Known side effects during shadow (documented in the harness):
- SessionManager::validate() runs twice per request. Each call may
update a session's last-activity timestamp. Acceptable on dev for a
2-4 week soak; not for production customers.
- One extra DB read for account + api key per request.
Shadow mode is intended for the Phase 1 cutover soak only — it verifies
HmacSessionStrategy produces identical auth results to the legacy inline
code before we flip to flag=on. After the soak, shadow is never used on
a given install again.
Smoke-tested via CLI with both the matching-state case (no diff logged,
state cleanly restored) and a forced-mismatch case (diff detected, log
attempt made, state still cleanly restored).
18/18 Phase 0 unit tests still green (flag defaults to 'off', shadow
code is dormant).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- tests/HmacSessionStrategyTest.php: 12 parallel characterization tests exercising HmacSessionStrategy::matches / preAuth / verify directly. Mirrors SignatureTest's coverage (same timestamp-boundary, unknown-key, unknown-account, undefined-session, malformed-identity cases) but goes through the new strategy surface instead of Api reflection. Together with SignatureTest this forms the Phase 1 equivalence contract: the strategy must behave identically to the legacy inline code in every branch tested. - phpunit.xml.dist: registers the new test file in the unit suite. - src/Core/Api.php: small VERBOSE_LOG-gated error_log of the selected strategy name when flag=on. Full per-request ActivityLogger entry was considered but deferred to Phase 2: with only one strategy today, an AUTH_STRATEGY_SELECTED row per request would be constant noise. When Phase 2 adds McpTokenStrategy and the selection becomes informative, this stub can graduate to a structured activity-log entry. 30/30 unit tests green (18 original + 12 new), 103 assertions, zero warnings, zero deprecations. This completes Phase 1 on branch feature/phase-1-strategy-dispatcher. Next steps before Phase 2: - Push branch, open PR, confirm CI green. - Dev-server soak with flag=shadow for 2-4 weeks; verify zero AUTH_SHADOW_DIFF log entries before flipping flag=on. - Tag v4.3.0 at Phase 1 exit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of the Kyte MCP / auth migration. Refactors the inline HMAC auth code in
Api::validateRequestinto a strategy-pattern abstraction, without changing behavior for existing clients. Four commits:src/Core/Auth/AuthStrategy.php(interface),HmacSessionStrategy.php(wraps the legacy three-layer HMAC-SHA256 flow line-for-line),AuthDispatcher.php(first-match-wins router). No wiring yet. Bumps$key,$signature,$utcDatefromprivatetopubliconApiso strategies can populate.AUTH_STRATEGY_DISPATCHERconstant added to$defaultEnvironmentConstantswith default'off'. When'off', legacy inline auth runs — byte-identical behavior to v4.2.0. When'on',AuthDispatcher::buildDefault()selects a strategy,preAuth()runs before URL parsing,verify()runs after response hydration — same two points where the legacy code calledparseIdentityStringandverifySignature, preserving response-body ordering on 403s.AuthShadowHarness::runAndCompare(). WhenAUTH_STRATEGY_DISPATCHER='shadow', legacy auth runs normally and serves the response. After it completes, the harness resetsApistate to pre-auth, re-runs the dispatcher, and compares fingerprints (key_id,account_id,user_id,response[session/token/uid/name/email],signature,utc_epoch). Divergences log viaActivityLoggerasAUTH_SHADOW_DIFF; new-path exceptions log asAUTH_SHADOW_EXCEPTION. Legacy state is restored in afinallyblock before the caller returns.tests/HmacSessionStrategyTest.phpwith 12 tests exercisingmatches / preAuth / verifydirectly — same edge cases as Phase 0'sSignatureTest(boundary, expired, malformed, unknown key/account, undefined-session quirk). A lightweightVERBOSE_LOG-gatederror_logof the selected strategy name.Flag mechanism decision
Design doc O2 originally proposed
AppConfigfor this flag. Revised here to a per-install PHP constant becauseAppConfigis keyed bykyte_account(required), which the auth flow itself is responsible for resolving — chicken-and-egg. Per-install constants match the pattern ofIS_PRIVATE,SIGNATURE_TIMEOUT, etc. Design doc O2 updated with the rationale.Known side effects of shadow mode
Documented in
AuthShadowHarness.php:SessionManager::validate()runs twice per request during shadow (legacy, then new). Each call may update a session'slast_activitytimestamp. Acceptable for the dev-only soak; should not be enabled on customer installs.KyteAccount+KyteAPIKeyper request.After the Phase 1 cutover on a given install,
shadowmode is never used again there.Testing
hmac_session, preAuth loads account + key, verify accepts signature.Explicitly deferred to Phase 2
AUTH_STRATEGY_SELECTEDactivity-log entries. With only one strategy today, per-request logging would be noise. When Phase 2 introducesMcpTokenStrategy, the stub inApi.phpcan graduate.Post-merge plan
v4.3.0.v4.3.0, setAUTH_STRATEGY_DISPATCHER='shadow'in itsconfig.php.SELECT * FROM KyteActivityLog WHERE action LIKE 'AUTH_SHADOW_%'returns zero rows (or only rows we can explain).AUTH_STRATEGY_DISPATCHER='on'. Soak another week on dev.feature/phase-2-mcp-tokensoff the tagged commit.Test plan
v4.3.0, flag=off → verify legacy path works (no-op upgrade)AUTH_SHADOW_*rows🤖 Generated with Claude Code