diff --git a/go.mod b/go.mod index 7fa6d3ca..2071fc89 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 7b544a11..19cb4bc5 100644 --- a/go.sum +++ b/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= diff --git a/storage/README.md b/storage/README.md index 8e03b68e..9bc5b1ef 100644 --- a/storage/README.md +++ b/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 diff --git a/storage/auth.go b/storage/auth.go new file mode 100644 index 00000000..e1384975 --- /dev/null +++ b/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) diff --git a/storage/http_handler.go b/storage/http_handler.go index 990356bc..d66a1da2 100644 --- a/storage/http_handler.go +++ b/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 diff --git a/storage/http_handler_test.go b/storage/http_handler_test.go index 1828e52b..e4059741 100644 --- a/storage/http_handler_test.go +++ b/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")