refactor(exchange): readability + maintainability sweep#63
Merged
Conversation
Move APITokenVerifier, APITokenOptions, APITokenRequest, and APITokenResult into apitoken.go alongside APITokenExchanger. Move IdentityOptions, IdentityRequest, and IdentityResult into identity.go alongside IdentityExchanger. Extract the shared AccessTokenIssuer port into a new tokens.go. Delete types.go. Public surface, behaviour, and error wording unchanged.
Move exchangeError, unauthenticated, and isContextError from apitoken.go into a new errors.go so identity.go (which also calls exchangeError) sees them in a neutral location. Add godocs documenting the sentinel-preservation contract and the context-error fast path.
Expand APITokenVerifier and AccessTokenIssuer godocs to spell out the sentinel-error contract and the signing/expiry responsibility. Annotate the ErrPrincipalNotFound branch in APITokenExchanger.Exchange and the nil/empty- ID branches in IdentityExchanger.Exchange to name what each check defends against (existence-leak, downstream-authorization key). Annotate the two early-return branches in exchangeError so the sentinel-preservation strategy is visible at the call site. Matches the access/middleware/authenticator.go inline-comment style from PR #61.
Lift testPrincipalID, testTokenID, fixedTime, newAccessJWTIssuerAndVerifier, newAPITokenExchanger, newIdentityExchanger, testExchangeIdentity, and testExchangePrincipal into a new helpers_test.go. Move the two adapter-local fakes (fakeAPITokenVerifier, fakeAccessTokenIssuer) alongside them since both test files reference them. Mechanical lift; no test logic changes. Matches the helpers_test.go precedent from PR #61 (access/jwt) and PR #62 (authz/casbin).
Add authkit.PrincipalResolver to .mockery.yaml and regenerate; the generated mock lands at mocks/authkit/principal_resolver.go in package authkitmocks. Swap fakePrincipalFinder for authkitmocks.PrincipalFinder in apitoken_test.go and fakeIdentityResolver for authkitmocks.PrincipalResolver in identity_test.go. Both hand-rolled types are deleted. fakeAPITokenVerifier and fakeAccessTokenIssuer stay because they back package-local adapter ports (the established mockery pattern). Drop TestAPITokenExchangerDoesNotRequireIdentityLink: its assertions are a strict subset of TestAPITokenExchangerExchangesTokenForAccessJWT (both create a principal with no identity link and exchange) and the test name promised a stronger property than the body actually verifies. Keep TestAPITokenExchangerExchangesTokenForAccessJWT integration-style with real memory.Store + real apikey.Service so one anchor still proves the full chain end-to-end.
A trailing blank line slipped into apitoken.go during the file-reorg commit and gofmt flags it. Pure formatting fix; no logic change.
This was referenced May 27, 2026
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
Third pass of the multi-session refactor sweep (after
access/PR #61 andauthz/PR #62). Liftsexchange/to the unified 10-criteria bar: godocs everywhere, self-explanatory names, consistent domain vocabulary, behaviour categorised behind contracts, helpful inline comments on security/defensive checks, small bespoke public API, hexagonal ports/adapters, mockery-based testing, pure domain logic separation, logical file organisation.Non-goals: bugs, correctness, performance. Public API surface and runtime behaviour unchanged.
Changes per commit
refactor(exchange): colocate request/result/options with their exchanger— splittypes.gosoapitoken.goandidentity.goeach hold the shapes that configure them; extract the sharedAccessTokenIssuerport into a newtokens.go; deletetypes.go.refactor(exchange): extract error helpers into errors.go— moveexchangeError,unauthenticated,isContextErrorout ofapitoken.gointo a newerrors.gosoidentity.go(which already callsexchangeError) sees them in a neutral location; godoc each with its sentinel-preservation contract.refactor(exchange): tighten port godocs and annotate defensive checks— expandAPITokenVerifierandAccessTokenIssuergodocs; annotate theErrPrincipalNotFoundexistence-leak branch inAPITokenExchanger.Exchange, the nil/empty-ID branches inIdentityExchanger.Exchange, and the sentinel-passthrough branches inexchangeError. Matches the inline-comment style fromaccess/middleware/authenticator.go(PR refactor(access): readability + maintainability sweep + introduce mockery #61).test(exchange): split shared helpers into helpers_test.go— mechanical lift of constants, helpers, and the two adapter-local fakes. Matches the precedent fromaccess/jwt/helpers_test.goandauthz/casbin/helpers_test.go.test(exchange): migrate root-port fakes to mockery— addauthkit.PrincipalResolverto.mockery.yaml, regenerate, swapfakePrincipalFinder/fakeIdentityResolverforauthkitmocks.PrincipalFinder/authkitmocks.PrincipalResolver. Adapter-local ports (APITokenVerifier,AccessTokenIssuer) stay as hand-rolled fakes per the established pattern. DropTestAPITokenExchangerDoesNotRequireIdentityLink(assertions identical to the happy-path test); keep the one happy-path integration test wired against realmemory.Store+apikey.Service+jwt.Issuer/Verifieras the end-to-end anchor.chore(exchange): drop trailing blank line introduced by file rewrite— pure format fix that gofmt flagged after commit 1.Deliberately deferred
APITokenRequest.Plaintextrename — waits for theproof/apikey/pass so the input field name realigns with whateverapikey.IssuedToken.Plaintextbecomes (TokenorSecret), avoiding mid-sweep asymmetry.Test plan
moon run root:check --summary minimal— format, lint, build, unit, Testcontainers integration. All green.go test ./exchange/... -count=1 -v— 12 tests pass; dropped one redundant test.go doc github.com/meigma/authkit/exchangeunchanged).testkit/internal/{authflow,httpui}unchanged).🤖 Generated with Claude Code