mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-02-23 11:55:48 -05:00
fix: consider HEAD requests to be pulls (#6750)
Previously an anonymous GET request to e.g. https://codeberg.org/forgejo/forgejo/HEAD was allowed, as GET requests are considered pulls and those don't need authentication for a public repository, but a HEAD request to the same URL was rejected with a 401. Since the result of a HEAD request is a subset of the result of a GET request it is safe to allow HEAD as well. This isn't really a practical issue for Forgejo itself, but I have encountered this in https://codeberg.org/forgejo-aneksajo/forgejo-aneksajo/issues/40. Since the fix isn't git-annex specific I am proposing it here. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [X] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6750 Reviewed-by: Michael Kriese <michael.kriese@gmx.de> Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Matthias Riße <m.risse@fz-juelich.de> Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
This commit is contained in:
Matthias Riße
Earl Warren
parent
5961db5aa7
commit
7f4f3434ec
2 changed files with 136 additions and 1 deletions
|
|
@ -78,7 +78,7 @@ func httpBase(ctx *context.Context) *serviceHandler {
|
|||
strings.HasSuffix(ctx.Req.URL.Path, "git-upload-archive") {
|
||||
isPull = true
|
||||
} else {
|
||||
isPull = ctx.Req.Method == "GET"
|
||||
isPull = ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD"
|
||||
}
|
||||
|
||||
var accessMode perm.AccessMode
|
||||
|
|
|
|||
|
|
@ -1,14 +1,24 @@
|
|||
// Copyright 2021 The Gitea Authors. All rights reserved.
|
||||
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
auth_model "code.gitea.io/gitea/models/auth"
|
||||
"code.gitea.io/gitea/models/db"
|
||||
"code.gitea.io/gitea/models/perm"
|
||||
unit_model "code.gitea.io/gitea/models/unit"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/tests"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
|
@ -67,3 +77,128 @@ func testGitSmartHTTP(t *testing.T, u *url.URL) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Test that the git http endpoints have the same authentication behavior irrespective of if it is a GET or a HEAD request.
|
||||
func TestGitHTTPSameStatusCodeForGetAndHeadRequests(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
|
||||
type caseType struct {
|
||||
User *user_model.User
|
||||
IsCollaborator bool
|
||||
RepoIsPrivate bool
|
||||
Endpoint string
|
||||
ExpectedStatusCode int
|
||||
}
|
||||
cases := []caseType{
|
||||
{owner, false, false, "HEAD", 200},
|
||||
{owner, false, false, "git-receive-pack", 405},
|
||||
{owner, false, false, "git-upload-pack", 405},
|
||||
{owner, false, false, "info/refs", 200},
|
||||
{owner, false, false, "objects/info/alternates", 404},
|
||||
{owner, false, false, "objects/info/http-alternates", 404},
|
||||
{owner, false, false, "objects/info/packs", 200},
|
||||
{owner, false, true, "HEAD", 200},
|
||||
{owner, false, true, "git-receive-pack", 405},
|
||||
{owner, false, true, "git-upload-pack", 405},
|
||||
{owner, false, true, "info/refs", 200},
|
||||
{owner, false, true, "objects/info/alternates", 404},
|
||||
{owner, false, true, "objects/info/http-alternates", 404},
|
||||
{owner, false, true, "objects/info/packs", 200},
|
||||
{user2, false, false, "HEAD", 200},
|
||||
{user2, false, false, "git-receive-pack", 405},
|
||||
{user2, false, false, "git-upload-pack", 405},
|
||||
{user2, false, false, "info/refs", 200},
|
||||
{user2, false, false, "objects/info/alternates", 404},
|
||||
{user2, false, false, "objects/info/http-alternates", 404},
|
||||
{user2, false, false, "objects/info/packs", 200},
|
||||
{user2, false, true, "HEAD", 404},
|
||||
{user2, false, true, "git-receive-pack", 405},
|
||||
{user2, false, true, "git-upload-pack", 405},
|
||||
{user2, false, true, "info/refs", 404},
|
||||
{user2, false, true, "objects/info/alternates", 404},
|
||||
{user2, false, true, "objects/info/http-alternates", 404},
|
||||
{user2, false, true, "objects/info/packs", 404},
|
||||
// user2 with IsCollaborator=true must come after IsCollaborator=false, because
|
||||
// the addition as a collaborator is not reset
|
||||
{user2, true, false, "HEAD", 200},
|
||||
{user2, true, false, "git-receive-pack", 405},
|
||||
{user2, true, false, "git-upload-pack", 405},
|
||||
{user2, true, false, "info/refs", 200},
|
||||
{user2, true, false, "objects/info/alternates", 404},
|
||||
{user2, true, false, "objects/info/http-alternates", 404},
|
||||
{user2, true, false, "objects/info/packs", 200},
|
||||
{user2, true, true, "HEAD", 200},
|
||||
{user2, true, true, "git-receive-pack", 405},
|
||||
{user2, true, true, "git-upload-pack", 405},
|
||||
{user2, true, true, "info/refs", 200},
|
||||
{user2, true, true, "objects/info/alternates", 404},
|
||||
{user2, true, true, "objects/info/http-alternates", 404},
|
||||
{user2, true, true, "objects/info/packs", 200},
|
||||
{nil, false, false, "HEAD", 200},
|
||||
{nil, false, false, "git-receive-pack", 405},
|
||||
{nil, false, false, "git-upload-pack", 405},
|
||||
{nil, false, false, "info/refs", 200},
|
||||
{nil, false, false, "objects/info/alternates", 404},
|
||||
{nil, false, false, "objects/info/http-alternates", 404},
|
||||
{nil, false, false, "objects/info/packs", 200},
|
||||
{nil, false, true, "HEAD", 401},
|
||||
{nil, false, true, "git-receive-pack", 405},
|
||||
{nil, false, true, "git-upload-pack", 405},
|
||||
{nil, false, true, "info/refs", 401},
|
||||
{nil, false, true, "objects/info/alternates", 401},
|
||||
{nil, false, true, "objects/info/http-alternates", 401},
|
||||
{nil, false, true, "objects/info/packs", 401},
|
||||
}
|
||||
|
||||
caseToTestName := func(c caseType) string {
|
||||
var user string
|
||||
if c.User == nil {
|
||||
user = "nil"
|
||||
} else if c.User == owner {
|
||||
user = "owner"
|
||||
} else {
|
||||
user = c.User.Name
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"User=%s,IsCollaborator=%t,RepoIsPrivate=%t,Endpoint=%s,ExpectedStatusCode=%d",
|
||||
user,
|
||||
c.IsCollaborator,
|
||||
c.RepoIsPrivate,
|
||||
c.Endpoint,
|
||||
c.ExpectedStatusCode,
|
||||
)
|
||||
}
|
||||
|
||||
repo, _, f := tests.CreateDeclarativeRepo(t, owner, "get-and-head-requests", []unit_model.Type{unit_model.TypeCode}, nil, nil)
|
||||
defer f()
|
||||
|
||||
for _, c := range cases {
|
||||
t.Run(caseToTestName(c), func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
session := emptyTestSession(t)
|
||||
if c.User != nil {
|
||||
session = loginUser(t, c.User.Name)
|
||||
}
|
||||
if c.IsCollaborator {
|
||||
testCtx := NewAPITestContext(t, owner.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
doAPIAddCollaborator(testCtx, c.User.Name, perm.AccessModeRead)(t)
|
||||
}
|
||||
repo.IsPrivate = c.RepoIsPrivate
|
||||
_, err := db.GetEngine(db.DefaultContext).Cols("is_private").Update(repo)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Given the test parameters check that the endpoint returns the same status
|
||||
// code for both GET and HEAD, which needs to equal the test cases expected
|
||||
// status code
|
||||
getReq := NewRequestf(t, "GET", "%s/%s", repo.Link(), c.Endpoint)
|
||||
getResp := session.MakeRequest(t, getReq, NoExpectedStatus)
|
||||
headReq := NewRequestf(t, "HEAD", "%s/%s", repo.Link(), c.Endpoint)
|
||||
headResp := session.MakeRequest(t, headReq, NoExpectedStatus)
|
||||
require.Equal(t, getResp.Result().StatusCode, headResp.Result().StatusCode)
|
||||
require.Equal(t, c.ExpectedStatusCode, headResp.Result().StatusCode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue