diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index e2e3582088..91ee060d50 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -22,6 +22,23 @@ type FileOptions struct { Signoff bool `json:"signoff"` } +type FileOptionsWithSHA struct { + FileOptions + // the blob ID (SHA) for the file that already exists, it is required for changing existing files + // required: true + SHA string `json:"sha" binding:"Required"` +} + +func (f *FileOptions) GetFileOptions() *FileOptions { + return f +} + +type FileOptionsInterface interface { + GetFileOptions() *FileOptions +} + +var _ FileOptionsInterface = (*FileOptions)(nil) + // CreateFileOptions options for creating files // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type CreateFileOptions struct { @@ -31,29 +48,16 @@ type CreateFileOptions struct { ContentBase64 string `json:"content"` } -// Branch returns branch name -func (o *CreateFileOptions) Branch() string { - return o.FileOptions.BranchName -} - // DeleteFileOptions options for deleting files (used for other File structs below) // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type DeleteFileOptions struct { - FileOptions - // sha is the SHA for the file that already exists - // required: true - SHA string `json:"sha" binding:"Required"` -} - -// Branch returns branch name -func (o *DeleteFileOptions) Branch() string { - return o.FileOptions.BranchName + FileOptionsWithSHA } // UpdateFileOptions options for updating files // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type UpdateFileOptions struct { - DeleteFileOptions + FileOptionsWithSHA // content must be base64 encoded // required: true ContentBase64 string `json:"content"` @@ -61,25 +65,21 @@ type UpdateFileOptions struct { FromPath string `json:"from_path" binding:"MaxSize(500)"` } -// Branch returns branch name -func (o *UpdateFileOptions) Branch() string { - return o.FileOptions.BranchName -} - -// FIXME: ChangeFileOperation.SHA is NOT required for update or delete if last commit is provided in the options. +// FIXME: there is no LastCommitID in FileOptions, actually it should be an alternative to the SHA in ChangeFileOperation // ChangeFileOperation for creating, updating or deleting a file type ChangeFileOperation struct { - // indicates what to do with the file + // indicates what to do with the file: "create" for creating a new file, "update" for updating an existing file, + // "upload" for creating or updating a file, "rename" for renaming a file, and "delete" for deleting an existing file. // required: true - // enum: create,update,delete + // enum: create,update,upload,rename,delete Operation string `json:"operation" binding:"Required"` // path to the existing or new file // required: true Path string `json:"path" binding:"Required;MaxSize(500)"` - // new or updated file content, must be base64 encoded + // new or updated file content, it must be base64 encoded ContentBase64 string `json:"content"` - // sha is the SHA for the file that already exists, required for update or delete + // the blob ID (SHA) for the file that already exists, required for changing existing files SHA string `json:"sha"` // old path of the file to move FromPath string `json:"from_path"` @@ -94,20 +94,10 @@ type ChangeFilesOptions struct { Files []*ChangeFileOperation `json:"files" binding:"Required"` } -// Branch returns branch name -func (o *ChangeFilesOptions) Branch() string { - return o.FileOptions.BranchName -} - -// FileOptionInterface provides a unified interface for the different file options -type FileOptionInterface interface { - Branch() string -} - // ApplyDiffPatchFileOptions options for applying a diff patch // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type ApplyDiffPatchFileOptions struct { - DeleteFileOptions + FileOptions // required: true Content string `json:"content"` } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 9171c0fadb..4a4bf12657 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -455,15 +455,6 @@ func reqRepoWriter(unitTypes ...unit.Type) func(ctx *context.APIContext) { } } -// reqRepoBranchWriter user should have a permission to write to a branch, or be a site admin -func reqRepoBranchWriter(ctx *context.APIContext) { - options, ok := web.GetForm(ctx).(api.FileOptionInterface) - if !ok || (!ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, options.Branch()) && !ctx.IsUserSiteAdmin()) { - ctx.APIError(http.StatusForbidden, "user should have a permission to write to this branch") - return - } -} - // reqRepoReader user should have specific read permission or be a repo admin or a site admin func reqRepoReader(unitType unit.Type) func(ctx *context.APIContext) { return func(ctx *context.APIContext) { @@ -744,9 +735,17 @@ func mustEnableWiki(ctx *context.APIContext) { } } +// FIXME: for consistency, maybe most mustNotBeArchived checks should be replaced with mustEnableEditor func mustNotBeArchived(ctx *context.APIContext) { if ctx.Repo.Repository.IsArchived { - ctx.APIError(http.StatusLocked, fmt.Errorf("%s is archived", ctx.Repo.Repository.LogString())) + ctx.APIError(http.StatusLocked, fmt.Errorf("%s is archived", ctx.Repo.Repository.FullName())) + return + } +} + +func mustEnableEditor(ctx *context.APIContext) { + if !ctx.Repo.Repository.CanEnableEditor() { + ctx.APIError(http.StatusLocked, fmt.Errorf("%s is not allowed to edit", ctx.Repo.Repository.FullName())) return } } @@ -1424,16 +1423,19 @@ func Routes() *web.Router { m.Get("/tags/{sha}", repo.GetAnnotatedTag) m.Get("/notes/{sha}", repo.GetNote) }, context.ReferencesGitRepo(true), reqRepoReader(unit.TypeCode)) - m.Post("/diffpatch", reqRepoWriter(unit.TypeCode), reqToken(), bind(api.ApplyDiffPatchFileOptions{}), mustNotBeArchived, repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) m.Get("/*", repo.GetContents) - m.Post("", reqToken(), bind(api.ChangeFilesOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.ChangeFiles) - m.Group("/*", func() { - m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.CreateFile) - m.Put("", bind(api.UpdateFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.UpdateFile) - m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, mustNotBeArchived, repo.DeleteFile) - }, reqToken()) + m.Group("", func() { + // "change file" operations, need permission to write to the target branch provided by the form + m.Post("", bind(api.ChangeFilesOptions{}), repo.ReqChangeRepoFileOptionsAndCheck, repo.ChangeFiles) + m.Group("/*", func() { + m.Post("", bind(api.CreateFileOptions{}), repo.ReqChangeRepoFileOptionsAndCheck, repo.CreateFile) + m.Put("", bind(api.UpdateFileOptions{}), repo.ReqChangeRepoFileOptionsAndCheck, repo.UpdateFile) + m.Delete("", bind(api.DeleteFileOptions{}), repo.ReqChangeRepoFileOptionsAndCheck, repo.DeleteFile) + }) + m.Post("/diffpatch", bind(api.ApplyDiffPatchFileOptions{}), repo.ReqChangeRepoFileOptionsAndCheck, repo.ApplyDiffPatch) + }, mustEnableEditor, reqToken()) }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) m.Group("/contents-ext", func() { m.Get("", repo.GetContentsExt) @@ -1441,7 +1443,7 @@ func Routes() *web.Router { }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) m.Combo("/file-contents", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()). Get(repo.GetFileContentsGet). - Post(bind(api.GetFilesOptions{}), repo.GetFileContentsPost) // POST method requires "write" permission, so we also support "GET" method above + Post(bind(api.GetFilesOptions{}), repo.GetFileContentsPost) // the POST method requires "write" permission, so we also support "GET" method above m.Get("/signing-key.gpg", misc.SigningKeyGPG) m.Get("/signing-key.pub", misc.SigningKeySSH) m.Group("/topics", func() { diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 76068c077f..8ce52c2cd4 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -62,7 +62,7 @@ func GetRawFile(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch" // type: string // required: false // responses: @@ -115,7 +115,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch" + // description: "The name of the commit/branch/tag. Default to the repository’s default branch" // type: string // required: false // responses: @@ -139,27 +139,27 @@ func GetRawFileOrLFS(ctx *context.APIContext) { ctx.RespHeader().Set(giteaObjectTypeHeader, string(files_service.GetObjectTypeFromTreeEntry(entry))) // LFS Pointer files are at most 1024 bytes - so any blob greater than 1024 bytes cannot be an LFS file - if blob.Size() > 1024 { + if blob.Size() > lfs.MetaFileMaxSize { // First handle caching for the blob if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { return } - // OK not cached - serve! + // If not cached - serve! if err := common.ServeBlob(ctx.Base, ctx.Repo.Repository, ctx.Repo.TreePath, blob, lastModified); err != nil { ctx.APIErrorInternal(err) } return } - // OK, now the blob is known to have at most 1024 bytes we can simply read this in one go (This saves reading it twice) + // OK, now the blob is known to have at most 1024 (lfs pointer max size) bytes, + // we can simply read this in one go (This saves reading it twice) dataRc, err := blob.DataAsync() if err != nil { ctx.APIErrorInternal(err) return } - // FIXME: code from #19689, what if the file is large ... OOM ... buf, err := io.ReadAll(dataRc) if err != nil { _ = dataRc.Close() @@ -181,7 +181,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } - // OK not cached - serve! + // If not cached - serve! common.ServeContentByReader(ctx.Base, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf)) return } @@ -405,13 +405,6 @@ func GetEditorconfig(ctx *context.APIContext) { ctx.JSON(http.StatusOK, def) } -// canWriteFiles returns true if repository is editable and user has proper access level. -func canWriteFiles(ctx *context.APIContext, branch string) bool { - return ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, branch) && - !ctx.Repo.Repository.IsMirror && - !ctx.Repo.Repository.IsArchived -} - func base64Reader(s string) (io.ReadSeeker, error) { b, err := base64.StdEncoding.DecodeString(s) if err != nil { @@ -420,6 +413,45 @@ func base64Reader(s string) (io.ReadSeeker, error) { return bytes.NewReader(b), nil } +func ReqChangeRepoFileOptionsAndCheck(ctx *context.APIContext) { + commonOpts := web.GetForm(ctx).(api.FileOptionsInterface).GetFileOptions() + commonOpts.BranchName = util.IfZero(commonOpts.BranchName, ctx.Repo.Repository.DefaultBranch) + commonOpts.NewBranchName = util.IfZero(commonOpts.NewBranchName, commonOpts.BranchName) + if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, commonOpts.NewBranchName) && !ctx.IsUserSiteAdmin() { + ctx.APIError(http.StatusForbidden, "user should have a permission to write to the target branch") + return + } + changeFileOpts := &files_service.ChangeRepoFilesOptions{ + Message: commonOpts.Message, + OldBranch: commonOpts.BranchName, + NewBranch: commonOpts.NewBranchName, + Committer: &files_service.IdentityOptions{ + GitUserName: commonOpts.Committer.Name, + GitUserEmail: commonOpts.Committer.Email, + }, + Author: &files_service.IdentityOptions{ + GitUserName: commonOpts.Author.Name, + GitUserEmail: commonOpts.Author.Email, + }, + Dates: &files_service.CommitDateOptions{ + Author: commonOpts.Dates.Author, + Committer: commonOpts.Dates.Committer, + }, + Signoff: commonOpts.Signoff, + } + if commonOpts.Dates.Author.IsZero() { + commonOpts.Dates.Author = time.Now() + } + if commonOpts.Dates.Committer.IsZero() { + commonOpts.Dates.Committer = time.Now() + } + ctx.Data["__APIChangeRepoFilesOptions"] = changeFileOpts +} + +func getAPIChangeRepoFileOptions[T api.FileOptionsInterface](ctx *context.APIContext) (apiOpts T, opts *files_service.ChangeRepoFilesOptions) { + return web.GetForm(ctx).(T), ctx.Data["__APIChangeRepoFilesOptions"].(*files_service.ChangeRepoFilesOptions) +} + // ChangeFiles handles API call for modifying multiple files func ChangeFiles(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/contents repository repoChangeFiles @@ -456,23 +488,18 @@ func ChangeFiles(ctx *context.APIContext) { // "$ref": "#/responses/error" // "423": // "$ref": "#/responses/repoArchivedError" - - apiOpts := web.GetForm(ctx).(*api.ChangeFilesOptions) - - if apiOpts.BranchName == "" { - apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch + apiOpts, opts := getAPIChangeRepoFileOptions[*api.ChangeFilesOptions](ctx) + if ctx.Written() { + return } - - var files []*files_service.ChangeRepoFile for _, file := range apiOpts.Files { contentReader, err := base64Reader(file.ContentBase64) if err != nil { ctx.APIError(http.StatusUnprocessableEntity, err) return } - // FIXME: actually now we support more operations like "rename", "upload" - // FIXME: ChangeFileOperation.SHA is NOT required for update or delete if last commit is provided in the options. - // Need to fully fix them in API + // FIXME: ChangeFileOperation.SHA is NOT required for update or delete if last commit is provided in the options + // But the LastCommitID is not provided in the API options, need to fully fix them in API changeRepoFile := &files_service.ChangeRepoFile{ Operation: file.Operation, TreePath: file.Path, @@ -480,41 +507,15 @@ func ChangeFiles(ctx *context.APIContext) { ContentReader: contentReader, SHA: file.SHA, } - files = append(files, changeRepoFile) - } - - opts := &files_service.ChangeRepoFilesOptions{ - Files: files, - Message: apiOpts.Message, - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, - Committer: &files_service.IdentityOptions{ - GitUserName: apiOpts.Committer.Name, - GitUserEmail: apiOpts.Committer.Email, - }, - Author: &files_service.IdentityOptions{ - GitUserName: apiOpts.Author.Name, - GitUserEmail: apiOpts.Author.Email, - }, - Dates: &files_service.CommitDateOptions{ - Author: apiOpts.Dates.Author, - Committer: apiOpts.Dates.Committer, - }, - Signoff: apiOpts.Signoff, - } - if opts.Dates.Author.IsZero() { - opts.Dates.Author = time.Now() - } - if opts.Dates.Committer.IsZero() { - opts.Dates.Committer = time.Now() + opts.Files = append(opts.Files, changeRepoFile) } if opts.Message == "" { - opts.Message = changeFilesCommitMessage(ctx, files) + opts.Message = changeFilesCommitMessage(ctx, opts.Files) } - if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { - handleCreateOrUpdateFileError(ctx, err) + if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { + handleChangeRepoFilesError(ctx, err) } else { ctx.JSON(http.StatusCreated, filesResponse) } @@ -562,56 +563,27 @@ func CreateFile(ctx *context.APIContext) { // "423": // "$ref": "#/responses/repoArchivedError" - apiOpts := web.GetForm(ctx).(*api.CreateFileOptions) - - if apiOpts.BranchName == "" { - apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch + apiOpts, opts := getAPIChangeRepoFileOptions[*api.CreateFileOptions](ctx) + if ctx.Written() { + return } - contentReader, err := base64Reader(apiOpts.ContentBase64) if err != nil { ctx.APIError(http.StatusUnprocessableEntity, err) return } - opts := &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: ctx.PathParam("*"), - ContentReader: contentReader, - }, - }, - Message: apiOpts.Message, - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, - Committer: &files_service.IdentityOptions{ - GitUserName: apiOpts.Committer.Name, - GitUserEmail: apiOpts.Committer.Email, - }, - Author: &files_service.IdentityOptions{ - GitUserName: apiOpts.Author.Name, - GitUserEmail: apiOpts.Author.Email, - }, - Dates: &files_service.CommitDateOptions{ - Author: apiOpts.Dates.Author, - Committer: apiOpts.Dates.Committer, - }, - Signoff: apiOpts.Signoff, - } - if opts.Dates.Author.IsZero() { - opts.Dates.Author = time.Now() - } - if opts.Dates.Committer.IsZero() { - opts.Dates.Committer = time.Now() - } - + opts.Files = append(opts.Files, &files_service.ChangeRepoFile{ + Operation: "create", + TreePath: ctx.PathParam("*"), + ContentReader: contentReader, + }) if opts.Message == "" { opts.Message = changeFilesCommitMessage(ctx, opts.Files) } - if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { - handleCreateOrUpdateFileError(ctx, err) + if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { + handleChangeRepoFilesError(ctx, err) } else { fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusCreated, fileResponse) @@ -659,96 +631,55 @@ func UpdateFile(ctx *context.APIContext) { // "$ref": "#/responses/error" // "423": // "$ref": "#/responses/repoArchivedError" - apiOpts := web.GetForm(ctx).(*api.UpdateFileOptions) - if ctx.Repo.Repository.IsEmpty { - ctx.APIError(http.StatusUnprocessableEntity, errors.New("repo is empty")) + + apiOpts, opts := getAPIChangeRepoFileOptions[*api.UpdateFileOptions](ctx) + if ctx.Written() { return } - - if apiOpts.BranchName == "" { - apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch - } - contentReader, err := base64Reader(apiOpts.ContentBase64) if err != nil { ctx.APIError(http.StatusUnprocessableEntity, err) return } - - opts := &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "update", - ContentReader: contentReader, - SHA: apiOpts.SHA, - FromTreePath: apiOpts.FromPath, - TreePath: ctx.PathParam("*"), - }, - }, - Message: apiOpts.Message, - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, - Committer: &files_service.IdentityOptions{ - GitUserName: apiOpts.Committer.Name, - GitUserEmail: apiOpts.Committer.Email, - }, - Author: &files_service.IdentityOptions{ - GitUserName: apiOpts.Author.Name, - GitUserEmail: apiOpts.Author.Email, - }, - Dates: &files_service.CommitDateOptions{ - Author: apiOpts.Dates.Author, - Committer: apiOpts.Dates.Committer, - }, - Signoff: apiOpts.Signoff, - } - if opts.Dates.Author.IsZero() { - opts.Dates.Author = time.Now() - } - if opts.Dates.Committer.IsZero() { - opts.Dates.Committer = time.Now() - } - + opts.Files = append(opts.Files, &files_service.ChangeRepoFile{ + Operation: "update", + ContentReader: contentReader, + SHA: apiOpts.SHA, + FromTreePath: apiOpts.FromPath, + TreePath: ctx.PathParam("*"), + }) if opts.Message == "" { opts.Message = changeFilesCommitMessage(ctx, opts.Files) } - if filesResponse, err := createOrUpdateFiles(ctx, opts); err != nil { - handleCreateOrUpdateFileError(ctx, err) + if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { + handleChangeRepoFilesError(ctx, err) } else { fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) } } -func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) { +func handleChangeRepoFilesError(ctx *context.APIContext, err error) { if files_service.IsErrUserCannotCommit(err) || pull_service.IsErrFilePathProtected(err) { ctx.APIError(http.StatusForbidden, err) return } if git_model.IsErrBranchAlreadyExists(err) || files_service.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || - files_service.IsErrFilePathInvalid(err) || files_service.IsErrRepoFileAlreadyExists(err) { + files_service.IsErrFilePathInvalid(err) || files_service.IsErrRepoFileAlreadyExists(err) || + files_service.IsErrCommitIDDoesNotMatch(err) || files_service.IsErrSHAOrCommitIDNotProvided(err) { ctx.APIError(http.StatusUnprocessableEntity, err) return } - if git_model.IsErrBranchNotExist(err) || git.IsErrBranchNotExist(err) { + if git.IsErrBranchNotExist(err) || files_service.IsErrRepoFileDoesNotExist(err) || git.IsErrNotExist(err) { ctx.APIError(http.StatusNotFound, err) return } - - ctx.APIErrorInternal(err) -} - -// Called from both CreateFile or UpdateFile to handle both -func createOrUpdateFiles(ctx *context.APIContext, opts *files_service.ChangeRepoFilesOptions) (*api.FilesResponse, error) { - if !canWriteFiles(ctx, opts.OldBranch) { - return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{ - UserID: ctx.Doer.ID, - RepoName: ctx.Repo.Repository.LowerName, - } + if errors.Is(err, util.ErrNotExist) { + ctx.APIError(http.StatusNotFound, err) + return } - - return files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts) + ctx.APIErrorInternal(err) } // format commit message if empty @@ -762,7 +693,7 @@ func changeFilesCommitMessage(ctx *context.APIContext, files []*files_service.Ch switch file.Operation { case "create": createFiles = append(createFiles, file.TreePath) - case "update": + case "update", "upload", "rename": // upload and rename works like "update", there is no translation for them at the moment updateFiles = append(updateFiles, file.TreePath) case "delete": deleteFiles = append(deleteFiles, file.TreePath) @@ -820,74 +751,27 @@ func DeleteFile(ctx *context.APIContext) { // "$ref": "#/responses/error" // "404": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/error" // "423": // "$ref": "#/responses/repoArchivedError" - apiOpts := web.GetForm(ctx).(*api.DeleteFileOptions) - if !canWriteFiles(ctx, apiOpts.BranchName) { - ctx.APIError(http.StatusForbidden, repo_model.ErrUserDoesNotHaveAccessToRepo{ - UserID: ctx.Doer.ID, - RepoName: ctx.Repo.Repository.LowerName, - }) + apiOpts, opts := getAPIChangeRepoFileOptions[*api.DeleteFileOptions](ctx) + if ctx.Written() { return } - if apiOpts.BranchName == "" { - apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch - } - - opts := &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "delete", - SHA: apiOpts.SHA, - TreePath: ctx.PathParam("*"), - }, - }, - Message: apiOpts.Message, - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, - Committer: &files_service.IdentityOptions{ - GitUserName: apiOpts.Committer.Name, - GitUserEmail: apiOpts.Committer.Email, - }, - Author: &files_service.IdentityOptions{ - GitUserName: apiOpts.Author.Name, - GitUserEmail: apiOpts.Author.Email, - }, - Dates: &files_service.CommitDateOptions{ - Author: apiOpts.Dates.Author, - Committer: apiOpts.Dates.Committer, - }, - Signoff: apiOpts.Signoff, - } - if opts.Dates.Author.IsZero() { - opts.Dates.Author = time.Now() - } - if opts.Dates.Committer.IsZero() { - opts.Dates.Committer = time.Now() - } - + opts.Files = append(opts.Files, &files_service.ChangeRepoFile{ + Operation: "delete", + SHA: apiOpts.SHA, + TreePath: ctx.PathParam("*"), + }) if opts.Message == "" { opts.Message = changeFilesCommitMessage(ctx, opts.Files) } if filesResponse, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, opts); err != nil { - if git.IsErrBranchNotExist(err) || files_service.IsErrRepoFileDoesNotExist(err) || git.IsErrNotExist(err) { - ctx.APIError(http.StatusNotFound, err) - return - } else if git_model.IsErrBranchAlreadyExists(err) || - files_service.IsErrFilenameInvalid(err) || - pull_service.IsErrSHADoesNotMatch(err) || - files_service.IsErrCommitIDDoesNotMatch(err) || - files_service.IsErrSHAOrCommitIDNotProvided(err) { - ctx.APIError(http.StatusBadRequest, err) - return - } else if files_service.IsErrUserCannotCommit(err) { - ctx.APIError(http.StatusForbidden, err) - return - } - ctx.APIErrorInternal(err) + handleChangeRepoFilesError(ctx, err) } else { fileResponse := files_service.GetFileResponseFromFilesResponse(filesResponse, 0) ctx.JSON(http.StatusOK, fileResponse) // FIXME on APIv2: return http.StatusNoContent @@ -911,6 +795,8 @@ func GetContentsExt(ctx *context.APIContext) { // summary: The extended "contents" API, to get file metadata and/or content, or list a directory. // description: It guarantees that only one of the response fields is set if the request succeeds. // Users can pass "includes=file_content" or "includes=lfs_metadata" to retrieve more fields. + // "includes=file_content" only works for single file, if you need to retrieve file contents in batch, + // use "file-contents" API after listing the directory. // produces: // - application/json // parameters: @@ -964,12 +850,11 @@ func GetContentsExt(ctx *context.APIContext) { ctx.JSON(http.StatusOK, getRepoContents(ctx, opts)) } -// GetContents Get the metadata and contents (if a file) of an entry in a repository, or a list of entries if a dir func GetContents(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/contents/{filepath} repository repoGetContents // --- // summary: Gets the metadata and contents (if a file) of an entry in a repository, or a list of entries if a dir. - // description: This API follows GitHub's design, and it is not easy to use. Recommend to use our "contents-ext" API instead. + // description: This API follows GitHub's design, and it is not easy to use. Recommend users to use the "contents-ext" API instead. // produces: // - application/json // parameters: @@ -1021,12 +906,11 @@ func getRepoContents(ctx *context.APIContext, opts files_service.GetContentsOrLi return &ret } -// GetContentsList Get the metadata of all the entries of the root dir func GetContentsList(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/contents repository repoGetContentsList // --- // summary: Gets the metadata of all the entries of the root dir. - // description: This API follows GitHub's design, and it is not easy to use. Recommend to use our "contents-ext" API instead. + // description: This API follows GitHub's design, and it is not easy to use. Recommend users to use our "contents-ext" API instead. // produces: // - application/json // parameters: @@ -1059,7 +943,7 @@ func GetFileContentsGet(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/file-contents repository repoGetFileContents // --- // summary: Get the metadata and contents of requested files - // description: See the POST method. This GET method supports to use JSON encoded request body in query parameter. + // description: See the POST method. This GET method supports using JSON encoded request body in query parameter. // produces: // - application/json // parameters: @@ -1089,7 +973,7 @@ func GetFileContentsGet(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - // POST method requires "write" permission, so we also support this "GET" method + // The POST method requires "write" permission, so we also support this "GET" method handleGetFileContents(ctx) } @@ -1133,7 +1017,7 @@ func GetFileContentsPost(ctx *context.APIContext) { // This is actually a "read" request, but we need to accept a "files" list, then POST method seems easy to use. // But the permission system requires that the caller must have "write" permission to use POST method. - // At the moment there is no other way to get around the permission check, so there is a "GET" workaround method above. + // At the moment, there is no other way to get around the permission check, so there is a "GET" workaround method above. handleGetFileContents(ctx) } diff --git a/routers/api/v1/repo/patch.go b/routers/api/v1/repo/patch.go index bcf498bf7e..e9f5cf5d90 100644 --- a/routers/api/v1/repo/patch.go +++ b/routers/api/v1/repo/patch.go @@ -5,15 +5,10 @@ package repo import ( "net/http" - "time" - git_model "code.gitea.io/gitea/models/git" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context" - pull_service "code.gitea.io/gitea/services/pull" "code.gitea.io/gitea/services/repository/files" ) @@ -49,63 +44,22 @@ func ApplyDiffPatch(ctx *context.APIContext) { // "$ref": "#/responses/notFound" // "423": // "$ref": "#/responses/repoArchivedError" - apiOpts := web.GetForm(ctx).(*api.ApplyDiffPatchFileOptions) - + apiOpts, changeRepoFileOpts := getAPIChangeRepoFileOptions[*api.ApplyDiffPatchFileOptions](ctx) opts := &files.ApplyDiffPatchOptions{ - Content: apiOpts.Content, - SHA: apiOpts.SHA, - Message: apiOpts.Message, - OldBranch: apiOpts.BranchName, - NewBranch: apiOpts.NewBranchName, - Committer: &files.IdentityOptions{ - GitUserName: apiOpts.Committer.Name, - GitUserEmail: apiOpts.Committer.Email, - }, - Author: &files.IdentityOptions{ - GitUserName: apiOpts.Author.Name, - GitUserEmail: apiOpts.Author.Email, - }, - Dates: &files.CommitDateOptions{ - Author: apiOpts.Dates.Author, - Committer: apiOpts.Dates.Committer, - }, - Signoff: apiOpts.Signoff, - } - if opts.Dates.Author.IsZero() { - opts.Dates.Author = time.Now() - } - if opts.Dates.Committer.IsZero() { - opts.Dates.Committer = time.Now() - } + Content: apiOpts.Content, + Message: util.IfZero(apiOpts.Message, "apply-patch"), - if opts.Message == "" { - opts.Message = "apply-patch" - } - - if !canWriteFiles(ctx, apiOpts.BranchName) { - ctx.APIErrorInternal(repo_model.ErrUserDoesNotHaveAccessToRepo{ - UserID: ctx.Doer.ID, - RepoName: ctx.Repo.Repository.LowerName, - }) - return + OldBranch: changeRepoFileOpts.OldBranch, + NewBranch: changeRepoFileOpts.NewBranch, + Committer: changeRepoFileOpts.Committer, + Author: changeRepoFileOpts.Author, + Dates: changeRepoFileOpts.Dates, + Signoff: changeRepoFileOpts.Signoff, } fileResponse, err := files.ApplyDiffPatch(ctx, ctx.Repo.Repository, ctx.Doer, opts) if err != nil { - if files.IsErrUserCannotCommit(err) || pull_service.IsErrFilePathProtected(err) { - ctx.APIError(http.StatusForbidden, err) - return - } - if git_model.IsErrBranchAlreadyExists(err) || files.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || - files.IsErrFilePathInvalid(err) || files.IsErrRepoFileAlreadyExists(err) { - ctx.APIError(http.StatusUnprocessableEntity, err) - return - } - if git_model.IsErrBranchNotExist(err) || git.IsErrBranchNotExist(err) { - ctx.APIError(http.StatusNotFound, err) - return - } - ctx.APIErrorInternal(err) + handleChangeRepoFilesError(ctx, err) } else { ctx.JSON(http.StatusCreated, fileResponse) } diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index ae0b74b019..9aee3d6a86 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -12,6 +12,7 @@ import ( "strings" git_model "code.gitea.io/gitea/models/git" + "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" @@ -138,6 +139,11 @@ func prepareEditorCommitSubmittedForm[T forms.CommitCommonFormInterface](ctx *co return nil } + if !issues.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, targetBranchName, ctx.Doer) { + ctx.NotFound(nil) + return nil + } + // Committer user info gitCommitter, valid := WebGitOperationGetCommitChosenEmailIdentity(ctx, commonForm.CommitEmail) if !valid { diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 5fbf748206..11a8744b7f 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -44,7 +44,6 @@ type ApplyDiffPatchOptions struct { NewBranch string Message string Content string - SHA string Author *IdentityOptions Committer *IdentityOptions Dates *CommitDateOptions diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 47951dc334..e871f777e5 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -113,7 +113,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } - // If no branch name is set, assume default branch + // If no branch name is set, assume the default branch if opts.OldBranch == "" { opts.OldBranch = repo.DefaultBranch } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index da0ba18b74..ff66bebfda 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7424,7 +7424,7 @@ }, "/repos/{owner}/{repo}/contents": { "get": { - "description": "This API follows GitHub's design, and it is not easy to use. Recommend to use our \"contents-ext\" API instead.", + "description": "This API follows GitHub's design, and it is not easy to use. Recommend users to use our \"contents-ext\" API instead.", "produces": [ "application/json" ], @@ -7521,7 +7521,7 @@ }, "/repos/{owner}/{repo}/contents-ext/{filepath}": { "get": { - "description": "It guarantees that only one of the response fields is set if the request succeeds. Users can pass \"includes=file_content\" or \"includes=lfs_metadata\" to retrieve more fields.", + "description": "It guarantees that only one of the response fields is set if the request succeeds. Users can pass \"includes=file_content\" or \"includes=lfs_metadata\" to retrieve more fields. \"includes=file_content\" only works for single file, if you need to retrieve file contents in batch, use \"file-contents\" API after listing the directory.", "produces": [ "application/json" ], @@ -7577,7 +7577,7 @@ }, "/repos/{owner}/{repo}/contents/{filepath}": { "get": { - "description": "This API follows GitHub's design, and it is not easy to use. Recommend to use our \"contents-ext\" API instead.", + "description": "This API follows GitHub's design, and it is not easy to use. Recommend users to use the \"contents-ext\" API instead.", "produces": [ "application/json" ], @@ -7802,6 +7802,9 @@ "404": { "$ref": "#/responses/error" }, + "422": { + "$ref": "#/responses/error" + }, "423": { "$ref": "#/responses/repoArchivedError" } @@ -7909,7 +7912,7 @@ }, "/repos/{owner}/{repo}/file-contents": { "get": { - "description": "See the POST method. This GET method supports to use JSON encoded request body in query parameter.", + "description": "See the POST method. This GET method supports using JSON encoded request body in query parameter.", "produces": [ "application/json" ], @@ -12876,7 +12879,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch", "name": "ref", "in": "query" } @@ -15020,7 +15023,7 @@ }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch", + "description": "The name of the commit/branch/tag. Default to the repository’s default branch", "name": "ref", "in": "query" } @@ -21867,7 +21870,7 @@ ], "properties": { "content": { - "description": "new or updated file content, must be base64 encoded", + "description": "new or updated file content, it must be base64 encoded", "type": "string", "x-go-name": "ContentBase64" }, @@ -21877,11 +21880,13 @@ "x-go-name": "FromPath" }, "operation": { - "description": "indicates what to do with the file", + "description": "indicates what to do with the file: \"create\" for creating a new file, \"update\" for updating an existing file,\n\"upload\" for creating or updating a file, \"rename\" for renaming a file, and \"delete\" for deleting an existing file.", "type": "string", "enum": [ "create", "update", + "upload", + "rename", "delete" ], "x-go-name": "Operation" @@ -21892,7 +21897,7 @@ "x-go-name": "Path" }, "sha": { - "description": "sha is the SHA for the file that already exists, required for update or delete", + "description": "the blob ID (SHA) for the file that already exists, required for changing existing files", "type": "string", "x-go-name": "SHA" } @@ -23657,7 +23662,7 @@ "x-go-name": "NewBranchName" }, "sha": { - "description": "sha is the SHA for the file that already exists", + "description": "the blob ID (SHA) for the file that already exists, it is required for changing existing files", "type": "string", "x-go-name": "SHA" }, @@ -28106,7 +28111,7 @@ "x-go-name": "NewBranchName" }, "sha": { - "description": "sha is the SHA for the file that already exists", + "description": "the blob ID (SHA) for the file that already exists, it is required for changing existing files", "type": "string", "x-go-name": "SHA" }, diff --git a/tests/integration/api_repo_file_delete_test.go b/tests/integration/api_repo_file_delete_test.go index 47730cd933..9dd47f93e6 100644 --- a/tests/integration/api_repo_file_delete_test.go +++ b/tests/integration/api_repo_file_delete_test.go @@ -20,20 +20,22 @@ import ( func getDeleteFileOptions() *api.DeleteFileOptions { return &api.DeleteFileOptions{ - FileOptions: api.FileOptions{ - BranchName: "master", - NewBranchName: "master", - Message: "Removing the file new/file.txt", - Author: api.Identity{ - Name: "John Doe", - Email: "johndoe@example.com", - }, - Committer: api.Identity{ - Name: "Jane Doe", - Email: "janedoe@example.com", + FileOptionsWithSHA: api.FileOptionsWithSHA{ + FileOptions: api.FileOptions{ + BranchName: "master", + NewBranchName: "master", + Message: "Removing the file new/file.txt", + Author: api.Identity{ + Name: "John Doe", + Email: "johndoe@example.com", + }, + Committer: api.Identity{ + Name: "Jane Doe", + Email: "janedoe@example.com", + }, }, + SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", }, - SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", } } @@ -110,7 +112,7 @@ func TestAPIDeleteFile(t *testing.T) { deleteFileOptions.SHA = "badsha" req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). AddTokenAuth(token2) - MakeRequest(t, req, http.StatusBadRequest) + MakeRequest(t, req, http.StatusUnprocessableEntity) // Test creating a file in repo16 by user4 who does not have write access fileID++ diff --git a/tests/integration/api_repo_file_update_test.go b/tests/integration/api_repo_file_update_test.go index 1605cfbd0b..6a7f7529a0 100644 --- a/tests/integration/api_repo_file_update_test.go +++ b/tests/integration/api_repo_file_update_test.go @@ -27,7 +27,7 @@ func getUpdateFileOptions() *api.UpdateFileOptions { content := "This is updated text" contentEncoded := base64.StdEncoding.EncodeToString([]byte(content)) return &api.UpdateFileOptions{ - DeleteFileOptions: api.DeleteFileOptions{ + FileOptionsWithSHA: api.FileOptionsWithSHA{ FileOptions: api.FileOptions{ BranchName: "master", NewBranchName: "master",