Fix session key conflict with database keyword (#28613)
This is a regression from #28220 . `builder.Cond` will not add `` ` `` automatically but xorm method `Get/Find` adds `` ` ``. This PR also adds tests to prevent the method from being implemented incorrectly. The tests are added in `integrations` to test every database.
This commit is contained in:
		
							parent
							
								
									a1dfffd723
								
							
						
					
					
						commit
						4c29c75968
					
				
					 2 changed files with 47 additions and 7 deletions
				
			
		| 
						 | 
				
			
			@ -41,12 +41,15 @@ func ReadSession(ctx context.Context, key string) (*Session, error) {
 | 
			
		|||
	}
 | 
			
		||||
	defer committer.Close()
 | 
			
		||||
 | 
			
		||||
	session, exist, err := db.Get[Session](ctx, builder.Eq{"key": key})
 | 
			
		||||
	session, exist, err := db.Get[Session](ctx, builder.Eq{"`key`": key})
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	} else if !exist {
 | 
			
		||||
		session.Expiry = timeutil.TimeStampNow()
 | 
			
		||||
		if err := db.Insert(ctx, &session); err != nil {
 | 
			
		||||
		session = &Session{
 | 
			
		||||
			Key:    key,
 | 
			
		||||
			Expiry: timeutil.TimeStampNow(),
 | 
			
		||||
		}
 | 
			
		||||
		if err := db.Insert(ctx, session); err != nil {
 | 
			
		||||
			return nil, err
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			@ -56,7 +59,7 @@ func ReadSession(ctx context.Context, key string) (*Session, error) {
 | 
			
		|||
 | 
			
		||||
// ExistSession checks if a session exists
 | 
			
		||||
func ExistSession(ctx context.Context, key string) (bool, error) {
 | 
			
		||||
	return db.Exist[Session](ctx, builder.Eq{"key": key})
 | 
			
		||||
	return db.Exist[Session](ctx, builder.Eq{"`key`": key})
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// DestroySession destroys a session
 | 
			
		||||
| 
						 | 
				
			
			@ -75,13 +78,13 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er
 | 
			
		|||
	}
 | 
			
		||||
	defer committer.Close()
 | 
			
		||||
 | 
			
		||||
	if has, err := db.Exist[Session](ctx, builder.Eq{"key": newKey}); err != nil {
 | 
			
		||||
	if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": newKey}); err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	} else if has {
 | 
			
		||||
		return nil, fmt.Errorf("session Key: %s already exists", newKey)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if has, err := db.Exist[Session](ctx, builder.Eq{"key": oldKey}); err != nil {
 | 
			
		||||
	if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": oldKey}); err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	} else if !has {
 | 
			
		||||
		if err := db.Insert(ctx, &Session{
 | 
			
		||||
| 
						 | 
				
			
			@ -96,7 +99,7 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er
 | 
			
		|||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	s, _, err := db.Get[Session](ctx, builder.Eq{"key": newKey})
 | 
			
		||||
	s, _, err := db.Get[Session](ctx, builder.Eq{"`key`": newKey})
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		// is not exist, it should be impossible
 | 
			
		||||
		return nil, err
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										37
									
								
								tests/integration/session_test.go
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										37
									
								
								tests/integration/session_test.go
									
										
									
									
									
										Normal file
									
								
							| 
						 | 
				
			
			@ -0,0 +1,37 @@
 | 
			
		|||
// Copyright 2023 The Gitea Authors. All rights reserved.
 | 
			
		||||
// SPDX-License-Identifier: MIT
 | 
			
		||||
 | 
			
		||||
package integration
 | 
			
		||||
 | 
			
		||||
import (
 | 
			
		||||
	"testing"
 | 
			
		||||
 | 
			
		||||
	"code.gitea.io/gitea/models/auth"
 | 
			
		||||
	"code.gitea.io/gitea/models/db"
 | 
			
		||||
	"code.gitea.io/gitea/models/unittest"
 | 
			
		||||
	"code.gitea.io/gitea/tests"
 | 
			
		||||
 | 
			
		||||
	"github.com/stretchr/testify/assert"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
func Test_RegenerateSession(t *testing.T) {
 | 
			
		||||
	defer tests.PrepareTestEnv(t)()
 | 
			
		||||
 | 
			
		||||
	assert.NoError(t, unittest.PrepareTestDatabase())
 | 
			
		||||
 | 
			
		||||
	key := "new_key890123456"  // it must be 16 characters long
 | 
			
		||||
	key2 := "new_key890123457" // it must be 16 characters
 | 
			
		||||
	exist, err := auth.ExistSession(db.DefaultContext, key)
 | 
			
		||||
	assert.NoError(t, err)
 | 
			
		||||
	assert.False(t, exist)
 | 
			
		||||
 | 
			
		||||
	sess, err := auth.RegenerateSession(db.DefaultContext, "", key)
 | 
			
		||||
	assert.NoError(t, err)
 | 
			
		||||
	assert.EqualValues(t, key, sess.Key)
 | 
			
		||||
	assert.Len(t, sess.Data, 0)
 | 
			
		||||
 | 
			
		||||
	sess, err = auth.ReadSession(db.DefaultContext, key2)
 | 
			
		||||
	assert.NoError(t, err)
 | 
			
		||||
	assert.EqualValues(t, key2, sess.Key)
 | 
			
		||||
	assert.Len(t, sess.Data, 0)
 | 
			
		||||
}
 | 
			
		||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue