Add some tests to clarify the "must-change-password" behavior (#30693)

Follow  #30472:

When a user is created by command line `./gitea admin user create`:

Old behavior before #30472: the first user (admin or non-admin) doesn't
need to change password.

Revert to the old behavior before #30472

diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go
index bd9063a8e4b8de4f24db327042586c837e5deb9f..f1ed46e70b0834f51da0bf8d70394432489f9620 100644 (file)
--- a/cmd/admin_user_change_password.go
+++ b/cmd/admin_user_change_password.go
@@ -35,7 +35,7 @@ var microcmdUserChangePassword = &cli.Command{
                },
                &cli.BoolFlag{
                        Name:  "must-change-password",
-                       Usage: "User must change password",
+                       Usage: "User must change password (can be disabled by --must-change-password=false)",
                        Value: true,
                },
        },

diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go
index 403e3ee8d82dc0eb8714be878c362fd1ca704dd4..f328b753d86dafb4f479504e0e8d6f152143f606 100644 (file)
--- a/cmd/admin_user_create.go
+++ b/cmd/admin_user_create.go
@@ -4,6 +4,7 @@
 package cmd
 
 import (
+       "context"
        "errors"
        "fmt"
 
@@ -48,7 +49,7 @@ var microcmdUserCreate = &cli.Command{
                },
                &cli.BoolFlag{
                        Name:               "must-change-password",
-                       Usage:              "Set to false to prevent forcing the user to change their password after initial login",
+                       Usage:              "User must change password after initial login, defaults to true for all users except the first one (can be disabled by --must-change-password=false)",
                        DisableDefaultText: true,
                },
                &cli.IntFlag{
@@ -91,11 +92,16 @@ func runCreateUser(c *cli.Context) error {
                _, _ = fmt.Fprintf(c.App.ErrWriter, "--name flag is deprecated. Use --username instead.\n")
        }
 
-       ctx, cancel := installSignals()
-       defer cancel()
-
-       if err := initDB(ctx); err != nil {
-               return err
+       ctx := c.Context
+       if !setting.IsInTesting {
+               // FIXME: need to refactor the "installSignals/initDB" related code later
+               // it doesn't make sense to call it in (almost) every command action function
+               var cancel context.CancelFunc
+               ctx, cancel = installSignals()
+               defer cancel()
+               if err := initDB(ctx); err != nil {
+                       return err
+               }
        }
 
        var password string
@@ -123,8 +129,8 @@ func runCreateUser(c *cli.Context) error {
                if err != nil {
                        return fmt.Errorf("IsTableNotEmpty: %w", err)
                }
-               if !hasUserRecord && isAdmin {
-                       // if this is the first admin being created, don't force to change password (keep the old behavior)
+               if !hasUserRecord {
+                       // if this is the first one being created, don't force to change password (keep the old behavior)
                        mustChangePassword = false
                }
        }

diff --git a/cmd/admin_user_create_test.go b/cmd/admin_user_create_test.go
new file mode 100644 (file)
index 0000000..83754e9
--- /dev/null
+++ b/cmd/admin_user_create_test.go
@@ -0,0 +1,44 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package cmd
+
+import (
+       "fmt"
+       "strings"
+       "testing"
+
+       "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/models/unittest"
+       user_model "code.gitea.io/gitea/models/user"
+
+       "github.com/stretchr/testify/assert"
+)
+
+func TestAdminUserCreate(t *testing.T) {
+       app := NewMainApp(AppVersion{})
+
+       reset := func() {
+               assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.User{}))
+               assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.EmailAddress{}))
+       }
+
+       type createCheck struct{ IsAdmin, MustChangePassword bool }
+       createUser := func(name, args string) createCheck {
+               assert.NoError(t, app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s --password foobar", name, name, args))))
+               u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: name})
+               return createCheck{u.IsAdmin, u.MustChangePassword}
+       }
+       reset()
+       assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u", ""), "first non-admin user doesn't need to change password")
+
+       reset()
+       assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u", "--admin"), "first admin user doesn't need to change password")
+
+       reset()
+       assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u", "--admin --must-change-password"))
+       assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u2", "--admin"))
+       assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u3", "--admin --must-change-password=false"))
+       assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: true}, createUser("u4", ""))
+       assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u5", "--must-change-password=false"))
+}

diff --git a/cmd/main.go b/cmd/main.go
index 02dd660e9eec424cb9b1dcb787d444e933cc0850..fd648946efa39a9333533e90905b61a9f47a124a 100644 (file)
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -112,13 +112,18 @@ func prepareWorkPathAndCustomConf(action cli.ActionFunc) func(ctx *cli.Context)
        }
 }
 
-func NewMainApp(version, versionExtra string) *cli.App {
+type AppVersion struct {
+       Version string
+       Extra   string
+}
+
+func NewMainApp(appVer AppVersion) *cli.App {
        app := cli.NewApp()
        app.Name = "Gitea"
        app.HelpName = "gitea"
        app.Usage = "A painless self-hosted Git service"
        app.Description = `Gitea program contains "web" and other subcommands. If no subcommand is given, it starts the web server by default. Use "web" subcommand for more web server arguments, use other subcommands for other purposes.`
-       app.Version = version + versionExtra
+       app.Version = appVer.Version + appVer.Extra
        app.EnableBashCompletion = true
 
        // these sub-commands need to use config file

diff --git a/cmd/main_test.go b/cmd/main_test.go
index a916c61f853d27f34dcc407d9588eff585fabb64..c182b440199d2e5a6f07cfdf69f0aaf8026cb8d6 100644 (file)
--- a/cmd/main_test.go
+++ b/cmd/main_test.go
@@ -28,7 +28,7 @@ func makePathOutput(workPath, customPath, customConf string) string {
 }
 
 func newTestApp(testCmdAction func(ctx *cli.Context) error) *cli.App {
-       app := NewMainApp("version", "version-extra")
+       app := NewMainApp(AppVersion{})
        testCmd := &cli.Command{Name: "test-cmd", Action: testCmdAction}
        prepareSubcommandWithConfig(testCmd, appGlobalFlags())
        app.Commands = append(app.Commands, testCmd)

diff --git a/main.go b/main.go
index 775c729c569ea5c1c8c2426bc8dc8ddddec72e9b..756c3e0f9ba12ab706a41bd6bb2919267188547a 100644 (file)
--- a/main.go
+++ b/main.go
@@ -42,7 +42,7 @@ func main() {
                log.GetManager().Close()
                os.Exit(code)
        }
-       app := cmd.NewMainApp(Version, formatBuiltWith())
+       app := cmd.NewMainApp(cmd.AppVersion{Version: Version, Extra: formatBuiltWith()})
        _ = cmd.RunMainApp(app, os.Args...) // all errors should have been handled by the RunMainApp
        log.GetManager().Close()
 }

diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go
index 51de18fa9bb39b8d6e3ddec32c684b6c9cf942e0..53c9dbdd77254514b2671d320df8ab39bac6f6ad 100644 (file)
--- a/models/unittest/testdb.go
+++ b/models/unittest/testdb.go
@@ -6,7 +6,6 @@ package unittest
 import (
        "context"
        "fmt"
-       "log"
        "os"
        "path/filepath"
        "strings"
@@ -18,6 +17,7 @@ import (
        "code.gitea.io/gitea/modules/base"
        "code.gitea.io/gitea/modules/cache"
        "code.gitea.io/gitea/modules/git"
+       "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/setting/config"
        "code.gitea.io/gitea/modules/storage"
@@ -46,6 +46,14 @@ func fatalTestError(fmtStr string, args ...any) {
 
 // InitSettings initializes config provider and load common settings for tests
 func InitSettings() {
+       setting.IsInTesting = true
+       log.OsExiter = func(code int) {
+               if code != 0 {
+                       // non-zero exit code (log.Fatal) shouldn't occur during testing, if it happens, show a full stacktrace for more details
+                       panic(fmt.Errorf("non-zero exit code during testing: %d", code))
+               }
+               os.Exit(0)
+       }
        if setting.CustomConf == "" {
                setting.CustomConf = filepath.Join(setting.CustomPath, "conf/app-unittest-tmp.ini")
                _ = os.Remove(setting.CustomConf)
@@ -54,7 +62,7 @@ func InitSettings() {
        setting.LoadCommonSettings()
 
        if err := setting.PrepareAppDataPath(); err != nil {
-               log.Fatalf("Can not prepare APP_DATA_PATH: %v", err)
+               log.Fatal("Can not prepare APP_DATA_PATH: %v", err)
        }
        // register the dummy hash algorithm function used in the test fixtures
        _ = hash.Register("dummy", hash.NewDummyHasher)

diff --git a/modules/log/logger_global.go b/modules/log/logger_global.go
index 994acfedbb3a16da90b17f512f85c2d69574415c..6ce8b70fed62028cc97e8a7ff2569a9694dcb9a9 100644 (file)
--- a/modules/log/logger_global.go
+++ b/modules/log/logger_global.go
@@ -57,11 +57,13 @@ func Critical(format string, v ...any) {
        Log(1, ERROR, format, v...)
 }
 
+var OsExiter = os.Exit
+
 // Fatal records fatal log and exit process
 func Fatal(format string, v ...any) {
        Log(1, FATAL, format, v...)
        GetManager().Close()
-       os.Exit(1)
+       OsExiter(1)
 }
 
 func GetLogger(name string) Logger {

diff --git a/tests/test_utils.go b/tests/test_utils.go
index 50049e73f01c9bd83a9ac5d75d624de01468cb1b..66a287ecad2622253dacfa141b43cc92f42a0472 100644 (file)
--- a/tests/test_utils.go
+++ b/tests/test_utils.go
@@ -46,7 +46,6 @@ func InitTest(requireGitea bool) {
        // TODO: Speedup tests that rely on the event source ticker, confirm whether there is any bug or failure.
        // setting.UI.Notification.EventSourceUpdateTime = time.Second
 
-       setting.IsInTesting = true
        setting.AppWorkPath = giteaRoot
        setting.CustomPath = filepath.Join(setting.AppWorkPath, "custom")
        if requireGitea {