Skip to content

Commit

Permalink
Refactor find forks and fix possible bugs that weak permissions check
Browse files Browse the repository at this point in the history
  • Loading branch information
lunny committed Nov 15, 2024
1 parent ecbb03d commit b744d1c
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 39 deletions.
15 changes: 0 additions & 15 deletions models/repo/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,6 @@ func GetUserFork(ctx context.Context, repoID, userID int64) (*Repository, error)
return &forkedRepo, nil
}

// GetForks returns all the forks of the repository
func GetForks(ctx context.Context, repo *Repository, listOptions db.ListOptions) ([]*Repository, error) {
sess := db.GetEngine(ctx)

var forks []*Repository
if listOptions.Page == 0 {
forks = make([]*Repository, 0, repo.NumForks)
} else {
forks = make([]*Repository, 0, listOptions.PageSize)
sess = db.SetSessionPagination(sess, &listOptions)
}

return forks, sess.Find(&forks, &Repository{ForkID: repo.ID})
}

// IncrementRepoForkNum increment repository fork number
func IncrementRepoForkNum(ctx context.Context, repoID int64) error {
_, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_forks=num_forks+1 WHERE id=?", repoID)
Expand Down
26 changes: 18 additions & 8 deletions models/repo/repo_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,14 @@ func (repos RepositoryList) IDs() []int64 {
return repoIDs
}

// LoadAttributes loads the attributes for the given RepositoryList
func (repos RepositoryList) LoadAttributes(ctx context.Context) error {
func (repos RepositoryList) LoadOwners(ctx context.Context) error {
if len(repos) == 0 {
return nil
}

userIDs := container.FilterSlice(repos, func(repo *Repository) (int64, bool) {
return repo.OwnerID, true
})
repoIDs := make([]int64, len(repos))
for i := range repos {
repoIDs[i] = repos[i].ID
}

// Load owners.
users := make(map[int64]*user_model.User, len(userIDs))
Expand All @@ -123,12 +118,19 @@ func (repos RepositoryList) LoadAttributes(ctx context.Context) error {
for i := range repos {
repos[i].Owner = users[repos[i].OwnerID]
}
return nil
}

func (repos RepositoryList) LoadLanguageStats(ctx context.Context) error {
if len(repos) == 0 {
return nil
}

// Load primary language.
stats := make(LanguageStatList, 0, len(repos))
if err := db.GetEngine(ctx).
Where("`is_primary` = ? AND `language` != ?", true, "other").
In("`repo_id`", repoIDs).
In("`repo_id`", repos.IDs()).
Find(&stats); err != nil {
return fmt.Errorf("find primary languages: %w", err)
}
Expand All @@ -141,10 +143,18 @@ func (repos RepositoryList) LoadAttributes(ctx context.Context) error {
}
}
}

return nil
}

// LoadAttributes loads the attributes for the given RepositoryList
func (repos RepositoryList) LoadAttributes(ctx context.Context) error {
if err := repos.LoadOwners(ctx); err != nil {
return err
}

return repos.LoadLanguageStats(ctx)
}

// SearchRepoOptions holds the search options
type SearchRepoOptions struct {
db.ListOptions
Expand Down
15 changes: 12 additions & 3 deletions routers/api/v1/repo/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,20 @@ func ListForks(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

forks, err := repo_model.GetForks(ctx, ctx.Repo.Repository, utils.GetListOptions(ctx))
forks, total, err := repo_service.FindForks(ctx, ctx.Repo.Repository, ctx.Doer, utils.GetListOptions(ctx))
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetForks", err)
ctx.Error(http.StatusInternalServerError, "FindForks", err)
return
}
if err := repo_model.RepositoryList(forks).LoadOwners(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadOwners", err)
return
}
if err := repo_model.RepositoryList(forks).LoadUnits(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadUnits", err)
return
}

apiForks := make([]*api.Repository, len(forks))
for i, fork := range forks {
permission, err := access_model.GetUserRepoPermission(ctx, fork, ctx.Doer)
Expand All @@ -70,7 +79,7 @@ func ListForks(ctx *context.APIContext) {
apiForks[i] = convert.ToRepo(ctx, fork, permission)
}

ctx.SetTotalCountHeader(int64(ctx.Repo.Repository.NumForks))
ctx.SetTotalCountHeader(total)
ctx.JSON(http.StatusOK, apiForks)
}

Expand Down
23 changes: 11 additions & 12 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -1151,26 +1151,25 @@ func Forks(ctx *context.Context) {
if page <= 0 {
page = 1
}
pageSize := setting.ItemsPerPage

pager := context.NewPagination(ctx.Repo.Repository.NumForks, setting.ItemsPerPage, page, 5)
ctx.Data["Page"] = pager

forks, err := repo_model.GetForks(ctx, ctx.Repo.Repository, db.ListOptions{
Page: pager.Paginater.Current(),
PageSize: setting.ItemsPerPage,
forks, total, err := repo_service.FindForks(ctx, ctx.Repo.Repository, ctx.Doer, db.ListOptions{
Page: page,
PageSize: pageSize,
})
if err != nil {
ctx.ServerError("GetForks", err)
ctx.ServerError("FindForks", err)
return
}

for _, fork := range forks {
if err = fork.LoadOwner(ctx); err != nil {
ctx.ServerError("LoadOwner", err)
return
}
if err := repo_model.RepositoryList(forks).LoadOwners(ctx); err != nil {
ctx.ServerError("LoadAttributes", err)
return
}

pager := context.NewPagination(int(total), pageSize, page, 5)
ctx.Data["Page"] = pager

ctx.Data["Forks"] = forks

ctx.HTML(http.StatusOK, tplForks)
Expand Down
2 changes: 1 addition & 1 deletion services/mirror/mirror_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
output := stderrBuilder.String()

if err := git.WriteCommitGraph(ctx, repoPath); err != nil {
log.Error("SyncMirrors [repo: %-v]: %v", m.Repo, err)
log.Error("SyncMirrors: WriteCommitGraph [repo: %-v]: %v", m.Repo, err)
}

gitRepo, err := gitrepo.OpenRepository(ctx, m.Repo)
Expand Down
23 changes: 23 additions & 0 deletions services/repository/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
Expand All @@ -20,6 +21,7 @@ import (
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
notify_service "code.gitea.io/gitea/services/notify"
"xorm.io/builder"
)

// ErrForkAlreadyExist represents a "ForkAlreadyExist" kind of error.
Expand Down Expand Up @@ -247,3 +249,24 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit

return err
}

type findForksOptions struct {
db.ListOptions
RepoID int64
Doer *user_model.User
}

func (opts findForksOptions) ToConds() builder.Cond {
return builder.Eq{"fork_id": opts.RepoID}.And(
repo_model.AccessibleRepositoryCondition(opts.Doer, unit.TypeInvalid),
)
}

// FindForks returns all the forks of the repository
func FindForks(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, listOptions db.ListOptions) ([]*repo_model.Repository, int64, error) {
return db.FindAndCount[repo_model.Repository](ctx, findForksOptions{
ListOptions: listOptions,
RepoID: repo.ID,
Doer: doer,
})
}

0 comments on commit b744d1c

Please # to comment.