diff --git a/models/actions/scoped_workflow.go b/models/actions/scoped_workflow.go index a2bce65ddad..829956d03ed 100644 --- a/models/actions/scoped_workflow.go +++ b/models/actions/scoped_workflow.go @@ -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 +} diff --git a/models/actions/scoped_workflow_test.go b/models/actions/scoped_workflow_test.go index f3b90f6cd17..b54d25a840d 100644 --- a/models/actions/scoped_workflow_test.go +++ b/models/actions/scoped_workflow_test.go @@ -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) diff --git a/models/fixtures/oauth2_grant.yml b/models/fixtures/oauth2_grant.yml index 54f4e45e62e..6b4cd66bab8 100644 --- a/models/fixtures/oauth2_grant.yml +++ b/models/fixtures/oauth2_grant.yml @@ -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" diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index afff02bcac3..491f1bfc46d 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -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) diff --git a/routers/private/hook_pre_receive_test.go b/routers/private/hook_pre_receive_test.go index 1ee2d57fdd2..6d753b25ced 100644 --- a/routers/private/hook_pre_receive_test.go +++ b/routers/private/hook_pre_receive_test.go @@ -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) { diff --git a/routers/private/hook_proc_receive.go b/routers/private/hook_proc_receive.go index 2f8ba5203f3..f1d6ea5835f 100644 --- a/routers/private/hook_proc_receive.go +++ b/routers/private/hook_proc_receive.go @@ -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 } diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index c2b44bd6620..ed73e24944e 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -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", diff --git a/routers/web/auth/oauth_test.go b/routers/web/auth/oauth_test.go index 6037f6e206c..da5808beaf7 100644 --- a/routers/web/auth/oauth_test.go +++ b/routers/web/auth/oauth_test.go @@ -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) { diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 616c2552831..2cf5711639c 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -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 { diff --git a/routers/web/repo/editor_util.go b/routers/web/repo/editor_util.go index 726f395f203..10a4a59ed3c 100644 --- a/routers/web/repo/editor_util.go +++ b/routers/web/repo/editor_util.go @@ -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), diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index 3236c428748..4e4037d667c 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -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) { diff --git a/services/lfs/server.go b/services/lfs/server.go index f093a940504..bd96757a7aa 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -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 } diff --git a/services/lfs/server_test.go b/services/lfs/server_test.go index 31830bf0280..150d2ce1122 100644 --- a/services/lfs/server_test.go +++ b/services/lfs/server_test.go @@ -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)) }) diff --git a/services/oauth2_provider/access_token.go b/services/oauth2_provider/access_token.go index 0ec10891bcc..6b19384c3a0 100644 --- a/services/oauth2_provider/access_token.go +++ b/services/oauth2_provider/access_token.go @@ -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, diff --git a/services/pull/commit_status_test.go b/services/pull/commit_status_test.go index 20889a76265..73197c3a259 100644 --- a/services/pull/commit_status_test.go +++ b/services/pull/commit_status_test.go @@ -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 diff --git a/services/pull/review.go b/services/pull/review.go index 7bf905f398b..1fa703385ca 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -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, diff --git a/services/pull/review_test.go b/services/pull/review_test.go index 9e9a398f669..db7fd29ee0e 100644 --- a/services/pull/review_test.go +++ b/services/pull/review_test.go @@ -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) { diff --git a/services/repository/merge_upstream.go b/services/repository/merge_upstream.go index 6820fca3372..449b504e08e 100644 --- a/services/repository/merge_upstream.go +++ b/services/repository/merge_upstream.go @@ -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` diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 92cbdea7fc9..e5470622043 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -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) diff --git a/services/repository/transfer_test.go b/services/repository/transfer_test.go index 95192b53018..1167cb225aa 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -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) diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index 6f442adb0f4..1f6f72b9609 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -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") diff --git a/tests/integration/git_misc_test.go b/tests/integration/git_misc_test.go index 9c476636429..27614a299fd 100644 --- a/tests/integration/git_misc_test.go +++ b/tests/integration/git_misc_test.go @@ -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 diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 5d74e748c92..8ae5822beb8 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -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) { diff --git a/tests/integration/repo_merge_upstream_test.go b/tests/integration/repo_merge_upstream_test.go index 59ffc6ab2cc..bbb11683a0a 100644 --- a/tests/integration/repo_merge_upstream_test.go +++ b/tests/integration/repo_merge_upstream_test.go @@ -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) }) }) }