Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement API endpoint to link a package to a repo #23851

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+218 −11
Open

Implement API endpoint to link a package to a repo #23851

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
339af37
Implement API endpoint to link a package to a repo
sclu1034 Mar 30, 2023
84a2bc3
Add integration test for package linking API
sclu1034 Mar 31, 2023
10df3c4
Rename function
sclu1034 Apr 2, 2023
6e2661b
Improve error reporting
sclu1034 May 2, 2023
f07cfa9
Fix package repository link permission check
sclu1034 Jan 17, 2024
9fb1f1c
Refactor router group
sclu1034 Jan 22, 2024
221dcdc
Allow {owner}/{name} repo format when linking package
sclu1034 Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions routers/api/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,6 +1412,7 @@ func Routes() *web.Route {
m.Delete("", reqToken(), reqPackageAccess(perm.AccessModeWrite), packages.DeletePackage)
m.Get("/files", reqToken(), packages.ListPackageFiles)
})
m.Post("/{type}/{name}/link", reqToken(), reqPackageAccess(perm.AccessModeWrite), packages.LinkPackage)
sclu1034 marked this conversation as resolved.
Show resolved Hide resolved
m.Get("/", reqToken(), packages.ListPackages)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryPackage), context_service.UserAssignmentAPI(), context.PackageAssignmentAPI(), reqPackageAccess(perm.AccessModeRead))

Expand Down
52 changes: 52 additions & 0 deletions routers/api/v1/packages/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package packages

import (
"errors"
"net/http"

"code.gitea.io/gitea/models/packages"
Expand Down Expand Up @@ -213,3 +214,54 @@ func ListPackageFiles(ctx *context.APIContext) {

ctx.JSON(http.StatusOK, apiPackageFiles)
}

// LinkPackage sets a repository link for a package
func LinkPackage(ctx *context.APIContext) {
// swagger:operation POST /packages/{owner}/{type}/{name}/link package linkPackage
// ---
// summary: Link a package to a repository
// parameters:
// - name: owner
// in: path
// description: owner of the package
// type: string
// required: true
// - name: type
// in: path
// description: type of the package
// type: string
// required: true
// - name: name
// in: path
// description: name of the package
// type: string
// required: true
// - name: repo
// in: query
// description: ID of the repository to link
// type: integer
// required: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if you can't just pass the repository id (which you don't have usually) but the repo name.
Could have two parameters repo_id=... and repo_name=.... The endpoint should choose the matching repo then. Could prefer the id over the name.

Copy link
Author

@sclu1034 sclu1034 Jan 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not particularly proud of how my code parsing those two parameters turned out, but I didn't find any useful utilities.
Any ideas to improve 221dcdc?

// responses:
// "201":
// "$ref": "#/responses/empty"
// "404":
// "$ref": "#/responses/notFound"

pkg, err := packages.GetPackageByName(ctx, ctx.ContextUser.ID, packages.Type(ctx.Params("type")), ctx.Params("name"))
if err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "GetPackageByName", err)
} else {
ctx.Error(http.StatusInternalServerError, "GetPackageByName", err)
}
return
}

err = packages_service.LinkPackageToRepository(ctx, ctx.Doer, pkg, ctx.FormInt64("repo"))
if err != nil {
ctx.Error(http.StatusInternalServerError, "LinkPackageToRepository", err)
return
}

ctx.Status(http.StatusNoContent)
}
34 changes: 34 additions & 0 deletions services/packages/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (

"code.gitea.io/gitea/models/db"
packages_model "code.gitea.io/gitea/models/packages"
access_model "code.gitea.io/gitea/models/perm/access"
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/json"
"code.gitea.io/gitea/modules/log"
Expand Down Expand Up @@ -657,3 +659,35 @@ func RemoveAllPackages(ctx context.Context, userID int64) (int, error) {
}
return count, nil
}

func LinkPackageToRepository(ctx context.Context, doer *user_model.User, p *packages_model.Package, repoID int64) error {
if repoID != 0 {
repo, err := repo_model.GetRepositoryByID(ctx, repoID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This MUST check if the repo belongs to the package owner. And there should be a test for linking a repo of another user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid further back and forth on permissions, and me doing best-effort guesses, can you give me a detailed run down on who should be allowed to do what?

if err != nil {
return fmt.Errorf("Error getting repository %d: %w", repoID, err)
}

canWrite := repo.OwnerID == doer.ID

if !canWrite {
perms, err := access_model.GetUserRepoPermission(ctx, repo, doer)
if err != nil {
return fmt.Errorf("Error getting repository permissions for %d on %d: %w", doer.ID, repo.ID, err)
}

canWrite = perms.CanWrite(unit.TypePackages)
}

if !canWrite {
return fmt.Errorf("No permissions to link this package and repository")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !canWrite {
perms, err := access_model.GetUserRepoPermission(ctx, repo, doer)
if err != nil {
return fmt.Errorf("Error getting repository permissions for %d on %d: %w", doer.ID, repo.ID, err)
}
canWrite = perms.CanWrite(unit.TypePackages)
}
if !canWrite {
return fmt.Errorf("No permissions to link this package and repository")
}
perms, err := access_model.GetUserRepoPermission(ctx, repo, doer)
if err != nil {
return fmt.Errorf("Error getting repository permissions for %d on %d: %w", doer.ID, repo.ID, err)
}
if !perms.CanWrite(unit.TypePackages) {
return fmt.Errorf("No permissions to link this package and repository")
}

It is also possible that packages are disabled for this repo, so I think we should check for all users, not just if user isn't the owner.


repoID = repo.ID
sclu1034 marked this conversation as resolved.
Show resolved Hide resolved
}

if err := packages_model.SetRepositoryLink(ctx, p.ID, repoID); err != nil {
return fmt.Errorf("Error updating package: %w", err)
}

return nil
}
47 changes: 47 additions & 0 deletions templates/swagger/v1_json.tmpl
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 32 additions & 6 deletions tests/integration/api_packages_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/db"
packages_model "code.gitea.io/gitea/models/packages"
container_model "code.gitea.io/gitea/models/packages/container"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -31,9 +32,17 @@ import (
func TestPackageAPI(t *testing.T) {
defer tests.PrepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
session := loginUser(t, user.Name)
tokenReadPackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage)
tokenWritePackage := getTokenForLoggedInUser(
t,
session,
auth_model.AccessTokenScopeWritePackage,
auth_model.AccessTokenScopeReadPackage,
auth_model.AccessTokenScopeWriteRepository,
)
tokenDeletePackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWritePackage)

packageName := "test-package"
Expand Down Expand Up @@ -99,7 +108,8 @@ func TestPackageAPI(t *testing.T) {
assert.Nil(t, ap1.Repository)

// link to public repository
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, 1))
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo=%d", user.Name, packageName, repo.ID)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusNoContent)

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).
AddTokenAuth(tokenReadPackage)
Expand All @@ -108,10 +118,15 @@ func TestPackageAPI(t *testing.T) {
var ap2 *api.Package
DecodeJSON(t, resp, &ap2)
assert.NotNil(t, ap2.Repository)
assert.EqualValues(t, 1, ap2.Repository.ID)
assert.EqualValues(t, repo.ID, ap2.Repository.ID)

// link to private repository
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, 2))
// link to repository without write access, should fail
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo=%d", user.Name, packageName, 3)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusInternalServerError)

// remove link
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo=%d", user.Name, packageName, 0)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusNoContent)

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).
AddTokenAuth(tokenReadPackage)
Expand All @@ -121,7 +136,18 @@ func TestPackageAPI(t *testing.T) {
DecodeJSON(t, resp, &ap3)
assert.Nil(t, ap3.Repository)

assert.NoError(t, packages_model.UnlinkRepositoryFromAllPackages(db.DefaultContext, 2))
// force link to a repository the currently logged-in user doesn't have access to
privateRepoID := int64(6)
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, privateRepoID))

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).AddTokenAuth(tokenReadPackage)
resp = MakeRequest(t, req, http.StatusOK)

var ap4 *api.Package
DecodeJSON(t, resp, &ap4)
assert.Nil(t, ap4.Repository)

assert.NoError(t, packages_model.UnlinkRepositoryFromAllPackages(db.DefaultContext, privateRepoID))
})
})

Expand Down