refactor(storage)!: decouple storage from authn via a Principal/Extractor interface - #158
Draft
sthanikan2000 wants to merge 1 commit into
Draft
refactor(storage)!: decouple storage from authn via a Principal/Extractor interface#158sthanikan2000 wants to merge 1 commit into
sthanikan2000 wants to merge 1 commit into
Conversation
…ctor interface
storage only ever used authn.GetAuthContext as a bare "is there an
authenticated caller" nil-check, never touching roles/scopes/claims. Mirror
the authz.Principal/Extractor pattern already established in this repo:
storage now defines its own minimal Principal{ Subject() string } interface
and Extractor func type, which *authn.AuthContext satisfies structurally
without storage importing that package.
NewHTTPHandler now takes an Extractor and returns (*HTTPHandler, error),
failing closed on a nil extractor (same convention as authz.New). Callers
bridge their auth library in at the composition root — see storage/README.md
for an authn example.
With storage decoupled, authn was the root module's only path to
golang-jwt/jwt/v5; `go mod tidy` drops both from go.mod/go.sum.
This also clears the way for a follow-up extracting storage into its own
Go module, since it no longer needs authn as a dependency at all.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 tasks
sthanikan2000
marked this pull request as draft
August 22, 2026 12:31
Collaborator
Author
|
Resequencing this behind #159: we're extracting `storage` into its own Go module first (non-breaking, releases as `storage/v0.1.0`) so downstream repos can pick up the module split without any code changes. This PR's `authn` → `Principal`/`Extractor` change is a real breaking API change, so it'll follow as a separate release (e.g. `v0.2.0`) once #159 has merged and been tagged — giving downstream repos time to migrate the two changes independently. Marking as draft until #159 lands; will rebase this branch onto the resulting `main` (which will include `storage/go.mod`) before marking ready again. |
sthanikan2000
added a commit
that referenced
this pull request
Aug 23, 2026
storage is the last remaining candidate matching the pattern already used for authn, authz, shared, remote, secret, database, payment, pagination, trace, and uiprojector: give it its own go.mod so it can be versioned and released independently of the root module. No source changes — storage keeps its current authn dependency for now, so this is purely a module-boundary change. Existing importers of github.com/OpenNSW/core/storage need to add an explicit `require github.com/OpenNSW/core/storage vX.Y.Z` to their own go.mod, but no code changes are needed on their end. Adds a storage-module CI job (mirroring authn-module) and lists /storage in dependabot.yml. Root's go.mod/go.sum drop authn and its transitive golang-jwt/jwt/v5 dependency, since storage was the only in-repo consumer and it now carries that requirement itself. Deliberately sequenced before the authn-decoupling change (#158): this lets storage ship as a non-breaking v0.1.0 first, so downstream repos can pick up the independent module before separately opting into the breaking Principal/Extractor interface change later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sthanikan2000
added a commit
that referenced
this pull request
Aug 23, 2026
storage is the last remaining candidate matching the pattern already used for authn, authz, shared, remote, secret, database, payment, pagination, trace, and uiprojector: give it its own go.mod so it can be versioned and released independently of the root module. No source changes — storage keeps its current authn dependency for now, so this is purely a module-boundary change. Existing importers of github.com/OpenNSW/core/storage need to add an explicit `require github.com/OpenNSW/core/storage vX.Y.Z` to their own go.mod, but no code changes are needed on their end. Adds a storage-module CI job (mirroring authn-module) and lists /storage in dependabot.yml. Root's go.mod/go.sum drop authn and its transitive golang-jwt/jwt/v5 dependency, since storage was the only in-repo consumer and it now carries that requirement itself. Deliberately sequenced before the authn-decoupling change (#158): this lets storage ship as a non-breaking v0.1.0 first, so downstream repos can pick up the independent module before separately opting into the breaking Principal/Extractor interface change later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sthanikan2000
added a commit
that referenced
this pull request
Aug 23, 2026
storage is the last remaining candidate matching the pattern already used for authn, authz, shared, remote, secret, database, payment, pagination, trace, and uiprojector: give it its own go.mod so it can be versioned and released independently of the root module. No source changes — storage keeps its current authn dependency for now, so this is purely a module-boundary change. Existing importers of github.com/OpenNSW/core/storage need to add an explicit `require github.com/OpenNSW/core/storage vX.Y.Z` to their own go.mod, but no code changes are needed on their end. Adds a storage-module CI job (mirroring authn-module) and lists /storage in dependabot.yml. Root's go.mod/go.sum drop authn and its transitive golang-jwt/jwt/v5 dependency, since storage was the only in-repo consumer and it now carries that requirement itself. Deliberately sequenced before the authn-decoupling change (#158): this lets storage ship as a non-breaking v0.1.0 first, so downstream repos can pick up the independent module before separately opting into the breaking Principal/Extractor interface change later.
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
storageonly ever usedauthn.GetAuthContextas a bare "is there an authenticated caller" nil-check — no role/scope/claim access. It now defines its own minimalPrincipal{ Subject() string }interface andExtractorfunc type, mirroring theauthz.Principal/Extractorpattern already established in this repo.*authn.AuthContextsatisfiesPrincipalstructurally, so callers bridge it in at their composition root withoutstorageimportingauthnat all.NewHTTPHandlernow takes anExtractorand returns(*HTTPHandler, error), failing closed on a nil extractor (same convention asauthz.New).storage/README.mddocuments the new wiring with anauthnexample.go.mod/go.sumno longer needauthn(or its transitivegolang-jwt/jwt/v5dep) at all, sincestoragewas the only consumer in this repo.This is a breaking change for
NewHTTPHandlercallers (new required parameter, new error return) — seestorage/README.mdfor the migration example.Follow-up (separate PR, not in this change): extract
storageinto its own Go module now that it no longer needsauthnas a dependency, following the same playbook used forauthz/pagination/trace/etc.Test plan
go build ./storage/.../go vet ./storage/...go test ./storage/... -v— all passgo build ./.../go test ./...at repo root — all passgolangci-lint run ./storage/...— 0 issuesgrep -rn authn storage/— only a doc comment and the README example remain, no Go importgo mod tidycheck, race tests) all passed locally