mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-15 21:31:29 +02:00
fix(security): enforce wiki git writes and LFS token access at request time (#37695)
This PR fixes two permission-checking gaps in Git and LFS request handling. ## What it changes - keep wiki Git HTTP pushes on the normal write-permission path, even when proc-receive support is enabled - revalidate LFS bearer token requests against the current user state and current repository permissions before allowing access - add regression coverage for unauthorized wiki HTTP pushes - add LFS tests for blocked users, revoked repository access, read-only upload attempts, and valid write access ## Why - wiki repositories should not inherit the relaxed refs/for handling used for normal code repositories - LFS authorization tokens should not remain usable after a user is disabled or loses repository access Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao
GitHub
parent
5b3575a8be
commit
f9b7b65371
@ -180,8 +180,8 @@ func httpBase(ctx *context.Context, optGitService ...string) *serviceHandler {
|
||||
}
|
||||
|
||||
if repoExist {
|
||||
// Because of special ref "refs/for" (agit) , need delay write permission check
|
||||
if git.DefaultFeatures().SupportProcReceive {
|
||||
// Only the main code repo accepts refs/for pushes, so wiki pushes must keep write checks.
|
||||
if git.DefaultFeatures().SupportProcReceive && !isWiki {
|
||||
accessMode = perm.AccessModeRead
|
||||
}
|
||||
|
||||
|
||||
@ -32,6 +32,7 @@ import (
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
"code.gitea.io/gitea/modules/storage"
|
||||
"code.gitea.io/gitea/modules/util"
|
||||
"code.gitea.io/gitea/services/context"
|
||||
|
||||
"github.com/golang-jwt/jwt/v5"
|
||||
@ -605,6 +606,18 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo
|
||||
log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err)
|
||||
return nil, err
|
||||
}
|
||||
if !u.IsActive || u.ProhibitLogin {
|
||||
return nil, util.NewPermissionDeniedErrorf("not allowed to access any repository")
|
||||
}
|
||||
|
||||
perm, err := access_model.GetDoerRepoPermission(ctx, target, u)
|
||||
if err != nil {
|
||||
log.Error("Unable to GetDoerRepoPermission for user[%d] repo[%d]: %v", claims.UserID, target.ID, err)
|
||||
return nil, err
|
||||
}
|
||||
if !perm.CanAccess(mode, unit.TypeCode) {
|
||||
return nil, util.NewPermissionDeniedErrorf("no permission to access the repository")
|
||||
}
|
||||
return u, nil
|
||||
}
|
||||
|
||||
|
||||
@ -7,9 +7,11 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
perm_model "code.gitea.io/gitea/models/perm"
|
||||
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/services/contexttest"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@ -22,11 +24,15 @@ func TestMain(m *testing.M) {
|
||||
|
||||
func TestAuthenticate(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
ctx, _ := contexttest.MockContext(t, "/")
|
||||
|
||||
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
|
||||
|
||||
token2, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: "download", UserID: 2, RepoID: 1})
|
||||
_, token2, _ = strings.Cut(token2, " ")
|
||||
ctx, _ := contexttest.MockContext(t, "/")
|
||||
getUserToken := func(op string, userID int64, repo *repo_model.Repository) string {
|
||||
s, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: op, UserID: userID, RepoID: repo.ID})
|
||||
_, token, _ := strings.Cut(s, " ")
|
||||
return token
|
||||
}
|
||||
|
||||
t.Run("handleLFSToken", func(t *testing.T) {
|
||||
u, err := handleLFSToken(ctx, "", repo1, perm_model.AccessModeRead)
|
||||
@ -37,15 +43,62 @@ func TestAuthenticate(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, u)
|
||||
|
||||
u, err = handleLFSToken(ctx, token2, repo1, perm_model.AccessModeRead)
|
||||
u, err = handleLFSToken(ctx, getUserToken("download", 2, repo1), repo1, perm_model.AccessModeRead)
|
||||
require.NoError(t, err)
|
||||
assert.EqualValues(t, 2, u.ID)
|
||||
})
|
||||
|
||||
t.Run("authenticate", func(t *testing.T) {
|
||||
const prefixBearer = "Bearer "
|
||||
token := getUserToken("download", 2, repo1)
|
||||
assert.False(t, authenticate(ctx, repo1, "", true, false))
|
||||
assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false))
|
||||
assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false))
|
||||
assert.True(t, authenticate(ctx, repo1, prefixBearer+token, true, false))
|
||||
})
|
||||
|
||||
handleLFSTokenTestPerm := func(op string, userID int64, repo *repo_model.Repository, accessMode perm_model.AccessMode) error {
|
||||
token := getUserToken(op, userID, repo)
|
||||
u, err := handleLFSToken(ctx, token, repo, accessMode)
|
||||
if err == nil {
|
||||
assert.Equal(t, userID, u.ID)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
t.Run("handleLFSToken blocks prohibited users", func(t *testing.T) {
|
||||
user37 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 37})
|
||||
|
||||
// prohibited user
|
||||
assert.True(t, user37.ProhibitLogin)
|
||||
err := handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
|
||||
assert.ErrorContains(t, err, "not allowed to access any repository")
|
||||
|
||||
// normal user
|
||||
_, _ = db.GetEngine(t.Context()).ID(37).Cols("prohibit_login").Update(&user_model.User{ProhibitLogin: false})
|
||||
err = handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// inactive user
|
||||
_, _ = db.GetEngine(t.Context()).ID(37).Cols("is_active").Update(&user_model.User{IsActive: false})
|
||||
err = handleLFSTokenTestPerm("download", 37, repo1, perm_model.AccessModeRead)
|
||||
assert.ErrorContains(t, err, "not allowed to access any repository")
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken blocks users without repo access", func(t *testing.T) {
|
||||
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
|
||||
err := handleLFSTokenTestPerm("download", 10, repo2, perm_model.AccessModeRead)
|
||||
assert.ErrorContains(t, err, "no permission to access the repository")
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken requires write access for uploads", func(t *testing.T) {
|
||||
err := handleLFSTokenTestPerm("download", 10, repo1, perm_model.AccessModeRead)
|
||||
assert.NoError(t, err)
|
||||
err = handleLFSTokenTestPerm("upload", 10, repo1, perm_model.AccessModeWrite)
|
||||
assert.ErrorContains(t, err, "no permission to access the repository")
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken allows writes for authorized users", func(t *testing.T) {
|
||||
err := handleLFSTokenTestPerm("upload", 2, repo1, perm_model.AccessModeWrite)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
@ -13,52 +13,17 @@ import (
|
||||
|
||||
auth_model "code.gitea.io/gitea/models/auth"
|
||||
"code.gitea.io/gitea/modules/git"
|
||||
api "code.gitea.io/gitea/modules/structs"
|
||||
"code.gitea.io/gitea/modules/util"
|
||||
"code.gitea.io/gitea/modules/git/gitcmd"
|
||||
"code.gitea.io/gitea/tests"
|
||||
|
||||
"github.com/PuerkitoBio/goquery"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func assertFileExist(t *testing.T, p string) {
|
||||
exist, err := util.IsExist(p)
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, exist)
|
||||
}
|
||||
|
||||
func assertFileEqual(t *testing.T, p string, content []byte) {
|
||||
bs, err := os.ReadFile(p)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, content, bs)
|
||||
}
|
||||
|
||||
func TestRepoCloneWiki(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
dstPath := t.TempDir()
|
||||
|
||||
r := u.String() + "user2/repo1.wiki.git"
|
||||
u, _ = url.Parse(r)
|
||||
u.User = url.UserPassword("user2", userPassword)
|
||||
t.Run("Clone", func(t *testing.T) {
|
||||
assert.NoError(t, git.Clone(t.Context(), u.String(), dstPath, git.CloneRepoOptions{}))
|
||||
assertFileEqual(t, filepath.Join(dstPath, "Home.md"), []byte("# Home page\n\nThis is the home page!\n"))
|
||||
assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md"))
|
||||
assertFileExist(t, filepath.Join(dstPath, "Page-With-Spaced-Name.md"))
|
||||
assertFileExist(t, filepath.Join(dstPath, "images"))
|
||||
assertFileExist(t, filepath.Join(dstPath, "files/Non-Renderable-File.zip"))
|
||||
assertFileExist(t, filepath.Join(dstPath, "jpeg.jpg"))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
func Test_RepoWikiPages(t *testing.T) {
|
||||
func TestRepoWikiPages(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
url := "/user2/repo1/wiki/?action=_pages"
|
||||
req := NewRequest(t, "GET", url)
|
||||
req := NewRequest(t, "GET", "/user2/repo1/wiki/?action=_pages")
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
doc := NewHTMLParser(t, resp.Body)
|
||||
@ -74,45 +39,72 @@ func Test_RepoWikiPages(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func Test_WikiClone(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
username := "user2"
|
||||
reponame := "repo1"
|
||||
wikiPath := username + "/" + reponame + ".wiki.git"
|
||||
keyname := "my-testing-key"
|
||||
baseAPITestContext := NewAPITestContext(t, username, "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
|
||||
func testRepoWikiCloneHTTP(t *testing.T, u *url.URL) {
|
||||
// When proc-receive support is enabled globally, the HTTP receive-pack pre-check
|
||||
// must still require write access for wiki repositories. Exercise this with a
|
||||
// normal wiki push because the regression is about the pre-check, not agit refs.
|
||||
require.True(t, git.DefaultFeatures().SupportProcReceive) // modern git should all support proc-receive
|
||||
|
||||
u.Path = wikiPath
|
||||
wikiURL := *u
|
||||
wikiURL.Path = "/user2/repo1.wiki.git"
|
||||
|
||||
t.Run("Clone HTTP", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
dstLocalPath := t.TempDir()
|
||||
|
||||
dstLocalPath := t.TempDir()
|
||||
assert.NoError(t, git.Clone(t.Context(), u.String(), dstLocalPath, git.CloneRepoOptions{}))
|
||||
content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
|
||||
})
|
||||
// reader can clone
|
||||
wikiURL.User = url.UserPassword("user20", userPassword)
|
||||
require.NoError(t, git.Clone(t.Context(), wikiURL.String(), dstLocalPath, git.CloneRepoOptions{}))
|
||||
_, _, runErr := gitcmd.NewCommand("fast-import").WithDir(dstLocalPath).WithStdinBytes([]byte(`commit refs/heads/master
|
||||
committer unauthorized-user <user20@example.com> 1714310400 +0000
|
||||
data <<EOM
|
||||
dummy-message
|
||||
EOM
|
||||
from refs/heads/master^0
|
||||
M 100644 inline Home.md
|
||||
data <<EOF
|
||||
changed-content
|
||||
EOF
|
||||
`)).RunStdString(t.Context())
|
||||
require.NoError(t, runErr)
|
||||
|
||||
t.Run("Clone SSH", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
|
||||
|
||||
dstLocalPath := t.TempDir()
|
||||
sshURL := createSSHUrl(wikiPath, u)
|
||||
// reader can't push
|
||||
_, _, runErr = gitcmd.NewCommand("push", "origin", "refs/heads/master").WithDir(dstLocalPath).RunStdString(t.Context())
|
||||
assert.True(t, gitcmd.StderrContains(runErr, "remote: Repository not found\n"))
|
||||
req := NewRequest(t, "GET", "/user2/repo1/wiki/raw/Home.md")
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
assert.Contains(t, resp.Body.String(), "This is the home page!")
|
||||
|
||||
withKeyFile(t, keyname, func(keyFile string) {
|
||||
var keyID int64
|
||||
t.Run("CreateUserKey", doAPICreateUserKey(baseAPITestContext, "test-key", keyFile, func(t *testing.T, key api.PublicKey) {
|
||||
keyID = key.ID
|
||||
}))
|
||||
assert.NotZero(t, keyID)
|
||||
// owner can push
|
||||
wikiURL.User = url.UserPassword("user2", userPassword)
|
||||
_, _, runErr = gitcmd.NewCommand("remote", "add", "origin-owner").AddDynamicArguments(wikiURL.String()).WithDir(dstLocalPath).RunStdString(t.Context())
|
||||
require.NoError(t, runErr)
|
||||
_, _, runErr = gitcmd.NewCommand("push", "origin-owner", "refs/heads/master").WithDir(dstLocalPath).RunStdString(t.Context())
|
||||
assert.NoError(t, runErr)
|
||||
req = NewRequest(t, "GET", "/user2/repo1/wiki/raw/Home.md")
|
||||
resp = MakeRequest(t, req, http.StatusOK)
|
||||
assert.Equal(t, "changed-content", strings.TrimSpace(resp.Body.String()))
|
||||
}
|
||||
|
||||
// Setup clone folder
|
||||
assert.NoError(t, git.Clone(t.Context(), sshURL.String(), dstLocalPath, git.CloneRepoOptions{}))
|
||||
content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
|
||||
})
|
||||
})
|
||||
func testRepoWikiCloneSSH(t *testing.T, u *url.URL) {
|
||||
dstLocalPath := t.TempDir()
|
||||
baseAPITestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
|
||||
sshURL := createSSHUrl("/user2/repo1.wiki.git", u)
|
||||
|
||||
withKeyFile(t, "my-testing-key", func(keyFile string) {
|
||||
t.Run("CreateUserKey", doAPICreateUserKey(baseAPITestContext, "test-key", keyFile))
|
||||
assert.NoError(t, git.Clone(t.Context(), sshURL.String(), dstLocalPath, git.CloneRepoOptions{}))
|
||||
content, err := os.ReadFile(filepath.Join(dstLocalPath, "Home.md"))
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "# Home page\n\nThis is the home page!\n", string(content))
|
||||
})
|
||||
}
|
||||
|
||||
func TestRepoWikiClonePush(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
t.Run("SSH", func(t *testing.T) { testRepoWikiCloneSSH(t, u) })
|
||||
t.Run("HTTP", func(t *testing.T) { testRepoWikiCloneHTTP(t, u) })
|
||||
})
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user