fix(sources): allow non-owners with read access to persist source sync status BE-13284 (#3329)

api/dataservices/interface.go

@@ -208,6 +208,7 @@ type (
 		Exists(context SourceServiceUserContext, ID portainer.SourceID) (bool, error)
 		ReadAll(context SourceServiceUserContext, predicates ...func(portainer.Source) bool) ([]portainer.Source, error)
 		Update(context SourceServiceUserContext, ID portainer.SourceID, source *portainer.Source) error
+		UpdateSyncStatus(context SourceServiceUserContext, ID portainer.SourceID, status portainer.SourceStatus, statusError string) error
 		Delete(context SourceServiceUserContext, ID portainer.SourceID) error
 		FindOrCreateGitSource(context SourceServiceUserContext, source *portainer.Source) (*portainer.Source, error)
 	}

api/dataservices/source/source.go

@@ -87,6 +87,12 @@ func (service *Service) Update(context UserContext, ID portainer.SourceID, sourc
 	})
 }
 
+func (service *Service) UpdateSyncStatus(context UserContext, ID portainer.SourceID, status portainer.SourceStatus, statusError string) error {
+	return service.base.Connection.UpdateTx(func(tx portainer.Transaction) error {
+		return service.Tx(tx).UpdateSyncStatus(context, ID, status, statusError)
+	})
+}
+
 func (service *Service) Delete(context UserContext, ID portainer.SourceID) error {
 	return service.base.Connection.UpdateTx(func(tx portainer.Transaction) error {
 		return service.Tx(tx).Delete(context, ID)

api/dataservices/source/sync_status_test.go

@@ -0,0 +1,100 @@
+package source_test
+
+import (
+	"testing"
+
+	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/dataservices/source"
+	"github.com/portainer/portainer/api/datastore"
+	gittypes "github.com/portainer/portainer/api/git/types"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestService_UpdateSyncStatus_HealthyBumpsLastSyncAndClearsError(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	adminContext := source.InsecureNewAdminContext()
+
+	src := &portainer.Source{
+		Type:        portainer.SourceTypeGit,
+		Git:         &gittypes.GitSource{URL: "https://example.com"},
+		Status:      portainer.SourceStatusError,
+		StatusError: "previous failure",
+	}
+	err := store.Source().Create(adminContext, src)
+	require.NoError(t, err)
+
+	err = store.Source().UpdateSyncStatus(adminContext, src.ID, portainer.SourceStatusHealthy, "")
+	require.NoError(t, err)
+
+	updated, err := store.Source().Read(adminContext, src.ID)
+	require.NoError(t, err)
+	require.Equal(t, portainer.SourceStatusHealthy, updated.Status)
+	require.Empty(t, updated.StatusError)
+	require.NotZero(t, updated.LastSync)
+}
+
+func TestService_UpdateSyncStatus_ErrorPreservesPriorLastSync(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	adminContext := source.InsecureNewAdminContext()
+
+	src := &portainer.Source{
+		Type:     portainer.SourceTypeGit,
+		Git:      &gittypes.GitSource{URL: "https://example.com"},
+		LastSync: 12345,
+	}
+	err := store.Source().Create(adminContext, src)
+	require.NoError(t, err)
+
+	err = store.Source().UpdateSyncStatus(adminContext, src.ID, portainer.SourceStatusError, "git fetch failed")
+	require.NoError(t, err)
+
+	updated, err := store.Source().Read(adminContext, src.ID)
+	require.NoError(t, err)
+	require.Equal(t, portainer.SourceStatusError, updated.Status)
+	require.Equal(t, "git fetch failed", updated.StatusError)
+	require.Equal(t, int64(12345), updated.LastSync)
+}
+
+// A standard user without any access to the source must not be able to persist a sync
+// status onto it, even though UpdateSyncStatus only requires read (not write) access.
+func TestService_UpdateSyncStatus_UserWithoutReadAccessIsDenied(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	adminContext := source.InsecureNewAdminContext()
+	standardUserContext := source.NewUserContext(&portainer.User{ID: 2, Role: portainer.StandardUserRole}, []portainer.TeamMembership{})
+
+	src := &portainer.Source{
+		Type:               portainer.SourceTypeGit,
+		Git:                &gittypes.GitSource{URL: "https://example.com"},
+		AdministratorsOnly: true,
+	}
+	err := store.Source().Create(adminContext, src)
+	require.NoError(t, err)
+
+	err = store.Source().UpdateSyncStatus(standardUserContext, src.ID, portainer.SourceStatusHealthy, "")
+	require.ErrorIs(t, err, source.ErrNotEnoughPermission)
+}
+
+func TestService_UpdateSyncStatus_UnknownSourceReturnsError(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	adminContext := source.InsecureNewAdminContext()
+
+	err := store.Source().UpdateSyncStatus(adminContext, portainer.SourceID(999), portainer.SourceStatusHealthy, "")
+	require.Error(t, err)
+}
+
+func TestService_UpdateSyncStatus_NilUserContextIsRejected(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	err := store.Source().UpdateSyncStatus(nil, portainer.SourceID(1), portainer.SourceStatusHealthy, "")
+	require.ErrorIs(t, err, source.ErrInvalidUserContext)
+}

api/dataservices/source/tx.go

@@ -2,6 +2,7 @@ package source
 
 import (
 	"slices"
+	"time"
 
 	portainer "github.com/portainer/portainer/api"
 	"github.com/portainer/portainer/api/dataservices"
@@ -119,6 +120,31 @@ func (service ServiceTx) Update(context UserContext, ID portainer.SourceID, sour
 	return service.base.Update(ID, source)
 }
 
+// UpdateSyncStatus updates only the status fields (Status, StatusError, LastSync) of a source.
+func (service ServiceTx) UpdateSyncStatus(context UserContext, ID portainer.SourceID, status portainer.SourceStatus, statusError string) error {
+	if err := validateUserContext(context); err != nil {
+		return err
+	}
+
+	source, err := service.base.Read(ID)
+	if err != nil {
+		return err
+	}
+
+	if err := enforceUserPermissions(context, source, actionRead); err != nil {
+		return err
+	}
+
+	source.Status = status
+	source.StatusError = statusError
+
+	if status == portainer.SourceStatusHealthy {
+		source.LastSync = time.Now().Unix()
+	}
+
+	return service.base.Update(ID, source)
+}
+
 // Delete deletes a source
 // It validates that the user has access to the source, and has enough permissions to perform the action
 func (service ServiceTx) Delete(context UserContext, ID portainer.SourceID) error {

api/gitops/workflows/source_artifact.go

@@ -2,7 +2,6 @@ package workflows
 
 import (
 	"fmt"
-	"time"
 
 	portainer "github.com/portainer/portainer/api"
 	"github.com/portainer/portainer/api/dataservices"
@@ -173,19 +172,7 @@ func UpdateSourceSyncStatus(tx gitSourceStore, userContext source.UserContext, s
 		return nil
 	}
 
-	src, err := tx.Source().Read(userContext, sourceID)
-	if err != nil {
-		return fmt.Errorf("failed to read source: %w", err)
-	}
-
-	src.Status = status
-	src.StatusError = statusError
-
-	if status == portainer.SourceStatusHealthy {
-		src.LastSync = time.Now().Unix()
-	}
-
-	return tx.Source().Update(userContext, src.ID, src)
+	return tx.Source().UpdateSyncStatus(userContext, sourceID, status, statusError)
 }
 
 func checkResultStatus(checkErr error) (portainer.SourceStatus, string) {
@@ -277,7 +264,7 @@ func SaveWorkflowGitConfig(tx gitSourceStore, userContext source.UserContext, wo
 		}
 
 		newSourceID = newSrc.ID
-	} else {
+	} else if !gitAuthEqual(cfg.Authentication, src.Git.Authentication) || cfg.TLSSkipVerify != src.Git.TLSSkipVerify {
 		src.Git.Authentication = cfg.Authentication
 		src.Git.TLSSkipVerify = cfg.TLSSkipVerify
 
@@ -294,6 +281,14 @@ func SaveWorkflowGitConfig(tx gitSourceStore, userContext source.UserContext, wo
 	})
 }
 
+func gitAuthEqual(a, b *gittypes.GitAuthentication) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+
+	return *a == *b
+}
+
 // SaveWorkflowArtifact replaces the ArtifactFile referencing oldSourceID on the Artifact matched by
 // matchArtifact with update (its SourceID may repoint the Artifact to a different Source). It does not
 // modify any Source's git config — the caller is responsible for ensuring update.SourceID

api/gitops/workflows/source_artifact_test.go

@@ -584,6 +584,77 @@ func TestSaveWorkflowGitConfig_UpdatesFileAndSourceWhenURLUnchanged(t *testing.T
 	require.True(t, src.Git.TLSSkipVerify)
 }
 
+// A plain redeploy (the stack_update_git_redeploy handler rebuilds cfg from the stack's own
+// stored Source, so URL/auth/TLS come back unchanged) must succeed for a non-owner standard
+// user with only read access to the Source: since nothing about the Source's config actually
+// changes, this must not require write/owner access.
+func TestSaveWorkflowGitConfig_NonOwnerRedeployWithUnchangedConfigSucceeds(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	standardUserContext := source.NewUserContext(&portainer.User{ID: 2, Role: portainer.StandardUserRole}, []portainer.TeamMembership{})
+
+	var workflowID portainer.WorkflowID
+	var sourceID portainer.SourceID
+	err := store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		src := &portainer.Source{
+			Type: portainer.SourceTypeGit,
+			Git: &gittypes.GitSource{
+				URL: "https://github.com/example/repo",
+				Authentication: &gittypes.GitAuthentication{
+					Username: "user",
+					Password: "pass",
+				},
+			},
+			Public: true,
+		}
+		err := tx.Source().Create(adminUserContext, src)
+		require.NoError(t, err)
+		sourceID = src.ID
+
+		wf := &portainer.Workflow{
+			Artifacts: []portainer.Artifact{{
+				StackID: 1,
+				Files: []portainer.ArtifactFile{{
+					SourceID: sourceID,
+					Path:     "docker-compose.yml",
+					Ref:      "refs/heads/main",
+					Hash:     "old-hash",
+				}},
+			}},
+		}
+		err = tx.Workflow().Create(wf)
+		require.NoError(t, err)
+		workflowID = wf.ID
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	// Same URL, auth and TLS setting as already stored: the shape loadGitConfigForStack
+	// produces on every redeploy that doesn't change the repository config.
+	redeployCfg := &gittypes.RepoConfig{
+		URL: "https://github.com/example/repo",
+		Authentication: &gittypes.GitAuthentication{
+			Username: "user",
+			Password: "pass",
+		},
+		ReferenceName: "refs/heads/main",
+		ConfigHash:    "new-hash",
+	}
+
+	err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return SaveWorkflowGitConfig(tx, standardUserContext, workflowID, func(a portainer.Artifact) bool {
+			return a.StackID == 1
+		}, sourceID, redeployCfg)
+	})
+	require.NoError(t, err)
+
+	wf, err := store.Workflow().Read(workflowID)
+	require.NoError(t, err)
+	require.Equal(t, "new-hash", wf.Artifacts[0].Files[0].Hash)
+}
+
 func TestSaveWorkflowGitConfig_CreatesNewSourceOnURLChange(t *testing.T) {
 	t.Parallel()
 	_, store := datastore.MustNewTestStore(t, false, true)
@@ -1109,6 +1180,86 @@ func TestUpdateSourceSyncStatus_ErrorPreservesPriorLastSync(t *testing.T) {
 	require.Equal(t, int64(12345), src.LastSync)
 }
 
+// A standard user deploying a stack from a Source they don't own must still be able to
+// persist sync status, for both a public Source and a restricted one they were granted
+// access to: syncing is a side effect of using the Source, not an edit of its config, and
+// must not require ownership or admin rights.
+func TestUpdateSourceSyncStatus_NonOwnerWithReadAccessCanUpdate(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	standardUserContext := source.NewUserContext(&portainer.User{ID: 2, Role: portainer.StandardUserRole}, []portainer.TeamMembership{})
+
+	var publicSourceID, restrictedSourceID portainer.SourceID
+	err := store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		public := &portainer.Source{
+			Type:   portainer.SourceTypeGit,
+			Git:    &gittypes.GitSource{URL: "https://github.com/example/public-repo"},
+			Public: true,
+		}
+		err := tx.Source().Create(adminUserContext, public)
+		require.NoError(t, err)
+		publicSourceID = public.ID
+
+		restricted := &portainer.Source{
+			Type:         portainer.SourceTypeGit,
+			Git:          &gittypes.GitSource{URL: "https://github.com/example/restricted-repo"},
+			UserAccesses: []portainer.UserID{2},
+		}
+		err = tx.Source().Create(adminUserContext, restricted)
+		require.NoError(t, err)
+		restrictedSourceID = restricted.ID
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return UpdateSourceSyncStatus(tx, standardUserContext, publicSourceID, portainer.SourceStatusHealthy, "")
+	})
+	require.NoError(t, err)
+
+	err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return UpdateSourceSyncStatus(tx, standardUserContext, restrictedSourceID, portainer.SourceStatusHealthy, "")
+	})
+	require.NoError(t, err)
+
+	public, err := store.Source().Read(adminUserContext, publicSourceID)
+	require.NoError(t, err)
+	require.Equal(t, portainer.SourceStatusHealthy, public.Status)
+
+	restricted, err := store.Source().Read(adminUserContext, restrictedSourceID)
+	require.NoError(t, err)
+	require.Equal(t, portainer.SourceStatusHealthy, restricted.Status)
+}
+
+func TestUpdateSourceSyncStatus_UserWithoutReadAccessIsDenied(t *testing.T) {
+	t.Parallel()
+	_, store := datastore.MustNewTestStore(t, false, true)
+
+	standardUserContext := source.NewUserContext(&portainer.User{ID: 2, Role: portainer.StandardUserRole}, []portainer.TeamMembership{})
+
+	var sourceID portainer.SourceID
+	err := store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		src := &portainer.Source{
+			Type:               portainer.SourceTypeGit,
+			Git:                &gittypes.GitSource{URL: "https://github.com/example/admin-only-repo"},
+			AdministratorsOnly: true,
+		}
+		err := tx.Source().Create(adminUserContext, src)
+		require.NoError(t, err)
+		sourceID = src.ID
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
+		return UpdateSourceSyncStatus(tx, standardUserContext, sourceID, portainer.SourceStatusHealthy, "")
+	})
+	require.ErrorIs(t, err, source.ErrNotEnoughPermission)
+}
+
 func TestFindOrCreateGitSource_StripsEmbeddedCredentialsFromURL(t *testing.T) {
 	t.Parallel()
 	_, store := datastore.MustNewTestStore(t, false, true)

api/http/handler/stacks/stack_update_git_redeploy_test.go

@@ -254,4 +254,40 @@ func TestDeployKubernetesStackInline(t *testing.T) {
 		require.True(t, postDeployCalled, "postDeploy should be called on failure too, consistent with the file-based inline deploy path")
 		require.ErrorIs(t, postDeployErr, deployErr)
 	})
+
+	t.Run("non-owner standard user with granted access can redeploy", func(t *testing.T) {
+		t.Parallel()
+
+		handler, stack, gitConfig, sourceID, _ := setupDeployKubernetesStackInlineTest(t, nil, portainer.StackStatusActive)
+
+		standardUser := &portainer.User{Username: "standarduser", Role: portainer.StandardUserRole}
+		require.NoError(t, handler.DataStore.User().Create(standardUser))
+
+		adminUserContext := source.InsecureNewAdminContext()
+		src, err := handler.DataStore.Source().Read(adminUserContext, sourceID)
+		require.NoError(t, err)
+		// AdministratorsOnly is a hard enforcement that defeats a UserAccesses grant, so it
+		// must be cleared for the grant below to actually take effect.
+		src.AdministratorsOnly = false
+		src.UserAccesses = []portainer.UserID{standardUser.ID}
+		require.NoError(t, handler.DataStore.Source().Update(adminUserContext, src.ID, src))
+
+		standardSecurityContext := &security.RestrictedRequestContext{
+			IsAdmin: false,
+			UserID:  standardUser.ID,
+			User:    standardUser,
+		}
+
+		req := mockDeployKubernetesStackInlineRequest()
+		stackutils.PrepareStackStatusForDeployment(stack)
+		deploymentConfig, httpErr := handler.deployStack(req, stack, false, &portainer.Endpoint{})
+		require.Nil(t, httpErr)
+
+		httpErr = handler.deployKubernetesStackInline(deploymentConfig, stack, standardSecurityContext, gitConfig, sourceID, nil)
+		require.Nil(t, httpErr)
+
+		updatedSrc, err := handler.DataStore.Source().Read(adminUserContext, sourceID)
+		require.NoError(t, err)
+		require.Equal(t, portainer.SourceStatusHealthy, updatedSrc.Status)
+	})
 }

api/stacks/stackbuilders/stack_git_builder_test.go

@@ -115,6 +115,48 @@ func TestGitMethodStackBuilder_WithSourceID_PersistsHealthyStatusOnSuccess(t *te
 	assert.NotZero(t, updatedSrc.LastSync)
 }
 
+func TestGitMethodStackBuilder_StandardUserWithReadAccessCanDeployFromAdminSource(t *testing.T) {
+	t.Parallel()
+	builder := newGitMethodBuilder(t, "abc123")
+
+	standardUser := &portainer.User{Username: "standarduser", Role: portainer.StandardUserRole}
+	require.NoError(t, builder.dataStore.User().Create(standardUser))
+
+	publicSrc := &portainer.Source{
+		Name:   "public-repo",
+		Type:   portainer.SourceTypeGit,
+		Git:    &gittypes.GitSource{URL: "https://github.com/org/public-repo"},
+		Public: true,
+	}
+	require.NoError(t, builder.dataStore.Source().Create(adminUserContext, publicSrc))
+
+	restrictedSrc := &portainer.Source{
+		Name:         "restricted-repo",
+		Type:         portainer.SourceTypeGit,
+		Git:          &gittypes.GitSource{URL: "https://github.com/org/restricted-repo"},
+		UserAccesses: []portainer.UserID{standardUser.ID},
+	}
+	require.NoError(t, builder.dataStore.Source().Create(adminUserContext, restrictedSrc))
+
+	for i, src := range []*portainer.Source{publicSrc, restrictedSrc} {
+		builder.stack.ID = portainer.StackID(10 + i)
+		payload := &StackPayload{
+			RepositoryConfigPayload: RepositoryConfigPayload{
+				SourceID:      src.ID,
+				ReferenceName: "refs/heads/main",
+			},
+		}
+
+		err := builder.prepare(t.Context(), payload, standardUser.ID)
+		require.NoError(t, err)
+
+		updatedSrc, err := builder.dataStore.Source().Read(adminUserContext, src.ID)
+		require.NoError(t, err)
+		assert.Equal(t, portainer.SourceStatusHealthy, updatedSrc.Status)
+		assert.NotZero(t, updatedSrc.LastSync)
+	}
+}
+
 func TestGitMethodStackBuilder_WithSourceID_PersistsErrorStatusOnCloneFailure(t *testing.T) {
 	t.Parallel()
 	builder := newGitMethodBuilder(t, "abc123")