From 21ef4cac53f0899cdcdd60ecbd8d08c689b9ff4e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 27 Nov 2025 09:39:41 +0100 Subject: [PATCH] On set/get of repo make sure forge_id is set and on fetch respected (#5717) --- server/api/hook.go | 7 +- server/api/login.go | 9 ++- server/api/repo.go | 5 +- server/api/user.go | 3 + server/forge/bitbucket/bitbucket.go | 6 +- server/forge/bitbucket/bitbucket_test.go | 6 +- .../bitbucketdatacenter.go | 6 +- .../bitbucketdatacenter_test.go | 2 +- server/forge/common/utils.go | 4 +- server/forge/forge.go | 2 + server/forge/forgejo/forgejo.go | 8 ++- server/forge/forgejo/forgejo_test.go | 10 +-- server/forge/gitea/gitea.go | 8 ++- server/forge/gitea/gitea_test.go | 10 +-- server/forge/github/github.go | 10 +-- server/forge/github/github_test.go | 10 +-- server/forge/gitlab/gitlab.go | 6 +- server/forge/setup/setup.go | 12 ++-- server/router/middleware/session/repo.go | 1 + server/store/datastore/permission.go | 2 +- server/store/datastore/repo.go | 16 ++--- server/store/datastore/repo_test.go | 4 +- server/store/mocks/mock_Store.go | 72 +++++++++++-------- server/store/store.go | 8 +-- 24 files changed, 131 insertions(+), 96 deletions(-) diff --git a/server/api/hook.go b/server/api/hook.go index cd43f5acd..d6fc4148f 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -347,13 +347,12 @@ func PostHook(c *gin.Context) { func getRepoFromToken(store store.Store, t *token.Token) (*model.Repo, error) { if t.Get("repo-forge-remote-id") != "" { - // TODO: use both the forge ID and repo forge remote ID - /*forgeID, err := strconv.ParseInt(t.Get("forge-id"), 10, 64) + forgeID, err := strconv.ParseInt(t.Get("forge-id"), 10, 64) if err != nil { return nil, err - }*/ + } - return store.GetRepoForgeID(model.ForgeRemoteID(t.Get("repo-forge-remote-id"))) + return store.GetRepoForgeID(forgeID, model.ForgeRemoteID(t.Get("repo-forge-remote-id"))) } // get the repo by the repo-id diff --git a/server/api/login.go b/server/api/login.go index 1a05ca3b5..598e0040f 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -291,7 +291,7 @@ func HandleAuth(c *gin.Context) { return } - err = updateRepoPermissions(c, user, _store, _forge) + err = updateRepoPermissions(c, user, _store, _forge, forgeID) if err != nil { log.Error().Err(err).Msgf("cannot update repo permissions for user %s", user.Login) c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") @@ -303,7 +303,7 @@ func HandleAuth(c *gin.Context) { c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/") } -func updateRepoPermissions(c *gin.Context, user *model.User, _store store.Store, _forge forge.Forge) error { +func updateRepoPermissions(c *gin.Context, user *model.User, _store store.Store, _forge forge.Forge, forgeID int64) error { repos, err := utils.Paginate(func(page int) ([]*model.Repo, error) { return _forge.Repos(c, user, &model.ListOptions{ Page: page, @@ -315,7 +315,10 @@ func updateRepoPermissions(c *gin.Context, user *model.User, _store store.Store, } for _, forgeRepo := range repos { - dbRepo, err := _store.GetRepoForgeID(forgeRepo.ForgeRemoteID) + // make sure forgeID is set + forgeRepo.ForgeID = forgeID + + dbRepo, err := _store.GetRepoForgeID(forgeID, forgeRepo.ForgeRemoteID) if err != nil && errors.Is(err, types.RecordNotExist) { continue } diff --git a/server/api/repo.go b/server/api/repo.go index 3f0b12b4a..0126f2acf 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -61,7 +61,7 @@ func PostRepo(c *gin.Context) { return } - repo, err := _store.GetRepoForgeID(forgeRemoteID) + repo, err := _store.GetRepoForgeID(user.ForgeID, forgeRemoteID) enabledOnce := err == nil // if there's no error, the repo was found and enabled once already if enabledOnce && repo.IsActive { c.String(http.StatusConflict, "Repository is already active.") @@ -78,6 +78,7 @@ func PostRepo(c *gin.Context) { c.String(http.StatusInternalServerError, "Could not fetch repository from forge.") return } + from.ForgeID = user.ForgeID if !from.Perm.Admin { c.String(http.StatusForbidden, "User has to be a admin of this repository") return @@ -540,6 +541,7 @@ func MoveRepo(c *gin.Context) { _ = c.AbortWithError(http.StatusInternalServerError, err) return } + from.ForgeID = repo.ForgeID if !from.Perm.Admin { c.AbortWithStatus(http.StatusUnauthorized) return @@ -697,6 +699,7 @@ func repairRepo(c *gin.Context, repo *model.Repo, updatePermissions bool) error log.Error().Err(err).Msgf("get repo '%s/%s' from forge", repo.Owner, repo.Name) return err } + from.ForgeID = repo.ForgeID if repo.FullName != from.FullName { // create a redirection diff --git a/server/api/user.go b/server/api/user.go index e7837ddef..9e048e6aa 100644 --- a/server/api/user.go +++ b/server/api/user.go @@ -127,6 +127,9 @@ func GetRepos(c *gin.Context) { var repos []*model.Repo for _, r := range _repos { + // make sure forgeID is set + r.ForgeID = user.ForgeID + if r.Perm.Push && server.Config.Permissions.OwnersAllowlist.IsAllowed(r) { if active[r.ForgeRemoteID] != nil { existingRepo := active[r.ForgeRemoteID] diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index db41981f2..f7fe72936 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -50,6 +50,7 @@ type Opts struct { } type config struct { + id int64 api string url string oAuthClientID string @@ -58,8 +59,9 @@ type config struct { // New returns a new forge Configuration for integrating with the Bitbucket // repository hosting service at https://bitbucket.org -func New(opts *Opts) (forge.Forge, error) { +func New(id int64, opts *Opts) (forge.Forge, error) { return &config{ + id: id, api: DefaultAPI, url: DefaultURL, oAuthClientID: opts.OAuthClientID, @@ -405,7 +407,7 @@ func (c *config) Hook(ctx context.Context, req *http.Request) (*model.Repo, *mod return nil, nil, err } - u, err := common.RepoUserForgeID(ctx, repo.ForgeRemoteID) + u, err := common.RepoUserForgeID(ctx, c.id, repo.ForgeRemoteID) if err != nil { return nil, nil, err } diff --git a/server/forge/bitbucket/bitbucket_test.go b/server/forge/bitbucket/bitbucket_test.go index 4c5c15c26..28921b9d4 100644 --- a/server/forge/bitbucket/bitbucket_test.go +++ b/server/forge/bitbucket/bitbucket_test.go @@ -34,7 +34,7 @@ import ( ) func TestNew(t *testing.T) { - forge, _ := New(&Opts{OAuthClientID: "4vyW6b49Z", OAuthClientSecret: "a5012f6c6"}) + forge, _ := New(0, &Opts{OAuthClientID: "4vyW6b49Z", OAuthClientSecret: "a5012f6c6"}) f, _ := forge.(*config) assert.Equal(t, DefaultURL, f.url) @@ -52,7 +52,7 @@ func TestBitbucket(t *testing.T) { ctx := t.Context() - forge, _ := New(&Opts{}) + forge, _ := New(0, &Opts{}) netrc, _ := forge.Netrc(fakeUser, fakeRepo) assert.Equal(t, "bitbucket.org", netrc.Machine) assert.Equal(t, "x-token-auth", netrc.Login) @@ -209,7 +209,7 @@ func TestBitbucket(t *testing.T) { mockStore := store_mocks.NewMockStore(t) ctx = store.InjectToContext(ctx, mockStore) mockStore.On("GetUser", mock.Anything).Return(fakeUser, nil) - mockStore.On("GetRepoForgeID", mock.Anything).Return(fakeRepoFromHook, nil) + mockStore.On("GetRepoForgeID", mock.Anything, mock.Anything).Return(fakeRepoFromHook, nil) r, b, err := c.Hook(ctx, req) assert.NoError(t, err) diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go index 80b717e48..438153d8f 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go @@ -54,6 +54,7 @@ type Opts struct { } type client struct { + id int64 url string urlAPI string clientID string @@ -66,8 +67,9 @@ type client struct { // New returns a Forge implementation that integrates with Bitbucket DataCenter/Server, // the on-premise edition of Bitbucket Cloud, formerly known as Stash. -func New(opts Opts) (forge.Forge, error) { +func New(id int64, opts Opts) (forge.Forge, error) { config := &client{ + id: id, url: opts.URL, urlAPI: fmt.Sprintf("%s/rest", opts.URL), clientID: opts.OAuthClientID, @@ -541,7 +543,7 @@ func (c *client) getUserAndRepo(ctx context.Context, r *model.Repo) (*model.User return nil, nil, fmt.Errorf("unable to get store from context") } - repo, err := _store.GetRepoForgeID(r.ForgeRemoteID) + repo, err := _store.GetRepoForgeID(c.id, r.ForgeRemoteID) if err != nil { return nil, nil, fmt.Errorf("unable to get repo: %w", err) } diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter_test.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter_test.go index d5d809c6f..9fe8e5b0b 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter_test.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter_test.go @@ -27,7 +27,7 @@ import ( ) func TestNew(t *testing.T) { - forge, err := New(Opts{ + forge, err := New(0, Opts{ URL: "http://localhost:8080", Username: "0ZXh0IjoiI", Password: "I1NiIsInR5", diff --git a/server/forge/common/utils.go b/server/forge/common/utils.go index 3026aa822..c86be61d6 100644 --- a/server/forge/common/utils.go +++ b/server/forge/common/utils.go @@ -74,12 +74,12 @@ func RepoUser(ctx context.Context, r *model.Repo) (*model.User, error) { return user, nil } -func RepoUserForgeID(ctx context.Context, repoForgeID model.ForgeRemoteID) (*model.User, error) { +func RepoUserForgeID(ctx context.Context, forgeID int64, remoteID model.ForgeRemoteID) (*model.User, error) { _store, ok := store.TryFromContext(ctx) if !ok { return nil, errors.New("could not get store from context") } - r, err := _store.GetRepoForgeID(repoForgeID) + r, err := _store.GetRepoForgeID(forgeID, remoteID) if err != nil { return nil, err } diff --git a/server/forge/forge.go b/server/forge/forge.go index cffc3ef08..fb5adb474 100644 --- a/server/forge/forge.go +++ b/server/forge/forge.go @@ -88,11 +88,13 @@ type Forge interface { // - Fallback to owner/name if remoteID empty // // Must verify user has at least read access. + // Caller must make sure ForgeID is set. Repo(ctx context.Context, u *model.User, remoteID model.ForgeRemoteID, owner, name string) (*model.Repo, error) // Repos fetches all repositories accessible to the user. // Should include user's permission level in Repo.Perm. // Should support pagination via ListOptions. + // Caller must make sure ForgeID is set. Repos(ctx context.Context, u *model.User, p *model.ListOptions) ([]*model.Repo, error) // File fetches a single file at a specific commit. diff --git a/server/forge/forgejo/forgejo.go b/server/forge/forgejo/forgejo.go index 68b227741..689f73a26 100644 --- a/server/forge/forgejo/forgejo.go +++ b/server/forge/forgejo/forgejo.go @@ -46,6 +46,7 @@ const ( ) type Forgejo struct { + id int64 url string oauth2URL string oAuthClientID string @@ -65,12 +66,13 @@ type Opts struct { // New returns a Forge implementation that integrates with Forgejo, // an open source Git service written in Go. See https://forgejo.org/ -func New(opts Opts) (forge.Forge, error) { +func New(id int64, opts Opts) (forge.Forge, error) { if opts.OAuth2URL == "" { opts.OAuth2URL = opts.URL } return &Forgejo{ + id: id, url: opts.URL, oauth2URL: opts.OAuth2URL, oAuthClientID: opts.OAuthClientID, @@ -628,7 +630,7 @@ func (c *Forgejo) getChangedFilesForPR(ctx context.Context, repo *model.Repo, in return []string{}, nil } - repo, err := _store.GetRepoNameFallback(repo.ForgeRemoteID, repo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, repo.ForgeRemoteID, repo.FullName) if err != nil { return nil, err } @@ -665,7 +667,7 @@ func (c *Forgejo) getTagCommitSHA(ctx context.Context, repo *model.Repo, tagName return "", nil } - repo, err := _store.GetRepoNameFallback(repo.ForgeRemoteID, repo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, repo.ForgeRemoteID, repo.FullName) if err != nil { return "", err } diff --git a/server/forge/forgejo/forgejo_test.go b/server/forge/forgejo/forgejo_test.go index 8a20b39cf..85a424d85 100644 --- a/server/forge/forgejo/forgejo_test.go +++ b/server/forge/forgejo/forgejo_test.go @@ -31,7 +31,7 @@ import ( ) func TestNew(t *testing.T) { - forge, _ := New(Opts{ + forge, _ := New(0, Opts{ URL: "http://localhost:8080", SkipVerify: true, }) @@ -46,7 +46,7 @@ func Test_forgejo(t *testing.T) { s := httptest.NewServer(fixtures.Handler()) defer s.Close() - c, _ := New(Opts{ + c, _ := New(0, Opts{ URL: s.URL, SkipVerify: true, }) @@ -55,7 +55,7 @@ func Test_forgejo(t *testing.T) { ctx := store.InjectToContext(t.Context(), mockStore) t.Run("netrc with user token", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(fakeUser, fakeRepo) assert.Equal(t, "forgejo.org", netrc.Machine) assert.Equal(t, fakeUser.Login, netrc.Login) @@ -63,7 +63,7 @@ func Test_forgejo(t *testing.T) { assert.Equal(t, model.ForgeTypeForgejo, netrc.Type) }) t.Run("netrc with machine account", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(nil, fakeRepo) assert.Equal(t, "forgejo.org", netrc.Machine) assert.Empty(t, netrc.Login) @@ -124,7 +124,7 @@ func Test_forgejo(t *testing.T) { req, _ := http.NewRequest(http.MethodPost, "/hook", buf) req.Header = http.Header{} req.Header.Set(hookEvent, hookPullRequest) - mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything).Return(fakeRepo, nil) + mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything, mock.Anything).Return(fakeRepo, nil) mockStore.On("GetUser", mock.Anything).Return(fakeUser, nil) r, b, err := c.Hook(ctx, req) assert.NotNil(t, r) diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index d8076c9b5..8a76505d3 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -48,6 +48,7 @@ const ( ) type Gitea struct { + id int64 url string oAuthClientID string oAuthClientSecret string @@ -67,8 +68,9 @@ type Opts struct { // New returns a Forge implementation that integrates with Gitea, // an open source Git service written in Go. See https://gitea.io/ -func New(opts Opts) (forge.Forge, error) { +func New(id int64, opts Opts) (forge.Forge, error) { return &Gitea{ + id: id, url: opts.URL, oAuthClientID: opts.OAuthClientID, oAuthClientSecret: opts.OAuthClientSecret, @@ -635,7 +637,7 @@ func (c *Gitea) getChangedFilesForPR(ctx context.Context, repo *model.Repo, inde return []string{}, nil } - repo, err := _store.GetRepoNameFallback(repo.ForgeRemoteID, repo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, repo.ForgeRemoteID, repo.FullName) if err != nil { return nil, err } @@ -672,7 +674,7 @@ func (c *Gitea) getTagCommitSHA(ctx context.Context, repo *model.Repo, tagName s return "", nil } - repo, err := _store.GetRepoNameFallback(repo.ForgeRemoteID, repo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, repo.ForgeRemoteID, repo.FullName) if err != nil { return "", err } diff --git a/server/forge/gitea/gitea_test.go b/server/forge/gitea/gitea_test.go index 2e2bb3c75..db6eccd50 100644 --- a/server/forge/gitea/gitea_test.go +++ b/server/forge/gitea/gitea_test.go @@ -32,7 +32,7 @@ import ( ) func TestNew(t *testing.T) { - forge, _ := New(Opts{ + forge, _ := New(0, Opts{ URL: "http://localhost:8080", SkipVerify: true, }) @@ -47,7 +47,7 @@ func Test_gitea(t *testing.T) { s := httptest.NewServer(fixtures.Handler()) defer s.Close() - c, _ := New(Opts{ + c, _ := New(0, Opts{ URL: s.URL, SkipVerify: true, }) @@ -56,7 +56,7 @@ func Test_gitea(t *testing.T) { ctx := store.InjectToContext(t.Context(), mockStore) t.Run("netrc with user token", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(fakeUser, fakeRepo) assert.Equal(t, "gitea.com", netrc.Machine) assert.Equal(t, fakeUser.Login, netrc.Login) @@ -64,7 +64,7 @@ func Test_gitea(t *testing.T) { assert.Equal(t, model.ForgeTypeGitea, netrc.Type) }) t.Run("netrc with machine account", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(nil, fakeRepo) assert.Equal(t, "gitea.com", netrc.Machine) assert.Empty(t, netrc.Login) @@ -125,7 +125,7 @@ func Test_gitea(t *testing.T) { req, _ := http.NewRequest(http.MethodPost, "/hook", buf) req.Header = http.Header{} req.Header.Set(hookEvent, hookPullRequest) - mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything).Return(fakeRepo, nil) + mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything, mock.Anything).Return(fakeRepo, nil) mockStore.On("GetUser", mock.Anything).Return(fakeUser, nil) r, b, err := c.Hook(ctx, req) assert.NotNil(t, r) diff --git a/server/forge/github/github.go b/server/forge/github/github.go index 0b6a632e2..1de738c72 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -63,8 +63,9 @@ type Opts struct { // New returns a Forge implementation that integrates with a GitHub Cloud or // GitHub Enterprise version control hosting provider. -func New(opts Opts) (forge.Forge, error) { +func New(id int64, opts Opts) (forge.Forge, error) { r := &client{ + id: id, API: defaultAPI, url: defaultURL, Client: opts.OAuthClientID, @@ -83,6 +84,7 @@ func New(opts Opts) (forge.Forge, error) { } type client struct { + id int64 url string API string Client string @@ -659,7 +661,7 @@ func (c *client) loadChangedFilesFromPullRequest(ctx context.Context, pull *gith return pipeline, nil } - repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, tmpRepo.ForgeRemoteID, tmpRepo.FullName) if err != nil { return nil, err } @@ -697,7 +699,7 @@ func (c *client) getTagCommitSHA(ctx context.Context, repo *model.Repo, tagName return "", nil } - repo, err := _store.GetRepoNameFallback(repo.ForgeRemoteID, repo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, repo.ForgeRemoteID, repo.FullName) if err != nil { return "", err } @@ -752,7 +754,7 @@ func (c *client) loadChangedFilesFromCommits(ctx context.Context, tmpRepo *model log.Trace().Msg("GitHub tag event, fetching changed files using current commit") } - repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName) + repo, err := _store.GetRepoNameFallback(c.id, tmpRepo.ForgeRemoteID, tmpRepo.FullName) if err != nil { return nil, err } diff --git a/server/forge/github/github_test.go b/server/forge/github/github_test.go index c997d81b0..01900f4c6 100644 --- a/server/forge/github/github_test.go +++ b/server/forge/github/github_test.go @@ -34,7 +34,7 @@ import ( ) func TestNew(t *testing.T) { - forge, _ := New(Opts{ + forge, _ := New(0, Opts{ URL: "http://localhost:8080/", OAuthClientID: "0ZXh0IjoiI", OAuthClientSecret: "I1NiIsInR5", @@ -52,7 +52,7 @@ func Test_github(t *testing.T) { gin.SetMode(gin.TestMode) s := httptest.NewServer(fixtures.Handler()) - c, _ := New(Opts{ + c, _ := New(0, Opts{ URL: s.URL, SkipVerify: true, }) @@ -62,7 +62,7 @@ func Test_github(t *testing.T) { ctx := t.Context() t.Run("netrc with user token", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(fakeUser, fakeRepo) assert.Equal(t, "github.com", netrc.Machine) assert.Equal(t, fakeUser.AccessToken, netrc.Login) @@ -70,7 +70,7 @@ func Test_github(t *testing.T) { assert.Equal(t, model.ForgeTypeGithub, netrc.Type) }) t.Run("netrc with machine account", func(t *testing.T) { - forge, _ := New(Opts{}) + forge, _ := New(0, Opts{}) netrc, _ := forge.Netrc(nil, fakeRepo) assert.Equal(t, "github.com", netrc.Machine) assert.Empty(t, netrc.Login) @@ -160,7 +160,7 @@ func TestHook(t *testing.T) { Login: "6543", AccessToken: "token", }, nil) - mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything).Return(&model.Repo{ + mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything, mock.Anything).Return(&model.Repo{ ID: 1, ForgeRemoteID: "1", Owner: "6543", diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index f68fbf404..b4e6eb79a 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -56,6 +56,7 @@ type Opts struct { // Gitlab implements "Forge" interface. type GitLab struct { + id int64 url string oAuthClientID string oAuthClientSecret string @@ -67,8 +68,9 @@ type GitLab struct { // New returns a Forge implementation that integrates with Gitlab, an open // source Git service. See https://gitlab.com -func New(opts Opts) (forge.Forge, error) { +func New(id int64, opts Opts) (forge.Forge, error) { return &GitLab{ + id: id, url: opts.URL, oAuthClientID: opts.OAuthClientID, oAuthClientSecret: opts.OAuthClientSecret, @@ -782,7 +784,7 @@ func (g *GitLab) loadMetadataFromMergeRequest(ctx context.Context, tmpRepo *mode return pipeline, nil } - repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName) + repo, err := _store.GetRepoNameFallback(g.id, tmpRepo.ForgeRemoteID, tmpRepo.FullName) if err != nil { return nil, err } diff --git a/server/forge/setup/setup.go b/server/forge/setup/setup.go index e079a6ead..95c4caa67 100644 --- a/server/forge/setup/setup.go +++ b/server/forge/setup/setup.go @@ -50,7 +50,7 @@ func setupBitbucket(forge *model.Forge) (forge.Forge, error) { Bool("oauth-client-secret-set", opts.OAuthClientSecret != ""). Str("type", string(forge.Type)). Msg("setting up forge") - return bitbucket.New(opts) + return bitbucket.New(forge.ID, opts) } func setupGitea(forge *model.Forge) (forge.Forge, error) { @@ -77,7 +77,7 @@ func setupGitea(forge *model.Forge) (forge.Forge, error) { Bool("oauth-secret-id-set", opts.OAuthClientSecret != ""). Str("type", string(forge.Type)). Msg("setting up forge") - return gitea.New(opts) + return gitea.New(forge.ID, opts) } func setupForgejo(forge *model.Forge) (forge.Forge, error) { @@ -104,7 +104,7 @@ func setupForgejo(forge *model.Forge) (forge.Forge, error) { Bool("oauth-client-secret-set", opts.OAuthClientSecret != ""). Str("type", string(forge.Type)). Msg("setting up forge") - return forgejo.New(opts) + return forgejo.New(forge.ID, opts) } func setupGitLab(forge *model.Forge) (forge.Forge, error) { @@ -123,7 +123,7 @@ func setupGitLab(forge *model.Forge) (forge.Forge, error) { Bool("oauth-client-secret-set", opts.OAuthClientSecret != ""). Str("type", string(forge.Type)). Msg("setting up forge") - return gitlab.New(opts) + return gitlab.New(forge.ID, opts) } func setupGitHub(forge *model.Forge) (forge.Forge, error) { @@ -150,7 +150,7 @@ func setupGitHub(forge *model.Forge) (forge.Forge, error) { Bool("oauth-client-secret-set", opts.OAuthClientSecret != ""). Str("type", string(forge.Type)). Msg("setting up forge") - return github.New(opts) + return github.New(forge.ID, opts) } func setupBitbucketDatacenter(forge *model.Forge) (forge.Forge, error) { @@ -185,7 +185,7 @@ func setupBitbucketDatacenter(forge *model.Forge) (forge.Forge, error) { Str("type", string(forge.Type)). Bool("oauth-enable-project-admin-scope", opts.OAuthEnableProjectAdminScope). Msg("setting up forge") - return bitbucketdatacenter.New(opts) + return bitbucketdatacenter.New(forge.ID, opts) } func setupAddon(forge *model.Forge) (forge.Forge, error) { diff --git a/server/router/middleware/session/repo.go b/server/router/middleware/session/repo.go index 788ae1792..55e5e620a 100644 --- a/server/router/middleware/session/repo.go +++ b/server/router/middleware/session/repo.go @@ -126,6 +126,7 @@ func SetPerm() gin.HandlerFunc { _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 perm = _repo.Perm perm.Repo = repo perm.RepoID = repo.ID diff --git a/server/store/datastore/permission.go b/server/store/datastore/permission.go index 63a6e3dc6..34adce724 100644 --- a/server/store/datastore/permission.go +++ b/server/store/datastore/permission.go @@ -51,7 +51,7 @@ func (s storage) permUpsert(sess *xorm.Session, perm *model.Perm) error { // lookup repo based on name or forge ID if possible if perm.RepoID == 0 && perm.Repo != nil { - r, err := s.getRepoNameFallback(sess, perm.Repo.ForgeRemoteID, perm.Repo.FullName) + r, err := s.getRepoNameFallback(sess, perm.Repo.ForgeID, perm.Repo.ForgeRemoteID, perm.Repo.FullName) if err != nil { return err } diff --git a/server/store/datastore/repo.go b/server/store/datastore/repo.go index 9c8dfb1d4..849bc9bf5 100644 --- a/server/store/datastore/repo.go +++ b/server/store/datastore/repo.go @@ -31,25 +31,25 @@ func (s storage) GetRepo(id int64) (*model.Repo, error) { return repo, wrapGet(s.engine.ID(id).Get(repo)) } -func (s storage) GetRepoForgeID(remoteID model.ForgeRemoteID) (*model.Repo, error) { +func (s storage) GetRepoForgeID(forgeID int64, remoteID model.ForgeRemoteID) (*model.Repo, error) { sess := s.engine.NewSession() defer sess.Close() - return s.getRepoForgeID(sess, remoteID) + return s.getRepoForgeID(sess, forgeID, remoteID) } -func (s storage) getRepoForgeID(e *xorm.Session, remoteID model.ForgeRemoteID) (*model.Repo, error) { +func (s storage) getRepoForgeID(e *xorm.Session, forgeID int64, remoteID model.ForgeRemoteID) (*model.Repo, error) { repo := new(model.Repo) - return repo, wrapGet(e.Where("forge_remote_id = ?", remoteID).Get(repo)) + return repo, wrapGet(e.Where("forge_id = ? AND forge_remote_id = ?", forgeID, remoteID).Get(repo)) } -func (s storage) GetRepoNameFallback(remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { +func (s storage) GetRepoNameFallback(forgeID int64, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { sess := s.engine.NewSession() defer sess.Close() - return s.getRepoNameFallback(sess, remoteID, fullName) + return s.getRepoNameFallback(sess, forgeID, remoteID, fullName) } -func (s storage) getRepoNameFallback(e *xorm.Session, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { - repo, err := s.getRepoForgeID(e, remoteID) +func (s storage) getRepoNameFallback(e *xorm.Session, forgeID int64, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { + repo, err := s.getRepoForgeID(e, forgeID, remoteID) if errors.Is(err, types.RecordNotExist) { return s.getRepoName(e, fullName) } diff --git a/server/store/datastore/repo_test.go b/server/store/datastore/repo_test.go index ff6b55a52..edc05b817 100644 --- a/server/store/datastore/repo_test.go +++ b/server/store/datastore/repo_test.go @@ -380,7 +380,7 @@ func TestRepoRedirection(t *testing.T) { })) // test redirection from old repo name - repoFromStore, err := store.GetRepoNameFallback("1", "bradrydzewski/test") + repoFromStore, err := store.GetRepoNameFallback(0, "1", "bradrydzewski/test") assert.NoError(t, err) assert.Equal(t, repoFromStore.FullName, repoUpdated.FullName) @@ -394,7 +394,7 @@ func TestRepoRedirection(t *testing.T) { } assert.NoError(t, store.CreateRepo(&repo)) - repoFromStore, err = store.GetRepoNameFallback("", "bradrydzewski/test-no-forge-id") + repoFromStore, err = store.GetRepoNameFallback(0, "", "bradrydzewski/test-no-forge-id") assert.NoError(t, err) assert.Equal(t, repoFromStore.FullName, repo.FullName) } diff --git a/server/store/mocks/mock_Store.go b/server/store/mocks/mock_Store.go index 6cff923dd..31c73f748 100644 --- a/server/store/mocks/mock_Store.go +++ b/server/store/mocks/mock_Store.go @@ -2397,8 +2397,8 @@ func (_c *MockStore_GetRepoCount_Call) RunAndReturn(run func() (int64, error)) * } // GetRepoForgeID provides a mock function for the type MockStore -func (_mock *MockStore) GetRepoForgeID(forgeRemoteID model.ForgeRemoteID) (*model.Repo, error) { - ret := _mock.Called(forgeRemoteID) +func (_mock *MockStore) GetRepoForgeID(n int64, forgeRemoteID model.ForgeRemoteID) (*model.Repo, error) { + ret := _mock.Called(n, forgeRemoteID) if len(ret) == 0 { panic("no return value specified for GetRepoForgeID") @@ -2406,18 +2406,18 @@ func (_mock *MockStore) GetRepoForgeID(forgeRemoteID model.ForgeRemoteID) (*mode var r0 *model.Repo var r1 error - if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID) (*model.Repo, error)); ok { - return returnFunc(forgeRemoteID) + if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID) (*model.Repo, error)); ok { + return returnFunc(n, forgeRemoteID) } - if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID) *model.Repo); ok { - r0 = returnFunc(forgeRemoteID) + if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID) *model.Repo); ok { + r0 = returnFunc(n, forgeRemoteID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Repo) } } - if returnFunc, ok := ret.Get(1).(func(model.ForgeRemoteID) error); ok { - r1 = returnFunc(forgeRemoteID) + if returnFunc, ok := ret.Get(1).(func(int64, model.ForgeRemoteID) error); ok { + r1 = returnFunc(n, forgeRemoteID) } else { r1 = ret.Error(1) } @@ -2430,19 +2430,25 @@ type MockStore_GetRepoForgeID_Call struct { } // GetRepoForgeID is a helper method to define mock.On call +// - n int64 // - forgeRemoteID model.ForgeRemoteID -func (_e *MockStore_Expecter) GetRepoForgeID(forgeRemoteID interface{}) *MockStore_GetRepoForgeID_Call { - return &MockStore_GetRepoForgeID_Call{Call: _e.mock.On("GetRepoForgeID", forgeRemoteID)} +func (_e *MockStore_Expecter) GetRepoForgeID(n interface{}, forgeRemoteID interface{}) *MockStore_GetRepoForgeID_Call { + return &MockStore_GetRepoForgeID_Call{Call: _e.mock.On("GetRepoForgeID", n, forgeRemoteID)} } -func (_c *MockStore_GetRepoForgeID_Call) Run(run func(forgeRemoteID model.ForgeRemoteID)) *MockStore_GetRepoForgeID_Call { +func (_c *MockStore_GetRepoForgeID_Call) Run(run func(n int64, forgeRemoteID model.ForgeRemoteID)) *MockStore_GetRepoForgeID_Call { _c.Call.Run(func(args mock.Arguments) { - var arg0 model.ForgeRemoteID + var arg0 int64 if args[0] != nil { - arg0 = args[0].(model.ForgeRemoteID) + arg0 = args[0].(int64) + } + var arg1 model.ForgeRemoteID + if args[1] != nil { + arg1 = args[1].(model.ForgeRemoteID) } run( arg0, + arg1, ) }) return _c @@ -2453,7 +2459,7 @@ func (_c *MockStore_GetRepoForgeID_Call) Return(repo *model.Repo, err error) *Mo return _c } -func (_c *MockStore_GetRepoForgeID_Call) RunAndReturn(run func(forgeRemoteID model.ForgeRemoteID) (*model.Repo, error)) *MockStore_GetRepoForgeID_Call { +func (_c *MockStore_GetRepoForgeID_Call) RunAndReturn(run func(n int64, forgeRemoteID model.ForgeRemoteID) (*model.Repo, error)) *MockStore_GetRepoForgeID_Call { _c.Call.Return(run) return _c } @@ -2583,8 +2589,8 @@ func (_c *MockStore_GetRepoName_Call) RunAndReturn(run func(s string) (*model.Re } // GetRepoNameFallback provides a mock function for the type MockStore -func (_mock *MockStore) GetRepoNameFallback(remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { - ret := _mock.Called(remoteID, fullName) +func (_mock *MockStore) GetRepoNameFallback(forgeID int64, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) { + ret := _mock.Called(forgeID, remoteID, fullName) if len(ret) == 0 { panic("no return value specified for GetRepoNameFallback") @@ -2592,18 +2598,18 @@ func (_mock *MockStore) GetRepoNameFallback(remoteID model.ForgeRemoteID, fullNa var r0 *model.Repo var r1 error - if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID, string) (*model.Repo, error)); ok { - return returnFunc(remoteID, fullName) + if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID, string) (*model.Repo, error)); ok { + return returnFunc(forgeID, remoteID, fullName) } - if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID, string) *model.Repo); ok { - r0 = returnFunc(remoteID, fullName) + if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID, string) *model.Repo); ok { + r0 = returnFunc(forgeID, remoteID, fullName) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Repo) } } - if returnFunc, ok := ret.Get(1).(func(model.ForgeRemoteID, string) error); ok { - r1 = returnFunc(remoteID, fullName) + if returnFunc, ok := ret.Get(1).(func(int64, model.ForgeRemoteID, string) error); ok { + r1 = returnFunc(forgeID, remoteID, fullName) } else { r1 = ret.Error(1) } @@ -2616,25 +2622,31 @@ type MockStore_GetRepoNameFallback_Call struct { } // GetRepoNameFallback is a helper method to define mock.On call +// - forgeID int64 // - remoteID model.ForgeRemoteID // - fullName string -func (_e *MockStore_Expecter) GetRepoNameFallback(remoteID interface{}, fullName interface{}) *MockStore_GetRepoNameFallback_Call { - return &MockStore_GetRepoNameFallback_Call{Call: _e.mock.On("GetRepoNameFallback", remoteID, fullName)} +func (_e *MockStore_Expecter) GetRepoNameFallback(forgeID interface{}, remoteID interface{}, fullName interface{}) *MockStore_GetRepoNameFallback_Call { + return &MockStore_GetRepoNameFallback_Call{Call: _e.mock.On("GetRepoNameFallback", forgeID, remoteID, fullName)} } -func (_c *MockStore_GetRepoNameFallback_Call) Run(run func(remoteID model.ForgeRemoteID, fullName string)) *MockStore_GetRepoNameFallback_Call { +func (_c *MockStore_GetRepoNameFallback_Call) Run(run func(forgeID int64, remoteID model.ForgeRemoteID, fullName string)) *MockStore_GetRepoNameFallback_Call { _c.Call.Run(func(args mock.Arguments) { - var arg0 model.ForgeRemoteID + var arg0 int64 if args[0] != nil { - arg0 = args[0].(model.ForgeRemoteID) + arg0 = args[0].(int64) } - var arg1 string + var arg1 model.ForgeRemoteID if args[1] != nil { - arg1 = args[1].(string) + arg1 = args[1].(model.ForgeRemoteID) + } + var arg2 string + if args[2] != nil { + arg2 = args[2].(string) } run( arg0, arg1, + arg2, ) }) return _c @@ -2645,7 +2657,7 @@ func (_c *MockStore_GetRepoNameFallback_Call) Return(repo *model.Repo, err error return _c } -func (_c *MockStore_GetRepoNameFallback_Call) RunAndReturn(run func(remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error)) *MockStore_GetRepoNameFallback_Call { +func (_c *MockStore_GetRepoNameFallback_Call) RunAndReturn(run func(forgeID int64, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error)) *MockStore_GetRepoNameFallback_Call { _c.Call.Return(run) return _c } diff --git a/server/store/store.go b/server/store/store.go index a922b51d9..68212e3b6 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -44,10 +44,10 @@ type Store interface { // Repos // GetRepo gets a repo by unique ID. GetRepo(int64) (*model.Repo, error) - // GetRepoForgeID gets a repo by its forge ID. - GetRepoForgeID(model.ForgeRemoteID) (*model.Repo, error) - // GetRepoNameFallback gets the repo by its forge ID and if this doesn't exist by its full name. - GetRepoNameFallback(remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) + // GetRepoForgeID gets a repo by its forge ID and forge remote ID. + GetRepoForgeID(int64, model.ForgeRemoteID) (*model.Repo, error) + // GetRepoNameFallback gets the repo by its forge ID and forge remote ID, and if this doesn't exist by its full name. + GetRepoNameFallback(forgeID int64, remoteID model.ForgeRemoteID, fullName string) (*model.Repo, error) // GetRepoName gets a repo by its full name. GetRepoName(string) (*model.Repo, error) // GetRepoCount gets a count of all repositories in the system.