fix: prune auth event types nothing emits and fix their consumers - #127
Merged
Conversation
15 of the 74 declared types had no emit site anywhere in src. Five were actively queried by the security anomaly detector (bearer_token_failed, jwks_failed, otp_failed, recovery_otp_failed, user_data_failed), so those failure categories always came back empty, while verify_otp_failed, totp_failed, magic_link_failed, and logout_failed were emitted and never searched for. OTP verification failures were therefore invisible to anomaly detection. The admin event filter had the same drift: otp expanded to a type nothing emits and missed every verify_otp_* and mfa_otp_* event, and suspicious named two types that are never written. Removes the bearer_token_* and jwks_* families, recovery_otp_*, user_data_failed, user_data_success, otp_failed, mfa_otp_suspicious, service_token_suspicious, and webauthn_login_suspicious. The failure and suspicious groupings are now derived from AUTH_EVENT_TYPES rather than hand-copied, so a new event type files itself in the right bucket. A test asserts every declared type has an emit site, which is what would have caught the bootstrap_admin_* entries when that feature was removed. Reading historical rows is unaffected: stored events are returned as z.string() and the query filter accepts any string, so events written under a removed name still appear. Closes #125
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
1 participant
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.
Closes #125
The audit found more than stale entries
The issue expected a few dead names left behind by removed features. The actual result: 15 of 74 declared types have no emit site anywhere in
src/, and several of them are things other code actively searches for. So this was not just dead weight in an enum, it was a monitoring gap.getSecurityAnomaliessearched for five types nothing writes:bearer_token_failedbearer_token_*family is ever emittedjwks_failedjwks_*otp_failedverify_otp_failedrecovery_otp_failedadmin_device_replacement_recoveryuser_data_faileduser_data_suspiciousis emittedMeanwhile
verify_otp_failed,totp_failed,magic_link_failed, andlogout_failedare emitted and were never searched for. The practical effect: OTP verification failures did not show up in anomaly detection at all.expandTypein the admin controller had the same drift. Theotpfilter expanded to['otp_success', 'otp_failed'], so it missed everyverify_otp_*andmfa_otp_*event, andsuspiciouslistedwebauthn_login_suspiciousandservice_token_suspicious, neither of which is ever written.Change
Removed (per your call): the
bearer_token_*andjwks_*families,recovery_otp_*,user_data_failed,user_data_success, and the four remaining never-emitted stragglersotp_failed,mfa_otp_suspicious,service_token_suspicious,webauthn_login_suspicious. 74 to 59.The groupings are now derived from
AUTH_EVENT_TYPESrather than hand-listed:FAILURE_EVENT_TYPESis everything ending in_failedSUSPICIOUS_EVENT_TYPESis everything ending in_suspiciousauthEventTypesFor(...prefixes)groups a flow by prefixBoth consumers now use those, so the copies cannot drift apart again. Prefix matching is anchored on
_soauthEventTypesFor('otp')does not sweep in the unrelatedtotp_*family, which a naiveincludeswould have done, and there is a test for exactly that.Guard
A test asserts every declared type has a
type: '...'emit site insrc/, excluding the declaration itself andsrc/generated/(both list every name verbatim and would make it pass vacuously). This is what would have caught thebootstrap_admin_*pair when that feature was removed.Verified it is not vacuous by adding a
never_emitted_probeentry and confirming the test failed naming it, then reverting.Compatibility
Reading historical rows is unaffected.
InternalAuthEventSchema.typeisz.string(), andAuthEventQuerySchema.typeis a union withz.string(), so an event row written under a removed name is still returned and still filterable. The enum is a compile-time constraint on emit sites plus an OpenAPI annotation, not a runtime gate on stored data. Removing a member makes emitting it a type error, which is the point.Relevant to fells-code/seamless-auth-types#4: the consolidated list there still carries these 15. Worth pruning before that release is adopted in #124, otherwise adopting it reintroduces them.
Checks
npm run typecheck,npm run lint,npm run format:check, and the suite pass (985 passed, 1 skipped).openapi.jsonand the generated types were regenerated; the drift test from #119 caught the stale spec on the first run, which is a nice confirmation it works.