// Copyright 2017 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
Prevent double use of `git cat-file` session. (#29298)
Fixes the reason why #29101 is hard to replicate.
Related #29297
Create a repo with a file with minimum size 4097 bytes (I use 10000) and
execute the following code:
```go
gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, <repo>)
assert.NoError(t, err)
commit, err := gitRepo.GetCommit(<sha>)
assert.NoError(t, err)
entry, err := commit.GetTreeEntryByPath(<file>)
assert.NoError(t, err)
b := entry.Blob()
// Create a reader
r, err := b.DataAsync()
assert.NoError(t, err)
defer r.Close()
// Create a second reader
r2, err := b.DataAsync()
assert.NoError(t, err) // Should be no error but is ErrNotExist
defer r2.Close()
```
The problem is the check in `CatFileBatch`:
https://github.com/go-gitea/gitea/blob/79217ea63c1f77de7ca79813ae45950724e63d02/modules/git/repo_base_nogogit.go#L81-L87
`Buffered() > 0` is used to check if there is a "operation" in progress
at the moment. This is a problem because we can't control the internal
buffer in the `bufio.Reader`. The code above demonstrates a sequence
which initiates an operation for which the code thinks there is no
active processing. The second call to `DataAsync()` therefore reuses the
existing instances instead of creating a new batch reader.
9 months ago
"bytes"
"encoding/hex"
"fmt"
"math/rand"
"net/http"
"net/url"
"os"
"path"
"path/filepath"
"strconv"
"testing"
"time"
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"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"
gitea_context "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/git"
Prevent double use of `git cat-file` session. (#29298)
Fixes the reason why #29101 is hard to replicate.
Related #29297
Create a repo with a file with minimum size 4097 bytes (I use 10000) and
execute the following code:
```go
gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, <repo>)
assert.NoError(t, err)
commit, err := gitRepo.GetCommit(<sha>)
assert.NoError(t, err)
entry, err := commit.GetTreeEntryByPath(<file>)
assert.NoError(t, err)
b := entry.Blob()
// Create a reader
r, err := b.DataAsync()
assert.NoError(t, err)
defer r.Close()
// Create a second reader
r2, err := b.DataAsync()
assert.NoError(t, err) // Should be no error but is ErrNotExist
defer r2.Close()
```
The problem is the check in `CatFileBatch`:
https://github.com/go-gitea/gitea/blob/79217ea63c1f77de7ca79813ae45950724e63d02/modules/git/repo_base_nogogit.go#L81-L87
`Buffered() > 0` is used to check if there is a "operation" in progress
at the moment. This is a problem because we can't control the internal
buffer in the `bufio.Reader`. The code above demonstrates a sequence
which initiates an operation for which the code thinks there is no
active processing. The second call to `DataAsync()` therefore reuses the
existing instances instead of creating a new batch reader.
9 months ago
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/lfs"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
Prevent double use of `git cat-file` session. (#29298)
Fixes the reason why #29101 is hard to replicate.
Related #29297
Create a repo with a file with minimum size 4097 bytes (I use 10000) and
execute the following code:
```go
gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, <repo>)
assert.NoError(t, err)
commit, err := gitRepo.GetCommit(<sha>)
assert.NoError(t, err)
entry, err := commit.GetTreeEntryByPath(<file>)
assert.NoError(t, err)
b := entry.Blob()
// Create a reader
r, err := b.DataAsync()
assert.NoError(t, err)
defer r.Close()
// Create a second reader
r2, err := b.DataAsync()
assert.NoError(t, err) // Should be no error but is ErrNotExist
defer r2.Close()
```
The problem is the check in `CatFileBatch`:
https://github.com/go-gitea/gitea/blob/79217ea63c1f77de7ca79813ae45950724e63d02/modules/git/repo_base_nogogit.go#L81-L87
`Buffered() > 0` is used to check if there is a "operation" in progress
at the moment. This is a problem because we can't control the internal
buffer in the `bufio.Reader`. The code above demonstrates a sequence
which initiates an operation for which the code thinks there is no
active processing. The second call to `DataAsync()` therefore reuses the
existing instances instead of creating a new batch reader.
9 months ago
files_service "code.gitea.io/gitea/services/repository/files"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
)
const (
littleSize = 1024 // 1ko
bigSize = 128 * 1024 * 1024 // 128Mo
)
func TestGit ( t * testing . T ) {
onGiteaRun ( t , testGit )
}
func testGit ( t * testing . T , u * url . URL ) {
username := "user2"
Redesign Scoped Access Tokens (#24767)
## Changes
- Adds the following high level access scopes, each with `read` and
`write` levels:
- `activitypub`
- `admin` (hidden if user is not a site admin)
- `misc`
- `notification`
- `organization`
- `package`
- `issue`
- `repository`
- `user`
- Adds new middleware function `tokenRequiresScopes()` in addition to
`reqToken()`
- `tokenRequiresScopes()` is used for each high-level api section
- _if_ a scoped token is present, checks that the required scope is
included based on the section and HTTP method
- `reqToken()` is used for individual routes
- checks that required authentication is present (but does not check
scope levels as this will already have been handled by
`tokenRequiresScopes()`
- Adds migration to convert old scoped access tokens to the new set of
scopes
- Updates the user interface for scope selection
### User interface example
<img width="903" alt="Screen Shot 2023-05-31 at 1 56 55 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/654766ec-2143-4f59-9037-3b51600e32f3">
<img width="917" alt="Screen Shot 2023-05-31 at 1 56 43 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/1ad64081-012c-4a73-b393-66b30352654c">
## tokenRequiresScopes Design Decision
- `tokenRequiresScopes()` was added to more reliably cover api routes.
For an incoming request, this function uses the given scope category
(say `AccessTokenScopeCategoryOrganization`) and the HTTP method (say
`DELETE`) and verifies that any scoped tokens in use include
`delete:organization`.
- `reqToken()` is used to enforce auth for individual routes that
require it. If a scoped token is not present for a request,
`tokenRequiresScopes()` will not return an error
## TODO
- [x] Alphabetize scope categories
- [x] Change 'public repos only' to a radio button (private vs public).
Also expand this to organizations
- [X] Disable token creation if no scopes selected. Alternatively, show
warning
- [x] `reqToken()` is missing from many `POST/DELETE` routes in the api.
`tokenRequiresScopes()` only checks that a given token has the correct
scope, `reqToken()` must be used to check that a token (or some other
auth) is present.
- _This should be addressed in this PR_
- [x] The migration should be reviewed very carefully in order to
minimize access changes to existing user tokens.
- _This should be addressed in this PR_
- [x] Link to api to swagger documentation, clarify what
read/write/delete levels correspond to
- [x] Review cases where more than one scope is needed as this directly
deviates from the api definition.
- _This should be addressed in this PR_
- For example:
```go
m.Group("/users/{username}/orgs", func() {
m.Get("", reqToken(), org.ListUserOrgs)
m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser,
auth_model.AccessTokenScopeCategoryOrganization),
context_service.UserAssignmentAPI())
```
## Future improvements
- [ ] Add required scopes to swagger documentation
- [ ] Redesign `reqToken()` to be opt-out rather than opt-in
- [ ] Subdivide scopes like `repository`
- [ ] Once a token is created, if it has no scopes, we should display
text instead of an empty bullet point
- [ ] If the 'public repos only' option is selected, should read
categories be selected by default
Closes #24501
Closes #24799
Co-authored-by: Jonathan Tran <jon@allspice.io>
Co-authored-by: Kyle D <kdumontnu@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
1 year ago
baseAPITestContext := NewAPITestContext ( t , username , "repo1" , auth_model . AccessTokenScopeWriteRepository , auth_model . AccessTokenScopeWriteUser )
u . Path = baseAPITestContext . GitPath ( )
Redesign Scoped Access Tokens (#24767)
## Changes
- Adds the following high level access scopes, each with `read` and
`write` levels:
- `activitypub`
- `admin` (hidden if user is not a site admin)
- `misc`
- `notification`
- `organization`
- `package`
- `issue`
- `repository`
- `user`
- Adds new middleware function `tokenRequiresScopes()` in addition to
`reqToken()`
- `tokenRequiresScopes()` is used for each high-level api section
- _if_ a scoped token is present, checks that the required scope is
included based on the section and HTTP method
- `reqToken()` is used for individual routes
- checks that required authentication is present (but does not check
scope levels as this will already have been handled by
`tokenRequiresScopes()`
- Adds migration to convert old scoped access tokens to the new set of
scopes
- Updates the user interface for scope selection
### User interface example
<img width="903" alt="Screen Shot 2023-05-31 at 1 56 55 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/654766ec-2143-4f59-9037-3b51600e32f3">
<img width="917" alt="Screen Shot 2023-05-31 at 1 56 43 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/1ad64081-012c-4a73-b393-66b30352654c">
## tokenRequiresScopes Design Decision
- `tokenRequiresScopes()` was added to more reliably cover api routes.
For an incoming request, this function uses the given scope category
(say `AccessTokenScopeCategoryOrganization`) and the HTTP method (say
`DELETE`) and verifies that any scoped tokens in use include
`delete:organization`.
- `reqToken()` is used to enforce auth for individual routes that
require it. If a scoped token is not present for a request,
`tokenRequiresScopes()` will not return an error
## TODO
- [x] Alphabetize scope categories
- [x] Change 'public repos only' to a radio button (private vs public).
Also expand this to organizations
- [X] Disable token creation if no scopes selected. Alternatively, show
warning
- [x] `reqToken()` is missing from many `POST/DELETE` routes in the api.
`tokenRequiresScopes()` only checks that a given token has the correct
scope, `reqToken()` must be used to check that a token (or some other
auth) is present.
- _This should be addressed in this PR_
- [x] The migration should be reviewed very carefully in order to
minimize access changes to existing user tokens.
- _This should be addressed in this PR_
- [x] Link to api to swagger documentation, clarify what
read/write/delete levels correspond to
- [x] Review cases where more than one scope is needed as this directly
deviates from the api definition.
- _This should be addressed in this PR_
- For example:
```go
m.Group("/users/{username}/orgs", func() {
m.Get("", reqToken(), org.ListUserOrgs)
m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser,
auth_model.AccessTokenScopeCategoryOrganization),
context_service.UserAssignmentAPI())
```
## Future improvements
- [ ] Add required scopes to swagger documentation
- [ ] Redesign `reqToken()` to be opt-out rather than opt-in
- [ ] Subdivide scopes like `repository`
- [ ] Once a token is created, if it has no scopes, we should display
text instead of an empty bullet point
- [ ] If the 'public repos only' option is selected, should read
categories be selected by default
Closes #24501
Closes #24799
Co-authored-by: Jonathan Tran <jon@allspice.io>
Co-authored-by: Kyle D <kdumontnu@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
1 year ago
forkedUserCtx := NewAPITestContext ( t , "user4" , "repo1" , auth_model . AccessTokenScopeWriteRepository , auth_model . AccessTokenScopeWriteUser )
t . Run ( "HTTP" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
ensureAnonymousClone ( t , u )
httpContext := baseAPITestContext
httpContext . Reponame = "repo-tmp-17"
forkedUserCtx . Reponame = httpContext . Reponame
dstPath := t . TempDir ( )
t . Run ( "CreateRepoInDifferentUser" , doAPICreateRepository ( forkedUserCtx , false ) )
t . Run ( "AddUserAsCollaborator" , doAPIAddCollaborator ( forkedUserCtx , httpContext . Username , perm . AccessModeRead ) )
t . Run ( "ForkFromDifferentUser" , doAPIForkRepository ( httpContext , forkedUserCtx . Username ) )
u . Path = httpContext . GitPath ( )
u . User = url . UserPassword ( username , userPassword )
t . Run ( "Clone" , doGitClone ( dstPath , u ) )
dstPath2 := t . TempDir ( )
t . Run ( "Partial Clone" , doPartialGitClone ( dstPath2 , u ) )
little , big := standardCommitAndPushTest ( t , dstPath )
littleLFS , bigLFS := lfsCommitAndPushTest ( t , dstPath )
rawTest ( t , & httpContext , little , big , littleLFS , bigLFS )
mediaTest ( t , & httpContext , little , big , littleLFS , bigLFS )
t . Run ( "CreateAgitFlowPull" , doCreateAgitFlowPull ( dstPath , & httpContext , "master" , "test/head" ) )
t . Run ( "BranchProtectMerge" , doBranchProtectPRMerge ( & httpContext , dstPath ) )
t . Run ( "AutoMerge" , doAutoPRMerge ( & httpContext , dstPath ) )
t . Run ( "CreatePRAndSetManuallyMerged" , doCreatePRAndSetManuallyMerged ( httpContext , httpContext , dstPath , "master" , "test-manually-merge" ) )
t . Run ( "MergeFork" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
t . Run ( "CreatePRAndMerge" , doMergeFork ( httpContext , forkedUserCtx , "master" , httpContext . Username + ":master" ) )
rawTest ( t , & forkedUserCtx , little , big , littleLFS , bigLFS )
mediaTest ( t , & forkedUserCtx , little , big , littleLFS , bigLFS )
} )
t . Run ( "PushCreate" , doPushCreate ( httpContext , u ) )
} )
t . Run ( "SSH" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
sshContext := baseAPITestContext
sshContext . Reponame = "repo-tmp-18"
keyname := "my-testing-key"
forkedUserCtx . Reponame = sshContext . Reponame
t . Run ( "CreateRepoInDifferentUser" , doAPICreateRepository ( forkedUserCtx , false ) )
t . Run ( "AddUserAsCollaborator" , doAPIAddCollaborator ( forkedUserCtx , sshContext . Username , perm . AccessModeRead ) )
t . Run ( "ForkFromDifferentUser" , doAPIForkRepository ( sshContext , forkedUserCtx . Username ) )
// Setup key the user ssh key
withKeyFile ( t , keyname , func ( keyFile string ) {
t . Run ( "CreateUserKey" , doAPICreateUserKey ( sshContext , "test-key" , keyFile ) )
// Setup remote link
// TODO: get url from api
sshURL := createSSHUrl ( sshContext . GitPath ( ) , u )
// Setup clone folder
dstPath := t . TempDir ( )
t . Run ( "Clone" , doGitClone ( dstPath , sshURL ) )
little , big := standardCommitAndPushTest ( t , dstPath )
littleLFS , bigLFS := lfsCommitAndPushTest ( t , dstPath )
rawTest ( t , & sshContext , little , big , littleLFS , bigLFS )
mediaTest ( t , & sshContext , little , big , littleLFS , bigLFS )
t . Run ( "CreateAgitFlowPull" , doCreateAgitFlowPull ( dstPath , & sshContext , "master" , "test/head2" ) )
t . Run ( "BranchProtectMerge" , doBranchProtectPRMerge ( & sshContext , dstPath ) )
t . Run ( "MergeFork" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
t . Run ( "CreatePRAndMerge" , doMergeFork ( sshContext , forkedUserCtx , "master" , sshContext . Username + ":master" ) )
rawTest ( t , & forkedUserCtx , little , big , littleLFS , bigLFS )
mediaTest ( t , & forkedUserCtx , little , big , littleLFS , bigLFS )
} )
t . Run ( "PushCreate" , doPushCreate ( sshContext , sshURL ) )
} )
} )
}
func ensureAnonymousClone ( t * testing . T , u * url . URL ) {
dstLocalPath := t . TempDir ( )
t . Run ( "CloneAnonymous" , doGitClone ( dstLocalPath , u ) )
}
func standardCommitAndPushTest ( t * testing . T , dstPath string ) ( little , big string ) {
t . Run ( "Standard" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
little , big = commitAndPushTest ( t , dstPath , "data-file-" )
} )
return little , big
}
func lfsCommitAndPushTest ( t * testing . T , dstPath string ) ( littleLFS , bigLFS string ) {
t . Run ( "LFS" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
prefix := "lfs-data-file-"
err := git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "install" ) . Run ( & git . RunOpts { Dir : dstPath } )
assert . NoError ( t , err )
_ , _ , err = git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "track" ) . AddDynamicArguments ( prefix + "*" ) . RunStdString ( & git . RunOpts { Dir : dstPath } )
assert . NoError ( t , err )
err = git . AddChanges ( dstPath , false , ".gitattributes" )
assert . NoError ( t , err )
err = git . CommitChangesWithArgs ( dstPath , git . AllowLFSFiltersArgs ( ) , git . CommitChangesOptions {
Committer : & git . Signature {
Email : "user2@example.com" ,
Name : "User Two" ,
When : time . Now ( ) ,
} ,
Author : & git . Signature {
Email : "user2@example.com" ,
Name : "User Two" ,
When : time . Now ( ) ,
} ,
Message : fmt . Sprintf ( "Testing commit @ %v" , time . Now ( ) ) ,
} )
assert . NoError ( t , err )
littleLFS , bigLFS = commitAndPushTest ( t , dstPath , prefix )
t . Run ( "Locks" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
lockTest ( t , dstPath )
} )
} )
return littleLFS , bigLFS
}
func commitAndPushTest ( t * testing . T , dstPath , prefix string ) ( little , big string ) {
t . Run ( "PushCommit" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
t . Run ( "Little" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
little = doCommitAndPush ( t , littleSize , dstPath , prefix )
} )
t . Run ( "Big" , func ( t * testing . T ) {
if testing . Short ( ) {
t . Skip ( "Skipping test in short mode." )
return
}
defer tests . PrintCurrentTest ( t ) ( )
big = doCommitAndPush ( t , bigSize , dstPath , prefix )
} )
} )
return little , big
}
func rawTest ( t * testing . T , ctx * APITestContext , little , big , littleLFS , bigLFS string ) {
t . Run ( "Raw" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
username := ctx . Username
reponame := ctx . Reponame
session := loginUser ( t , username )
// Request raw paths
req := NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/raw/branch/master/" , little ) )
resp := session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , littleSize , resp . Length )
if setting . LFS . StartServer {
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/raw/branch/master/" , littleLFS ) )
resp := session . MakeRequest ( t , req , http . StatusOK )
assert . NotEqual ( t , littleSize , resp . Body . Len ( ) )
assert . LessOrEqual ( t , resp . Body . Len ( ) , 1024 )
if resp . Body . Len ( ) != littleSize && resp . Body . Len ( ) <= 1024 {
assert . Contains ( t , resp . Body . String ( ) , lfs . MetaFileIdentifier )
}
}
if ! testing . Short ( ) {
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/raw/branch/master/" , big ) )
resp := session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , bigSize , resp . Length )
if setting . LFS . StartServer {
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/raw/branch/master/" , bigLFS ) )
resp := session . MakeRequest ( t , req , http . StatusOK )
assert . NotEqual ( t , bigSize , resp . Body . Len ( ) )
if resp . Body . Len ( ) != bigSize && resp . Body . Len ( ) <= 1024 {
assert . Contains ( t , resp . Body . String ( ) , lfs . MetaFileIdentifier )
}
}
}
} )
}
func mediaTest ( t * testing . T , ctx * APITestContext , little , big , littleLFS , bigLFS string ) {
t . Run ( "Media" , func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
username := ctx . Username
reponame := ctx . Reponame
session := loginUser ( t , username )
// Request media paths
req := NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/media/branch/master/" , little ) )
resp := session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , littleSize , resp . Length )
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/media/branch/master/" , littleLFS ) )
resp = session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , littleSize , resp . Length )
if ! testing . Short ( ) {
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/media/branch/master/" , big ) )
resp = session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , bigSize , resp . Length )
if setting . LFS . StartServer {
req = NewRequest ( t , "GET" , path . Join ( "/" , username , reponame , "/media/branch/master/" , bigLFS ) )
resp = session . MakeRequestNilResponseRecorder ( t , req , http . StatusOK )
assert . Equal ( t , bigSize , resp . Length )
}
}
} )
}
func lockTest ( t * testing . T , repoPath string ) {
lockFileTest ( t , "README.md" , repoPath )
}
func lockFileTest ( t * testing . T , filename , repoPath string ) {
_ , _ , err := git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "locks" ) . RunStdString ( & git . RunOpts { Dir : repoPath } )
assert . NoError ( t , err )
_ , _ , err = git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "lock" ) . AddDynamicArguments ( filename ) . RunStdString ( & git . RunOpts { Dir : repoPath } )
assert . NoError ( t , err )
_ , _ , err = git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "locks" ) . RunStdString ( & git . RunOpts { Dir : repoPath } )
assert . NoError ( t , err )
_ , _ , err = git . NewCommand ( git . DefaultContext , "lfs" ) . AddArguments ( "unlock" ) . AddDynamicArguments ( filename ) . RunStdString ( & git . RunOpts { Dir : repoPath } )
assert . NoError ( t , err )
}
func doCommitAndPush ( t * testing . T , size int , repoPath , prefix string ) string {
name , err := generateCommitWithNewData ( size , repoPath , "user2@example.com" , "User Two" , prefix )
assert . NoError ( t , err )
_ , _ , err = git . NewCommand ( git . DefaultContext , "push" , "origin" , "master" ) . RunStdString ( & git . RunOpts { Dir : repoPath } ) // Push
assert . NoError ( t , err )
return name
}
func generateCommitWithNewData ( size int , repoPath , email , fullName , prefix string ) ( string , error ) {
// Generate random file
bufSize := 4 * 1024
if bufSize > size {
bufSize = size
}
buffer := make ( [ ] byte , bufSize )
tmpFile , err := os . CreateTemp ( repoPath , prefix )
if err != nil {
return "" , err
}
defer tmpFile . Close ( )
written := 0
for written < size {
n := size - written
if n > bufSize {
n = bufSize
}
_ , err := rand . Read ( buffer [ : n ] )
if err != nil {
return "" , err
}
n , err = tmpFile . Write ( buffer [ : n ] )
if err != nil {
return "" , err
}
written += n
}
if err != nil {
return "" , err
}
// Commit
// Now here we should explicitly allow lfs filters to run
globalArgs := git . AllowLFSFiltersArgs ( )
err = git . AddChangesWithArgs ( repoPath , globalArgs , false , filepath . Base ( tmpFile . Name ( ) ) )
if err != nil {
return "" , err
}
err = git . CommitChangesWithArgs ( repoPath , globalArgs , git . CommitChangesOptions {
Committer : & git . Signature {
Email : email ,
Name : fullName ,
When : time . Now ( ) ,
} ,
Author : & git . Signature {
Email : email ,
Name : fullName ,
When : time . Now ( ) ,
} ,
Message : fmt . Sprintf ( "Testing commit @ %v" , time . Now ( ) ) ,
} )
return filepath . Base ( tmpFile . Name ( ) ) , err
}
func doBranchProtectPRMerge ( baseCtx * APITestContext , dstPath string ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
t . Run ( "CreateBranchProtected" , doGitCreateBranch ( dstPath , "protected" ) )
t . Run ( "PushProtectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "protected" ) )
Redesign Scoped Access Tokens (#24767)
## Changes
- Adds the following high level access scopes, each with `read` and
`write` levels:
- `activitypub`
- `admin` (hidden if user is not a site admin)
- `misc`
- `notification`
- `organization`
- `package`
- `issue`
- `repository`
- `user`
- Adds new middleware function `tokenRequiresScopes()` in addition to
`reqToken()`
- `tokenRequiresScopes()` is used for each high-level api section
- _if_ a scoped token is present, checks that the required scope is
included based on the section and HTTP method
- `reqToken()` is used for individual routes
- checks that required authentication is present (but does not check
scope levels as this will already have been handled by
`tokenRequiresScopes()`
- Adds migration to convert old scoped access tokens to the new set of
scopes
- Updates the user interface for scope selection
### User interface example
<img width="903" alt="Screen Shot 2023-05-31 at 1 56 55 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/654766ec-2143-4f59-9037-3b51600e32f3">
<img width="917" alt="Screen Shot 2023-05-31 at 1 56 43 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/1ad64081-012c-4a73-b393-66b30352654c">
## tokenRequiresScopes Design Decision
- `tokenRequiresScopes()` was added to more reliably cover api routes.
For an incoming request, this function uses the given scope category
(say `AccessTokenScopeCategoryOrganization`) and the HTTP method (say
`DELETE`) and verifies that any scoped tokens in use include
`delete:organization`.
- `reqToken()` is used to enforce auth for individual routes that
require it. If a scoped token is not present for a request,
`tokenRequiresScopes()` will not return an error
## TODO
- [x] Alphabetize scope categories
- [x] Change 'public repos only' to a radio button (private vs public).
Also expand this to organizations
- [X] Disable token creation if no scopes selected. Alternatively, show
warning
- [x] `reqToken()` is missing from many `POST/DELETE` routes in the api.
`tokenRequiresScopes()` only checks that a given token has the correct
scope, `reqToken()` must be used to check that a token (or some other
auth) is present.
- _This should be addressed in this PR_
- [x] The migration should be reviewed very carefully in order to
minimize access changes to existing user tokens.
- _This should be addressed in this PR_
- [x] Link to api to swagger documentation, clarify what
read/write/delete levels correspond to
- [x] Review cases where more than one scope is needed as this directly
deviates from the api definition.
- _This should be addressed in this PR_
- For example:
```go
m.Group("/users/{username}/orgs", func() {
m.Get("", reqToken(), org.ListUserOrgs)
m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser,
auth_model.AccessTokenScopeCategoryOrganization),
context_service.UserAssignmentAPI())
```
## Future improvements
- [ ] Add required scopes to swagger documentation
- [ ] Redesign `reqToken()` to be opt-out rather than opt-in
- [ ] Subdivide scopes like `repository`
- [ ] Once a token is created, if it has no scopes, we should display
text instead of an empty bullet point
- [ ] If the 'public repos only' option is selected, should read
categories be selected by default
Closes #24501
Closes #24799
Co-authored-by: Jonathan Tran <jon@allspice.io>
Co-authored-by: Kyle D <kdumontnu@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
1 year ago
ctx := NewAPITestContext ( t , baseCtx . Username , baseCtx . Reponame , auth_model . AccessTokenScopeWriteRepository )
t . Run ( "ProtectProtectedBranchNoWhitelist" , doProtectBranch ( ctx , "protected" , "" , "" ) )
t . Run ( "GenerateCommit" , func ( t * testing . T ) {
_ , err := generateCommitWithNewData ( littleSize , dstPath , "user2@example.com" , "User Two" , "branch-data-file-" )
assert . NoError ( t , err )
} )
t . Run ( "FailToPushToProtectedBranch" , doGitPushTestRepositoryFail ( dstPath , "origin" , "protected" ) )
t . Run ( "PushToUnprotectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "protected:unprotected" ) )
var pr api . PullRequest
var err error
t . Run ( "CreatePullRequest" , func ( t * testing . T ) {
pr , err = doAPICreatePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , "protected" , "unprotected" ) ( t )
assert . NoError ( t , err )
} )
t . Run ( "GenerateCommit" , func ( t * testing . T ) {
_ , err := generateCommitWithNewData ( littleSize , dstPath , "user2@example.com" , "User Two" , "branch-data-file-" )
assert . NoError ( t , err )
} )
t . Run ( "PushToUnprotectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "protected:unprotected-2" ) )
var pr2 api . PullRequest
t . Run ( "CreatePullRequest" , func ( t * testing . T ) {
pr2 , err = doAPICreatePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , "unprotected" , "unprotected-2" ) ( t )
assert . NoError ( t , err )
} )
t . Run ( "MergePR2" , doAPIMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr2 . Index ) )
t . Run ( "MergePR" , doAPIMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
t . Run ( "PullProtected" , doGitPull ( dstPath , "origin" , "protected" ) )
t . Run ( "ProtectProtectedBranchUnprotectedFilePaths" , doProtectBranch ( ctx , "protected" , "" , "unprotected-file-*" ) )
t . Run ( "GenerateCommit" , func ( t * testing . T ) {
_ , err := generateCommitWithNewData ( littleSize , dstPath , "user2@example.com" , "User Two" , "unprotected-file-" )
assert . NoError ( t , err )
} )
t . Run ( "PushUnprotectedFilesToProtectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "protected" ) )
t . Run ( "ProtectProtectedBranchWhitelist" , doProtectBranch ( ctx , "protected" , baseCtx . Username , "" ) )
t . Run ( "CheckoutMaster" , doGitCheckoutBranch ( dstPath , "master" ) )
t . Run ( "CreateBranchForced" , doGitCreateBranch ( dstPath , "toforce" ) )
t . Run ( "GenerateCommit" , func ( t * testing . T ) {
_ , err := generateCommitWithNewData ( littleSize , dstPath , "user2@example.com" , "User Two" , "branch-data-file-" )
assert . NoError ( t , err )
} )
t . Run ( "FailToForcePushToProtectedBranch" , doGitPushTestRepositoryFail ( dstPath , "-f" , "origin" , "toforce:protected" ) )
t . Run ( "MergeProtectedToToforce" , doGitMerge ( dstPath , "protected" ) )
t . Run ( "PushToProtectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "toforce:protected" ) )
t . Run ( "CheckoutMasterAgain" , doGitCheckoutBranch ( dstPath , "master" ) )
}
}
func doProtectBranch ( ctx APITestContext , branch , userToWhitelist , unprotectedFilePatterns string ) func ( t * testing . T ) {
// We are going to just use the owner to set the protection.
return func ( t * testing . T ) {
csrf := GetCSRF ( t , ctx . Session , fmt . Sprintf ( "/%s/%s/settings/branches" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) ) )
if userToWhitelist == "" {
// Change branch to protected
req := NewRequestWithValues ( t , "POST" , fmt . Sprintf ( "/%s/%s/settings/branches/edit" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) ) , map [ string ] string {
"_csrf" : csrf ,
"rule_name" : branch ,
"unprotected_file_patterns" : unprotectedFilePatterns ,
} )
ctx . Session . MakeRequest ( t , req , http . StatusSeeOther )
} else {
user , err := user_model . GetUserByName ( db . DefaultContext , userToWhitelist )
assert . NoError ( t , err )
// Change branch to protected
req := NewRequestWithValues ( t , "POST" , fmt . Sprintf ( "/%s/%s/settings/branches/edit" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) ) , map [ string ] string {
"_csrf" : csrf ,
"rule_name" : branch ,
"enable_push" : "whitelist" ,
"enable_whitelist" : "on" ,
"whitelist_users" : strconv . FormatInt ( user . ID , 10 ) ,
"unprotected_file_patterns" : unprotectedFilePatterns ,
} )
ctx . Session . MakeRequest ( t , req , http . StatusSeeOther )
}
// Check if master branch has been locked successfully
flashCookie := ctx . Session . GetCookie ( gitea_context . CookieNameFlash )
assert . NotNil ( t , flashCookie )
assert . EqualValues ( t , "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2522" + url . QueryEscape ( branch ) + "%2522%2Bhas%2Bbeen%2Bupdated." , flashCookie . Value )
}
}
func doMergeFork ( ctx , baseCtx APITestContext , baseBranch , headBranch string ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
var pr api . PullRequest
var err error
// Create a test pullrequest
t . Run ( "CreatePullRequest" , func ( t * testing . T ) {
pr , err = doAPICreatePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , baseBranch , headBranch ) ( t )
assert . NoError ( t , err )
} )
// Ensure the PR page works
t . Run ( "EnsureCanSeePull" , doEnsureCanSeePull ( baseCtx , pr ) )
// Then get the diff string
var diffHash string
var diffLength int
t . Run ( "GetDiff" , func ( t * testing . T ) {
req := NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d.diff" , url . PathEscape ( baseCtx . Username ) , url . PathEscape ( baseCtx . Reponame ) , pr . Index ) )
resp := ctx . Session . MakeRequestNilResponseHashSumRecorder ( t , req , http . StatusOK )
diffHash = string ( resp . Hash . Sum ( nil ) )
diffLength = resp . Length
} )
// Now: Merge the PR & make sure that doesn't break the PR page or change its diff
t . Run ( "MergePR" , doAPIMergePullRequest ( baseCtx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
t . Run ( "EnsureCanSeePull" , doEnsureCanSeePull ( baseCtx , pr ) )
t . Run ( "CheckPR" , func ( t * testing . T ) {
oldMergeBase := pr . MergeBase
pr2 , err := doAPIGetPullRequest ( baseCtx , baseCtx . Username , baseCtx . Reponame , pr . Index ) ( t )
assert . NoError ( t , err )
assert . Equal ( t , oldMergeBase , pr2 . MergeBase )
} )
t . Run ( "EnsurDiffNoChange" , doEnsureDiffNoChange ( baseCtx , pr , diffHash , diffLength ) )
// Then: Delete the head branch & make sure that doesn't break the PR page or change its diff
t . Run ( "DeleteHeadBranch" , doBranchDelete ( baseCtx , baseCtx . Username , baseCtx . Reponame , headBranch ) )
t . Run ( "EnsureCanSeePull" , doEnsureCanSeePull ( baseCtx , pr ) )
t . Run ( "EnsureDiffNoChange" , doEnsureDiffNoChange ( baseCtx , pr , diffHash , diffLength ) )
// Delete the head repository & make sure that doesn't break the PR page or change its diff
t . Run ( "DeleteHeadRepository" , doAPIDeleteRepository ( ctx ) )
t . Run ( "EnsureCanSeePull" , doEnsureCanSeePull ( baseCtx , pr ) )
t . Run ( "EnsureDiffNoChange" , doEnsureDiffNoChange ( baseCtx , pr , diffHash , diffLength ) )
}
}
func doCreatePRAndSetManuallyMerged ( ctx , baseCtx APITestContext , dstPath , baseBranch , headBranch string ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
var (
pr api . PullRequest
err error
lastCommitID string
)
trueBool := true
falseBool := false
t . Run ( "AllowSetManuallyMergedAndSwitchOffAutodetectManualMerge" , doAPIEditRepository ( baseCtx , & api . EditRepoOption {
HasPullRequests : & trueBool ,
AllowManualMerge : & trueBool ,
AutodetectManualMerge : & falseBool ,
} ) )
t . Run ( "CreateHeadBranch" , doGitCreateBranch ( dstPath , headBranch ) )
Refactor git command package to improve security and maintainability (#22678)
This PR follows #21535 (and replace #22592)
## Review without space diff
https://github.com/go-gitea/gitea/pull/22678/files?diff=split&w=1
## Purpose of this PR
1. Make git module command completely safe (risky user inputs won't be
passed as argument option anymore)
2. Avoid low-level mistakes like
https://github.com/go-gitea/gitea/pull/22098#discussion_r1045234918
3. Remove deprecated and dirty `CmdArgCheck` function, hide the `CmdArg`
type
4. Simplify code when using git command
## The main idea of this PR
* Move the `git.CmdArg` to the `internal` package, then no other package
except `git` could use it. Then developers could never do
`AddArguments(git.CmdArg(userInput))` any more.
* Introduce `git.ToTrustedCmdArgs`, it's for user-provided and already
trusted arguments. It's only used in a few cases, for example: use git
arguments from config file, help unit test with some arguments.
* Introduce `AddOptionValues` and `AddOptionFormat`, they make code more
clear and simple:
* Before: `AddArguments("-m").AddDynamicArguments(message)`
* After: `AddOptionValues("-m", message)`
* -
* Before: `AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'",
sig.Name, sig.Email)))`
* After: `AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email)`
## FAQ
### Why these changes were not done in #21535 ?
#21535 is mainly a search&replace, it did its best to not change too
much logic.
Making the framework better needs a lot of changes, so this separate PR
is needed as the second step.
### The naming of `AddOptionXxx`
According to git's manual, the `--xxx` part is called `option`.
### How can it guarantee that `internal.CmdArg` won't be not misused?
Go's specification guarantees that. Trying to access other package's
internal package causes compilation error.
And, `golangci-lint` also denies the git/internal package. Only the
`git/command.go` can use it carefully.
### There is still a `ToTrustedCmdArgs`, will it still allow developers
to make mistakes and pass untrusted arguments?
Generally speaking, no. Because when using `ToTrustedCmdArgs`, the code
will be very complex (see the changes for examples). Then developers and
reviewers can know that something might be unreasonable.
### Why there was a `CmdArgCheck` and why it's removed?
At the moment of #21535, to reduce unnecessary changes, `CmdArgCheck`
was introduced as a hacky patch. Now, almost all code could be written
as `cmd := NewCommand(); cmd.AddXxx(...)`, then there is no need for
`CmdArgCheck` anymore.
### Why many codes for `signArg == ""` is deleted?
Because in the old code, `signArg` could never be empty string, it's
either `-S[key-id]` or `--no-gpg-sign`. So the `signArg == ""` is just
dead code.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
2 years ago
t . Run ( "PushToHeadBranch" , doGitPushTestRepository ( dstPath , "origin" , headBranch ) )
t . Run ( "CreateEmptyPullRequest" , func ( t * testing . T ) {
pr , err = doAPICreatePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , baseBranch , headBranch ) ( t )
assert . NoError ( t , err )
} )
lastCommitID = pr . Base . Sha
t . Run ( "ManuallyMergePR" , doAPIManuallyMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , lastCommitID , pr . Index ) )
}
}
func doEnsureCanSeePull ( ctx APITestContext , pr api . PullRequest ) func ( t * testing . T ) {
return func ( t * testing . T ) {
req := NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) , pr . Index ) )
ctx . Session . MakeRequest ( t , req , http . StatusOK )
req = NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d/files" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) , pr . Index ) )
ctx . Session . MakeRequest ( t , req , http . StatusOK )
req = NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d/commits" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) , pr . Index ) )
ctx . Session . MakeRequest ( t , req , http . StatusOK )
}
}
func doEnsureDiffNoChange ( ctx APITestContext , pr api . PullRequest , diffHash string , diffLength int ) func ( t * testing . T ) {
return func ( t * testing . T ) {
req := NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d.diff" , url . PathEscape ( ctx . Username ) , url . PathEscape ( ctx . Reponame ) , pr . Index ) )
resp := ctx . Session . MakeRequestNilResponseHashSumRecorder ( t , req , http . StatusOK )
actual := string ( resp . Hash . Sum ( nil ) )
actualLength := resp . Length
equal := diffHash == actual
assert . True ( t , equal , "Unexpected change in the diff string: expected hash: %s size: %d but was actually: %s size: %d" , hex . EncodeToString ( [ ] byte ( diffHash ) ) , diffLength , hex . EncodeToString ( [ ] byte ( actual ) ) , actualLength )
}
}
func doPushCreate ( ctx APITestContext , u * url . URL ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
// create a context for a currently non-existent repository
ctx . Reponame = fmt . Sprintf ( "repo-tmp-push-create-%s" , u . Scheme )
u . Path = ctx . GitPath ( )
// Create a temporary directory
tmpDir := t . TempDir ( )
// Now create local repository to push as our test and set its origin
t . Run ( "InitTestRepository" , doGitInitTestRepository ( tmpDir ) )
t . Run ( "AddRemote" , doGitAddRemote ( tmpDir , "origin" , u ) )
// Disable "Push To Create" and attempt to push
setting . Repository . EnablePushCreateUser = false
t . Run ( "FailToPushAndCreateTestRepository" , doGitPushTestRepositoryFail ( tmpDir , "origin" , "master" ) )
// Enable "Push To Create"
setting . Repository . EnablePushCreateUser = true
// Assert that cloning from a non-existent repository does not create it and that it definitely wasn't create above
t . Run ( "FailToCloneFromNonExistentRepository" , doGitCloneFail ( u ) )
// Then "Push To Create"x
t . Run ( "SuccessfullyPushAndCreateTestRepository" , doGitPushTestRepository ( tmpDir , "origin" , "master" ) )
// Finally, fetch repo from database and ensure the correct repository has been created
repo , err := repo_model . GetRepositoryByOwnerAndName ( db . DefaultContext , ctx . Username , ctx . Reponame )
assert . NoError ( t , err )
assert . False ( t , repo . IsEmpty )
assert . True ( t , repo . IsPrivate )
// Now add a remote that is invalid to "Push To Create"
invalidCtx := ctx
invalidCtx . Reponame = fmt . Sprintf ( "invalid/repo-tmp-push-create-%s" , u . Scheme )
u . Path = invalidCtx . GitPath ( )
t . Run ( "AddInvalidRemote" , doGitAddRemote ( tmpDir , "invalid" , u ) )
// Fail to "Push To Create" the invalid
t . Run ( "FailToPushAndCreateInvalidTestRepository" , doGitPushTestRepositoryFail ( tmpDir , "invalid" , "master" ) )
}
}
func doBranchDelete ( ctx APITestContext , owner , repo , branch string ) func ( * testing . T ) {
return func ( t * testing . T ) {
csrf := GetCSRF ( t , ctx . Session , fmt . Sprintf ( "/%s/%s/branches" , url . PathEscape ( owner ) , url . PathEscape ( repo ) ) )
req := NewRequestWithValues ( t , "POST" , fmt . Sprintf ( "/%s/%s/branches/delete?name=%s" , url . PathEscape ( owner ) , url . PathEscape ( repo ) , url . QueryEscape ( branch ) ) , map [ string ] string {
"_csrf" : csrf ,
} )
ctx . Session . MakeRequest ( t , req , http . StatusOK )
}
}
func doAutoPRMerge ( baseCtx * APITestContext , dstPath string ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
Redesign Scoped Access Tokens (#24767)
## Changes
- Adds the following high level access scopes, each with `read` and
`write` levels:
- `activitypub`
- `admin` (hidden if user is not a site admin)
- `misc`
- `notification`
- `organization`
- `package`
- `issue`
- `repository`
- `user`
- Adds new middleware function `tokenRequiresScopes()` in addition to
`reqToken()`
- `tokenRequiresScopes()` is used for each high-level api section
- _if_ a scoped token is present, checks that the required scope is
included based on the section and HTTP method
- `reqToken()` is used for individual routes
- checks that required authentication is present (but does not check
scope levels as this will already have been handled by
`tokenRequiresScopes()`
- Adds migration to convert old scoped access tokens to the new set of
scopes
- Updates the user interface for scope selection
### User interface example
<img width="903" alt="Screen Shot 2023-05-31 at 1 56 55 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/654766ec-2143-4f59-9037-3b51600e32f3">
<img width="917" alt="Screen Shot 2023-05-31 at 1 56 43 PM"
src="https://github.com/go-gitea/gitea/assets/23248839/1ad64081-012c-4a73-b393-66b30352654c">
## tokenRequiresScopes Design Decision
- `tokenRequiresScopes()` was added to more reliably cover api routes.
For an incoming request, this function uses the given scope category
(say `AccessTokenScopeCategoryOrganization`) and the HTTP method (say
`DELETE`) and verifies that any scoped tokens in use include
`delete:organization`.
- `reqToken()` is used to enforce auth for individual routes that
require it. If a scoped token is not present for a request,
`tokenRequiresScopes()` will not return an error
## TODO
- [x] Alphabetize scope categories
- [x] Change 'public repos only' to a radio button (private vs public).
Also expand this to organizations
- [X] Disable token creation if no scopes selected. Alternatively, show
warning
- [x] `reqToken()` is missing from many `POST/DELETE` routes in the api.
`tokenRequiresScopes()` only checks that a given token has the correct
scope, `reqToken()` must be used to check that a token (or some other
auth) is present.
- _This should be addressed in this PR_
- [x] The migration should be reviewed very carefully in order to
minimize access changes to existing user tokens.
- _This should be addressed in this PR_
- [x] Link to api to swagger documentation, clarify what
read/write/delete levels correspond to
- [x] Review cases where more than one scope is needed as this directly
deviates from the api definition.
- _This should be addressed in this PR_
- For example:
```go
m.Group("/users/{username}/orgs", func() {
m.Get("", reqToken(), org.ListUserOrgs)
m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser,
auth_model.AccessTokenScopeCategoryOrganization),
context_service.UserAssignmentAPI())
```
## Future improvements
- [ ] Add required scopes to swagger documentation
- [ ] Redesign `reqToken()` to be opt-out rather than opt-in
- [ ] Subdivide scopes like `repository`
- [ ] Once a token is created, if it has no scopes, we should display
text instead of an empty bullet point
- [ ] If the 'public repos only' option is selected, should read
categories be selected by default
Closes #24501
Closes #24799
Co-authored-by: Jonathan Tran <jon@allspice.io>
Co-authored-by: Kyle D <kdumontnu@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
1 year ago
ctx := NewAPITestContext ( t , baseCtx . Username , baseCtx . Reponame , auth_model . AccessTokenScopeWriteRepository )
t . Run ( "CheckoutProtected" , doGitCheckoutBranch ( dstPath , "protected" ) )
t . Run ( "PullProtected" , doGitPull ( dstPath , "origin" , "protected" ) )
t . Run ( "GenerateCommit" , func ( t * testing . T ) {
_ , err := generateCommitWithNewData ( littleSize , dstPath , "user2@example.com" , "User Two" , "branch-data-file-" )
assert . NoError ( t , err )
} )
t . Run ( "PushToUnprotectedBranch" , doGitPushTestRepository ( dstPath , "origin" , "protected:unprotected3" ) )
var pr api . PullRequest
var err error
t . Run ( "CreatePullRequest" , func ( t * testing . T ) {
pr , err = doAPICreatePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , "protected" , "unprotected3" ) ( t )
assert . NoError ( t , err )
} )
// Request repository commits page
req := NewRequest ( t , "GET" , fmt . Sprintf ( "/%s/%s/pulls/%d/commits" , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
resp := ctx . Session . MakeRequest ( t , req , http . StatusOK )
doc := NewHTMLParser ( t , resp . Body )
// Get first commit URL
commitURL , exists := doc . doc . Find ( "#commits-table tbody tr td.sha a" ) . Last ( ) . Attr ( "href" )
assert . True ( t , exists )
assert . NotEmpty ( t , commitURL )
commitID := path . Base ( commitURL )
addCommitStatus := func ( status api . CommitStatusState ) func ( * testing . T ) {
return doAPICreateCommitStatus ( ctx , commitID , api . CreateStatusOption {
State : status ,
TargetURL : "http://test.ci/" ,
Description : "" ,
Context : "testci" ,
} )
}
// Call API to add Pending status for commit
t . Run ( "CreateStatus" , addCommitStatus ( api . CommitStatusPending ) )
// Cancel not existing auto merge
ctx . ExpectedCode = http . StatusNotFound
t . Run ( "CancelAutoMergePR" , doAPICancelAutoMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
// Add auto merge request
ctx . ExpectedCode = http . StatusCreated
t . Run ( "AutoMergePR" , doAPIAutoMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
// Can not create schedule twice
ctx . ExpectedCode = http . StatusConflict
t . Run ( "AutoMergePRTwice" , doAPIAutoMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
// Cancel auto merge request
ctx . ExpectedCode = http . StatusNoContent
t . Run ( "CancelAutoMergePR" , doAPICancelAutoMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
// Add auto merge request
ctx . ExpectedCode = http . StatusCreated
t . Run ( "AutoMergePR" , doAPIAutoMergePullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) )
// Check pr status
ctx . ExpectedCode = 0
pr , err = doAPIGetPullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) ( t )
assert . NoError ( t , err )
assert . False ( t , pr . HasMerged )
// Call API to add Failure status for commit
t . Run ( "CreateStatus" , addCommitStatus ( api . CommitStatusFailure ) )
// Check pr status
pr , err = doAPIGetPullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) ( t )
assert . NoError ( t , err )
assert . False ( t , pr . HasMerged )
// Call API to add Success status for commit
t . Run ( "CreateStatus" , addCommitStatus ( api . CommitStatusSuccess ) )
// wait to let gitea merge stuff
time . Sleep ( time . Second )
// test pr status
pr , err = doAPIGetPullRequest ( ctx , baseCtx . Username , baseCtx . Reponame , pr . Index ) ( t )
assert . NoError ( t , err )
assert . True ( t , pr . HasMerged )
}
}
func doCreateAgitFlowPull ( dstPath string , ctx * APITestContext , baseBranch , headBranch string ) func ( t * testing . T ) {
return func ( t * testing . T ) {
defer tests . PrintCurrentTest ( t ) ( )
// skip this test if git version is low
if git . CheckGitVersionAtLeast ( "2.29" ) != nil {
return
}
gitRepo , err := git . OpenRepository ( git . DefaultContext , dstPath )
if ! assert . NoError ( t , err ) {
return
}
defer gitRepo . Close ( )
var (
pr1 , pr2 * issues_model . PullRequest
commit string
)
repo , err := repo_model . GetRepositoryByOwnerAndName ( db . DefaultContext , ctx . Username , ctx . Reponame )
if ! assert . NoError ( t , err ) {
return
}
pullNum := unittest . GetCount ( t , & issues_model . PullRequest { } )
t . Run ( "CreateHeadBranch" , doGitCreateBranch ( dstPath , headBranch ) )
t . Run ( "AddCommit" , func ( t * testing . T ) {
err := os . WriteFile ( path . Join ( dstPath , "test_file" ) , [ ] byte ( "## test content" ) , 0 o666 )
if ! assert . NoError ( t , err ) {
return
}
err = git . AddChanges ( dstPath , true )
assert . NoError ( t , err )
err = git . CommitChanges ( dstPath , git . CommitChangesOptions {
Committer : & git . Signature {
Email : "user2@example.com" ,
Name : "user2" ,
When : time . Now ( ) ,
} ,
Author : & git . Signature {
Email : "user2@example.com" ,
Name : "user2" ,
When : time . Now ( ) ,
} ,
Message : "Testing commit 1" ,
} )
assert . NoError ( t , err )
commit , err = gitRepo . GetRefCommitID ( "HEAD" )
assert . NoError ( t , err )
} )
t . Run ( "Push" , func ( t * testing . T ) {
err := git . NewCommand ( git . DefaultContext , "push" , "origin" , "HEAD:refs/for/master" , "-o" ) . AddDynamicArguments ( "topic=" + headBranch ) . Run ( & git . RunOpts { Dir : dstPath } )
if ! assert . NoError ( t , err ) {
return
}
unittest . AssertCount ( t , & issues_model . PullRequest { } , pullNum + 1 )
pr1 = unittest . AssertExistsAndLoadBean ( t , & issues_model . PullRequest {
HeadRepoID : repo . ID ,
Flow : issues_model . PullRequestFlowAGit ,
} )
if ! assert . NotEmpty ( t , pr1 ) {
return
}
prMsg , err := doAPIGetPullRequest ( * ctx , ctx . Username , ctx . Reponame , pr1 . Index ) ( t )
if ! assert . NoError ( t , err ) {
return
}
assert . Equal ( t , "user2/" + headBranch , pr1 . HeadBranch )
assert . False ( t , prMsg . HasMerged )
assert . Contains ( t , "Testing commit 1" , prMsg . Body )
assert . Equal ( t , commit , prMsg . Head . Sha )
_ , _ , err = git . NewCommand ( git . DefaultContext , "push" , "origin" ) . AddDynamicArguments ( "HEAD:refs/for/master/test/" + headBranch ) . RunStdString ( & git . RunOpts { Dir : dstPath } )
if ! assert . NoError ( t , err ) {
return
}
unittest . AssertCount ( t , & issues_model . PullRequest { } , pullNum + 2 )
pr2 = unittest . AssertExistsAndLoadBean ( t , & issues_model . PullRequest {
HeadRepoID : repo . ID ,
Index : pr1 . Index + 1 ,
Flow : issues_model . PullRequestFlowAGit ,
} )
if ! assert . NotEmpty ( t , pr2 ) {
return
}
prMsg , err = doAPIGetPullRequest ( * ctx , ctx . Username , ctx . Reponame , pr2 . Index ) ( t )
if ! assert . NoError ( t , err ) {
return
}
assert . Equal ( t , "user2/test/" + headBranch , pr2 . HeadBranch )
assert . False ( t , prMsg . HasMerged )
} )
if pr1 == nil || pr2 == nil {
return
}
t . Run ( "AddCommit2" , func ( t * testing . T ) {
err := os . WriteFile ( path . Join ( dstPath , "test_file" ) , [ ] byte ( "## test content \n ## test content 2" ) , 0 o666 )
if ! assert . NoError ( t , err ) {
return
}
err = git . AddChanges ( dstPath , true )
assert . NoError ( t , err )
err = git . CommitChanges ( dstPath , git . CommitChangesOptions {
Committer : & git . Signature {
Email : "user2@example.com" ,
Name : "user2" ,
When : time . Now ( ) ,
} ,
Author : & git . Signature {
Email : "user2@example.com" ,
Name : "user2" ,
When : time . Now ( ) ,
} ,
Message : "Testing commit 2" ,
} )
assert . NoError ( t , err )
commit , err = gitRepo . GetRefCommitID ( "HEAD" )
assert . NoError ( t , err )
} )
t . Run ( "Push2" , func ( t * testing . T ) {
err := git . NewCommand ( git . DefaultContext , "push" , "origin" , "HEAD:refs/for/master" , "-o" ) . AddDynamicArguments ( "topic=" + headBranch ) . Run ( & git . RunOpts { Dir : dstPath } )
if ! assert . NoError ( t , err ) {
return
}
unittest . AssertCount ( t , & issues_model . PullRequest { } , pullNum + 2 )
prMsg , err := doAPIGetPullRequest ( * ctx , ctx . Username , ctx . Reponame , pr1 . Index ) ( t )
if ! assert . NoError ( t , err ) {
return
}
assert . False ( t , prMsg . HasMerged )
assert . Equal ( t , commit , prMsg . Head . Sha )
_ , _ , err = git . NewCommand ( git . DefaultContext , "push" , "origin" ) . AddDynamicArguments ( "HEAD:refs/for/master/test/" + headBranch ) . RunStdString ( & git . RunOpts { Dir : dstPath } )
if ! assert . NoError ( t , err ) {
return
}
unittest . AssertCount ( t , & issues_model . PullRequest { } , pullNum + 2 )
prMsg , err = doAPIGetPullRequest ( * ctx , ctx . Username , ctx . Reponame , pr2 . Index ) ( t )
if ! assert . NoError ( t , err ) {
return
}
assert . False ( t , prMsg . HasMerged )
assert . Equal ( t , commit , prMsg . Head . Sha )
} )
t . Run ( "Merge" , doAPIMergePullRequest ( * ctx , ctx . Username , ctx . Reponame , pr1 . Index ) )
t . Run ( "CheckoutMasterAgain" , doGitCheckoutBranch ( dstPath , "master" ) )
}
}
Prevent double use of `git cat-file` session. (#29298)
Fixes the reason why #29101 is hard to replicate.
Related #29297
Create a repo with a file with minimum size 4097 bytes (I use 10000) and
execute the following code:
```go
gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, <repo>)
assert.NoError(t, err)
commit, err := gitRepo.GetCommit(<sha>)
assert.NoError(t, err)
entry, err := commit.GetTreeEntryByPath(<file>)
assert.NoError(t, err)
b := entry.Blob()
// Create a reader
r, err := b.DataAsync()
assert.NoError(t, err)
defer r.Close()
// Create a second reader
r2, err := b.DataAsync()
assert.NoError(t, err) // Should be no error but is ErrNotExist
defer r2.Close()
```
The problem is the check in `CatFileBatch`:
https://github.com/go-gitea/gitea/blob/79217ea63c1f77de7ca79813ae45950724e63d02/modules/git/repo_base_nogogit.go#L81-L87
`Buffered() > 0` is used to check if there is a "operation" in progress
at the moment. This is a problem because we can't control the internal
buffer in the `bufio.Reader`. The code above demonstrates a sequence
which initiates an operation for which the code thinks there is no
active processing. The second call to `DataAsync()` therefore reuses the
existing instances instead of creating a new batch reader.
9 months ago
func TestDataAsync_Issue29101 ( t * testing . T ) {
onGiteaRun ( t , func ( t * testing . T , u * url . URL ) {
user := unittest . AssertExistsAndLoadBean ( t , & user_model . User { ID : 2 } )
repo := unittest . AssertExistsAndLoadBean ( t , & repo_model . Repository { ID : 1 } )
resp , err := files_service . ChangeRepoFiles ( db . DefaultContext , repo , user , & files_service . ChangeRepoFilesOptions {
Files : [ ] * files_service . ChangeRepoFile {
{
Operation : "create" ,
TreePath : "test.txt" ,
ContentReader : bytes . NewReader ( make ( [ ] byte , 10000 ) ) ,
} ,
} ,
OldBranch : repo . DefaultBranch ,
NewBranch : repo . DefaultBranch ,
} )
assert . NoError ( t , err )
sha := resp . Commit . SHA
gitRepo , err := gitrepo . OpenRepository ( db . DefaultContext , repo )
assert . NoError ( t , err )
commit , err := gitRepo . GetCommit ( sha )
assert . NoError ( t , err )
entry , err := commit . GetTreeEntryByPath ( "test.txt" )
assert . NoError ( t , err )
b := entry . Blob ( )
r , err := b . DataAsync ( )
assert . NoError ( t , err )
defer r . Close ( )
r2 , err := b . DataAsync ( )
assert . NoError ( t , err )
defer r2 . Close ( )
} )
}