diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index a08b351bb..23a3fd876 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -84,19 +84,15 @@ func (share *LinkSharing) GetID() int64 { return share.ID } -// GetLinkShareFromClaims builds a link sharing object from jwt claims -func GetLinkShareFromClaims(claims jwt.MapClaims) (share *LinkSharing, err error) { - projectID, is := claims["project_id"].(float64) - if !is { - return nil, &ErrLinkShareTokenInvalid{} - } - - permissionFloat, is := claims["permission"].(float64) - if !is { - return nil, &ErrLinkShareTokenInvalid{} - } - - id, is := claims["id"].(float64) +// GetLinkShareFromClaims resolves a link share JWT against the DB on every +// call so that deletion or permission downgrades take effect immediately +// instead of persisting for the (up to 72h) JWT TTL. The `permission` and +// `sharedByID` claims NewLinkShareJWTAuthtoken writes are intentionally +// ignored: the DB row is authoritative. +// +// Fixes GHSA-96q5-xm3p-7m84 / CVE-2026-35594. +func GetLinkShareFromClaims(s *xorm.Session, claims jwt.MapClaims) (share *LinkSharing, err error) { + idFloat, is := claims["id"].(float64) if !is { return nil, &ErrLinkShareTokenInvalid{} } @@ -104,18 +100,27 @@ func GetLinkShareFromClaims(claims jwt.MapClaims) (share *LinkSharing, err error if !is { return nil, &ErrLinkShareTokenInvalid{} } - sharedByID, is := claims["sharedByID"].(float64) - if !is { + + share, err = GetLinkShareByID(s, int64(idFloat)) + if err != nil { + // Only a missing row means the token is stale; everything else + // (DB down, etc.) must bubble up as 500 instead of 400. + if IsErrProjectShareDoesNotExist(err) { + return nil, &ErrLinkShareTokenInvalid{} + } + return nil, err + } + + // Defense in depth against a future bug that mints a token with a + // mismatched id/hash pair — the JWT signature alone already prevents + // tampering at rest. + if share.Hash != hash { return nil, &ErrLinkShareTokenInvalid{} } - share = &LinkSharing{} - share.ID = int64(id) - share.Hash = hash - share.ProjectID = int64(projectID) - share.Permission = Permission(permissionFloat) - share.SharedByID = int64(sharedByID) - return + // Never leak the hashed password through the auth object. + share.Password = "" + return share, nil } func (share *LinkSharing) getUserID() int64 { diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go index 268024e60..53482d7b2 100644 --- a/pkg/models/link_sharing_test.go +++ b/pkg/models/link_sharing_test.go @@ -23,6 +23,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/user" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -216,3 +217,132 @@ func TestLinkSharing_toUser(t *testing.T) { assert.Equal(t, int64(-2), user.ID) }) } + +func TestGetLinkShareFromClaims(t *testing.T) { + // Mirrors NewLinkShareJWTAuthtoken, including the legacy `permission` + // and `sharedByID` claims so the tests below can prove they're ignored. + buildClaims := func(id int64, hash string, projectID int64, permission Permission, sharedByID int64) jwt.MapClaims { + return jwt.MapClaims{ + "type": float64(2), // AuthTypeLinkShare + "id": float64(id), + "hash": hash, + "project_id": float64(projectID), + "permission": float64(permission), + "sharedByID": float64(sharedByID), + } + } + + t.Run("valid share returns DB values", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + dbShare, err := GetLinkShareByID(s, 1) + require.NoError(t, err) + + claims := buildClaims(dbShare.ID, dbShare.Hash, dbShare.ProjectID, dbShare.Permission, dbShare.SharedByID) + + got, err := GetLinkShareFromClaims(s, claims) + require.NoError(t, err) + assert.Equal(t, dbShare.ID, got.ID) + assert.Equal(t, dbShare.Hash, got.Hash) + assert.Equal(t, dbShare.ProjectID, got.ProjectID) + assert.Equal(t, dbShare.Permission, got.Permission) + assert.Equal(t, dbShare.SharedByID, got.SharedByID) + }) + + t.Run("deleted share is rejected", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + dbShare, err := GetLinkShareByID(s, 1) + require.NoError(t, err) + claims := buildClaims(dbShare.ID, dbShare.Hash, dbShare.ProjectID, dbShare.Permission, dbShare.SharedByID) + + _, err = s.Where("id = ?", dbShare.ID).Delete(&LinkSharing{}) + require.NoError(t, err) + + _, err = GetLinkShareFromClaims(s, claims) + require.Error(t, err) + assert.True(t, IsErrLinkShareTokenInvalid(err), + "expected ErrLinkShareTokenInvalid for deleted share, got %T: %v", err, err) + }) + + t.Run("permission downgrade takes effect immediately", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + dbShare, err := GetLinkShareByID(s, 3) + require.NoError(t, err) + require.Equal(t, PermissionAdmin, dbShare.Permission, + "fixture precondition: share id=3 must start as admin") + + // Capture claims while the share is still admin, then downgrade. + claims := buildClaims(dbShare.ID, dbShare.Hash, dbShare.ProjectID, PermissionAdmin, dbShare.SharedByID) + + _, err = s.Where("id = ?", dbShare.ID).Cols("permission").Update(&LinkSharing{Permission: PermissionRead}) + require.NoError(t, err) + + got, err := GetLinkShareFromClaims(s, claims) + require.NoError(t, err) + assert.Equal(t, PermissionRead, got.Permission, + "permission must come from DB, not from the (stale) JWT claim") + }) + + t.Run("hash mismatch is rejected", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + dbShare, err := GetLinkShareByID(s, 1) + require.NoError(t, err) + + claims := buildClaims(dbShare.ID, "not-the-real-hash", dbShare.ProjectID, dbShare.Permission, dbShare.SharedByID) + + _, err = GetLinkShareFromClaims(s, claims) + require.Error(t, err) + assert.True(t, IsErrLinkShareTokenInvalid(err)) + }) + + t.Run("sharedByID comes from DB not from claim", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + dbShare, err := GetLinkShareByID(s, 1) + require.NoError(t, err) + + // Bogus sharedByID in the claim must be ignored in favor of the DB value. + claims := buildClaims(dbShare.ID, dbShare.Hash, dbShare.ProjectID, dbShare.Permission, 9999999) + + got, err := GetLinkShareFromClaims(s, claims) + require.NoError(t, err) + assert.Equal(t, dbShare.SharedByID, got.SharedByID) + }) + + t.Run("missing id claim is rejected", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + claims := jwt.MapClaims{ + "hash": "whatever", + } + _, err := GetLinkShareFromClaims(s, claims) + require.Error(t, err) + assert.True(t, IsErrLinkShareTokenInvalid(err)) + }) + + t.Run("missing hash claim is rejected", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + claims := jwt.MapClaims{ + "id": float64(1), + } + _, err := GetLinkShareFromClaims(s, claims) + require.Error(t, err) + assert.True(t, IsErrLinkShareTokenInvalid(err)) + }) +} diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 402de54e9..4e2ebb94e 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -184,7 +184,9 @@ func GetAuthFromClaims(c *echo.Context) (a web.Auth, err error) { } typ := int(typFloat) if typ == AuthTypeLinkShare && config.ServiceEnableLinkSharing.GetBool() { - return models.GetLinkShareFromClaims(claims) + s := db.NewSession() + defer s.Close() + return models.GetLinkShareFromClaims(s, claims) } if typ == AuthTypeUser { return user.GetUserFromClaims(claims)