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

Make NuGet service index publicly accessible #21242

Merged
merged 7 commits into from
Sep 24, 2022
Merged

Make NuGet service index publicly accessible #21242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ac3ccc8
Make NuGet service index public.
KN4CK3R Sep 22, 2022
5576f9e
Merge branch 'main' into fix-20717-2
KN4CK3R Sep 23, 2022
f5693d0
Add comment.
KN4CK3R Sep 24, 2022
459ff08
Merge branch 'main' of https://github.com/go-gitea/gitea into fix-207…
KN4CK3R Sep 24, 2022
dfd45fb
Merge branch 'main' of https://github.com/go-gitea/gitea into fix-207…
KN4CK3R Sep 24, 2022
f7484b1
.
KN4CK3R Sep 24, 2022
c6fe79b
Merge branch 'main' into fix-20717-2
wxiaoguang Sep 24, 2022
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
54 changes: 28 additions & 26 deletions routers/api/packages/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func Routes(ctx gocontext.Context) *web.Route {
r.Get("/p2/{vendorname}/{projectname}.json", composer.PackageMetadata)
r.Get("/files/{package}/{version}/{filename}", composer.DownloadPackageFile)
r.Put("", reqPackageAccess(perm.AccessModeWrite), composer.UploadPackage)
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/conan", func() {
r.Group("/v1", func() {
r.Get("/ping", conan.Ping)
Expand Down Expand Up @@ -157,7 +157,7 @@ func Routes(ctx gocontext.Context) *web.Route {
}, conan.ExtractPathParameters)
})
})
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/generic", func() {
r.Group("/{packagename}/{packageversion}", func() {
r.Delete("", reqPackageAccess(perm.AccessModeWrite), generic.DeletePackage)
Expand All @@ -169,33 +169,35 @@ func Routes(ctx gocontext.Context) *web.Route {
}, reqPackageAccess(perm.AccessModeWrite))
})
})
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/helm", func() {
r.Get("/index.yaml", helm.Index)
r.Get("/{filename}", helm.DownloadPackageFile)
r.Post("/api/charts", reqPackageAccess(perm.AccessModeWrite), helm.UploadPackage)
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/maven", func() {
r.Put("/*", reqPackageAccess(perm.AccessModeWrite), maven.UploadPackageFile)
r.Get("/*", maven.DownloadPackageFile)
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/nuget", func() {
r.Get("/index.json", nuget.ServiceIndex)
r.Get("/query", nuget.SearchService)
r.Group("/registration/{id}", func() {
r.Get("/index.json", nuget.RegistrationIndex)
r.Get("/{version}", nuget.RegistrationLeaf)
})
r.Group("/package/{id}", func() {
r.Get("/index.json", nuget.EnumeratePackageVersions)
r.Get("/{version}/{filename}", nuget.DownloadPackageFile)
})
r.Get("/index.json", nuget.ServiceIndex) // Needs to be unauthenticated for the NuGet client.
r.Group("", func() {
r.Put("/", nuget.UploadPackage)
r.Put("/symbolpackage", nuget.UploadSymbolPackage)
r.Delete("/{id}/{version}", nuget.DeletePackage)
}, reqPackageAccess(perm.AccessModeWrite))
r.Get("/symbols/{filename}/{guid:[0-9a-f]{32}}FFFFFFFF/{filename2}", nuget.DownloadSymbolFile)
r.Get("/query", nuget.SearchService)
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, while I think it needs more comments about why there is no permission check for it.

And, maybe someone will worry about "attackers can detect private user names" again, while I think it's acceptable and that's cost of using NuGet package registry.

Copy link
Member Author

Choose a reason for hiding this comment

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

And, maybe someone will worry about "attackers can detect private user names" again, while I think it's acceptable and that's cost of using NuGet package registry.

You can detect that with the /users/{user} api too, so I see no difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, so LGTM 😁

Copy link
Member Author

@KN4CK3R KN4CK3R Sep 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think that's a definition problem of private. Private like "there is a locked door I know something is behind" or private like "which door?".

r.Group("/registration/{id}", func() {
r.Get("/index.json", nuget.RegistrationIndex)
r.Get("/{version}", nuget.RegistrationLeaf)
})
r.Group("/package/{id}", func() {
r.Get("/index.json", nuget.EnumeratePackageVersions)
r.Get("/{version}/{filename}", nuget.DownloadPackageFile)
})
r.Group("", func() {
r.Put("/", nuget.UploadPackage)
r.Put("/symbolpackage", nuget.UploadSymbolPackage)
r.Delete("/{id}/{version}", nuget.DeletePackage)
}, reqPackageAccess(perm.AccessModeWrite))
r.Get("/symbols/{filename}/{guid:[0-9a-f]{32}}FFFFFFFF/{filename2}", nuget.DownloadSymbolFile)
}, reqPackageAccess(perm.AccessModeRead))
})
r.Group("/npm", func() {
r.Group("/@{scope}/{id}", func() {
Expand Down Expand Up @@ -239,7 +241,7 @@ func Routes(ctx gocontext.Context) *web.Route {
r.Group("/-/v1/search", func() {
r.Get("", npm.PackageSearch)
})
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/pub", func() {
r.Group("/api/packages", func() {
r.Group("/versions/new", func() {
Expand All @@ -253,12 +255,12 @@ func Routes(ctx gocontext.Context) *web.Route {
r.Get("/{version}", pub.PackageVersionMetadata)
})
})
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/pypi", func() {
r.Post("/", reqPackageAccess(perm.AccessModeWrite), pypi.UploadPackageFile)
r.Get("/files/{id}/{version}/{filename}", pypi.DownloadPackageFile)
r.Get("/simple/{id}", pypi.PackageMetadata)
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/rubygems", func() {
r.Get("/specs.4.8.gz", rubygems.EnumeratePackages)
r.Get("/latest_specs.4.8.gz", rubygems.EnumeratePackagesLatest)
Expand All @@ -269,7 +271,7 @@ func Routes(ctx gocontext.Context) *web.Route {
r.Post("/", rubygems.UploadPackageFile)
r.Delete("/yank", rubygems.DeletePackage)
}, reqPackageAccess(perm.AccessModeWrite))
})
}, reqPackageAccess(perm.AccessModeRead))
r.Group("/vagrant", func() {
r.Group("/authenticate", func() {
r.Get("", vagrant.CheckAuthenticate)
Expand All @@ -282,8 +284,8 @@ func Routes(ctx gocontext.Context) *web.Route {
r.Put("", reqPackageAccess(perm.AccessModeWrite), vagrant.UploadPackageFile)
})
})
})
}, context_service.UserAssignmentWeb(), context.PackageAssignment(), reqPackageAccess(perm.AccessModeRead))
}, reqPackageAccess(perm.AccessModeRead))
}, context_service.UserAssignmentWeb(), context.PackageAssignment())

return r
}
Expand Down
80 changes: 50 additions & 30 deletions tests/integration/api_packages_nuget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
nuget_module "code.gitea.io/gitea/modules/packages/nuget"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/routers/api/packages/nuget"
"code.gitea.io/gitea/tests"

Expand Down Expand Up @@ -66,39 +67,58 @@ func TestPackageNuGet(t *testing.T) {
t.Run("ServiceIndex", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

req := NewRequest(t, "GET", fmt.Sprintf("%s/index.json", url))
req = AddBasicAuthHeader(req, user.Name)
MakeRequest(t, req, http.StatusOK)
privateUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{Visibility: structs.VisibleTypePrivate})

req = NewRequest(t, "GET", fmt.Sprintf("%s/index.json", url))
req = addNuGetAPIKeyHeader(req, token)
resp := MakeRequest(t, req, http.StatusOK)
cases := []struct {
Owner string
UseBasicAuth bool
UseTokenAuth bool
}{
{privateUser.Name, false, false},
{privateUser.Name, true, false},
{privateUser.Name, false, true},
{user.Name, false, false},
{user.Name, true, false},
{user.Name, false, true},
}

var result nuget.ServiceIndexResponse
DecodeJSON(t, resp, &result)
for _, c := range cases {
url := fmt.Sprintf("/api/packages/%s/nuget", c.Owner)

assert.Equal(t, "3.0.0", result.Version)
assert.NotEmpty(t, result.Resources)

root := setting.AppURL + url[1:]
for _, r := range result.Resources {
switch r.Type {
case "SearchQueryService":
fallthrough
case "SearchQueryService/3.0.0-beta":
fallthrough
case "SearchQueryService/3.0.0-rc":
assert.Equal(t, root+"/query", r.ID)
case "RegistrationsBaseUrl":
fallthrough
case "RegistrationsBaseUrl/3.0.0-beta":
fallthrough
case "RegistrationsBaseUrl/3.0.0-rc":
assert.Equal(t, root+"/registration", r.ID)
case "PackageBaseAddress/3.0.0":
assert.Equal(t, root+"/package", r.ID)
case "PackagePublish/2.0.0":
assert.Equal(t, root, r.ID)
req := NewRequest(t, "GET", fmt.Sprintf("%s/index.json", url))
if c.UseBasicAuth {
req = AddBasicAuthHeader(req, user.Name)
} else if c.UseTokenAuth {
req = addNuGetAPIKeyHeader(req, token)
}
resp := MakeRequest(t, req, http.StatusOK)

var result nuget.ServiceIndexResponse
DecodeJSON(t, resp, &result)

assert.Equal(t, "3.0.0", result.Version)
assert.NotEmpty(t, result.Resources)

root := setting.AppURL + url[1:]
for _, r := range result.Resources {
switch r.Type {
case "SearchQueryService":
fallthrough
case "SearchQueryService/3.0.0-beta":
fallthrough
case "SearchQueryService/3.0.0-rc":
assert.Equal(t, root+"/query", r.ID)
case "RegistrationsBaseUrl":
fallthrough
case "RegistrationsBaseUrl/3.0.0-beta":
fallthrough
case "RegistrationsBaseUrl/3.0.0-rc":
assert.Equal(t, root+"/registration", r.ID)
case "PackageBaseAddress/3.0.0":
assert.Equal(t, root+"/package", r.ID)
case "PackagePublish/2.0.0":
assert.Equal(t, root, r.ID)
}
}
}
})
Expand Down