bfabric-asgi-auth: customizable renderer, error classification#493
Open
leoschwarz wants to merge 8 commits intomainfrom
Open
bfabric-asgi-auth: customizable renderer, error classification#493leoschwarz wants to merge 8 commits intomainfrom
leoschwarz wants to merge 8 commits intomainfrom
Conversation
…list Driven by the zip-browser port. Six bundled improvements: - Configurable redirect status (default 303 See Other) on both renderers. - Path allowlist on the middleware (str + re.Pattern) for healthchecks. - Token validation error classification: BfabricTokenValidationFailedError gains is_expired; the strategy maps to expired/invalid/network/unknown; ErrorResponse.invalid_token emits a discriminated error_type. - HTMLRenderer customization: callable error_message / error_title hooks let apps inject per-error-type copy. - Default-on B-Fabric branding (wordmark, accent, return-to-B-Fabric link) with bfabric_url, logo_html, footer_html, back_link overrides. - AuthHooks.on_reject now returns ErrorResponse | RedirectResponse | None instead of bool. Removes the broken silent-handle path. - README updates the VisibleException pattern for app-defined error_types raised inside on_success.
…ricTokenInvalidError subclasses Idiomatic exception hierarchy in place of the boolean discriminator. Callers that care about a specific kind catch the subclass; callers that handle any validation failure still catch the parent. Strategy dispatch becomes ordered except clauses with no per-attribute branching.
…ubclasses Two reportCallIssue / reportArgumentType entries no longer trigger now that BfabricTokenExpiredError / BfabricTokenInvalidError have typed constructors, so the baseline shrinks accordingly.
…criminator Type-safe enum members (TokenErrorKind.EXPIRED, etc.) at the producer side (strategies + middleware); StrEnum equality keeps string-keyed lookups working inside ErrorResponse.invalid_token. Re-exported from the package root so apps can import it directly.
A better solution exists for the public-paths use case; remove the allowlist parameter, helper, and accompanying tests/docs to keep the middleware surface focused.
…enErrorKind Replace the plain-str signature with the StrEnum so misspellings fail at the type level instead of silently falling back to "unknown". Switch the classification dict to enum keys and look up with [] so a future enum addition that forgets to register copy fails loudly.
leoschwarz
changed the title
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.
Summary
Widens extension points in
bfabric_asgi_authso downstream apps (zip-browser was the driver) can supply their own copy, branding, and error semantics without subclassing the renderer. Six bundled improvements:redirect_statuskwarg onPlainTextRendererandHTMLRenderer, default303 See Other.Path allowlist —unauthenticated_paths: Sequence[str | re.Pattern]onBfabricAuthMiddlewareso healthchecks/static assets bypass auth.BfabricTokenValidationFailedError.is_expiredflag (inbfabriccore) + the strategy classifies intoexpired/invalid/network/unknown, surfaced as discriminatederror_types (token_expired,token_invalid,token_network,token_unknown).error_message(error_type, default)anderror_title(error_type, status, default)onHTMLRendererfor per-error-type copy.bfabric_branding=Trueships a wordmark + accent colour + return-to-B-Fabric footer;bfabric_url,logo_html,footer_html,back_linkoverride;bfabric_branding=Falseopts out.on_rejectcontract — now returnsErrorResponse | RedirectResponse | Noneinstead ofbool. The oldTrue"exit silently" path was broken (response never sent); removing it cleans up the contract.README updated with the
VisibleExceptionpattern for app-defined error_types raised insideon_success(B5 from the port notes — no middleware change needed, just docs).Review notes
bfabric/src/bfabric/errors.pyadding theis_expiredattribute.bfabric_asgi_authtests pass; 388bfabriccore tests pass; basedpyright clean on touched files./Users/leo/code/web-apps/zip-browser/docs/bfabric-asgi-auth-port-notes.md.Breaking changes
AuthHooks.on_rejectreturn type changed (bool→ErrorResponse | RedirectResponse | None).AuthHooksis aProtocolwith a default implementation, so only apps that actively overrideon_rejectneed to update — currently zip-browser plus internal usage.302to303. Setredirect_status=302on the renderer to keep legacy behaviour.