From 4c29c75968f520123f125e8305b2c29198664251 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 27 Dec 2023 15:24:23 +0800 Subject: [PATCH] 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. --- models/auth/session.go | 17 ++++++++------ tests/integration/session_test.go | 37 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 tests/integration/session_test.go diff --git a/models/auth/session.go b/models/auth/session.go index 60fdeaba7c2..75a205f702b 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -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 diff --git a/tests/integration/session_test.go b/tests/integration/session_test.go new file mode 100644 index 00000000000..d47148efa23 --- /dev/null +++ b/tests/integration/session_test.go @@ -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) +}