diff --git a/api/dataservices/source/access_rules.go b/api/dataservices/source/access_rules.go index e2f29f3d7a..d36a186006 100644 --- a/api/dataservices/source/access_rules.go +++ b/api/dataservices/source/access_rules.go @@ -76,17 +76,14 @@ func userCanReadSource(source *portainer.Source, context UserContext) bool { return true } - // An explicit user grant wins over AdministratorsOnly: sanitizeAccesses clears - // UserAccesses whenever an admin marks a source admins-only, so an entry here - // can only come from the deliberate auto-grant in FindOrCreateGitSource. - if slices.Contains(source.UserAccesses, context.ID()) { - return true - } - if source.AdministratorsOnly { return false } + if slices.Contains(source.UserAccesses, context.ID()) { + return true + } + if len(userTeams) == 0 || len(source.TeamAccesses) == 0 { return false } diff --git a/api/dataservices/source/access_rules_test.go b/api/dataservices/source/access_rules_test.go index 71b7347c90..fe1c2c86e7 100644 --- a/api/dataservices/source/access_rules_test.go +++ b/api/dataservices/source/access_rules_test.go @@ -15,13 +15,12 @@ func Test_UserCanReadSource_AdministratorsOnly(t *testing.T) { adminOnly := &portainer.Source{AdministratorsOnly: true} require.False(t, userCanReadSource(adminOnly, standardUser)) - // An explicit UserAccesses entry (the FindOrCreateGitSource auto-grant) wins - // over AdministratorsOnly. + // AdministratorsOnly is a hard enforcement: no user or team access overrides + // it. Sharing a source with a non-admin requires demoting the flag, as the + // FindOrCreateGitSource auto-grant does. granted := &portainer.Source{AdministratorsOnly: true, UserAccesses: []portainer.UserID{2}} - require.True(t, userCanReadSource(granted, standardUser)) - require.False(t, userCanReadSource(granted, teamMember)) + require.False(t, userCanReadSource(granted, standardUser)) - // Team accesses do not override AdministratorsOnly — only user grants do. teamGranted := &portainer.Source{AdministratorsOnly: true, TeamAccesses: []portainer.TeamID{7}} require.False(t, userCanReadSource(teamGranted, teamMember)) } diff --git a/api/dataservices/source/tx.go b/api/dataservices/source/tx.go index 864e86ebba..de8f8589db 100644 --- a/api/dataservices/source/tx.go +++ b/api/dataservices/source/tx.go @@ -1,6 +1,8 @@ package source import ( + "slices" + portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" gittypes "github.com/portainer/portainer/api/git/types" @@ -174,7 +176,12 @@ func (service ServiceTx) FindOrCreateGitSource(context UserContext, src *portain // give user access to the first source if he doesn't have access // to any of the sources that have the same url+auth - existing[0].UserAccesses = append(existing[0].UserAccesses, context.ID()) + if !slices.Contains(existing[0].UserAccesses, context.ID()) { + existing[0].UserAccesses = append(existing[0].UserAccesses, context.ID()) + } + // AdministratorsOnly is a hard enforcement that would defeat the grant, and + // a source shared with a non-admin is factually no longer admins-only. + existing[0].AdministratorsOnly = false if err := service.base.Update(existing[0].ID, &existing[0]); err != nil { return nil, err } diff --git a/api/gitops/workflows/source_artifact_test.go b/api/gitops/workflows/source_artifact_test.go index 6835148d6c..1cbdc8e136 100644 --- a/api/gitops/workflows/source_artifact_test.go +++ b/api/gitops/workflows/source_artifact_test.go @@ -487,7 +487,9 @@ func TestFindOrCreateGitSource_AutoGrantLetsStandardUserReadAdminOnlySource(t *t // A standard user supplying the same URL+auth is auto-granted access to the // existing source and must be able to read it afterwards (the stack builder - // reads it back in the same transaction to persist sync status). + // reads it back in the same transaction to persist sync status). The grant + // demotes AdministratorsOnly, which is a hard enforcement and would + // otherwise defeat the granted access. err = store.UpdateTx(func(tx dataservices.DataStoreTx) error { s, err := makeSource(standardUserContext)(tx) if err != nil { @@ -500,6 +502,11 @@ func TestFindOrCreateGitSource_AutoGrantLetsStandardUserReadAdminOnlySource(t *t return err }) require.NoError(t, err) + + shared, err := store.Source().Read(standardUserContext, sourceID) + require.NoError(t, err) + require.False(t, shared.AdministratorsOnly) + require.Equal(t, []portainer.UserID{2}, shared.UserAccesses) } func TestSaveWorkflowGitConfig_UpdatesFileAndSourceWhenURLUnchanged(t *testing.T) {