Merge branch 'main' into update-actions-route

Author: Zettat123

go.mod

@@ -174,7 +174,7 @@ require (
 	github.com/caddyserver/zerossl v0.1.4 // indirect
 	github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
-	github.com/cloudflare/circl v1.6.1 // indirect
+	github.com/cloudflare/circl v1.6.3 // indirect
 	github.com/couchbase/go-couchbase v0.1.1 // indirect
 	github.com/couchbase/gomemcached v0.3.3 // indirect
 	github.com/couchbase/goutils v0.1.2 // indirect

go.sum

@@ -227,8 +227,8 @@ github.com/chzyer/readline v1.5.1/go.mod h1:Eh+b79XXUwfKfcPLepksvw2tcLE/Ct21YObk
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
 github.com/chzyer/test v1.0.0/go.mod h1:2JlltgoNkt4TW/z9V/IzDdFaMTM2JPIi26O1pF38GC8=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
-github.com/cloudflare/circl v1.6.1 h1:zqIqSPIndyBh1bjLVVDHMPpVKqp8Su/V+6MeDzzQBQ0=
-github.com/cloudflare/circl v1.6.1/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs=
+github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8=
+github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
 github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=

models/auth/oauth2.go

@@ -14,6 +14,7 @@ import (
 	"net/url"
 	"slices"
 	"strings"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/modules/container"
@@ -27,6 +28,11 @@ import (
 	"xorm.io/xorm"
 )
 
+// Authorization codes should expire within 10 minutes per https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2
+const oauth2AuthorizationCodeValidity = 10 * time.Minute
+
+var ErrOAuth2AuthorizationCodeInvalidated = errors.New("oauth2 authorization code already invalidated")
+
 // OAuth2Application represents an OAuth2 client (RFC 6749)
 type OAuth2Application struct {
 	ID           int64 `xorm:"pk autoincr"`
@@ -386,6 +392,14 @@ func (code *OAuth2AuthorizationCode) TableName() string {
 	return "oauth2_authorization_code"
 }
 
+// IsExpired reports whether the authorization code is expired.
+func (code *OAuth2AuthorizationCode) IsExpired() bool {
+	if code.ValidUntil.IsZero() {
+		return true
+	}
+	return code.ValidUntil <= timeutil.TimeStampNow()
+}
+
 // GenerateRedirectURI generates a redirect URI for a successful authorization request. State will be used if not empty.
 func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL, error) {
 	redirect, err := url.Parse(code.RedirectURI)
@@ -403,8 +417,14 @@ func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL
 
 // Invalidate deletes the auth code from the database to invalidate this code
 func (code *OAuth2AuthorizationCode) Invalidate(ctx context.Context) error {
-	_, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code)
-	return err
+	affected, err := db.GetEngine(ctx).ID(code.ID).NoAutoCondition().Delete(code)
+	if err != nil {
+		return err
+	}
+	if affected == 0 {
+		return ErrOAuth2AuthorizationCodeInvalidated
+	}
+	return nil
 }
 
 // ValidateCodeChallenge validates the given verifier against the saved code challenge. This is part of the PKCE implementation.
@@ -472,13 +492,15 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redi
 	// for code scanners to grab sensitive tokens.
 	codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)
 
+	validUntil := time.Now().Add(oauth2AuthorizationCodeValidity)
 	code = &OAuth2AuthorizationCode{
 		Grant:               grant,
 		GrantID:             grant.ID,
 		RedirectURI:         redirectURI,
 		Code:                codeSecret,
 		CodeChallenge:       codeChallenge,
 		CodeChallengeMethod: codeChallengeMethod,
+		ValidUntil:          timeutil.TimeStamp(validUntil.Unix()),
 	}
 	if err := db.Insert(ctx, code); err != nil {
 		return nil, err

models/auth/oauth2_test.go

@@ -5,13 +5,45 @@ package auth_test
 
 import (
 	"testing"
+	"time"
 
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 )
 
+func TestOAuth2AuthorizationCodeValidity(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	t.Run("GenerateSetsValidUntil", func(t *testing.T) {
+		grant := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Grant{ID: 1})
+		expectedValidUntil := timeutil.TimeStamp(time.Now().Unix() + 600)
+		code, err := grant.GenerateNewAuthorizationCode(t.Context(), "http://127.0.0.1/", "", "")
+		assert.NoError(t, err)
+		assert.Equal(t, expectedValidUntil, code.ValidUntil)
+		assert.False(t, code.IsExpired())
+		assert.NoError(t, code.Invalidate(t.Context()))
+	})
+
+	t.Run("Expired", func(t *testing.T) {
+		defer timeutil.MockSet(time.Unix(2, 0).UTC())()
+
+		code := &auth_model.OAuth2AuthorizationCode{ValidUntil: timeutil.TimeStamp(1)}
+		assert.True(t, code.IsExpired())
+	})
+
+	t.Run("InvalidateTwice", func(t *testing.T) {
+		code, err := auth_model.GetOAuth2AuthorizationByCode(t.Context(), "authcode")
+		assert.NoError(t, err)
+		if assert.NotNil(t, code) {
+			assert.NoError(t, code.Invalidate(t.Context()))
+			assert.ErrorIs(t, code.Invalidate(t.Context()), auth_model.ErrOAuth2AuthorizationCodeInvalidated)
+		}
+	})
+}
+
 func TestOAuth2Application_GenerateClientSecret(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	app := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1})

models/dbfs/dbfile.go

@@ -75,7 +75,7 @@ func (f *file) readAt(fileMeta *dbfsMeta, offset int64, p []byte) (n int, err er
 }
 
 func (f *file) Read(p []byte) (n int, err error) {
-	if f.metaID == 0 || !f.allowRead {
+	if !f.allowRead {
 		return 0, os.ErrInvalid
 	}
 
@@ -89,7 +89,7 @@ func (f *file) Read(p []byte) (n int, err error) {
 }
 
 func (f *file) Write(p []byte) (n int, err error) {
-	if f.metaID == 0 || !f.allowWrite {
+	if !f.allowWrite {
 		return 0, os.ErrInvalid
 	}
 
@@ -184,10 +184,6 @@ func (f *file) Close() error {
 }
 
 func (f *file) Stat() (os.FileInfo, error) {
-	if f.metaID == 0 {
-		return nil, os.ErrInvalid
-	}
-
 	fileMeta, err := findFileMetaByID(f.ctx, f.metaID)
 	if err != nil {
 		return nil, err
@@ -232,15 +228,17 @@ func (f *file) open(flag int) (err error) {
 				if f.metaID != 0 {
 					return os.ErrExist
 				}
-			} else {
-				// create a new file if none exists.
-				if f.metaID == 0 {
-					if err = f.createEmpty(); err != nil {
-						return err
-					}
+			}
+			// create a new file if not exists.
+			if f.metaID == 0 {
+				if err = f.createEmpty(); err != nil {
+					return err
 				}
 			}
 		}
+		if f.metaID == 0 {
+			return os.ErrNotExist
+		}
 		if flag&os.O_TRUNC != 0 {
 			if err = f.truncate(); err != nil {
 				return err
@@ -252,7 +250,7 @@ func (f *file) open(flag int) (err error) {
 			}
 		}
 		return nil
-	}
+	} // end if: allowWrite
 
 	// read only mode
 	if f.metaID == 0 {
@@ -322,9 +320,6 @@ func (f *file) delete() error {
 }
 
 func (f *file) size() (int64, error) {
-	if f.metaID == 0 {
-		return 0, os.ErrNotExist
-	}
 	fileMeta, err := findFileMetaByID(f.ctx, f.metaID)
 	if err != nil {
 		return 0, err
@@ -339,7 +334,7 @@ func findFileMetaByID(ctx context.Context, metaID int64) (*dbfsMeta, error) {
 	} else if ok {
 		return &fileMeta, nil
 	}
-	return nil, nil //nolint:nilnil // return nil to indicate that the object does not exist
+	return nil, os.ErrNotExist
 }
 
 func buildPath(path string) string {

models/dbfs/dbfs.go

@@ -40,6 +40,9 @@ The DBFS solution:
 * In the future, when Gitea action needs to limit the log size (other CI/CD services also do so), it's easier to calculate the log file size.
 * Even sometimes the UI needs to render the tailing lines, the tailing lines can be found be counting the "\n" from the end of the file by seek.
   The seeking and finding is not the fastest way, but it's still acceptable and won't affect the performance too much.
+
+Limitations of the DBFS solution:
+* Not fully POSIX-compliant, some behaviors may be different from the real filesystem, especially for concurrent read/write
 */
 
 type dbfsMeta struct {

models/dbfs/dbfs_test.go

@@ -9,19 +9,14 @@ import (
 	"os"
 	"testing"
 
+	"code.gitea.io/gitea/modules/test"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func changeDefaultFileBlockSize(n int64) (restore func()) {
-	old := defaultFileBlockSize
-	defaultFileBlockSize = n
-	return func() {
-		defaultFileBlockSize = old
-	}
-}
-
 func TestDbfsBasic(t *testing.T) {
-	defer changeDefaultFileBlockSize(4)()
+	defer test.MockVariableValue(&defaultFileBlockSize, 4)()
 
 	// test basic write/read
 	f, err := OpenFile(t.Context(), "test.txt", os.O_RDWR|os.O_CREATE)
@@ -122,10 +117,55 @@ func TestDbfsBasic(t *testing.T) {
 	stat, err = f.Stat()
 	assert.NoError(t, err)
 	assert.EqualValues(t, 10, stat.Size())
+
+	t.Run("NonExisting", func(t *testing.T) {
+		f, err := OpenFile(t.Context(), "non-existing.txt", os.O_RDONLY)
+		assert.ErrorIs(t, err, os.ErrNotExist)
+		assert.Nil(t, f)
+
+		f, err = OpenFile(t.Context(), "non-existing.txt", os.O_WRONLY)
+		assert.ErrorIs(t, err, os.ErrNotExist)
+		assert.Nil(t, f)
+
+		f, err = OpenFile(t.Context(), "non-existing.txt", os.O_WRONLY|os.O_APPEND|os.O_TRUNC)
+		assert.ErrorIs(t, err, os.ErrNotExist)
+		assert.Nil(t, f)
+	})
+
+	t.Run("Existing", func(t *testing.T) {
+		assertFileContent := func(f File, expected string) {
+			_, err := f.Seek(0, io.SeekStart)
+			require.NoError(t, err)
+			buf, err := io.ReadAll(f)
+			require.NoError(t, err)
+			assert.Equal(t, expected, string(buf))
+		}
+
+		f, err := OpenFile(t.Context(), "existing.txt", os.O_RDWR|os.O_CREATE)
+		require.NoError(t, err)
+		_, _ = f.Write([]byte("test"))
+		assertFileContent(f, "test")
+		assert.NoError(t, f.Close())
+
+		f, err = OpenFile(t.Context(), "existing.txt", os.O_RDWR|os.O_CREATE|os.O_APPEND)
+		require.NoError(t, err)
+		_, _ = f.Write([]byte("\nnew"))
+		assertFileContent(f, "test\nnew")
+		assert.NoError(t, f.Close())
+
+		f, err = OpenFile(t.Context(), "existing.txt", os.O_RDWR|os.O_TRUNC)
+		require.NoError(t, err)
+		assertFileContent(f, "")
+		assert.NoError(t, f.Close())
+
+		f, err = OpenFile(t.Context(), "existing.txt", os.O_RDWR|os.O_CREATE|os.O_EXCL)
+		assert.ErrorIs(t, err, os.ErrExist)
+		assert.Nil(t, f)
+	})
 }
 
 func TestDbfsReadWrite(t *testing.T) {
-	defer changeDefaultFileBlockSize(4)()
+	defer test.MockVariableValue(&defaultFileBlockSize, 4)()
 
 	f1, err := OpenFile(t.Context(), "test.log", os.O_RDWR|os.O_CREATE)
 	assert.NoError(t, err)
@@ -157,30 +197,32 @@ func TestDbfsReadWrite(t *testing.T) {
 }
 
 func TestDbfsSeekWrite(t *testing.T) {
-	defer changeDefaultFileBlockSize(4)()
+	defer test.MockVariableValue(&defaultFileBlockSize, 4)()
 
-	f, err := OpenFile(t.Context(), "test2.log", os.O_RDWR|os.O_CREATE)
-	assert.NoError(t, err)
-	defer f.Close()
+	// write something
+	fw, err := OpenFile(t.Context(), "test2.log", os.O_RDWR|os.O_CREATE)
+	require.NoError(t, err)
+	defer fw.Close()
 
-	n, err := f.Write([]byte("111"))
+	n, err := fw.Write([]byte("111"))
 	assert.NoError(t, err)
 
-	_, err = f.Seek(int64(n), io.SeekStart)
+	_, err = fw.Seek(int64(n), io.SeekStart)
 	assert.NoError(t, err)
 
-	_, err = f.Write([]byte("222"))
+	_, err = fw.Write([]byte("222"))
 	assert.NoError(t, err)
 
-	_, err = f.Seek(int64(n), io.SeekStart)
+	_, err = fw.Seek(int64(n), io.SeekStart)
 	assert.NoError(t, err)
 
-	_, err = f.Write([]byte("333"))
+	_, err = fw.Write([]byte("333"))
 	assert.NoError(t, err)
 
+	// then read it
 	fr, err := OpenFile(t.Context(), "test2.log", os.O_RDONLY)
-	assert.NoError(t, err)
-	defer f.Close()
+	require.NoError(t, err)
+	defer fr.Close()
 
 	buf, err := io.ReadAll(fr)
 	assert.NoError(t, err)