Login: Fix mismatching label on auth_module in user list (#49177)

* Login: Fix mismatching label on user auth_module

* Login: ensure previous auth was set to differing

* Login: ensure only one entry of auth is updated

* compare both entries

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* remove noop

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

pkg/services/login/authinfoservice/database/database.go

@@ -146,6 +146,22 @@ func (s *AuthInfoStore) SetAuthInfo(ctx context.Context, cmd *models.SetAuthInfo
 	})
 }
 
+// UpdateAuthInfo updates the auth info for the user with the latest date. Avoids overlapping entries hiding
+// the last used one (ex: LDAP->SAML->LDAP)
+func (s *AuthInfoStore) UpdateAuthInfoDate(ctx context.Context, authInfo *models.UserAuth) error {
+	authInfo.Created = GetTime()
+
+	cond := &models.UserAuth{
+		Id:         authInfo.Id,
+		UserId:     authInfo.UserId,
+		AuthModule: authInfo.AuthModule,
+	}
+	return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		_, err := sess.Update(authInfo, cond)
+		return err
+	})
+}
+
 func (s *AuthInfoStore) UpdateAuthInfo(ctx context.Context, cmd *models.UpdateAuthInfoCommand) error {
 	authUser := &models.UserAuth{
 		UserId:     cmd.UserId,

pkg/services/login/authinfoservice/service.go

@@ -158,14 +158,20 @@ func (s *Implementation) LookupAndUpdate(ctx context.Context, query *models.GetU
 		authInfo = ai
 	}
 
-	if authInfo == nil && query.AuthModule != "" {
-		cmd := &models.SetAuthInfoCommand{
-			UserId:     user.Id,
-			AuthModule: query.AuthModule,
-			AuthId:     query.AuthId,
-		}
-		if err := s.authInfoStore.SetAuthInfo(ctx, cmd); err != nil {
-			return nil, err
+	if query.AuthModule != "" {
+		if authInfo == nil {
+			cmd := &models.SetAuthInfoCommand{
+				UserId:     user.Id,
+				AuthModule: query.AuthModule,
+				AuthId:     query.AuthId,
+			}
+			if err := s.authInfoStore.SetAuthInfo(ctx, cmd); err != nil {
+				return nil, err
+			}
+		} else {
+			if err := s.authInfoStore.UpdateAuthInfoDate(ctx, authInfo); err != nil {
+				return nil, err
+			}
 		}
 	}
 

pkg/services/login/authinfoservice/user_auth_test.go

@@ -230,6 +230,85 @@ func TestUserAuth(t *testing.T) {
 			require.Equal(t, getAuthQuery.Result.AuthModule, "test1")
 		})
 
+		t.Run("Keeps track of last used auth_module when not using oauth", func(t *testing.T) {
+			// Restore after destructive operation
+			sqlStore = sqlstore.InitTestDB(t)
+
+			for i := 0; i < 5; i++ {
+				cmd := models.CreateUserCommand{
+					Email: fmt.Sprint("user", i, "@test.com"),
+					Name:  fmt.Sprint("user", i),
+					Login: fmt.Sprint("loginuser", i),
+				}
+				_, err := sqlStore.CreateUser(context.Background(), cmd)
+				require.Nil(t, err)
+			}
+
+			// Find a user to set tokens on
+			login := "loginuser0"
+
+			fixedTime := time.Now()
+			// Calling srv.LookupAndUpdateQuery on an existing user will populate an entry in the user_auth table
+			// Make the first log-in during the past
+			database.GetTime = func() time.Time { return fixedTime.AddDate(0, 0, -2) }
+			queryOne := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test1", AuthId: "test1"}
+			user, err := srv.LookupAndUpdate(context.Background(), queryOne)
+			database.GetTime = time.Now
+
+			require.Nil(t, err)
+			require.Equal(t, user.Login, login)
+
+			// Add a second auth module for this user
+			// Have this module's last log-in be more recent
+			database.GetTime = func() time.Time { return fixedTime.AddDate(0, 0, -1) }
+			queryTwo := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test2", AuthId: "test2"}
+			user, err = srv.LookupAndUpdate(context.Background(), queryTwo)
+			require.Nil(t, err)
+			require.Equal(t, user.Login, login)
+
+			// Get the latest entry by not supply an authmodule or authid
+			getAuthQuery := &models.GetAuthInfoQuery{
+				UserId: user.Id,
+			}
+
+			err = authInfoStore.GetAuthInfo(context.Background(), getAuthQuery)
+
+			require.Nil(t, err)
+			require.Equal(t, "test2", getAuthQuery.Result.AuthModule)
+
+			// Now reuse first auth module and make sure it's updated to the most recent
+			database.GetTime = func() time.Time { return fixedTime }
+			user, err = srv.LookupAndUpdate(context.Background(), queryOne)
+
+			require.Nil(t, err)
+			require.Equal(t, user.Login, login)
+
+			err = authInfoStore.GetAuthInfo(context.Background(), getAuthQuery)
+
+			require.Nil(t, err)
+			require.Equal(t, "test1", getAuthQuery.Result.AuthModule)
+
+			// Now reuse second auth module and make sure it's updated to the most recent
+			database.GetTime = func() time.Time { return fixedTime.AddDate(0, 0, 1) }
+			user, err = srv.LookupAndUpdate(context.Background(), queryTwo)
+			require.Nil(t, err)
+			require.Equal(t, user.Login, login)
+
+			err = authInfoStore.GetAuthInfo(context.Background(), getAuthQuery)
+			require.Nil(t, err)
+			require.Equal(t, "test2", getAuthQuery.Result.AuthModule)
+
+			// Ensure test 1 did not have its entry modified
+			getAuthQueryUnchanged := &models.GetAuthInfoQuery{
+				UserId:     user.Id,
+				AuthModule: "test1",
+			}
+			err = authInfoStore.GetAuthInfo(context.Background(), getAuthQueryUnchanged)
+			require.Nil(t, err)
+			require.Equal(t, "test1", getAuthQueryUnchanged.Result.AuthModule)
+			require.Less(t, getAuthQueryUnchanged.Result.Created, getAuthQuery.Result.Created)
+		})
+
 		t.Run("Can set & locate by generic oauth auth module and user id", func(t *testing.T) {
 			// Find a user to set tokens on
 			login := "loginuser0"

pkg/services/login/userprotection.go

@@ -15,6 +15,7 @@ type Store interface {
 	GetAuthInfo(ctx context.Context, query *models.GetAuthInfoQuery) error
 	SetAuthInfo(ctx context.Context, cmd *models.SetAuthInfoCommand) error
 	UpdateAuthInfo(ctx context.Context, cmd *models.UpdateAuthInfoCommand) error
+	UpdateAuthInfoDate(ctx context.Context, authInfo *models.UserAuth) error
 	DeleteAuthInfo(ctx context.Context, cmd *models.DeleteAuthInfoCommand) error
 	GetUserById(ctx context.Context, id int64) (*models.User, error)
 	GetUserByLogin(ctx context.Context, login string) (*models.User, error)