Users: Expire old user invites (#27361)

* expire with existng cleanup service

* expire with new temp user service

* make Drone happy :)

* add expiry status

* remove other approach

* cleanup

* add test for idempotency

* add migration from datetime to unix ts

* update cmd names

* change lifetime config to duration

* remove unnecessart formatting

* add comment

* update docs

* remove max bound and introduce min error

* simplify sql

* remove comment

* allow any outstanding to exist for at least 24 hours

* revert created ts change

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* add extra state check to cleanup step

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

pkg/services/sqlstore/migrations/temp_user.go

@@ -1,6 +1,11 @@
 package migrations
 
-import . "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+import (
+	"time"
+
+	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+	"xorm.io/xorm"
+)
 
 func addTempUserMigrations(mg *Migrator) {
 	tempUserV1 := Table{
@@ -44,4 +49,65 @@ func addTempUserMigrations(mg *Migrator) {
 		{Name: "status", Type: DB_Varchar, Length: 20},
 		{Name: "remote_addr", Type: DB_Varchar, Length: 255, Nullable: true},
 	}))
+
+	tempUserV2 := Table{
+		Name: "temp_user",
+		Columns: []*Column{
+			{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
+			{Name: "org_id", Type: DB_BigInt, Nullable: false},
+			{Name: "version", Type: DB_Int, Nullable: false},
+			{Name: "email", Type: DB_NVarchar, Length: 190},
+			{Name: "name", Type: DB_NVarchar, Length: 255, Nullable: true},
+			{Name: "role", Type: DB_NVarchar, Length: 20, Nullable: true},
+			{Name: "code", Type: DB_NVarchar, Length: 190},
+			{Name: "status", Type: DB_Varchar, Length: 20},
+			{Name: "invited_by_user_id", Type: DB_BigInt, Nullable: true},
+			{Name: "email_sent", Type: DB_Bool},
+			{Name: "email_sent_on", Type: DB_DateTime, Nullable: true},
+			{Name: "remote_addr", Type: DB_Varchar, Length: 255, Nullable: true},
+			{Name: "created", Type: DB_Int, Default: "0", Nullable: false},
+			{Name: "updated", Type: DB_Int, Default: "0", Nullable: false},
+		},
+		Indices: []*Index{
+			{Cols: []string{"email"}, Type: IndexType},
+			{Cols: []string{"org_id"}, Type: IndexType},
+			{Cols: []string{"code"}, Type: IndexType},
+			{Cols: []string{"status"}, Type: IndexType},
+		},
+	}
+
+	addTableReplaceMigrations(mg, tempUserV1, tempUserV2, 2, map[string]string{
+		"id":                 "id",
+		"org_id":             "org_id",
+		"version":            "version",
+		"email":              "email",
+		"name":               "name",
+		"role":               "role",
+		"code":               "code",
+		"status":             "status",
+		"invited_by_user_id": "invited_by_user_id",
+		"email_sent":         "email_sent",
+		"email_sent_on":      "email_sent_on",
+		"remote_addr":        "remote_addr",
+	})
+
+	// Ensure outstanding invites are given a valid lifetime post-migration
+	mg.AddMigration("Set created for temp users that will otherwise prematurely expire", &SetCreatedForOutstandingInvites{})
+}
+
+type SetCreatedForOutstandingInvites struct {
+	MigrationBase
+}
+
+func (m *SetCreatedForOutstandingInvites) Sql(dialect Dialect) string {
+	return "code migration"
+}
+
+func (m *SetCreatedForOutstandingInvites) Exec(sess *xorm.Session, mg *Migrator) error {
+	created := time.Now().Unix()
+	if _, err := sess.Exec("UPDATE "+mg.Dialect.Quote("temp_user")+
+		" SET created = ?, updated = ? WHERE created = '0' AND status in ('SignUpStarted', 'InvitePending')", created, created); err != nil {
+		return err
+	}
+	return nil
 }

pkg/services/sqlstore/temp_user.go

@@ -13,6 +13,7 @@ func init() {
 	bus.AddHandler("sql", UpdateTempUserStatus)
 	bus.AddHandler("sql", GetTempUserByCode)
 	bus.AddHandler("sql", UpdateTempUserWithEmailSent)
+	bus.AddHandler("sql", ExpireOldUserInvites)
 }
 
 func UpdateTempUserStatus(cmd *models.UpdateTempUserStatusCommand) error {
@@ -36,8 +37,8 @@ func CreateTempUser(cmd *models.CreateTempUserCommand) error {
 			RemoteAddr:      cmd.RemoteAddr,
 			InvitedByUserId: cmd.InvitedByUserId,
 			EmailSentOn:     time.Now(),
-			Created:         time.Now(),
-			Updated:         time.Now(),
+			Created:         time.Now().Unix(),
+			Updated:         time.Now().Unix(),
 		}
 
 		if _, err := sess.Insert(user); err != nil {
@@ -132,3 +133,15 @@ func GetTempUserByCode(query *models.GetTempUserByCodeQuery) error {
 	query.Result = &tempUser
 	return err
 }
+
+func ExpireOldUserInvites(cmd *models.ExpireTempUsersCommand) error {
+	return inTransaction(func(sess *DBSession) error {
+		var rawSql = "UPDATE temp_user SET status = ?, updated = ? WHERE created <= ? AND status in (?, ?)"
+		if result, err := sess.Exec(rawSql, string(models.TmpUserExpired), time.Now().Unix(), cmd.OlderThan.Unix(), string(models.TmpUserSignUpStarted), string(models.TmpUserInvitePending)); err != nil {
+			return err
+		} else if cmd.NumExpired, err = result.RowsAffected(); err != nil {
+			return err
+		}
+		return nil
+	})
+}

pkg/services/sqlstore/temp_user_test.go

@@ -53,8 +53,8 @@ func TestTempUserCommandsAndQueries(t *testing.T) {
 			})
 
 			Convey("Should be able update email sent and email sent on", func() {
-				cmd3 := models.UpdateTempUserWithEmailSentCommand{Code: cmd.Result.Code}
-				err := UpdateTempUserWithEmailSent(&cmd3)
+				cmd2 := models.UpdateTempUserWithEmailSentCommand{Code: cmd.Result.Code}
+				err := UpdateTempUserWithEmailSent(&cmd2)
 				So(err, ShouldBeNil)
 
 				query := models.GetTempUsersQuery{OrgId: 2256, Status: models.TmpUserInvitePending}
@@ -62,7 +62,21 @@ func TestTempUserCommandsAndQueries(t *testing.T) {
 
 				So(err, ShouldBeNil)
 				So(query.Result[0].EmailSent, ShouldBeTrue)
-				So(query.Result[0].EmailSentOn, ShouldHappenOnOrAfter, (query.Result[0].Created))
+				So(query.Result[0].EmailSentOn.UTC(), ShouldHappenOnOrAfter, query.Result[0].Created.UTC())
+			})
+
+			Convey("Should be able expire temp user", func() {
+				cmd2 := models.ExpireTempUsersCommand{OlderThan: timeNow()}
+				err := ExpireOldUserInvites(&cmd2)
+				So(err, ShouldBeNil)
+				So(cmd2.NumExpired, ShouldEqual, 1)
+
+				Convey("Should do nothing when no temp users to expire", func() {
+					cmd2 = models.ExpireTempUsersCommand{OlderThan: timeNow()}
+					err := ExpireOldUserInvites(&cmd2)
+					So(err, ShouldBeNil)
+					So(cmd2.NumExpired, ShouldEqual, 0)
+				})
 			})
 		})
 	})