diff --git a/server/api/org.go b/server/api/org.go index aaa54fe93..8122eea44 100644 --- a/server/api/org.go +++ b/server/api/org.go @@ -74,6 +74,11 @@ func GetOrgPermissions(c *gin.Context) { user := session.User(c) org := session.Org(c) + if user == nil { + c.JSON(http.StatusOK, &model.OrgPerm{}) + return + } + _forge, err := server.Config.Services.Manager.ForgeFromUser(user) if err != nil { log.Error().Err(err).Msg("Cannot get forge from user") @@ -81,12 +86,7 @@ func GetOrgPermissions(c *gin.Context) { return } - if user == nil { - c.JSON(http.StatusOK, &model.OrgPerm{}) - return - } - - if (org.IsUser && org.Name == user.Login) || (user.Admin && !org.IsUser) { + if (org.IsUser && org.Name == user.Login && org.ForgeID == user.ForgeID) || (user.Admin && !org.IsUser) { c.JSON(http.StatusOK, &model.OrgPerm{ Member: true, Admin: true, @@ -97,6 +97,13 @@ func GetOrgPermissions(c *gin.Context) { return } + // orgs of other forges can share a name with orgs of the user's forge, + // so a membership looked up on the user's forge proves nothing about them + if org.ForgeID != user.ForgeID { + c.JSON(http.StatusOK, &model.OrgPerm{}) + return + } + perm, err := server.Config.Services.Membership.Get(c, _forge, user, org.Name) if err != nil { c.String(http.StatusInternalServerError, "Error getting membership for %d. %s", org.ID, err) diff --git a/server/api/org_test.go b/server/api/org_test.go new file mode 100644 index 000000000..ac3e51c13 --- /dev/null +++ b/server/api/org_test.go @@ -0,0 +1,167 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build test + +package api + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "go.woodpecker-ci.org/woodpecker/v3/server" + "go.woodpecker-ci.org/woodpecker/v3/server/forge" + forge_mocks "go.woodpecker-ci.org/woodpecker/v3/server/forge/mocks" + "go.woodpecker-ci.org/woodpecker/v3/server/model" + manager_mocks "go.woodpecker-ci.org/woodpecker/v3/server/services/mocks" +) + +// fakeMembership is a canned cache.MembershipService. It records whether it +// was consulted so tests can assert the forge was (not) asked. +type fakeMembership struct { + perm *model.OrgPerm + called bool +} + +func (f *fakeMembership) Get(_ context.Context, _ forge.Forge, _ *model.User, _ string) (*model.OrgPerm, error) { + f.called = true + if f.perm == nil { + return &model.OrgPerm{}, nil + } + return f.perm, nil +} + +// installOrgForgeManager wires a mock manager whose ForgeFromUser returns a +// bare mock forge (no expectations; membership is faked separately). +func installOrgForgeManager(t *testing.T) { + t.Helper() + mgr := manager_mocks.NewMockManager(t) + _forge := forge_mocks.NewMockForge(t) + mgr.On("ForgeFromUser", mock.Anything).Return(_forge, nil).Maybe() + server.Config.Services.Manager = mgr +} + +func TestGetOrgPermissions(t *testing.T) { + s := newTestStore(t) + + t.Run("anonymous user gets empty permissions without forge lookup", func(t *testing.T) { + // no ForgeFromUser expectation: mock manager fails the test if the + // handler dereferences the nil user to resolve a forge + mgr := manager_mocks.NewMockManager(t) + server.Config.Services.Manager = mgr + membership := &fakeMembership{} + server.Config.Services.Membership = membership + + tc := newTestContext(t, s) + org := &model.Org{ID: 1, Name: "some-org", ForgeID: 1} + tc.Ctx.Set("org", org) + + GetOrgPermissions(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code) + perm := new(model.OrgPerm) + tc.decodeJSON(t, perm) + assert.False(t, perm.Member) + assert.False(t, perm.Admin) + assert.False(t, membership.called) + }) + + t.Run("same login on another forge gets no admin on foreign user-org", func(t *testing.T) { + installOrgForgeManager(t) + membership := &fakeMembership{} + server.Config.Services.Membership = membership + + tc := newTestContext(t, s) + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 2, Name: "alice", ForgeID: 2, IsUser: true} + withUser(user)(tc) + tc.Ctx.Set("org", org) + + GetOrgPermissions(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code) + perm := new(model.OrgPerm) + tc.decodeJSON(t, perm) + assert.False(t, perm.Member) + assert.False(t, perm.Admin) + }) + + t.Run("membership of same-named org on user's forge does not leak to foreign org", func(t *testing.T) { + installOrgForgeManager(t) + // the user IS an admin of org "acme" on their own forge (1) ... + membership := &fakeMembership{perm: &model.OrgPerm{Member: true, Admin: true}} + server.Config.Services.Membership = membership + + tc := newTestContext(t, s) + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + // ... but this org "acme" lives on forge 2 + org := &model.Org{ID: 3, Name: "acme", ForgeID: 2} + withUser(user)(tc) + tc.Ctx.Set("org", org) + + GetOrgPermissions(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code) + perm := new(model.OrgPerm) + tc.decodeJSON(t, perm) + assert.False(t, perm.Member, "membership on forge 1 must not apply to org of forge 2") + assert.False(t, perm.Admin) + assert.False(t, membership.called, "forge of the user must not be asked about a foreign forge's org") + }) + + t.Run("own user-org on same forge grants admin", func(t *testing.T) { + installOrgForgeManager(t) + membership := &fakeMembership{} + server.Config.Services.Membership = membership + + tc := newTestContext(t, s) + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 4, Name: "alice", ForgeID: 1, IsUser: true} + withUser(user)(tc) + tc.Ctx.Set("org", org) + + GetOrgPermissions(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code) + perm := new(model.OrgPerm) + tc.decodeJSON(t, perm) + assert.True(t, perm.Member) + assert.True(t, perm.Admin) + }) + + t.Run("membership check runs for org on user's own forge", func(t *testing.T) { + installOrgForgeManager(t) + membership := &fakeMembership{perm: &model.OrgPerm{Member: true}} + server.Config.Services.Membership = membership + + tc := newTestContext(t, s) + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 5, Name: "acme", ForgeID: 1} + withUser(user)(tc) + tc.Ctx.Set("org", org) + + GetOrgPermissions(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code) + perm := new(model.OrgPerm) + tc.decodeJSON(t, perm) + assert.True(t, perm.Member) + assert.True(t, membership.called) + }) +} diff --git a/server/api/repo.go b/server/api/repo.go index 2c8083057..96dd28655 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -37,6 +37,31 @@ import ( "go.woodpecker-ci.org/woodpecker/v3/shared/token" ) +// getOrCreateOrg resolves the org named owner on the given forge, creating it +// from forge data if it is not known yet. +func getOrCreateOrg(c *gin.Context, _store store.Store, _forge forge.Forge, user *model.User, forgeID int64, owner string) (*model.Org, error) { + org, err := _store.OrgFindByName(owner, forgeID) + if err != nil && !errors.Is(err, types.ErrRecordNotExist) { + return nil, err + } + + // create an org if it doesn't exist yet + if errors.Is(err, types.ErrRecordNotExist) { + org, err = _forge.Org(c, user, owner) + if err != nil { + return nil, fmt.Errorf("failed to fetch organization from forge: %w", err) + } + + org.ForgeID = forgeID + err = _store.OrgCreate(org) + if err != nil { + return nil, fmt.Errorf("failed to create organization: %w", err) + } + } + + return org, nil +} + // PostRepo // // @Summary Activate a repository @@ -123,33 +148,14 @@ func PostRepo(c *gin.Context) { } // find org of repo - var org *model.Org - org, err = _store.OrgFindByName(repo.Owner, user.ForgeID) - if err != nil && !errors.Is(err, types.ErrRecordNotExist) { - c.String(http.StatusInternalServerError, err.Error()) + org, err := getOrCreateOrg(c, _store, _forge, user, user.ForgeID, repo.Owner) + if err != nil { + msg := fmt.Sprintf("Could not find or create organization %s.", repo.Owner) + log.Error().Err(err).Msg(msg) + c.String(http.StatusInternalServerError, msg) return } - // create an org if it doesn't exist yet - if errors.Is(err, types.ErrRecordNotExist) { - org, err = _forge.Org(c, user, repo.Owner) - if err != nil { - msg := fmt.Sprintf("Organization %s not found in DB. Attempting to create new one.", repo.Owner) - log.Error().Err(err).Msg(msg) - c.String(http.StatusInternalServerError, msg) - return - } - - org.ForgeID = user.ForgeID - err = _store.OrgCreate(org) - if err != nil { - msg := fmt.Sprintf("Failed to create organization %s.", repo.Owner) - log.Error().Err(err).Msg(msg) - c.String(http.StatusInternalServerError, msg) - return - } - } - repo.OrgID = org.ID // creates the jwt token used to verify the repository @@ -601,6 +607,18 @@ func MoveRepo(c *gin.Context) { } repo.Update(from) + + // the owner changed, so re-resolve the org of the repo: org-level + // secrets and registries of the old owner must no longer apply + org, err := getOrCreateOrg(c, _store, _forge, user, repo.ForgeID, repo.Owner) + if err != nil { + msg := fmt.Sprintf("Could not find or create organization %s.", repo.Owner) + log.Error().Err(err).Msg(msg) + c.String(http.StatusInternalServerError, msg) + return + } + repo.OrgID = org.ID + errStore := _store.UpdateRepo(repo) if errStore != nil { _ = c.AbortWithError(http.StatusInternalServerError, errStore) diff --git a/server/api/repo_test.go b/server/api/repo_test.go index 641aa9953..6f73131c0 100644 --- a/server/api/repo_test.go +++ b/server/api/repo_test.go @@ -22,6 +22,7 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "go.woodpecker-ci.org/woodpecker/v3/server" forge_mocks "go.woodpecker-ci.org/woodpecker/v3/server/forge/mocks" @@ -80,3 +81,107 @@ func TestPostRepoReturnsConflictOnDuplicateRepository(t *testing.T) { assert.Contains(t, w.Body.String(), "Remove the stale repository entry") mockStore.AssertNotCalled(t, "PermUpsert", mock.Anything) } + +func moveRepoForge(t *testing.T) *forge_mocks.MockForge { + t.Helper() + mgr := manager_mocks.NewMockManager(t) + mockForge := forge_mocks.NewMockForge(t) + mgr.On("ForgeFromRepo", mock.Anything).Return(mockForge, nil) + server.Config.Services.Manager = mgr + server.Config.Server.WebhookHost = "https://woodpecker.example" + return mockForge +} + +func TestMoveRepoUpdatesOrg(t *testing.T) { + s := newTestStore(t) + + seed := func(t *testing.T, forgeID int64, owner, name string) (*model.User, *model.Org, *model.Repo) { + t.Helper() + user := &model.User{Login: "alice-" + name, ForgeID: forgeID, ForgeRemoteID: model.ForgeRemoteID("u-" + name), Hash: "userhash-" + name} + require.NoError(t, s.CreateUser(user)) + oldOrg := &model.Org{Name: owner, ForgeID: forgeID} + require.NoError(t, s.OrgCreate(oldOrg)) + repo := &model.Repo{ + ForgeID: forgeID, + ForgeRemoteID: model.ForgeRemoteID("r-" + name), + Owner: owner, + Name: name, + FullName: owner + "/" + name, + OrgID: oldOrg.ID, + UserID: user.ID, + Hash: "hash-" + name, + IsActive: true, + } + require.NoError(t, s.CreateRepo(repo)) + return user, oldOrg, repo + } + + move := func(t *testing.T, user *model.User, repo *model.Repo, to string) (int, string) { + t.Helper() + tc := newTestContext(t, s) + withUser(user)(tc) + withRepo(repo, &model.Perm{Admin: true})(tc) + tc.Ctx.Request = httptest.NewRequest(http.MethodPost, "/repos/1/move?to="+to, nil) + MoveRepo(tc.Ctx) + return tc.Ctx.Writer.Status(), tc.Recorder.Body.String() + } + + t.Run("move to unknown owner creates the org and relinks the repo", func(t *testing.T) { + user, oldOrg, repo := seed(t, 1, "oldcorp", "rocket") + mockForge := moveRepoForge(t) + + from := &model.Repo{ + ForgeRemoteID: repo.ForgeRemoteID, + Owner: "newcorp", + Name: "rocket", + FullName: "newcorp/rocket", + Perm: &model.Perm{Admin: true}, + } + mockForge.On("Repo", mock.Anything, user, model.ForgeRemoteID(""), "newcorp", "rocket").Return(from, nil) + mockForge.On("Org", mock.Anything, user, "newcorp").Return(&model.Org{Name: "newcorp"}, nil) + mockForge.On("Deactivate", mock.Anything, user, mock.Anything, mock.Anything).Return(nil) + mockForge.On("Activate", mock.Anything, user, mock.Anything, mock.Anything).Return(nil) + + code, body := move(t, user, repo, "newcorp/rocket") + require.Equal(t, http.StatusNoContent, code, body) + + stored, err := s.GetRepo(repo.ID) + require.NoError(t, err) + assert.Equal(t, "newcorp/rocket", stored.FullName) + assert.NotEqual(t, oldOrg.ID, stored.OrgID, "repo must not stay linked to the old org") + + newOrg, err := s.OrgFindByName("newcorp", repo.ForgeID) + require.NoError(t, err) + assert.Equal(t, newOrg.ID, stored.OrgID) + assert.EqualValues(t, repo.ForgeID, newOrg.ForgeID) + }) + + t.Run("move to known owner links the existing org of the repo's forge", func(t *testing.T) { + user, oldOrg, repo := seed(t, 1, "oldinc", "probe") + // same-named org on ANOTHER forge that must not be picked + require.NoError(t, s.OrgCreate(&model.Org{Name: "newinc", ForgeID: 2})) + target := &model.Org{Name: "newinc", ForgeID: 1} + require.NoError(t, s.OrgCreate(target)) + + mockForge := moveRepoForge(t) + from := &model.Repo{ + ForgeRemoteID: repo.ForgeRemoteID, + Owner: "newinc", + Name: "probe", + FullName: "newinc/probe", + Perm: &model.Perm{Admin: true}, + } + // no Org() expectation: forge must not be asked for a known org + mockForge.On("Repo", mock.Anything, user, model.ForgeRemoteID(""), "newinc", "probe").Return(from, nil) + mockForge.On("Deactivate", mock.Anything, user, mock.Anything, mock.Anything).Return(nil) + mockForge.On("Activate", mock.Anything, user, mock.Anything, mock.Anything).Return(nil) + + code, body := move(t, user, repo, "newinc/probe") + require.Equal(t, http.StatusNoContent, code, body) + + stored, err := s.GetRepo(repo.ID) + require.NoError(t, err) + assert.Equal(t, target.ID, stored.OrgID) + assert.NotEqual(t, oldOrg.ID, stored.OrgID) + }) +} diff --git a/server/api/users.go b/server/api/users.go index 9f45f4a1b..f4f26c190 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -162,6 +162,11 @@ func PostUser(c *gin.Context) { c.String(http.StatusBadRequest, err.Error()) return } + + if in.ForgeID < defaultForgeID { + in.ForgeID = defaultForgeID + } + user := &model.User{ Login: in.Login, Email: in.Email, diff --git a/server/api/users_test.go b/server/api/users_test.go new file mode 100644 index 000000000..2d319a55e --- /dev/null +++ b/server/api/users_test.go @@ -0,0 +1,62 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build test + +package api + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.woodpecker-ci.org/woodpecker/v3/server/model" +) + +func TestPostUser(t *testing.T) { + s := newTestStore(t) + + t.Run("missing forge_id falls back to the default forge", func(t *testing.T) { + tc := newTestContext(t, s) + withRequest(http.MethodPost, &model.User{Login: "carol"})(tc) + + PostUser(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code, tc.Recorder.Body.String()) + + created := new(model.User) + tc.decodeJSON(t, created) + assert.EqualValues(t, defaultForgeID, created.ForgeID, "user must never be created with forge id 0") + + // the user's org must be forge-scoped as well + org, err := s.OrgGet(created.OrgID) + require.NoError(t, err) + assert.EqualValues(t, defaultForgeID, org.ForgeID, "org must never be created with forge id 0") + }) + + t.Run("explicit forge_id is kept", func(t *testing.T) { + tc := newTestContext(t, s) + withRequest(http.MethodPost, &model.User{Login: "dave", ForgeID: 2})(tc) + + PostUser(tc.Ctx) + + require.Equal(t, http.StatusOK, tc.Recorder.Code, tc.Recorder.Body.String()) + + created := new(model.User) + tc.decodeJSON(t, created) + assert.EqualValues(t, 2, created.ForgeID) + }) +} diff --git a/server/cache/membership.go b/server/cache/membership.go index cf1e09390..3a2a40311 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -52,7 +52,9 @@ func NewMembershipService(_store store.Store) MembershipService { // Get returns if the user is a member of the organization. func (c *membershipCache) Get(ctx context.Context, _forge forge.Forge, u *model.User, org string) (*model.OrgPerm, error) { - key := fmt.Sprintf("%s-%s", u.ForgeRemoteID, org) + // ForgeRemoteID is only unique per forge, so the key must be scoped by + // the (globally unique) user ID to not leak permissions across forges + key := fmt.Sprintf("%d-%s", u.ID, org) item := c.cache.Get(key) if item != nil && !item.IsExpired() { return item.Value(), nil diff --git a/server/cache/membership_test.go b/server/cache/membership_test.go new file mode 100644 index 000000000..261e8437e --- /dev/null +++ b/server/cache/membership_test.go @@ -0,0 +1,65 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build test + +package cache + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + forge_mocks "go.woodpecker-ci.org/woodpecker/v3/server/forge/mocks" + "go.woodpecker-ci.org/woodpecker/v3/server/model" +) + +func TestMembershipCacheForgeScoping(t *testing.T) { + cache := NewMembershipService(nil) + + // two distinct users on two forges that happen to share the same + // forge-local remote id + userForge1 := &model.User{ID: 1, Login: "alice", ForgeID: 1, ForgeRemoteID: "42"} + userForge2 := &model.User{ID: 2, Login: "bob", ForgeID: 2, ForgeRemoteID: "42"} + + forge1 := forge_mocks.NewMockForge(t) + forge1.On("OrgMembership", mock.Anything, userForge1, "acme"). + Return(&model.OrgPerm{Member: true, Admin: true}, nil).Once() + + forge2 := forge_mocks.NewMockForge(t) + forge2.On("OrgMembership", mock.Anything, userForge2, "acme"). + Return(&model.OrgPerm{}, nil).Once() + + perm1, err := cache.Get(t.Context(), forge1, userForge1, "acme") + require.NoError(t, err) + assert.True(t, perm1.Admin) + + // must NOT be served from userForge1's cache entry + perm2, err := cache.Get(t.Context(), forge2, userForge2, "acme") + require.NoError(t, err) + assert.False(t, perm2.Member, "membership of a same-remote-id user on another forge leaked from the cache") + assert.False(t, perm2.Admin) + + // repeated lookups are served from the per-user cache entries (the + // .Once() expectations above fail the test on a second forge call) + perm1again, err := cache.Get(t.Context(), forge1, userForge1, "acme") + require.NoError(t, err) + assert.True(t, perm1again.Admin) + + perm2again, err := cache.Get(t.Context(), forge2, userForge2, "acme") + require.NoError(t, err) + assert.False(t, perm2again.Member) +} diff --git a/server/router/middleware/session/repo.go b/server/router/middleware/session/repo.go index 097185fe9..e5f289f21 100644 --- a/server/router/middleware/session/repo.go +++ b/server/router/middleware/session/repo.go @@ -122,11 +122,13 @@ func SetPerm() gin.HandlerFunc { log.Error().Err(err).Msgf("error fetching permission for %s %s", user.Login, repo.FullName) } - if time.Unix(perm.Synced, 0).Add(time.Hour).Before(time.Now()) { + // a user of another forge has no permissions on this repo and + // their token must not be sent to the repo's forge + if user.ForgeID == repo.ForgeID && time.Unix(perm.Synced, 0).Add(time.Hour).Before(time.Now()) { _repo, err := _forge.Repo(c, user, repo.ForgeRemoteID, repo.Owner, repo.Name) if err == nil { log.Debug().Msgf("synced user permission for %s %s", user.Login, repo.FullName) - _repo.ForgeID = user.ForgeID + _repo.ForgeID = repo.ForgeID perm = _repo.Perm perm.RepoID = repo.ID perm.UserID = user.ID diff --git a/server/router/middleware/session/repo_test.go b/server/router/middleware/session/repo_test.go new file mode 100644 index 000000000..08caabbac --- /dev/null +++ b/server/router/middleware/session/repo_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build test + +package session + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "go.woodpecker-ci.org/woodpecker/v3/server" + forge_mocks "go.woodpecker-ci.org/woodpecker/v3/server/forge/mocks" + "go.woodpecker-ci.org/woodpecker/v3/server/model" + manager_mocks "go.woodpecker-ci.org/woodpecker/v3/server/services/mocks" + "go.woodpecker-ci.org/woodpecker/v3/server/store/datastore" +) + +func TestSetPerm(t *testing.T) { + s := datastore.NewTestStore(t) + + newCtx := func(user *model.User, repo *model.Repo) (*gin.Context, *httptest.ResponseRecorder) { + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodGet, "/", nil) + c.Set("store", s) + if user != nil { + c.Set("user", user) + } + c.Set("repo", repo) + return c, rec + } + + t.Run("stale perm of user on another forge is not synced via the repo's forge", func(t *testing.T) { + mgr := manager_mocks.NewMockManager(t) + // forge of the repo; no Repo() expectation: the mock fails the test + // if the middleware sends the foreign user's token to this forge + _forge := forge_mocks.NewMockForge(t) + mgr.On("ForgeFromRepo", mock.Anything).Return(_forge, nil) + server.Config.Services.Manager = mgr + + user := &model.User{ID: 10, Login: "alice", ForgeID: 1} + repo := &model.Repo{ID: 20, FullName: "acme/spanner", ForgeID: 2, ForgeRemoteID: "7"} + c, _ := newCtx(user, repo) + + SetPerm()(c) + + perm := Perm(c) + require.NotNil(t, perm) + assert.False(t, perm.Pull) + assert.False(t, perm.Push) + assert.False(t, perm.Admin) + }) + + t.Run("stale perm of user on the repo's forge is synced", func(t *testing.T) { + mgr := manager_mocks.NewMockManager(t) + _forge := forge_mocks.NewMockForge(t) + mgr.On("ForgeFromRepo", mock.Anything).Return(_forge, nil) + server.Config.Services.Manager = mgr + + user := &model.User{ID: 11, Login: "bob", ForgeID: 2} + repo := &model.Repo{ID: 21, FullName: "acme/spanner", ForgeID: 2, ForgeRemoteID: "7"} + + _forge.On("Repo", mock.Anything, user, repo.ForgeRemoteID, repo.Owner, repo.Name). + Return(&model.Repo{ + ForgeRemoteID: repo.ForgeRemoteID, + ForgeID: repo.ForgeID, + Perm: &model.Perm{Pull: true, Push: true}, + }, nil).Once() + + c, _ := newCtx(user, repo) + + SetPerm()(c) + + perm := Perm(c) + require.NotNil(t, perm) + assert.True(t, perm.Pull) + assert.True(t, perm.Push) + assert.EqualValues(t, repo.ID, perm.RepoID) + assert.EqualValues(t, user.ID, perm.UserID) + assert.InDelta(t, time.Now().Unix(), perm.Synced, 5) + }) +} diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index 744f09ddc..495b8f688 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -137,11 +137,20 @@ func MustOrgMember(admin bool) gin.HandlerFunc { } // User can access his own, admin can access all - if (org.Name == user.Login) || user.Admin { + if (org.Name == user.Login && org.ForgeID == user.ForgeID) || user.Admin { c.Next() return } + // orgs of other forges can share a name with orgs of the user's + // forge, so a membership looked up on the user's forge proves + // nothing about them + if org.ForgeID != user.ForgeID { + c.String(http.StatusForbidden, "user not authorized") + c.Abort() + return + } + _forge, err := server.Config.Services.Manager.ForgeFromUser(user) if err != nil { log.Error().Err(err).Msg("Cannot get forge from user") diff --git a/server/router/middleware/session/user_test.go b/server/router/middleware/session/user_test.go new file mode 100644 index 000000000..9efcab149 --- /dev/null +++ b/server/router/middleware/session/user_test.go @@ -0,0 +1,149 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build test + +package session + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "go.woodpecker-ci.org/woodpecker/v3/server" + "go.woodpecker-ci.org/woodpecker/v3/server/forge" + forge_mocks "go.woodpecker-ci.org/woodpecker/v3/server/forge/mocks" + "go.woodpecker-ci.org/woodpecker/v3/server/model" + manager_mocks "go.woodpecker-ci.org/woodpecker/v3/server/services/mocks" +) + +// fakeMembership is a canned cache.MembershipService that records whether it +// was consulted. +type fakeMembership struct { + perm *model.OrgPerm + called bool +} + +func (f *fakeMembership) Get(_ context.Context, _ forge.Forge, _ *model.User, _ string) (*model.OrgPerm, error) { + f.called = true + if f.perm == nil { + return &model.OrgPerm{}, nil + } + return f.perm, nil +} + +func newOrgMemberTestContext(user *model.User, org *model.Org) (*gin.Context, *httptest.ResponseRecorder) { + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodGet, "/", nil) + if user != nil { + c.Set("user", user) + } + if org != nil { + c.Set("org", org) + } + return c, rec +} + +func installMemberForgeManager(t *testing.T) { + t.Helper() + mgr := manager_mocks.NewMockManager(t) + _forge := forge_mocks.NewMockForge(t) + mgr.On("ForgeFromUser", mock.Anything).Return(_forge, nil).Maybe() + server.Config.Services.Manager = mgr +} + +func TestMustOrgMember(t *testing.T) { + t.Run("same login on another forge is denied access to foreign user-org", func(t *testing.T) { + installMemberForgeManager(t) + membership := &fakeMembership{} + server.Config.Services.Membership = membership + + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 2, Name: "alice", ForgeID: 2, IsUser: true} + c, rec := newOrgMemberTestContext(user, org) + + MustOrgMember(true)(c) + + assert.Equal(t, http.StatusForbidden, rec.Code) + }) + + t.Run("membership on user's forge does not grant access to same-named org on another forge", func(t *testing.T) { + installMemberForgeManager(t) + // admin of "acme" on forge 1 ... + membership := &fakeMembership{perm: &model.OrgPerm{Member: true, Admin: true}} + server.Config.Services.Membership = membership + + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + // ... must not grant anything on "acme" of forge 2 + org := &model.Org{ID: 3, Name: "acme", ForgeID: 2} + c, rec := newOrgMemberTestContext(user, org) + + MustOrgMember(true)(c) + + assert.Equal(t, http.StatusForbidden, rec.Code) + assert.False(t, membership.called, "forge of the user must not be asked about a foreign forge's org") + }) + + t.Run("own user-org on same forge is allowed", func(t *testing.T) { + installMemberForgeManager(t) + membership := &fakeMembership{} + server.Config.Services.Membership = membership + + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 4, Name: "alice", ForgeID: 1, IsUser: true} + c, rec := newOrgMemberTestContext(user, org) + + MustOrgMember(true)(c) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.False(t, c.IsAborted()) + }) + + t.Run("instance admin is allowed across forges", func(t *testing.T) { + installMemberForgeManager(t) + server.Config.Services.Membership = &fakeMembership{} + + user := &model.User{ID: 1, Login: "root", ForgeID: 1, Admin: true} + org := &model.Org{ID: 5, Name: "acme", ForgeID: 2} + c, rec := newOrgMemberTestContext(user, org) + + MustOrgMember(true)(c) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.False(t, c.IsAborted()) + }) + + t.Run("org member on same forge is allowed", func(t *testing.T) { + installMemberForgeManager(t) + membership := &fakeMembership{perm: &model.OrgPerm{Member: true, Admin: true}} + server.Config.Services.Membership = membership + + user := &model.User{ID: 1, Login: "alice", ForgeID: 1} + org := &model.Org{ID: 6, Name: "acme", ForgeID: 1} + c, rec := newOrgMemberTestContext(user, org) + + MustOrgMember(true)(c) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.False(t, c.IsAborted()) + assert.True(t, membership.called) + }) +}