fix: add missing checks to more handlers and services (#40131)

Several code paths skipped checks that their siblings already perform.

- Code comment replies are rejected when they target a review of another
pull request or another user's pending review
- OAuth2 refuses tokens for users who can't sign in, and introspection
reports their tokens and already used refresh tokens as inactive
- AGit proc-receive runs the same pull request permission and wiki
checks as pre-receive
- Repository transfer drops the repository's owner-level scoped workflow
source registrations, and registrations whose source the owner no longer
owns are ignored
- The web editor's base branch copy into a fork and the fork's upstream
divergence info require read access to the base repository
- LFS downloads require sign-in under strict `REQUIRE_SIGNIN_VIEW`, like
git smart HTTP
- Unknown AGit target branches are reported as an unexpected ref again
This commit is contained in:
silverwind
2026-11-08 09:02:51 +00:00
committed by GitHub
parent cd92c6a008
commit 8cb6fb2001
24 changed files with 208 additions and 121 deletions

View File

@@ -65,26 +65,22 @@ func (opts FindScopedWorkflowSourceOpts) ToConds() builder.Cond {
if opts.SourceRepoID != 0 {
cond = cond.And(builder.Eq{"source_repo_id": opts.SourceRepoID})
}
return cond
return cond.And(scopedWorkflowSourceOwnedCond())
}
func scopedWorkflowSourceOwnedCond() builder.Cond {
return builder.Eq{"owner_id": 0}.Or(builder.Expr("EXISTS (SELECT 1 FROM repository WHERE repository.id = action_scoped_workflow_source.source_repo_id AND repository.owner_id = action_scoped_workflow_source.owner_id)"))
}
// GetEffectiveScopedWorkflowSources returns the scoped-workflow sources effective for a repo owned by repoOwnerID:
// the owner's own sources plus instance-level (owner_id=0) sources.
func GetEffectiveScopedWorkflowSources(ctx context.Context, repoOwnerID int64) ([]*ActionScopedWorkflowSource, error) {
owners := []int64{0}
if repoOwnerID != 0 {
owners = append(owners, repoOwnerID)
}
return db.Find[ActionScopedWorkflowSource](ctx, FindScopedWorkflowSourceOpts{OwnerIDs: owners})
return db.Find[ActionScopedWorkflowSource](ctx, FindScopedWorkflowSourceOpts{OwnerIDs: []int64{0, repoOwnerID}})
}
// IsScopedWorkflowSourceEffective reports whether sourceRepoID is a scoped-workflow source effective for a repo owned by repoOwnerID.
func IsScopedWorkflowSourceEffective(ctx context.Context, repoOwnerID, sourceRepoID int64) (bool, error) {
owners := []int64{0}
if repoOwnerID != 0 {
owners = append(owners, repoOwnerID)
}
return db.Exist[ActionScopedWorkflowSource](ctx, FindScopedWorkflowSourceOpts{OwnerIDs: owners, SourceRepoID: sourceRepoID}.ToConds())
return db.Exist[ActionScopedWorkflowSource](ctx, FindScopedWorkflowSourceOpts{OwnerIDs: []int64{0, repoOwnerID}, SourceRepoID: sourceRepoID}.ToConds())
}
// IsWorkflowRequiredInSources reports whether workflowID from sourceRepoID is required by any of the given sources.
@@ -138,12 +134,10 @@ func GetScopedWorkflowSourcesByOwner(ctx context.Context, ownerID int64) ([]*Act
// GetScopedWorkflowSource returns the (owner, repo) source registration or a NotExist error.
func GetScopedWorkflowSource(ctx context.Context, ownerID, repoID int64) (*ActionScopedWorkflowSource, error) {
src := &ActionScopedWorkflowSource{}
has, err := db.GetEngine(ctx).Where("owner_id = ? AND source_repo_id = ?", ownerID, repoID).Get(src)
src, has, err := db.Get[ActionScopedWorkflowSource](ctx, builder.Eq{"owner_id": ownerID, "source_repo_id": repoID}.And(scopedWorkflowSourceOwnedCond()))
if err != nil {
return nil, err
}
if !has {
} else if !has {
return nil, util.NewNotExistErrorf("scoped workflow source (owner %d, repo %d) does not exist", ownerID, repoID)
}
return src, nil
@@ -181,3 +175,9 @@ func RemoveScopedWorkflowSource(ctx context.Context, ownerID, repoID int64) erro
_, err := db.GetEngine(ctx).Where("owner_id = ? AND source_repo_id = ?", ownerID, repoID).Delete(new(ActionScopedWorkflowSource))
return err
}
// RemoveOwnerScopedWorkflowSources removes every owner-level registration of repoID, as only its current owner may hold one.
func RemoveOwnerScopedWorkflowSources(ctx context.Context, repoID int64) error {
_, err := db.GetEngine(ctx).Where("owner_id <> 0 AND source_repo_id = ?", repoID).Delete(new(ActionScopedWorkflowSource))
return err
}

View File

@@ -48,26 +48,28 @@ func TestGetEffectiveScopedWorkflowSources(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
ctx := t.Context()
rows := []*ActionScopedWorkflowSource{
{OwnerID: 2, SourceRepoID: 100, WorkflowConfigs: nil}, // org 2 registers repo 100 (optional)
{OwnerID: 0, SourceRepoID: 100, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"a.yml": {Required: true, Patterns: []string{"p"}}}}, // instance also registers repo 100 (required)
{OwnerID: 0, SourceRepoID: 200, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"b.yml": {Required: true, Patterns: []string{"p"}}}}, // instance source 200
{OwnerID: 3, SourceRepoID: 300, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"c.yml": {Required: true, Patterns: []string{"p"}}}}, // a different owner's source
}
for _, r := range rows {
require.NoError(t, db.Insert(ctx, r))
for _, source := range []*ActionScopedWorkflowSource{
{OwnerID: 2, SourceRepoID: 1},
{OwnerID: 0, SourceRepoID: 1, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"a.yml": {Required: true}}},
{OwnerID: 0, SourceRepoID: 200, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"b.yml": {Required: true}}},
{OwnerID: 3, SourceRepoID: 3, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"c.yml": {Required: true}}},
{OwnerID: 2, SourceRepoID: 3, WorkflowConfigs: map[string]*ScopedWorkflowConfig{"c.yml": {Required: true}}},
} {
require.NoError(t, db.Insert(ctx, source))
}
// owner 2 sees its own sources plus instance-level ones, but not owner 3's.
owner2, err := GetEffectiveScopedWorkflowSources(ctx, 2)
require.NoError(t, err)
assert.Len(t, owner2, 3)
_, err = GetScopedWorkflowSource(ctx, 2, 3)
assert.ErrorIs(t, err, util.ErrNotExist)
required, err := IsScopedWorkflowRequired(ctx, 2, 100, "a.yml")
required, err := IsScopedWorkflowRequired(ctx, 2, 1, "a.yml")
require.NoError(t, err)
assert.True(t, required, "instance marks a.yml required → required for owner 2 even though org left it optional")
required, err = IsScopedWorkflowRequired(ctx, 2, 100, "x.yml")
required, err = IsScopedWorkflowRequired(ctx, 2, 1, "x.yml")
require.NoError(t, err)
assert.False(t, required)
@@ -76,12 +78,12 @@ func TestGetEffectiveScopedWorkflowSources(t *testing.T) {
assert.True(t, required)
// owner 3's source must not be effective for owner 2.
required, err = IsScopedWorkflowRequired(ctx, 2, 300, "c.yml")
required, err = IsScopedWorkflowRequired(ctx, 2, 3, "c.yml")
require.NoError(t, err)
assert.False(t, required)
// IsScopedWorkflowSourceEffective: owner-level and instance-level sources are effective; another owner's is not.
effective, err := IsScopedWorkflowSourceEffective(ctx, 2, 100)
effective, err := IsScopedWorkflowSourceEffective(ctx, 2, 1)
require.NoError(t, err)
assert.True(t, effective, "owner 2's own source")
@@ -89,7 +91,7 @@ func TestGetEffectiveScopedWorkflowSources(t *testing.T) {
require.NoError(t, err)
assert.True(t, effective, "instance-level source is effective for any owner")
effective, err = IsScopedWorkflowSourceEffective(ctx, 2, 300)
effective, err = IsScopedWorkflowSourceEffective(ctx, 2, 3)
require.NoError(t, err)
assert.False(t, effective, "owner 3's source is not effective for owner 2")
@@ -97,7 +99,7 @@ func TestGetEffectiveScopedWorkflowSources(t *testing.T) {
require.NoError(t, err)
assert.False(t, effective, "unknown source repo")
effective, err = IsScopedWorkflowSourceEffective(ctx, 3, 300)
effective, err = IsScopedWorkflowSourceEffective(ctx, 3, 3)
require.NoError(t, err)
assert.True(t, effective, "owner 3's own source is effective for owner 3")
}
@@ -107,8 +109,8 @@ func TestScopedWorkflowSourceCRUD(t *testing.T) {
ctx := t.Context()
// add is idempotent
require.NoError(t, AddScopedWorkflowSource(ctx, 5, 10))
require.NoError(t, AddScopedWorkflowSource(ctx, 5, 10))
require.NoError(t, AddScopedWorkflowSource(ctx, 5, 4))
require.NoError(t, AddScopedWorkflowSource(ctx, 5, 4))
sources, err := GetScopedWorkflowSourcesByOwner(ctx, 5)
require.NoError(t, err)
assert.Len(t, sources, 1)
@@ -118,20 +120,20 @@ func TestScopedWorkflowSourceCRUD(t *testing.T) {
"a.yml": {Required: true, Patterns: []string{"src: a.yml / *"}},
"b.yml": {Required: false, Patterns: []string{"src: b.yml / build (push)"}},
}
require.NoError(t, SetScopedWorkflowSourceConfigs(ctx, 5, 10, configs))
src, err := GetScopedWorkflowSource(ctx, 5, 10)
require.NoError(t, SetScopedWorkflowSourceConfigs(ctx, 5, 4, configs))
src, err := GetScopedWorkflowSource(ctx, 5, 4)
require.NoError(t, err)
assert.Equal(t, configs, src.WorkflowConfigs)
// clearing the configs works
require.NoError(t, SetScopedWorkflowSourceConfigs(ctx, 5, 10, nil))
src, err = GetScopedWorkflowSource(ctx, 5, 10)
require.NoError(t, SetScopedWorkflowSourceConfigs(ctx, 5, 4, nil))
src, err = GetScopedWorkflowSource(ctx, 5, 4)
require.NoError(t, err)
assert.Empty(t, src.WorkflowConfigs)
// remove
require.NoError(t, RemoveScopedWorkflowSource(ctx, 5, 10))
_, err = GetScopedWorkflowSource(ctx, 5, 10)
require.NoError(t, RemoveScopedWorkflowSource(ctx, 5, 4))
_, err = GetScopedWorkflowSource(ctx, 5, 4)
assert.ErrorIs(t, err, util.ErrNotExist)
sources, err = GetScopedWorkflowSourcesByOwner(ctx, 5)
require.NoError(t, err)

View File

@@ -7,7 +7,7 @@
updated_unix: 1546869730
- id: 2
user_id: 3
user_id: 14
application_id: 1
counter: 1
scope: "openid"
@@ -23,7 +23,7 @@
updated_unix: 1546869730
- id: 4
user_id: 99
user_id: 2
application_id: 2
counter: 1
scope: "whatever"

View File

@@ -28,7 +28,6 @@ type preReceiveContext struct {
// this context should only contain shared variables, mutable variables like "current branch name" shouldn't be put here
canWriteCodeUnitCached *bool
canCreatePullRequest *bool
protectedTags []*git_model.ProtectedTag
}
@@ -63,21 +62,10 @@ func (ctx *preReceiveContext) assertCanWriteRef(refFullName git.RefName) bool {
return true
}
// CanCreatePullRequest returns true if pusher can create pull requests
func (ctx *preReceiveContext) CanCreatePullRequest() bool {
if ctx.canCreatePullRequest == nil {
ctx.canCreatePullRequest = new(ctx.Repo.Permission.CanRead(unit.TypePullRequests))
}
return *ctx.canCreatePullRequest
}
// AssertCreatePullRequest returns true if can create pull requests
func (ctx *preReceiveContext) AssertCreatePullRequest() bool {
if !ctx.CanCreatePullRequest() {
if ctx.Written() {
return false
}
ctx.PrivateUserErrorf(http.StatusForbidden, "User permission denied for creating pull-request.")
func assertCanCreatePullRequest(ctx *gitea_context.PrivateContext, opts *private.HookOptions) bool {
canReadPullRequests := ctx.Repo.Permission.CanRead(unit.TypePullRequests)
if !canReadPullRequests || opts.IsWiki {
ctx.PrivateUserErrorf(http.StatusForbidden, "%s", util.Iif(canReadPullRequests, "Pull requests are not supported on the wiki.", "User permission denied for creating pull-request."))
return false
}
return true
@@ -341,23 +329,12 @@ func preReceiveTag(ctx *preReceiveContext, refFullName git.RefName) {
}
func preReceiveFor(ctx *preReceiveContext, refFullName git.RefName) {
if !ctx.AssertCreatePullRequest() {
if !assertCanCreatePullRequest(ctx.PrivateContext, ctx.opts) {
return
}
if ctx.Repo.Repository.IsEmpty {
} else if ctx.Repo.Repository.IsEmpty {
ctx.PrivateUserErrorf(http.StatusForbidden, "Can't create pull request for an empty repository.")
return
}
if ctx.opts.IsWiki {
ctx.PrivateUserErrorf(http.StatusForbidden, "Pull requests are not supported on the wiki.")
return
}
_, _, err := agit.GetAgitBranchInfo(ctx, ctx.Repo.Repository.ID, refFullName.ForBranchName())
if err != nil {
if !errors.Is(err, util.ErrNotExist) {
} else if _, _, err := agit.GetAgitBranchInfo(ctx, ctx.Repo.Repository.ID, refFullName.ForBranchName()); err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.PrivateUserErrorf(http.StatusForbidden, "Unexpected ref: %s", refFullName)
} else {
ctx.PrivateInternalErrorf("Unable to get branch info for ref %s: %v", refFullName, err)

View File

@@ -18,6 +18,7 @@ import (
user_model "gitea.dev/models/user"
"gitea.dev/modules/git"
"gitea.dev/modules/private"
"gitea.dev/modules/web"
"gitea.dev/services/contexttest"
"github.com/stretchr/testify/assert"
@@ -29,6 +30,21 @@ func TestPreReceive(t *testing.T) {
t.Run("Branch", testPreReceiveActionsBranch)
t.Run("Tag", testPreReceiveSystemUserTag)
t.Run("CanWrite", testPreReceiveCanWriteCodePerBranch)
t.Run("ProcReceiveNeedsPullRequests", func(t *testing.T) {
ctx, resp := contexttest.MockPrivateContext(t, "/")
ctx.SetPathParam("owner", "user2")
ctx.SetPathParam("repo", "repo2")
RepoAssignment(ctx)
require.False(t, ctx.Written())
defer ctx.Repo.GitRepo.Close()
web.SetForm(ctx, &private.HookOptions{
UserID: 2,
RefFullNames: []git.RefName{"refs/for/master"},
})
HookProcReceive(ctx)
assert.Equal(t, http.StatusForbidden, resp.Code, resp.Body.String())
})
}
func testPreReceiveActionsBranch(t *testing.T) {

View File

@@ -6,9 +6,11 @@ package private
import (
"errors"
"net/http"
"slices"
issues_model "gitea.dev/models/issues"
user_model "gitea.dev/models/user"
"gitea.dev/modules/git"
"gitea.dev/modules/private"
"gitea.dev/modules/web"
"gitea.dev/services/agit"
@@ -18,7 +20,8 @@ import (
// HookProcReceive proc-receive hook - only handles agit Proc-Receive requests at present
func HookProcReceive(ctx *gitea_context.PrivateContext) {
opts := web.GetForm[*private.HookOptions](ctx)
if !loadContextDoerPermission(ctx, opts.UserID, opts.UserExtDoerData) {
if !loadContextDoerPermission(ctx, opts.UserID, opts.UserExtDoerData) ||
(slices.ContainsFunc(opts.RefFullNames, git.RefName.IsFor) && !assertCanCreatePullRequest(ctx, opts)) {
return
}

View File

@@ -188,22 +188,22 @@ func IntrospectOAuth(ctx *context.Context) {
ctx.ServerError("GetOAuth2GrantByID", err)
return
}
if grant == nil || grant.ApplicationID != introspectingApp.ID {
if grant == nil || grant.ApplicationID != introspectingApp.ID || (token.Kind == oauth2_provider.KindRefreshToken && isRefreshTokenUsed(grant, token)) {
// RFC 7662 allows the server to reply inactive when the caller must not learn more.
// https://www.rfc-editor.org/rfc/rfc7662.html#section-2.2
ctx.JSON(http.StatusOK, response)
return
}
response.Active = true
response.Scope = oauth2_provider.NormalizeGrantScope(grant.Scope)
response.RegisteredClaims = oauth2_provider.NewJwtRegisteredClaimsFromUser(introspectingApp.ClientID, grant.UserID, nil /*exp*/)
user, err := user_model.GetUserByID(ctx, grant.UserID)
if err != nil {
ctx.ServerError("GetUserByID", err)
if user, err := user_model.GetValidDoerUserByID(ctx, grant.UserID); err == nil {
response.Active = true
response.Scope = oauth2_provider.NormalizeGrantScope(grant.Scope)
response.RegisteredClaims = oauth2_provider.NewJwtRegisteredClaimsFromUser(introspectingApp.ClientID, grant.UserID, nil /*exp*/)
response.Username = user.Name
} else if !errors.Is(err, util.ErrNotExist) && !errors.Is(err, util.ErrPermissionDenied) {
ctx.ServerError("GetValidDoerUserByID", err)
return
}
response.Username = user.Name
ctx.JSON(http.StatusOK, response)
}
@@ -563,6 +563,10 @@ func AccessTokenOAuth(ctx *context.Context) {
}
}
func isRefreshTokenUsed(grant *auth.OAuth2Grant, token *oauth2_provider.Token) bool {
return setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0)
}
func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, serverKey, clientKey oauth2_provider.JWTSigningKey) {
app, err := auth.GetOAuth2ApplicationByClientID(ctx, form.ClientID)
if err != nil {
@@ -614,7 +618,7 @@ func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, server
}
// check if token got already used
if setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0) {
if isRefreshTokenUsed(grant, token) {
handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{
ErrorCode: oauth2_provider.AccessTokenErrorCodeUnauthorizedClient,
ErrorDescription: "token was already used",

View File

@@ -15,6 +15,8 @@ import (
user_model "gitea.dev/models/user"
"gitea.dev/modules/egress/policy"
"gitea.dev/modules/session"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"gitea.dev/modules/web"
"gitea.dev/services/contexttest"
"gitea.dev/services/forms"
@@ -53,7 +55,7 @@ func createAndParseToken(t *testing.T, grant *auth.OAuth2Grant) *oauth2_provider
func TestNewAccessTokenResponse_OIDCToken(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
grants, err := auth.GetOAuth2GrantsByUserID(t.Context(), 3)
grants, err := auth.GetOAuth2GrantsByUserID(t.Context(), 14)
assert.NoError(t, err)
assert.Len(t, grants, 1)
@@ -83,6 +85,19 @@ func TestNewAccessTokenResponse_OIDCToken(t *testing.T) {
assert.Equal(t, user.UpdatedUnix, oidcToken.UpdatedAt)
assert.Equal(t, user.Email, oidcToken.Email)
assert.Equal(t, user.IsActive, oidcToken.EmailVerified)
defer test.MockVariableValue(&setting.OAuth2.InvalidateRefreshTokens, true)()
require.NoError(t, user_model.UpdateUserCols(t.Context(), &user_model.User{ID: user.ID}, "is_active"))
defer func() {
require.NoError(t, user_model.UpdateUserCols(t.Context(), user, "is_active"))
}()
signingKey, err := oauth2_provider.CreateJWTSigningKey("HS256", make([]byte, 32))
require.NoError(t, err)
counter := grants[0].Counter
response, tokenErr := oauth2_provider.NewAccessTokenResponse(t.Context(), grants[0], signingKey, signingKey)
assert.Nil(t, response)
assert.Equal(t, &oauth2_provider.AccessTokenError{ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidGrant, ErrorDescription: "user is not allowed to sign in"}, tokenErr)
assert.Equal(t, counter, unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Grant{ID: grants[0].ID}).Counter)
}
func TestOAuth2AvatarClientBlocksLoopback(t *testing.T) {

View File

@@ -134,6 +134,10 @@ func prepareEditorCommitSubmittedForm[T forms.CommitCommonFormInterface](ctx *co
// check commit behavior
fromBaseBranch := ctx.FormString("from_base_branch")
if fromBaseBranch != "" && ctx.Repo.Repository.BaseRepo == nil {
ctx.NotFound(nil)
return nil
}
commitToNewBranch := commonForm.CommitChoice == editorCommitChoiceNewBranch || fromBaseBranch != ""
targetBranchName := util.Iif(commitToNewBranch, commonForm.NewBranchName, ctx.Repo.BranchName)
if targetBranchName == ctx.Repo.BranchName && !commitFormOptions.CanCommitToBranch {

View File

@@ -11,7 +11,9 @@ import (
"strings"
git_model "gitea.dev/models/git"
access_model "gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
user_model "gitea.dev/models/user"
"gitea.dev/modules/git"
"gitea.dev/modules/log"
@@ -127,6 +129,11 @@ func getUniqueRepositoryName(ctx context.Context, ownerID int64, name string) st
}
func editorPushBranchToForkedRepository(ctx context.Context, doer *user_model.User, baseRepo *repo_model.Repository, baseBranchName string, targetRepo *repo_model.Repository, targetBranchName string) error {
if basePerm, err := access_model.GetDoerRepoPermission(ctx, baseRepo, doer); err != nil {
return err
} else if !basePerm.CanRead(unit.TypeCode) {
return util.NewPermissionDeniedErrorf("permission denied to read base repo %d", baseRepo.ID)
}
return git.PushManaged(ctx, baseRepo, targetRepo, git.PushOptions{
Branch: baseBranchName + ":" + targetBranchName,
Env: repo_module.PushingEnvironment(doer, targetRepo),

View File

@@ -224,14 +224,11 @@ func prepareUpstreamDivergingInfo(ctx *context.Context) {
if !ctx.Repo.Repository.IsFork || !ctx.Repo.RefFullName.IsBranch() || ctx.Repo.TreePath != "" {
return
}
upstreamDivergingInfo, err := repo_service.GetUpstreamDivergingInfo(ctx, ctx.Repo.Repository, ctx.Repo.BranchName)
if err != nil {
if !errors.Is(err, util.ErrNotExist) && !errors.Is(err, util.ErrInvalidArgument) {
log.Error("GetUpstreamDivergingInfo: %v", err)
}
return
if upstreamDivergingInfo, err := repo_service.GetUpstreamDivergingInfo(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName); err == nil {
ctx.Data["UpstreamDivergingInfo"] = upstreamDivergingInfo
} else if !errors.Is(err, util.ErrNotExist) && !errors.Is(err, util.ErrInvalidArgument) && !errors.Is(err, util.ErrPermissionDenied) {
log.Error("GetUpstreamDivergingInfo: %v", err)
}
ctx.Data["UpstreamDivergingInfo"] = upstreamDivergingInfo
}
func updateContextRepoEmptyAndStatus(ctx *context.Context, empty bool, status repo_model.RepositoryStatus) {

View File

@@ -557,10 +557,9 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho
return false
}
canAccess := perm.CanAccess(accessMode, unit.TypeCode)
// if it doesn't require sign-in and anonymous user has access, return true
// if the user is already signed in (for example: by session auth method), and the doer can access, return true
if canAccess && (!requireSigned || ctx.IsSigned) {
if perm.CanAccess(accessMode, unit.TypeCode) && ((!requireSigned && !setting.Service.RequireSignInViewStrict) || ctx.IsSigned) {
return true
}

View File

@@ -13,6 +13,8 @@ import (
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"gitea.dev/services/contexttest"
"github.com/stretchr/testify/assert"
@@ -51,6 +53,10 @@ func TestAuthenticate(t *testing.T) {
const prefixBearer = "Bearer "
token := getUserToken("download", 2, repo1, "")
assert.False(t, authenticate(ctx, repo1, "", true, false))
assert.True(t, authenticate(ctx, repo1, "", false, false))
restoreStrict := test.MockVariableValue(&setting.Service.RequireSignInViewStrict, true)
assert.False(t, authenticate(ctx, repo1, "", false, false))
restoreStrict()
assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false))
assert.True(t, authenticate(ctx, repo1, prefixBearer+token, true, false))
})

View File

@@ -5,6 +5,7 @@ package oauth2_provider
import (
"context"
"errors"
"fmt"
"slices"
"strconv"
@@ -152,6 +153,20 @@ func NewJwtRegisteredClaimsFromUser(clientID string, grantUserID int64, exp *jwt
}
func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, serverKey, clientKey JWTSigningKey) (*AccessTokenResponse, *AccessTokenError) {
user, err := user_model.GetValidDoerUserByID(ctx, grant.UserID)
if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrPermissionDenied) {
return nil, &AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidGrant,
ErrorDescription: "user is not allowed to sign in",
}
} else if err != nil {
log.Error("Error loading user: %v", err)
return nil, &AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "server error",
}
}
if setting.OAuth2.InvalidateRefreshTokens {
if err := grant.IncreaseCounter(ctx); err != nil {
return nil, &AccessTokenError{
@@ -205,21 +220,6 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server
ErrorDescription: "cannot find application",
}
}
user, err := user_model.GetUserByID(ctx, grant.UserID)
if err != nil {
if user_model.IsErrUserNotExist(err) {
return nil, &AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "cannot find user",
}
}
log.Error("Error loading user: %v", err)
return nil, &AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "server error",
}
}
idToken := &OIDCToken{
RegisteredClaims: NewJwtRegisteredClaimsFromUser(app.ClientID, grant.UserID, jwt.NewNumericDate(expirationDate.AsTime())),
Nonce: grant.Nonce,

View File

@@ -126,7 +126,7 @@ func TestEffectiveRequiredContexts(t *testing.T) {
t.Run("required workflow patterns appended", func(t *testing.T) {
require.NoError(t, db.Insert(t.Context(), &actions_model.ActionScopedWorkflowSource{
OwnerID: consumer.OwnerID,
SourceRepoID: 1,
SourceRepoID: consumer.ID,
WorkflowConfigs: map[string]*actions_model.ScopedWorkflowConfig{
"ci.yaml": {Required: true, Patterns: []string{"org/src: ci.yaml / build (pull_request)", "org/src: ci.yaml / lint (pull_request)"}},
"old.yaml": {Required: false, Patterns: []string{"org/src: old.yaml / *"}}, // kept as history, must NOT be enforced

View File

@@ -112,6 +112,12 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
// Comments that are replies don't require a review header to show up in the issue view
// Add the code comment to existing submitted review
if replyToSubmittedReview {
if replyReview, err := issues_model.GetReviewByID(ctx, replyReviewID); err != nil {
return nil, err
} else if replyReview.IssueID != issue.ID || (replyReview.Type == issues_model.ReviewTypePending && replyReview.ReviewerID != doer.ID) {
return nil, issues_model.ErrReviewNotExist{ID: replyReviewID}
}
comment, err := createCodeComment(ctx,
doer,
issue.Repo,

View File

@@ -112,6 +112,15 @@ func TestCreateCodeCommentContextOfUnchangedLine(t *testing.T) {
}
})
}
t.Run("ReplyToOtherUsersPendingReview", func(t *testing.T) {
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: pr.IssueID})
require.NoError(t, issue.LoadRepo(ctx))
pendingComment, err := pull_service.CreateCodeComment(ctx, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}), gitRepo, issue, 17, "test", treePath, true, 0, "", nil)
require.NoError(t, err)
_, err = pull_service.CreateCodeComment(ctx, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}), gitRepo, issue, 17, "test", treePath, false, pendingComment.ReviewID, "", nil)
assert.True(t, issues_model.IsErrReviewNotExist(err), "got: %v", err)
})
}
func TestCreateCodeCommentUnknownCommitID(t *testing.T) {

View File

@@ -24,21 +24,8 @@ func MergeUpstream(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_
if err = repo.MustNotBeArchived(); err != nil {
return "", err
}
if err = repo.GetBaseRepo(ctx); err != nil {
return "", err
}
// The doer must still be able to read the base repository's code. Otherwise a fork created
// while the base repo was public could keep pulling commits after it turned private.
basePerm, err := access_model.GetDoerRepoPermission(ctx, repo.BaseRepo, doer)
if err != nil {
return "", err
}
if !basePerm.CanRead(unit.TypeCode) {
return "", util.NewPermissionDeniedErrorf("permission denied to read base repo %d", repo.BaseRepo.ID)
}
divergingInfo, err := GetUpstreamDivergingInfo(ctx, repo, branch)
divergingInfo, err := GetUpstreamDivergingInfo(ctx, doer, repo, branch)
if err != nil {
return "", err
}
@@ -102,7 +89,7 @@ type UpstreamDivergingInfo struct {
}
// GetUpstreamDivergingInfo returns the information about the divergence between the fork repository's branch and the base repository's default branch.
func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, forkRepo *repo_model.Repository, forkBranch string) (*UpstreamDivergingInfo, error) {
func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, doer *user_model.User, forkRepo *repo_model.Repository, forkBranch string) (*UpstreamDivergingInfo, error) {
if !forkRepo.IsFork {
return nil, util.NewInvalidArgumentErrorf("repo is not a fork")
}
@@ -115,6 +102,14 @@ func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, forkRepo *repo_model.Re
return nil, err
}
// The doer must still be able to read the base repository's code. Otherwise a fork created
// while the base repo was public could keep pulling commits after it turned private.
if basePerm, err := access_model.GetDoerRepoPermission(ctx, forkRepo.BaseRepo, doer); err != nil {
return nil, err
} else if !basePerm.CanRead(unit.TypeCode) {
return nil, util.NewPermissionDeniedErrorf("permission denied to read base repo %d", forkRepo.BaseRepo.ID)
}
// Do the best to follow the GitHub's behavior, suppose there is a `branch-a` in fork repo:
// * if `branch-a` exists in base repo: try to sync `base:branch-a` to `fork:branch-a`
// * if `branch-a` doesn't exist in base repo: try to sync `base:main` to `fork:branch-a`

View File

@@ -262,6 +262,10 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName
return fmt.Errorf("GetOwnerActionsConfig: %w", err)
}
if err := actions_model.RemoveOwnerScopedWorkflowSources(ctx, repo.ID); err != nil {
return fmt.Errorf("transferOwnership: %w", err)
}
// Update repository count.
if _, err := sess.Exec("UPDATE `user` SET num_repos=num_repos+1 WHERE id=?", newOwner.ID); err != nil {
return fmt.Errorf("increase new owner repository count: %w", err)

View File

@@ -7,6 +7,7 @@ import (
"sync"
"testing"
actions_model "gitea.dev/models/actions"
activities_model "gitea.dev/models/activities"
"gitea.dev/models/organization"
"gitea.dev/models/perm"
@@ -22,6 +23,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"xorm.io/builder"
)
var notifySync sync.Once
@@ -43,11 +45,16 @@ func TestTransferOwnership(t *testing.T) {
assert.NoError(t, sourceRepo.LoadOwner(t.Context()))
repoTransfer := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoTransfer{ID: 1})
assert.NoError(t, repoTransfer.LoadAttributes(t.Context()))
for _, ownerID := range []int64{0, sourceRepo.OwnerID, 1} {
require.NoError(t, actions_model.AddScopedWorkflowSource(t.Context(), ownerID, sourceRepo.ID))
}
assert.NoError(t, AcceptTransferOwnership(t.Context(), sourceRepo, doer))
transferredRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
assert.EqualValues(t, 1, transferredRepo.OwnerID) // repo_transfer.yml id=1
unittest.AssertNotExistsBean(t, &repo_model.RepoTransfer{ID: 1})
unittest.AssertCount(t, &actions_model.ActionScopedWorkflowSource{SourceRepoID: 3}, 1)
unittest.AssertCountByCond(t, "action_scoped_workflow_source", builder.Eq{"owner_id": 0, "source_repo_id": 3}, 1)
exist, err := git.IsRepositoryExist(t.Context(), sourceRepoBak)
assert.NoError(t, err)

View File

@@ -38,6 +38,10 @@ func TestEditor(t *testing.T) {
t.Run("EditFile", func(t *testing.T) {
testEditFile(t, sessionUser2, "user2", "repo1", "master", "README.md", "Hello, World (direct)\n")
testEditFileToNewBranch(t, sessionUser2, "user2", "repo1", "master", "feature/test", "README.md", "Hello, World (commit-to-new-branch)\n")
assert.Equal(t, http.StatusNotFound, testEditorActionPostRequest(t, sessionUser2, "/user2/repo1/_edit/master/README.md?from_base_branch=master", map[string]string{
"commit_choice": "direct",
"new_branch_name": "from-base-on-non-fork",
}).Code)
})
t.Run("EditorconfigOfBranch", func(t *testing.T) {
testCreateFile(t, sessionUser2, "user2", "repo1", "master", "editorconfig", ".editorconfig", "[*]\nindent_size = 2\n")

View File

@@ -155,6 +155,11 @@ func TestAgitPullPush(t *testing.T) {
WithDir(dstPath).
Run(t.Context())
assert.NoError(t, err)
_, stderr, _ = gitcmd.NewCommand("push", "origin", "HEAD:refs/for/no-such-branch/test-agit-push").
WithDir(dstPath).
RunStdString(t.Context())
assert.Contains(t, stderr, "Unexpected ref: refs/for/no-such-branch/test-agit-push")
} else {
// Get initial commit ID for the review
initialCommitID := pr.HeadCommitID

View File

@@ -773,6 +773,10 @@ func testRefreshTokenInvalidation(t *testing.T) {
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
assert.Equal(t, "token was already used", parsedError.ErrorDescription)
req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{"token": parsed.RefreshToken}).
AddBasicAuth("da7da3ba-9a13-4167-856f-3899de0b0138", "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=")
assert.JSONEq(t, `{"active":false}`, MakeRequest(t, req, http.StatusOK).Body.String())
}
func testRefreshTokenCrossClientUsage(t *testing.T) {
@@ -879,6 +883,16 @@ func testOAuthIntrospection(t *testing.T) {
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpK")
resp = MakeRequest(t, req, http.StatusUnauthorized)
assert.Contains(t, resp.Body.String(), "no valid authorization")
require.NoError(t, user_model.UpdateUserCols(t.Context(), &user_model.User{ID: 1, ProhibitLogin: true}, "prohibit_login"))
defer func() {
require.NoError(t, user_model.UpdateUserCols(t.Context(), &user_model.User{ID: 1}, "prohibit_login"))
}()
for _, token := range []string{parsed.AccessToken, parsed.RefreshToken} {
req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{"token": token}).
AddBasicAuth("da7da3ba-9a13-4167-856f-3899de0b0138", "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=")
assert.JSONEq(t, `{"active":false}`, MakeRequest(t, req, http.StatusOK).Body.String())
}
}
func testOAuthIntrospectionCrossClientIsolation(t *testing.T) {

View File

@@ -169,6 +169,10 @@ func TestRepoMergeUpstream(t *testing.T) {
t.Run("BasePrivateBlocksSync", func(t *testing.T) {
// add a new commit to the base repo, then make the base repo private
require.NoError(t, createOrReplaceFileInBranch(baseUser, baseRepo, "secret.txt", "master", "private-content"))
forkBranchMergeUpstreamLink := func() string {
return queryMergeUpstreamButtonLink(NewHTMLParser(t, session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK).Body))
}
require.Eventually(t, func() bool { return forkBranchMergeUpstreamLink() != "" }, 5*time.Second, 100*time.Millisecond)
baseRepo.IsPrivate = true
_, err := db.GetEngine(t.Context()).ID(baseRepo.ID).Cols("is_private").Update(baseRepo)
require.NoError(t, err)
@@ -177,6 +181,15 @@ func TestRepoMergeUpstream(t *testing.T) {
Branch: "fork-branch",
}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusForbidden)
assert.Empty(t, forkBranchMergeUpstreamLink())
testEditorActionPostRequestError(t, session, fmt.Sprintf("/%s/test-repo-fork/_edit/master/README.md?from_base_branch=master", forkUser.Name), map[string]string{
"tree_path": "README.md",
"content": "edited",
"commit_choice": "commit-to-new-branch",
"new_branch_name": "from-private-base",
}, "Failed to push branch from-private-base to your repository.")
session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/from-private-base", forkUser.Name), http.StatusNotFound)
})
})
}