diff --git a/Makefile b/Makefile index bd65eb185..78afd6de8 100644 --- a/Makefile +++ b/Makefile @@ -72,15 +72,15 @@ watch: watch-js build-all: build-go build-js pull: docker-pull test: test-js test-go -test-go: reset-sqlite run-test-go -test-pkg: reset-sqlite run-test-pkg -test-ai: reset-sqlite run-test-ai -test-api: reset-sqlite run-test-api -test-video: reset-sqlite run-test-video -test-entity: reset-sqlite run-test-entity -test-commands: reset-sqlite run-test-commands -test-photoprism: reset-sqlite run-test-photoprism -test-short: reset-sqlite run-test-short +test-go: run-test-go +test-pkg: run-test-pkg +test-ai: run-test-ai +test-api: run-test-api +test-video: run-test-video +test-entity: run-test-entity +test-commands: run-test-commands +test-photoprism: run-test-photoprism +test-short: run-test-short test-mariadb: reset-acceptance run-test-mariadb acceptance-run-chromium: storage/acceptance acceptance-auth-sqlite-restart wait acceptance-auth acceptance-auth-sqlite-stop acceptance-sqlite-restart wait-2 acceptance acceptance-sqlite-stop acceptance-run-chromium-short: storage/acceptance acceptance-auth-sqlite-restart wait acceptance-auth-short acceptance-auth-sqlite-stop acceptance-sqlite-restart wait-2 acceptance-short acceptance-sqlite-stop diff --git a/internal/api/api_auth_jwt.go b/internal/api/api_auth_jwt.go index e96db76cc..a1ffb2024 100644 --- a/internal/api/api_auth_jwt.go +++ b/internal/api/api_auth_jwt.go @@ -1,7 +1,9 @@ package api import ( + "context" "fmt" + "net" "net/http" "strings" @@ -16,78 +18,132 @@ import ( "github.com/photoprism/photoprism/pkg/clean" ) -// authAnyJWT attempts to authenticate a Portal-issued JWT when a cluster -// node receives a request without an existing session. It verifies the token -// against the node's cached JWKS, ensures the issuer/audience/scope match the -// expected portal values, and, if valid, returns a client session mirroring the -// JWT claims. It returns nil on any validation failure so the caller can fall -// back to existing auth flows. Currently cluster and vision resources are -// eligible for JWT-based authorization; vision access requires the `vision` -// scope whereas cluster access requires the `cluster` scope. +// authAnyJWT attempts to authenticate a Portal-issued JWT when a cluster node +// receives a request without an existing session. It verifies the token against +// the node's cached JWKS, ensures the issuer/audience/scope match the expected +// portal values, and, if valid, returns a client session mirroring the JWT +// claims. It returns nil on any validation failure so the caller can fall back +// to existing auth flows. By default, only cluster and vision resources are +// eligible, but nodes may opt in to additional scopes via PHOTOPRISM_JWT_SCOPE. func authAnyJWT(c *gin.Context, clientIP, authToken string, resource acl.Resource, perms acl.Permissions) *entity.Session { - if c == nil || authToken == "" { - return nil - } - - _ = perms - - if resource != acl.ResourceCluster && resource != acl.ResourceVision { - return nil - } - - // Basic sanity check for JWT structure. - if strings.Count(authToken, ".") != 2 { + // Check if token may be a JWT. + if !shouldAttemptJWT(c, authToken) { return nil } conf := get.Config() - if conf == nil || conf.IsPortal() { + // Determine whether JWT authentication is possible + // based on the local config and client IP address. + if !shouldAllowJWT(conf, clientIP) { return nil } + requiredScope := resource.String() + expected := expectedClaimsFor(conf, requiredScope) + + // verifyTokenFromPortal handles cryptographic validation (signature, issuer, + // audience, temporal claims) and enforces that the token includes any scopes + // listed in expected.Scope. Local authorization still happens below so nodes + // can apply their own allow-list semantics. + claims := verifyTokenFromPortal(c.Request.Context(), authToken, expected, jwtIssuerCandidates(conf)) + + if claims == nil { + return nil + } + + // Check if config allows resource access to be authorized with JWT. + allowedScopes := conf.JWTAllowedScopes() + if !acl.ScopeAttrPermits(allowedScopes, resource, perms) { + return nil + } + + // Check if token allows access to specified resource. + tokenScopes := acl.ScopeAttr(claims.Scope) + if !acl.ScopeAttrPermits(tokenScopes, resource, perms) { + return nil + } + + claims.Scope = tokenScopes.String() + + return sessionFromJWTClaims(claims, clientIP) +} + +// shouldAttemptJWT reports whether JWT verification should run for the supplied +// request context and token. +func shouldAttemptJWT(c *gin.Context, token string) bool { + if c == nil { + return false + } + + if token == "" || strings.Count(token, ".") != 2 { + return false + } + + return true +} + +// shouldAllowJWT reports whether the current node configuration permits JWT +// authentication for the request originating from clientIP. +func shouldAllowJWT(conf *config.Config, clientIP string) bool { + if conf == nil || conf.IsPortal() { + return false + } + if conf.JWKSUrl() == "" { - return nil + return false } - requiredScopes := []string{"cluster"} - if resource == acl.ResourceVision { - requiredScopes = []string{"vision"} + cidr := strings.TrimSpace(conf.ClusterCIDR()) + if cidr == "" { + return true } + ip := net.ParseIP(clientIP) + _, block, err := net.ParseCIDR(cidr) + if err != nil || ip == nil { + return false + } + + return block.Contains(ip) +} + +// expectedClaimsFor builds the ExpectedClaims used to validate JWTs for the +// current node and required scope. +func expectedClaimsFor(conf *config.Config, requiredScope string) clusterjwt.ExpectedClaims { expected := clusterjwt.ExpectedClaims{ Audience: fmt.Sprintf("node:%s", conf.NodeUUID()), - Scope: requiredScopes, JWKSURL: conf.JWKSUrl(), } - issuers := jwtIssuerCandidates(conf) + if requiredScope != "" { + expected.Scope = []string{requiredScope} + } + return expected +} + +// verifyTokenFromPortal checks the token against each candidate issuer and +// returns the verified claims on success. +func verifyTokenFromPortal(ctx context.Context, token string, expected clusterjwt.ExpectedClaims, issuers []string) *clusterjwt.Claims { if len(issuers) == 0 { return nil } - var ( - claims *clusterjwt.Claims - err error - ) - - ctx := c.Request.Context() - for _, issuer := range issuers { expected.Issuer = issuer - claims, err = get.VerifyJWT(ctx, authToken, expected) + claims, err := get.VerifyJWT(ctx, token, expected) if err == nil { - break + return claims } } - if err != nil { - return nil - } else if claims == nil { - return nil - } + return nil +} +// sessionFromJWTClaims constructs a Session populated with fields derived from +// the verified JWT claims. +func sessionFromJWTClaims(claims *clusterjwt.Claims, clientIP string) *entity.Session { sess := &entity.Session{ Status: http.StatusOK, ClientUID: claims.Subject, diff --git a/internal/api/api_auth_jwt_test.go b/internal/api/api_auth_jwt_test.go index 2db684397..2281a6a78 100644 --- a/internal/api/api_auth_jwt_test.go +++ b/internal/api/api_auth_jwt_test.go @@ -1,17 +1,22 @@ package api import ( + "context" + "fmt" "net/http" "net/http/httptest" "testing" "github.com/gin-gonic/gin" + gojwt "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/photoprism/photoprism/internal/auth/acl" + clusterjwt "github.com/photoprism/photoprism/internal/auth/jwt" "github.com/photoprism/photoprism/internal/config" "github.com/photoprism/photoprism/internal/photoprism/get" + "github.com/photoprism/photoprism/pkg/clean" ) func TestAuthAnyJWT(t *testing.T) { @@ -35,7 +40,149 @@ func TestAuthAnyJWT(t *testing.T) { assert.Contains(t, session.AuthScope, "cluster") assert.Equal(t, spec.Issuer, session.AuthIssuer) }) + t.Run("ClusterCIDRAllowed", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-cidr-allow") + spec := fx.defaultClaimsSpec() + token := fx.issue(t, spec) + origCIDR := fx.nodeConf.Options().ClusterCIDR + fx.nodeConf.Options().ClusterCIDR = "192.0.2.0/24" + get.SetConfig(fx.nodeConf) + t.Cleanup(func() { + fx.nodeConf.Options().ClusterCIDR = origCIDR + get.SetConfig(fx.nodeConf) + }) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/cluster/theme", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "192.0.2.10:2222" + c.Request = req + + session := authAnyJWT(c, "192.0.2.10", token, acl.ResourceCluster, nil) + require.NotNil(t, session) + assert.Equal(t, spec.Subject, session.ClientUID) + }) + t.Run("ClusterCIDRBlocked", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-cidr-block") + spec := fx.defaultClaimsSpec() + token := fx.issue(t, spec) + + origCIDR := fx.nodeConf.Options().ClusterCIDR + fx.nodeConf.Options().ClusterCIDR = "192.0.2.0/24" + get.SetConfig(fx.nodeConf) + t.Cleanup(func() { + fx.nodeConf.Options().ClusterCIDR = origCIDR + get.SetConfig(fx.nodeConf) + }) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/cluster/theme", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "203.0.113.10:2222" + c.Request = req + + assert.Nil(t, authAnyJWT(c, "203.0.113.10", token, acl.ResourceCluster, nil)) + }) + t.Run("JWTScopeDefaultRejectsOtherResources", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-scope-default-reject") + spec := fx.defaultClaimsSpec() + spec.Scope = []string{"photos"} + token := fx.issue(t, spec) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/photos", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "192.0.2.60:1001" + c.Request = req + + assert.Nil(t, authAnyJWT(c, "192.0.2.60", token, acl.ResourcePhotos, nil)) + }) + t.Run("JWTScopeAllowed", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-scope-allow") + token := fx.issue(t, fx.defaultClaimsSpec()) + + orig := fx.nodeConf.Options().JWTScope + fx.nodeConf.Options().JWTScope = "cluster vision" + get.SetConfig(fx.nodeConf) + t.Cleanup(func() { + fx.nodeConf.Options().JWTScope = orig + get.SetConfig(fx.nodeConf) + }) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/cluster/theme", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "192.0.2.30:1001" + c.Request = req + + sess := authAnyJWT(c, "192.0.2.30", token, acl.ResourceCluster, nil) + require.NotNil(t, sess) + }) + t.Run("JWTScopeAllowsSuperset", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-scope-reject") + token := fx.issue(t, fx.defaultClaimsSpec()) + + orig := fx.nodeConf.Options().JWTScope + fx.nodeConf.Options().JWTScope = "cluster" + get.SetConfig(fx.nodeConf) + t.Cleanup(func() { + fx.nodeConf.Options().JWTScope = orig + get.SetConfig(fx.nodeConf) + }) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/cluster/theme", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "192.0.2.40:1001" + c.Request = req + + sess := authAnyJWT(c, "192.0.2.40", token, acl.ResourceCluster, nil) + require.NotNil(t, sess) + }) + t.Run("JWTScopeCustomResource", func(t *testing.T) { + fx := newPortalJWTFixture(t, "cluster-jwt-scope-custom") + spec := fx.defaultClaimsSpec() + spec.Scope = []string{"photos"} + token := fx.issue(t, spec) + + origScope := fx.nodeConf.Options().JWTScope + fx.nodeConf.Options().JWTScope = "photos" + get.SetConfig(fx.nodeConf) + t.Cleanup(func() { + fx.nodeConf.Options().JWTScope = origScope + get.SetConfig(fx.nodeConf) + }) + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/api/v1/photos", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "192.0.2.50:2001" + c.Request = req + + _, verifyErr := get.VerifyJWT(c.Request.Context(), token, clusterjwt.ExpectedClaims{ + Issuer: fmt.Sprintf("portal:%s", fx.clusterUUID), + Audience: fmt.Sprintf("node:%s", fx.nodeUUID), + Scope: []string{"photos"}, + JWKSURL: fx.nodeConf.JWKSUrl(), + }) + require.NoError(t, verifyErr) + + sess := authAnyJWT(c, "192.0.2.50", token, acl.ResourcePhotos, nil) + require.NotNil(t, sess) + }) t.Run("VisionScope", func(t *testing.T) { fx := newPortalJWTFixture(t, "cluster-jwt-vision") spec := fx.defaultClaimsSpec() @@ -146,3 +293,83 @@ func TestJwtIssuerCandidates(t *testing.T) { assert.Equal(t, []string{"http://localhost:2342"}, jwtIssuerCandidates(conf)) }) } + +func TestShouldAttemptJWT(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req, _ := http.NewRequest(http.MethodGet, "/ping", nil) + c.Request = req + + assert.True(t, shouldAttemptJWT(c, "a.b.c")) + assert.False(t, shouldAttemptJWT(nil, "a.b.c")) + assert.False(t, shouldAttemptJWT(c, "invalidtoken")) + assert.False(t, shouldAttemptJWT(c, "")) +} + +func TestNodeAllowsJWT(t *testing.T) { + fx := newPortalJWTFixture(t, "node-allows") + conf := fx.nodeConf + + assert.True(t, shouldAllowJWT(conf, "192.0.2.9")) + + origCIDR := conf.Options().ClusterCIDR + conf.Options().ClusterCIDR = "192.0.2.0/24" + assert.True(t, shouldAllowJWT(conf, "192.0.2.25")) + assert.False(t, shouldAllowJWT(conf, "203.0.113.1")) + conf.Options().ClusterCIDR = origCIDR + + origJWKS := conf.JWKSUrl() + conf.SetJWKSUrl("") + assert.False(t, shouldAllowJWT(conf, "192.0.2.25")) + conf.SetJWKSUrl(origJWKS) + + assert.False(t, shouldAllowJWT(nil, "192.0.2.25")) +} + +func TestExpectedClaimsFor(t *testing.T) { + fx := newPortalJWTFixture(t, "expected-claims") + + claims := expectedClaimsFor(fx.nodeConf, "cluster") + assert.Equal(t, fmt.Sprintf("node:%s", fx.nodeUUID), claims.Audience) + assert.Equal(t, []string{"cluster"}, claims.Scope) + assert.Equal(t, fx.nodeConf.JWKSUrl(), claims.JWKSURL) + + noScope := expectedClaimsFor(fx.nodeConf, "") + assert.Nil(t, noScope.Scope) +} + +func TestVerifyTokenFromPortal(t *testing.T) { + fx := newPortalJWTFixture(t, "verify-token") + spec := fx.defaultClaimsSpec() + token := fx.issue(t, spec) + + expected := expectedClaimsFor(fx.nodeConf, clean.Scope("cluster")) + claims := verifyTokenFromPortal(context.Background(), token, expected, []string{"wrong", spec.Issuer}) + require.NotNil(t, claims) + assert.Equal(t, spec.Issuer, claims.Issuer) + assert.Equal(t, spec.Subject, claims.Subject) + + nilClaims := verifyTokenFromPortal(context.Background(), token, expected, []string{"wrong"}) + assert.Nil(t, nilClaims) +} + +func TestSessionFromJWTClaims(t *testing.T) { + claims := &clusterjwt.Claims{ + Scope: "cluster vision", + RegisteredClaims: gojwt.RegisteredClaims{ + Issuer: "portal:test", + Subject: "portal:client", + ID: "token-id", + }, + } + + sess := sessionFromJWTClaims(claims, "192.0.2.100") + require.NotNil(t, sess) + assert.Equal(t, http.StatusOK, sess.HttpStatus()) + assert.Equal(t, "portal:client", sess.ClientUID) + assert.Equal(t, clean.Scope("cluster vision"), sess.AuthScope) + assert.Equal(t, "portal:test", sess.AuthIssuer) + assert.Equal(t, "token-id", sess.AuthID) + assert.Equal(t, "192.0.2.100", sess.ClientIP) +} diff --git a/internal/auth/acl/scopes.go b/internal/auth/acl/scopes.go index 18d8bfdbc..6219848d2 100644 --- a/internal/auth/acl/scopes.go +++ b/internal/auth/acl/scopes.go @@ -1,5 +1,11 @@ package acl +import ( + "strings" + + "github.com/photoprism/photoprism/pkg/list" +) + // Permission scopes to Grant multiple Permissions for a Resource. const ( ScopeRead Permission = "read" @@ -35,3 +41,60 @@ var ( ActionManageOwn: true, } ) + +// ScopeAttr parses an authentication scope string and returns it as list.Attr. +func ScopeAttr(s string) list.Attr { + if s == "" { + return list.Attr{} + } + + return list.ParseAttr(strings.ToLower(s)) +} + +// ScopePermits verifies if the authorized scope permits access to the specified resource. +func ScopePermits(scope string, resource Resource, perms Permissions) bool { + if scope == "" { + return false + } + + // Parse scope to check for resources and permissions. + return ScopeAttrPermits(ScopeAttr(scope), resource, perms) +} + +// ScopeAttrPermits verifies if the authorized scope permits access to the specified resource. +func ScopeAttrPermits(attr list.Attr, resource Resource, perms Permissions) bool { + if len(attr) == 0 { + return false + } + + scope := attr.String() + + // Skip detailed check and allow all if scope is "*". + if scope == list.Any { + return true + } + + // Skip resource check if scope includes all read operations. + if scope == ScopeRead.String() { + return !GrantScopeRead.DenyAny(perms) + } + + // Check if resource is within scope. + if granted := attr.Contains(resource.String()); !granted { + return false + } + + // Check if permission is within scope. + if len(perms) == 0 { + return true + } + + // Check if scope is limited to read or write operations. + if a := attr.Find(ScopeRead.String()); a.Value == list.True && GrantScopeRead.DenyAny(perms) { + return false + } else if a = attr.Find(ScopeWrite.String()); a.Value == list.True && GrantScopeWrite.DenyAny(perms) { + return false + } + + return true +} diff --git a/internal/auth/acl/scopes_test.go b/internal/auth/acl/scopes_test.go index 8b06d1015..14d65f5a1 100644 --- a/internal/auth/acl/scopes_test.go +++ b/internal/auth/acl/scopes_test.go @@ -35,3 +35,136 @@ func TestGrantScopeWrite(t *testing.T) { assert.False(t, GrantScopeWrite.DenyAny(Permissions{AccessAll})) }) } + +func TestScopePermits(t *testing.T) { + t.Run("AnyScope", func(t *testing.T) { + assert.True(t, ScopePermits("*", "", nil)) + }) + t.Run("ReadScope", func(t *testing.T) { + assert.True(t, ScopePermits("read", "metrics", nil)) + assert.True(t, ScopePermits("read", "sessions", nil)) + assert.True(t, ScopePermits("read", "metrics", Permissions{ActionView, AccessAll})) + assert.False(t, ScopePermits("read", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read", "settings", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read", "settings", Permissions{ActionCreate})) + assert.False(t, ScopePermits("read", "sessions", Permissions{ActionDelete})) + }) + t.Run("ReadAny", func(t *testing.T) { + assert.True(t, ScopePermits("read *", "metrics", nil)) + assert.True(t, ScopePermits("read *", "sessions", nil)) + assert.True(t, ScopePermits("read *", "metrics", Permissions{ActionView, AccessAll})) + assert.False(t, ScopePermits("read *", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read *", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read *", "settings", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read *", "settings", Permissions{ActionCreate})) + assert.False(t, ScopePermits("read *", "sessions", Permissions{ActionDelete})) + }) + t.Run("ReadSettings", func(t *testing.T) { + assert.True(t, ScopePermits("read settings", "settings", Permissions{ActionView})) + assert.False(t, ScopePermits("read settings", "metrics", nil)) + assert.False(t, ScopePermits("read settings", "sessions", nil)) + assert.False(t, ScopePermits("read settings", "metrics", Permissions{ActionView, AccessAll})) + assert.False(t, ScopePermits("read settings", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read settings", "metrics", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read settings", "settings", Permissions{ActionUpdate})) + assert.False(t, ScopePermits("read settings", "sessions", Permissions{ActionDelete})) + assert.False(t, ScopePermits("read settings", "sessions", Permissions{ActionDelete})) + }) +} + +func TestScopeAttr(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + {name: "Empty", input: "", expected: nil}, + {name: "Lowercase", input: "read metrics", expected: []string{"metrics", "read"}}, + {name: "Uppercase", input: "READ SETTINGS", expected: []string{"read", "settings"}}, + {name: "WithNoise", input: " Read\tSessions\nmetrics", expected: []string{"metrics", "read", "sessions"}}, + {name: "Deduplicates", input: "metrics metrics", expected: []string{"metrics"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + attr := ScopeAttr(tc.input) + if len(tc.expected) == 0 { + assert.Len(t, attr, 0) + return + } + assert.ElementsMatch(t, tc.expected, attr.Strings()) + }) + } +} + +func TestScopePermitsEdgeCases(t *testing.T) { + tests := []struct { + name string + scope string + resource Resource + perms Permissions + want bool + }{ + {name: "EmptyScope", scope: "", resource: "metrics", perms: nil, want: false}, + {name: "OnlyInvalidChars", scope: "()", resource: "metrics", perms: nil, want: false}, + {name: "WildcardMixedOrder", scope: "* read metrics", resource: "metrics", perms: Permissions{ActionUpdate}, want: false}, + {name: "WildcardOverridesReadRestrictions", scope: "read metrics *", resource: "metrics", perms: Permissions{ActionDelete}, want: false}, + {name: "WildcardWithFalseValueIgnored", scope: "*:false read", resource: "metrics", perms: Permissions{ActionUpdate}, want: false}, + {name: "ExplicitFalseResource", scope: "metrics:false", resource: "metrics", perms: nil, want: false}, + {name: "ExplicitTrueResource", scope: "metrics:true", resource: "metrics", perms: nil, want: true}, + {name: "CaseInsensitiveScopeAndResource", scope: "READ SETTINGS", resource: Resource("Settings"), perms: Permissions{ActionView}, want: true}, + {name: "WhitespaceAndTabs", scope: "\tread\tsettings\n", resource: "settings", perms: Permissions{ActionView}, want: true}, + {name: "DefaultResourceRead", scope: "read default", resource: "", perms: Permissions{ActionView}, want: true}, + {name: "DefaultResourceUpdateDenied", scope: "read default", resource: "", perms: Permissions{ActionUpdate}, want: false}, + {name: "WriteAllowsMutation", scope: "write settings", resource: "settings", perms: Permissions{ActionUpdate}, want: true}, + {name: "WriteBlocksReadOnly", scope: "write settings", resource: "settings", perms: Permissions{ActionView}, want: false}, + {name: "ReadGrantAllowsAccessAll", scope: "read", resource: "metrics", perms: Permissions{AccessAll}, want: true}, + {name: "ReadGrantDeniesManage", scope: "read metrics", resource: "metrics", perms: Permissions{ActionManage}, want: false}, + {name: "WriteGrantAllowsManage", scope: "write metrics", resource: "metrics", perms: Permissions{ActionManage}, want: true}, + {name: "ResourceWildcard", scope: "metrics:*", resource: "metrics", perms: Permissions{ActionDelete}, want: true}, + {name: "GlobalWildcardWithoutRead", scope: "* metrics", resource: "metrics", perms: Permissions{ActionDelete}, want: true}, + {name: "ResourceWildcardWithRead", scope: "read metrics:*", resource: "metrics", perms: Permissions{ActionView}, want: true}, + {name: "ResourceWildcardWriteDenied", scope: "read metrics:*", resource: "metrics", perms: Permissions{ActionUpdate}, want: false}, + {name: "DuplicateAndNoise", scope: " read metrics metrics ", resource: "metrics", perms: nil, want: true}, + {name: "FalseOverridesTrue", scope: "metrics metrics:false", resource: "metrics", perms: nil, want: false}, + {name: "CaseInsensitiveResourceLookup", scope: "read metrics", resource: Resource("METRICS"), perms: Permissions{ActionView}, want: true}, + {name: "MixedReadWriteConflict", scope: "read write settings", resource: "settings", perms: Permissions{ActionUpdate}, want: false}, + {name: "PermissionsEmptySlice", scope: "read metrics", resource: "metrics", perms: Permissions{}, want: true}, + {name: "SimpleNonReadScopeAllows", scope: "cluster vision", resource: "cluster", perms: nil, want: true}, + {name: "SimpleNonReadScopeRejectsMissing", scope: "cluster vision", resource: "portal", perms: nil, want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := ScopePermits(tc.scope, tc.resource, tc.perms) + assert.Equalf(t, tc.want, got, "scope %q resource %q perms %v", tc.scope, tc.resource, tc.perms) + }) + } +} + +func TestScopeAttrPermits(t *testing.T) { + tests := []struct { + name string + scope string + resource Resource + perms Permissions + want bool + }{ + {name: "EmptyAttr", scope: "", resource: "metrics", perms: nil, want: false}, + {name: "Wildcard", scope: "*", resource: "metrics", perms: Permissions{ActionUpdate}, want: true}, + {name: "ReadAllowsView", scope: "read", resource: "settings", perms: Permissions{ActionView}, want: true}, + {name: "ReadBlocksUpdate", scope: "read", resource: "settings", perms: Permissions{ActionUpdate}, want: false}, + {name: "ResourceMismatch", scope: "read metrics", resource: "settings", perms: nil, want: false}, + {name: "WriteAllowsManage", scope: "write metrics", resource: "metrics", perms: Permissions{ActionManage}, want: true}, + {name: "WriteBlocksView", scope: "write metrics", resource: "metrics", perms: Permissions{ActionView}, want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + attr := ScopeAttr(tc.scope) + got := ScopeAttrPermits(attr, tc.resource, tc.perms) + assert.Equalf(t, tc.want, got, "scope %q resource %q perms %v", tc.scope, tc.resource, tc.perms) + }) + } +} diff --git a/internal/commands/clients_add.go b/internal/commands/clients_add.go index cb9fcd89f..7bebb8e54 100644 --- a/internal/commands/clients_add.go +++ b/internal/commands/clients_add.go @@ -71,7 +71,7 @@ func clientsAddAction(ctx *cli.Context) error { // Set a default client name if no specific name has been provided. if frm.AuthScope == "" { - frm.AuthScope = list.All + frm.AuthScope = list.Any } client, addErr := entity.AddClient(frm) diff --git a/internal/config/config_cluster.go b/internal/config/config_cluster.go index ba1d923a9..de18c9793 100644 --- a/internal/config/config_cluster.go +++ b/internal/config/config_cluster.go @@ -13,6 +13,7 @@ import ( "github.com/photoprism/photoprism/internal/service/cluster" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/list" "github.com/photoprism/photoprism/pkg/rnd" "github.com/photoprism/photoprism/pkg/service/http/header" ) @@ -278,6 +279,18 @@ func (c *Config) JWTLeeway() int { return c.options.JWTLeeway } +// JWTAllowedScopes returns an optional allow-list of accepted JWT scopes. +func (c *Config) JWTAllowedScopes() list.Attr { + if s := strings.TrimSpace(c.options.JWTScope); s != "" { + parsed := list.ParseAttr(strings.ToLower(s)) + if len(parsed) > 0 { + return parsed + } + } + + return list.ParseAttr("cluster vision metrics") +} + // AdvertiseUrl returns the advertised node URL for intra-cluster calls (scheme://host[:port]). func (c *Config) AdvertiseUrl() string { if c.options.AdvertiseUrl != "" { diff --git a/internal/config/config_cluster_test.go b/internal/config/config_cluster_test.go index de9baff92..31f281df7 100644 --- a/internal/config/config_cluster_test.go +++ b/internal/config/config_cluster_test.go @@ -11,6 +11,7 @@ import ( "github.com/photoprism/photoprism/internal/service/cluster" "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/list" "github.com/photoprism/photoprism/pkg/rnd" ) @@ -145,6 +146,13 @@ func TestConfig_Cluster(t *testing.T) { }) } }) + t.Run("JWTAllowedScopes", func(t *testing.T) { + c := NewConfig(CliTestContext()) + c.options.JWTScope = "cluster vision" + assert.Equal(t, list.ParseAttr("cluster vision"), c.JWTAllowedScopes()) + c.options.JWTScope = "" + assert.Equal(t, list.ParseAttr("cluster vision metrics"), c.JWTAllowedScopes()) + }) t.Run("Paths", func(t *testing.T) { c := NewConfig(CliTestContext()) diff --git a/internal/config/customize/scope.go b/internal/config/customize/scope.go index d68d5087e..5011e889f 100644 --- a/internal/config/customize/scope.go +++ b/internal/config/customize/scope.go @@ -9,7 +9,7 @@ import ( // ApplyScope updates the current settings based on the authorization scope passed. func (s *Settings) ApplyScope(scope string) *Settings { - if scope == "" || scope == list.All { + if scope == "" || scope == list.Any { return s } diff --git a/internal/config/flags.go b/internal/config/flags.go index 959ce03d9..5d9e2d1b1 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -731,6 +731,11 @@ var Flags = CliFlags{ Value: 300, EnvVars: EnvVars("JWKS_CACHE_TTL"), }}, { + Flag: &cli.StringFlag{ + Name: "jwt-scope", + Usage: "allowed JWT `SCOPES` (space separated). Leave empty to accept defaults", + EnvVars: EnvVars("JWT_SCOPE"), + }}, { Flag: &cli.IntFlag{ Name: "jwt-leeway", Usage: "JWT clock skew allowance in `SECONDS` (default 60, max 300)", diff --git a/internal/config/options.go b/internal/config/options.go index ea9895fc3..5e1dc4a46 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -154,6 +154,7 @@ type Options struct { NodeClientSecret string `yaml:"NodeClientSecret" json:"-" flag:"node-client-secret"` JWKSUrl string `yaml:"JWKSUrl" json:"-" flag:"jwks-url"` JWKSCacheTTL int `yaml:"JWKSCacheTTL" json:"-" flag:"jwks-cache-ttl"` + JWTScope string `yaml:"JWTScope" json:"-" flag:"jwt-scope"` JWTLeeway int `yaml:"JWTLeeway" json:"-" flag:"jwt-leeway"` AdvertiseUrl string `yaml:"AdvertiseUrl" json:"-" flag:"advertise-url"` HttpsProxy string `yaml:"HttpsProxy" json:"HttpsProxy" flag:"https-proxy"` diff --git a/internal/config/report.go b/internal/config/report.go index edcf5f31b..2e42fee70 100644 --- a/internal/config/report.go +++ b/internal/config/report.go @@ -190,6 +190,7 @@ func (c *Config) Report() (rows [][]string, cols []string) { {"node-client-secret", fmt.Sprintf("%s", strings.Repeat("*", utf8.RuneCountInString(c.NodeClientSecret())))}, {"jwks-url", c.JWKSUrl()}, {"jwks-cache-ttl", fmt.Sprintf("%d", c.JWKSCacheTTL())}, + {"jwt-scope", c.JWTAllowedScopes().String()}, {"jwt-leeway", fmt.Sprintf("%d", c.JWTLeeway())}, {"advertise-url", c.AdvertiseUrl()}, diff --git a/internal/entity/auth_session.go b/internal/entity/auth_session.go index 7c427639e..fad1993e5 100644 --- a/internal/entity/auth_session.go +++ b/internal/entity/auth_session.go @@ -17,7 +17,6 @@ import ( "github.com/photoprism/photoprism/pkg/authn" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/i18n" - "github.com/photoprism/photoprism/pkg/list" "github.com/photoprism/photoprism/pkg/rnd" "github.com/photoprism/photoprism/pkg/service/http/header" "github.com/photoprism/photoprism/pkg/time/unix" @@ -492,40 +491,7 @@ func (m *Session) Scope() string { // ValidateScope checks if the scope does not exclude access to specified resource. func (m *Session) ValidateScope(resource acl.Resource, perms acl.Permissions) bool { - // Get scope string. - scope := m.Scope() - - // Skip detailed check and allow all if scope is "*". - if scope == list.All { - return true - } - - // Skip resource check if scope includes all read operations. - if scope == acl.ScopeRead.String() { - return !acl.GrantScopeRead.DenyAny(perms) - } - - // Parse scope to check for resources and permissions. - attr := list.ParseAttr(scope) - - // Check if resource is within scope. - if granted := attr.Contains(resource.String()); !granted { - return false - } - - // Check if permission is within scope. - if len(perms) == 0 { - return true - } - - // Check if scope is limited to read or write operations. - if a := attr.Find(acl.ScopeRead.String()); a.Value == list.True && acl.GrantScopeRead.DenyAny(perms) { - return false - } else if a = attr.Find(acl.ScopeWrite.String()); a.Value == list.True && acl.GrantScopeWrite.DenyAny(perms) { - return false - } - - return true + return acl.ScopePermits(m.AuthScope, resource, perms) } // InsufficientScope checks if the scope does not include access to specified resource. diff --git a/pkg/list/attribute.go b/pkg/list/attribute.go index 57a0ac820..acb626977 100644 --- a/pkg/list/attribute.go +++ b/pkg/list/attribute.go @@ -74,7 +74,7 @@ func (f *KeyValue) Parse(s string) *KeyValue { } // Default? - if f.Key == All { + if f.Key == Any { return f } else if v = Value(v); v == "" { f.Value = True @@ -97,8 +97,8 @@ func (f *KeyValue) String() string { return "" } - if f.Key == All { - return All + if f.Key == Any { + return Any } if Bool[strings.ToLower(f.Value)] == True { @@ -111,3 +111,8 @@ func (f *KeyValue) String() string { return "" } + +// Any checks if this represents any value (asterisk). +func (f *KeyValue) Any() bool { + return f.Key == Any +} diff --git a/pkg/list/attributes.go b/pkg/list/attributes.go index 77b0b9648..341029d30 100644 --- a/pkg/list/attributes.go +++ b/pkg/list/attributes.go @@ -68,9 +68,9 @@ func (list Attr) Sort() Attr { sort.Slice(list, func(i, j int) bool { if list[i].Key == list[j].Key { return list[i].Value < list[j].Value - } else if list[i].Key == All { + } else if list[i].Key == Any { return false - } else if list[j].Key == All { + } else if list[j].Key == Any { return true } else { return list[i].Key < list[j].Key @@ -95,23 +95,25 @@ func (list Attr) Contains(s string) bool { func (list Attr) Find(s string) (a KeyValue) { if len(list) == 0 || s == "" { return a - } else if s == All { - return KeyValue{Key: All, Value: ""} + } else if s == Any { + return KeyValue{Key: Any, Value: ""} } attr := ParseKeyValue(s) - // Return nil if key is invalid or all. - if attr.Key == "" { + // Return if key is invalid. + if attr == nil { + return a + } else if attr.Key == "" { return a } // Find and return first match. - if attr.Value == "" || attr.Value == All { + if attr.Value == "" || attr.Value == Any { for i := range list { if strings.EqualFold(attr.Key, list[i].Key) { return *list[i] - } else if list[i].Key == All { + } else if list[i].Key == Any { a = *list[i] } } @@ -122,10 +124,10 @@ func (list Attr) Find(s string) (a KeyValue) { return KeyValue{Key: "", Value: ""} } else if attr.Value == list[i].Value { return *list[i] - } else if list[i].Value == All { + } else if list[i].Value == Any { a = *list[i] } - } else if list[i].Key == All && attr.Value != False { + } else if list[i].Key == Any && attr.Value != False { a = *list[i] } } diff --git a/pkg/list/attributes_test.go b/pkg/list/attributes_test.go index f8b3de400..38edd6f95 100644 --- a/pkg/list/attributes_test.go +++ b/pkg/list/attributes_test.go @@ -164,7 +164,7 @@ func TestAttr_Find(t *testing.T) { assert.Len(t, attr, 1) result := attr.Find("metrics") - assert.Equal(t, All, result.Key) + assert.Equal(t, Any, result.Key) assert.Equal(t, "", result.Value) }) t.Run("Empty", func(t *testing.T) { @@ -182,6 +182,7 @@ func TestAttr_Find(t *testing.T) { assert.Len(t, attr, 1) result := attr.Find("*") + assert.Equal(t, Any, result.Key) assert.Equal(t, All, result.Key) assert.Equal(t, "", result.Value) }) @@ -191,6 +192,7 @@ func TestAttr_Find(t *testing.T) { assert.Len(t, attr, 1) result := attr.Find("6VU:*") + assert.Equal(t, Any, result.Key) assert.Equal(t, All, result.Key) assert.Equal(t, "", result.Value) }) @@ -230,7 +232,7 @@ func TestAttr_Find(t *testing.T) { assert.Len(t, attr, 2) result := attr.Find("read") - assert.Equal(t, All, result.Key) + assert.Equal(t, Any, result.Key) assert.Equal(t, "", result.Value) result = attr.Find("read:other") @@ -238,7 +240,7 @@ func TestAttr_Find(t *testing.T) { assert.Equal(t, "other", result.Value) result = attr.Find("read:true") - assert.Equal(t, All, result.Key) + assert.Equal(t, Any, result.Key) assert.Equal(t, "", result.Value) result = attr.Find("read:false") diff --git a/pkg/list/contains.go b/pkg/list/contains.go index f45b976b9..8ea88685d 100644 --- a/pkg/list/contains.go +++ b/pkg/list/contains.go @@ -1,18 +1,22 @@ package list -const All = "*" +// Any matches everything. +const Any = "*" + +// All is kept for backward compatibility, but deprecated. +const All = Any // Contains tests if a string is contained in the list. func Contains(list []string, s string) bool { if len(list) == 0 || s == "" { return false - } else if s == All { + } else if s == Any { return true } // Find matches. for i := range list { - if s == list[i] || list[i] == All { + if s == list[i] || list[i] == Any { return true } } @@ -27,11 +31,11 @@ func ContainsAny(l, s []string) bool { } // If second list contains All, it's a wildcard match. - if s[0] == All { + if s[0] == Any { return true } for j := 1; j < len(s); j++ { - if s[j] == All { + if s[j] == Any { return true } } diff --git a/pkg/list/remove.go b/pkg/list/remove.go index fab41e43c..d8b6f4275 100644 --- a/pkg/list/remove.go +++ b/pkg/list/remove.go @@ -4,7 +4,7 @@ package list func Remove(list []string, s string) []string { if len(list) == 0 || s == "" { return list - } else if s == All { + } else if s == Any { return []string{} }