feat: Optional path-prefix and method scoping for Filer HTTP JWT (#8014)
* Implement optional path-prefix and method scoping for Filer HTTP JWT * Fix security vulnerability and improve test error handling * Address PR feedback: replace debug logging and improve tests * Use URL.Path in logs to avoid leaking query params
This commit is contained in:
@@ -24,6 +24,8 @@ type SeaweedFileIdClaims struct {
|
|||||||
// Right now, it only contains the standard claims; but this might be extended later
|
// Right now, it only contains the standard claims; but this might be extended later
|
||||||
// for more fine-grained permissions.
|
// for more fine-grained permissions.
|
||||||
type SeaweedFilerClaims struct {
|
type SeaweedFilerClaims struct {
|
||||||
|
AllowedPrefixes []string `json:"allowed_prefixes,omitempty"`
|
||||||
|
AllowedMethods []string `json:"allowed_methods,omitempty"`
|
||||||
jwt.RegisteredClaims
|
jwt.RegisteredClaims
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -56,7 +58,7 @@ func GenJwtForFilerServer(signingKey SigningKey, expiresAfterSec int) EncodedJwt
|
|||||||
}
|
}
|
||||||
|
|
||||||
claims := SeaweedFilerClaims{
|
claims := SeaweedFilerClaims{
|
||||||
jwt.RegisteredClaims{},
|
RegisteredClaims: jwt.RegisteredClaims{},
|
||||||
}
|
}
|
||||||
if expiresAfterSec > 0 {
|
if expiresAfterSec > 0 {
|
||||||
claims.ExpiresAt = jwt.NewNumericDate(time.Now().Add(time.Second * time.Duration(expiresAfterSec)))
|
claims.ExpiresAt = jwt.NewNumericDate(time.Now().Add(time.Second * time.Duration(expiresAfterSec)))
|
||||||
|
|||||||
143
weed/server/filer_jwt_test.go
Normal file
143
weed/server/filer_jwt_test.go
Normal file
@@ -0,0 +1,143 @@
|
|||||||
|
package weed_server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/golang-jwt/jwt/v5"
|
||||||
|
"github.com/seaweedfs/seaweedfs/weed/security"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFilerServer_maybeCheckJwtAuthorization_Scoped(t *testing.T) {
|
||||||
|
signingKey := "secret"
|
||||||
|
filerGuard := security.NewGuard(nil, signingKey, 0, signingKey, 0)
|
||||||
|
fs := &FilerServer{
|
||||||
|
filerGuard: filerGuard,
|
||||||
|
}
|
||||||
|
|
||||||
|
// Helper to generate token
|
||||||
|
genToken := func(allowedPrefixes []string, allowedMethods []string) string {
|
||||||
|
claims := security.SeaweedFilerClaims{
|
||||||
|
AllowedPrefixes: allowedPrefixes,
|
||||||
|
AllowedMethods: allowedMethods,
|
||||||
|
RegisteredClaims: jwt.RegisteredClaims{
|
||||||
|
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
|
||||||
|
str, err := token.SignedString([]byte(signingKey))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to sign token: %v", err)
|
||||||
|
}
|
||||||
|
return str
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
token string
|
||||||
|
method string
|
||||||
|
path string
|
||||||
|
isWrite bool
|
||||||
|
expectAuthorized bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "no restrictions",
|
||||||
|
token: genToken(nil, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "allowed prefix match",
|
||||||
|
token: genToken([]string{"/data"}, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "allowed prefix mismatch",
|
||||||
|
token: genToken([]string{"/private"}, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "allowed method match",
|
||||||
|
token: genToken(nil, []string{"GET"}),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "allowed method mismatch",
|
||||||
|
token: genToken(nil, []string{"POST"}),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "both match",
|
||||||
|
token: genToken([]string{"/data"}, []string{"GET"}),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "prefix match, method mismatch",
|
||||||
|
token: genToken([]string{"/data"}, []string{"POST"}),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "multiple prefixes match",
|
||||||
|
token: genToken([]string{"/other", "/data"}, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data/test",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "write operation with method restriction",
|
||||||
|
token: genToken(nil, []string{"POST", "PUT"}),
|
||||||
|
method: "POST",
|
||||||
|
path: "/data/upload",
|
||||||
|
isWrite: true,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "root path with prefix restriction",
|
||||||
|
token: genToken([]string{"/data"}, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "exact prefix match",
|
||||||
|
token: genToken([]string{"/data"}, nil),
|
||||||
|
method: "GET",
|
||||||
|
path: "/data",
|
||||||
|
isWrite: false,
|
||||||
|
expectAuthorized: true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
req := httptest.NewRequest(tt.method, tt.path, nil)
|
||||||
|
req.Header.Set("Authorization", "Bearer "+tt.token)
|
||||||
|
if authorized := fs.maybeCheckJwtAuthorization(req, tt.isWrite); authorized != tt.expectAuthorized {
|
||||||
|
t.Errorf("expected authorized=%v, got %v", tt.expectAuthorized, authorized)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -4,7 +4,6 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
@@ -148,7 +147,7 @@ func (fs *FilerServer) readonlyFilerHandler(w http.ResponseWriter, r *http.Reque
|
|||||||
statusRecorder := stats.NewStatusResponseWriter(w)
|
statusRecorder := stats.NewStatusResponseWriter(w)
|
||||||
w = statusRecorder
|
w = statusRecorder
|
||||||
|
|
||||||
os.Stdout.WriteString("Request: " + r.Method + " " + r.URL.String() + "\n")
|
glog.V(4).Infof("Request: %s %s", r.Method, r.URL.Path)
|
||||||
|
|
||||||
origin := r.Header.Get("Origin")
|
origin := r.Header.Get("Origin")
|
||||||
if origin != "" {
|
if origin != "" {
|
||||||
@@ -242,9 +241,42 @@ func (fs *FilerServer) maybeCheckJwtAuthorization(r *http.Request, isWrite bool)
|
|||||||
if !token.Valid {
|
if !token.Valid {
|
||||||
glog.V(1).Infof("jwt invalid from %s: %v", r.RemoteAddr, tokenStr)
|
glog.V(1).Infof("jwt invalid from %s: %v", r.RemoteAddr, tokenStr)
|
||||||
return false
|
return false
|
||||||
} else {
|
|
||||||
return true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
claims, ok := token.Claims.(*security.SeaweedFilerClaims)
|
||||||
|
if !ok {
|
||||||
|
glog.V(1).Infof("jwt claims not of type *SeaweedFilerClaims from %s", r.RemoteAddr)
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(claims.AllowedPrefixes) > 0 {
|
||||||
|
hasPrefix := false
|
||||||
|
for _, prefix := range claims.AllowedPrefixes {
|
||||||
|
if strings.HasPrefix(r.URL.Path, prefix) {
|
||||||
|
hasPrefix = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !hasPrefix {
|
||||||
|
glog.V(1).Infof("jwt path not allowed from %s: %v", r.RemoteAddr, r.URL.Path)
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(claims.AllowedMethods) > 0 {
|
||||||
|
hasMethod := false
|
||||||
|
for _, method := range claims.AllowedMethods {
|
||||||
|
if method == r.Method {
|
||||||
|
hasMethod = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !hasMethod {
|
||||||
|
glog.V(1).Infof("jwt method not allowed from %s: %v", r.RemoteAddr, r.Method)
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
func (fs *FilerServer) filerHealthzHandler(w http.ResponseWriter, r *http.Request) {
|
func (fs *FilerServer) filerHealthzHandler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|||||||
Reference in New Issue
Block a user