fix(sources): demote AdministratorsOnly when self-granting access [BE-13225] (#3219)

This commit is contained in:
RHCowan
2026-07-21 09:15:47 +12:00
committed by GitHub
parent 9cae64cc3e
commit 33b4772cfb
4 changed files with 24 additions and 14 deletions
+4 -7
View File
@@ -76,17 +76,14 @@ func userCanReadSource(source *portainer.Source, context UserContext) bool {
return true 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 { if source.AdministratorsOnly {
return false return false
} }
if slices.Contains(source.UserAccesses, context.ID()) {
return true
}
if len(userTeams) == 0 || len(source.TeamAccesses) == 0 { if len(userTeams) == 0 || len(source.TeamAccesses) == 0 {
return false return false
} }
+4 -5
View File
@@ -15,13 +15,12 @@ func Test_UserCanReadSource_AdministratorsOnly(t *testing.T) {
adminOnly := &portainer.Source{AdministratorsOnly: true} adminOnly := &portainer.Source{AdministratorsOnly: true}
require.False(t, userCanReadSource(adminOnly, standardUser)) require.False(t, userCanReadSource(adminOnly, standardUser))
// An explicit UserAccesses entry (the FindOrCreateGitSource auto-grant) wins // AdministratorsOnly is a hard enforcement: no user or team access overrides
// over AdministratorsOnly. // 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}} granted := &portainer.Source{AdministratorsOnly: true, UserAccesses: []portainer.UserID{2}}
require.True(t, userCanReadSource(granted, standardUser)) require.False(t, userCanReadSource(granted, standardUser))
require.False(t, userCanReadSource(granted, teamMember))
// Team accesses do not override AdministratorsOnly — only user grants do.
teamGranted := &portainer.Source{AdministratorsOnly: true, TeamAccesses: []portainer.TeamID{7}} teamGranted := &portainer.Source{AdministratorsOnly: true, TeamAccesses: []portainer.TeamID{7}}
require.False(t, userCanReadSource(teamGranted, teamMember)) require.False(t, userCanReadSource(teamGranted, teamMember))
} }
+8 -1
View File
@@ -1,6 +1,8 @@
package source package source
import ( import (
"slices"
portainer "github.com/portainer/portainer/api" portainer "github.com/portainer/portainer/api"
"github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/dataservices"
gittypes "github.com/portainer/portainer/api/git/types" 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 // give user access to the first source if he doesn't have access
// to any of the sources that have the same url+auth // 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 { if err := service.base.Update(existing[0].ID, &existing[0]); err != nil {
return nil, err return nil, err
} }
+8 -1
View File
@@ -487,7 +487,9 @@ func TestFindOrCreateGitSource_AutoGrantLetsStandardUserReadAdminOnlySource(t *t
// A standard user supplying the same URL+auth is auto-granted access to the // 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 // 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 { err = store.UpdateTx(func(tx dataservices.DataStoreTx) error {
s, err := makeSource(standardUserContext)(tx) s, err := makeSource(standardUserContext)(tx)
if err != nil { if err != nil {
@@ -500,6 +502,11 @@ func TestFindOrCreateGitSource_AutoGrantLetsStandardUserReadAdminOnlySource(t *t
return err return err
}) })
require.NoError(t, 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) { func TestSaveWorkflowGitConfig_UpdatesFileAndSourceWhenURLUnchanged(t *testing.T) {