refactor(storage)!: decouple storage from authn via a Principal/Extractor interface

go.mod

@@ -3,7 +3,6 @@ module github.com/OpenNSW/core
 go 1.26
 
 require (
-	github.com/OpenNSW/core/authn v0.2.0
 	github.com/OpenNSW/core/remote v0.6.0
 	github.com/OpenNSW/core/secret v0.2.0
 	github.com/OpenNSW/core/shared v0.3.0
@@ -37,7 +36,6 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang-jwt/jwt/v5 v5.3.1 // indirect
 	github.com/golang/mock v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 // indirect

go.sum

@@ -1,5 +1,3 @@
-github.com/OpenNSW/core/authn v0.2.0 h1:hNiD7H/f/qln/E9zq8rbnC+Sru1CvC/Z1rDhR/qrUhE=
-github.com/OpenNSW/core/authn v0.2.0/go.mod h1:9/yGWkx5t5u0+YBUH4FsaZO6wpq/5WK5tdyOD+yL2Ek=
 github.com/OpenNSW/core/remote v0.6.0 h1:TvhZW3G+XowiCmbPNvZBaoND6xOfryz1iKiMMHNQdtk=
 github.com/OpenNSW/core/remote v0.6.0/go.mod h1:BzfC7ppzEAd5fUQmIsgeR2lZ6LzygNU4prNjvDpDH+4=
 github.com/OpenNSW/core/secret v0.2.0 h1:7kxJYbVNJhN9X9/YyTUci4bv9oYKcm0nT/wBVrxBChQ=
@@ -58,8 +56,6 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
 github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
-github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY=
-github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE=
 github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
 github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
 github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=

storage/README.md

@@ -50,6 +50,31 @@ meta, err := svc.GetDownloadURL(ctx, fileKey)
 err := svc.Delete(ctx, fileKey)
 ```
 
+## HTTP handler & authentication
+
+`HTTPHandler` wraps a `Service` with ready-made upload/download/delete endpoints. It gates
+`Upload` and `Delete` behind an authenticated caller, but stays decoupled from any specific
+authentication library — you supply an `Extractor` that resolves the caller from the
+request context:
+
+```go
+import "github.com/OpenNSW/core/authn"
+
+extract := func(ctx context.Context) (storage.Principal, bool) {
+    ac := authn.GetAuthContext(ctx)
+    if ac == nil {
+        return nil, false
+    }
+    return ac, true // *authn.AuthContext satisfies storage.Principal structurally
+}
+
+handler, err := storage.NewHTTPHandler(svc, extract)
+```
+
+`storage.Principal` only requires a `Subject() string` method, so any authentication
+context type that exposes one can be wired in directly, without `storage` importing that
+package.
+
 ## Implementing a custom driver
 
 ```go

storage/auth.go

@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (c) 2026 Lanka Software Foundation
+
+package storage
+
+import "context"
+
+// Principal is the minimal identity contract storage needs to gate authenticated
+// endpoints. It is deliberately tiny so an authentication layer's context type
+// (e.g. *authn.AuthContext) satisfies it structurally, without storage importing
+// that package — mirrors the authz.Principal/Extractor pattern.
+type Principal interface {
+	// Subject returns a stable identifier for the caller, used for audit logging.
+	Subject() string
+}
+
+// Extractor retrieves the authenticated Principal from a request context. It is
+// injected at construction so this package stays decoupled from any specific
+// authentication implementation. It must return (nil, false) when the request is
+// unauthenticated.
+type Extractor func(ctx context.Context) (Principal, bool)

storage/http_handler.go

@@ -14,7 +14,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/OpenNSW/core/authn"
 	"github.com/OpenNSW/core/storage/drivers"
 )
 
@@ -43,11 +42,15 @@ func isAllowedContentType(ct string) bool {
 }
 
 type HTTPHandler struct {
-	Service *Service
+	Service     *Service
+	authExtract Extractor
 }
 
-func NewHTTPHandler(service *Service) *HTTPHandler {
-	return &HTTPHandler{Service: service}
+func NewHTTPHandler(service *Service, authExtract Extractor) (*HTTPHandler, error) {
+	if authExtract == nil {
+		return nil, errors.New("storage: NewHTTPHandler requires a non-nil Extractor")
+	}
+	return &HTTPHandler{Service: service, authExtract: authExtract}, nil
 }
 
 // writeJSONError sets Content-Type: application/json and writes a consistent JSON error body.
@@ -58,7 +61,7 @@ func writeJSONError(w http.ResponseWriter, status int, message string) {
 }
 
 func (h *HTTPHandler) Upload(w http.ResponseWriter, r *http.Request) {
-	if authn.GetAuthContext(r.Context()) == nil {
+	if _, ok := h.authExtract(r.Context()); !ok {
 		slog.WarnContext(r.Context(), "authentication required but not provided for upload")
 		writeJSONError(w, http.StatusUnauthorized, "Unauthorized")
 		return
@@ -206,7 +209,7 @@ func (h *HTTPHandler) UploadContentLocal(w http.ResponseWriter, r *http.Request)
 
 func (h *HTTPHandler) Download(w http.ResponseWriter, r *http.Request) {
 	// TODO: Uncomment when M2M AUTH Implemented.
-	//if authn.GetAuthContext(r.Context()) == nil {
+	//if _, ok := h.authExtract(r.Context()); !ok {
 	//	slog.WarnContext(r.Context(), "authentication required but not provided for download")
 	//	writeJSONError(w, http.StatusUnauthorized, "Unauthorized")
 	//	return
@@ -315,7 +318,7 @@ func (h *HTTPHandler) DownloadContent(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *HTTPHandler) Delete(w http.ResponseWriter, r *http.Request) {
-	if authn.GetAuthContext(r.Context()) == nil {
+	if _, ok := h.authExtract(r.Context()); !ok {
 		slog.WarnContext(r.Context(), "authentication required but not provided for delete")
 		writeJSONError(w, http.StatusUnauthorized, "Unauthorized")
 		return

storage/http_handler_test.go

@@ -15,7 +15,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/OpenNSW/core/authn"
 	"github.com/OpenNSW/core/storage/drivers"
 )
 
@@ -25,7 +24,7 @@ func TestDownloadContent_LocalDriver_Success(t *testing.T) {
 	tempDir := t.TempDir()
 	driver, _ := drivers.NewLocalFSDriver(tempDir, "/api/v1/storage", "local-dev-secret", 15*time.Minute)
 	service := NewService(driver)
-	handler := NewHTTPHandler(service)
+	handler := mustHTTPHandler(t, service, authenticatedAs("trader-1"))
 
 	ctx := context.Background()
 	key := "550e8400-e29b-41d4-a716-446655440000.pdf"
@@ -65,20 +64,32 @@ func TestDownloadContent_LocalDriver_Success(t *testing.T) {
 	}
 }
 
-// withAuthContext returns a context with the given AuthContext injected.
-func withAuthContext(ctx context.Context, ac *authn.AuthContext) context.Context {
-	return context.WithValue(ctx, authn.AuthContextKey, ac)
+type fakePrincipal struct{ subject string }
+
+func (f fakePrincipal) Subject() string { return f.subject }
+
+func authenticatedAs(subject string) Extractor {
+	return func(context.Context) (Principal, bool) { return fakePrincipal{subject}, true }
+}
+
+func unauthenticated() Extractor {
+	return func(context.Context) (Principal, bool) { return nil, false }
+}
+
+func mustHTTPHandler(t *testing.T, service *Service, extract Extractor) *HTTPHandler {
+	t.Helper()
+	h, err := NewHTTPHandler(service, extract)
+	if err != nil {
+		t.Fatalf("NewHTTPHandler: %v", err)
+	}
+	return h
 }
 
 func TestDownload_MissingKey(t *testing.T) {
-	handler := NewHTTPHandler(NewService(&MockDriver{}))
+	handler := mustHTTPHandler(t, NewService(&MockDriver{}), authenticatedAs("trader-1"))
 
 	req := httptest.NewRequest(http.MethodGet, "/files/", nil)
 	// Auth present, but no path value for "key".
-	ctx := withAuthContext(req.Context(), &authn.AuthContext{
-		User: &authn.UserContext{ID: "trader-1"},
-	})
-	req = req.WithContext(ctx)
 	rec := httptest.NewRecorder()
 
 	handler.Download(rec, req)
@@ -90,17 +101,13 @@ func TestDownload_MissingKey(t *testing.T) {
 
 func TestDownload_Success(t *testing.T) {
 	mock := &MockDriver{}
-	handler := NewHTTPHandler(NewService(mock))
+	handler := mustHTTPHandler(t, NewService(mock), authenticatedAs("trader-1"))
 
-	// Build request with auth context and path value.
+	// Build request with path value.
 	mux := http.NewServeMux()
 	mux.HandleFunc("GET /files/{key}", handler.Download)
 
 	req := httptest.NewRequest(http.MethodGet, "/files/550e8400-e29b-41d4-a716-446655440000.pdf", nil)
-	ctx := withAuthContext(req.Context(), &authn.AuthContext{
-		User: &authn.UserContext{ID: "trader-1"},
-	})
-	req = req.WithContext(ctx)
 	rec := httptest.NewRecorder()
 
 	mux.ServeHTTP(rec, req)
@@ -131,16 +138,12 @@ func TestDownload_GenerateURLError(t *testing.T) {
 	mock := &MockDriver{
 		GenerateURLErr: errors.New("presign failure"),
 	}
-	handler := NewHTTPHandler(NewService(mock))
+	handler := mustHTTPHandler(t, NewService(mock), authenticatedAs("trader-1"))
 
 	mux := http.NewServeMux()
 	mux.HandleFunc("GET /files/{key}", handler.Download)
 
 	req := httptest.NewRequest(http.MethodGet, "/files/550e8400-e29b-41d4-a716-446655440000", nil)
-	ctx := withAuthContext(req.Context(), &authn.AuthContext{
-		User: &authn.UserContext{ID: "trader-1"},
-	})
-	req = req.WithContext(ctx)
 	rec := httptest.NewRecorder()
 
 	mux.ServeHTTP(rec, req)
@@ -156,17 +159,13 @@ func TestDownload_GenerateURLError(t *testing.T) {
 }
 
 func TestDownload_InvalidKeyFormat(t *testing.T) {
-	handler := NewHTTPHandler(NewService(&MockDriver{}))
+	handler := mustHTTPHandler(t, NewService(&MockDriver{}), authenticatedAs("trader-1"))
 
 	mux := http.NewServeMux()
 	mux.HandleFunc("GET /files/{key}", handler.Download)
 
 	// Key that is not UUID or UUID.ext (validStorageKey rejects it)
 	req := httptest.NewRequest(http.MethodGet, "/files/invalid-key-format", nil)
-	ctx := withAuthContext(req.Context(), &authn.AuthContext{
-		User: &authn.UserContext{ID: "trader-1"},
-	})
-	req = req.WithContext(ctx)
 	rec := httptest.NewRecorder()
 
 	mux.ServeHTTP(rec, req)
@@ -177,7 +176,7 @@ func TestDownload_InvalidKeyFormat(t *testing.T) {
 }
 
 func TestUpload_Unauthorized(t *testing.T) {
-	handler := NewHTTPHandler(NewService(&MockDriver{}))
+	handler := mustHTTPHandler(t, NewService(&MockDriver{}), unauthenticated())
 
 	body := map[string]any{
 		"filename":  "test.pdf",
@@ -211,7 +210,7 @@ func TestUpload_ContentTypes(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			handler := NewHTTPHandler(NewService(&MockDriver{}))
+			handler := mustHTTPHandler(t, NewService(&MockDriver{}), authenticatedAs("trader-1"))
 
 			body := map[string]any{
 				"filename":  tt.filename,
@@ -222,10 +221,6 @@ func TestUpload_ContentTypes(t *testing.T) {
 
 			req := httptest.NewRequest(http.MethodPost, "/uploads", bytes.NewReader(jsonBody))
 			req.Header.Set("Content-Type", "application/json")
-			ctx := withAuthContext(req.Context(), &authn.AuthContext{
-				User: &authn.UserContext{ID: "trader-1"},
-			})
-			req = req.WithContext(ctx)
 			rec := httptest.NewRecorder()
 
 			handler.Upload(rec, req)
@@ -239,7 +234,7 @@ func TestUpload_ContentTypes(t *testing.T) {
 
 func TestUpload_Success(t *testing.T) {
 	mock := &MockDriver{}
-	handler := NewHTTPHandler(NewService(mock))
+	handler := mustHTTPHandler(t, NewService(mock), authenticatedAs("trader-1"))
 
 	body := map[string]any{
 		"filename":  "test.pdf",
@@ -250,10 +245,6 @@ func TestUpload_Success(t *testing.T) {
 
 	req := httptest.NewRequest(http.MethodPost, "/uploads", bytes.NewReader(jsonBody))
 	req.Header.Set("Content-Type", "application/json")
-	ctx := withAuthContext(req.Context(), &authn.AuthContext{
-		User: &authn.UserContext{ID: "trader-1"},
-	})
-	req = req.WithContext(ctx)
 	rec := httptest.NewRecorder()
 
 	handler.Upload(rec, req)
@@ -279,7 +270,7 @@ func TestUploadContentLocal_Success(t *testing.T) {
 	tempDir := t.TempDir()
 	driver, _ := drivers.NewLocalFSDriver(tempDir, "/api/v1/storage", "local-dev-secret", 15*time.Minute)
 	service := NewService(driver)
-	handler := NewHTTPHandler(service)
+	handler := mustHTTPHandler(t, service, authenticatedAs("trader-1"))
 
 	key := "550e8400-e29b-41d4-a716-446655440000.pdf"
 	content := []byte("pdf content")
@@ -325,7 +316,7 @@ func TestUploadContentLocal_Success(t *testing.T) {
 }
 
 func TestDelete_Unauthorized(t *testing.T) {
-	handler := NewHTTPHandler(NewService(&MockDriver{}))
+	handler := mustHTTPHandler(t, NewService(&MockDriver{}), unauthenticated())
 
 	req := httptest.NewRequest(http.MethodDelete, "/storage/550e8400-e29b-41d4-a716-446655440000.pdf", nil)
 	req.SetPathValue("key", "550e8400-e29b-41d4-a716-446655440000.pdf")
@@ -340,7 +331,7 @@ func TestDelete_Unauthorized(t *testing.T) {
 
 func TestDownloadContent_NonLocalDriver_NotFound(t *testing.T) {
 	// For non-local drivers, DownloadContent should be disabled and return 404
-	handler := NewHTTPHandler(NewService(&MockDriver{}))
+	handler := mustHTTPHandler(t, NewService(&MockDriver{}), authenticatedAs("trader-1"))
 
 	req := httptest.NewRequest(http.MethodGet, "/storage/550e8400-e29b-41d4-a716-446655440000.pdf/content", nil)
 	req.SetPathValue("key", "550e8400-e29b-41d4-a716-446655440000.pdf")