Merge pull request #547 from stellar/release/3.5.1

Release `3.5.1` to `main`
stellar · Feb 13, 2025 · 3b5b38d · 3b5b38d
2 parents 3b6bd64 + e9b8495
commit 3b5b38d
Show file tree

Hide file tree

Showing 13 changed files with 92 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
+## [3.5.1](https://github.com/stellar/stellar-disbursement-platform-backend/releases/tag/3.5.1) ([diff](https://github.com/stellar/stellar-disbursement-platform-backend/compare/3.5.0...3.5.1))
+
+### Fixed
+
+- GET `/disbursements` breaks when one of the users is deactivated. [#550](https://github.com/stellar/stellar-disbursement-platform-frontend/pull/550)
+
 ## [3.5.0](https://github.com/stellar/stellar-disbursement-platform-backend/releases/tag/3.5.0) ([diff](https://github.com/stellar/stellar-disbursement-platform-backend/compare/3.4.0...3.5.0))
 
 > [!WARNING]

diff --git a/helmchart/sdp/Chart.yaml b/helmchart/sdp/Chart.yaml
@@ -1,8 +1,8 @@
 apiVersion: v2
 name: stellar-disbursement-platform
 description: A Helm chart for the Stellar Disbursement Platform Backend (A.K.A. `sdp`)
-version: "3.5.0"
-appVersion: "3.5.0"
+version: "3.5.1"
+appVersion: "3.5.1"
 type: application
 maintainers:
   - name: Stellar Development Foundation

diff --git a/helmchart/sdp/README.md b/helmchart/sdp/README.md
@@ -106,7 +106,7 @@ Configuration parameters for the SDP Core Service which is the core backend serv
 | `sdp.image`                                                       | Configuration related to the Docker image used by the SDP service.                                                                                             |                                                 |
 | `sdp.image.repository`                                            | Docker image repository for the SDP backend service.                                                                                                           | `stellar/stellar-disbursement-platform-backend` |
 | `sdp.image.pullPolicy`                                            | Image pull policy for the SDP service. For locally built images, consider using "Never" or "IfNotPresent".                                                     | `Always`                                        |
-| `sdp.image.tag`                                                   | Docker image tag for the SDP service. If set, this overrides the default value from `.Chart.AppVersion`.                                                       | `3.5.0`                                         |
+| `sdp.image.tag`                                                   | Docker image tag for the SDP service. If set, this overrides the default value from `.Chart.AppVersion`.                                                       | `3.5.1`                                         |
 | `sdp.deployment`                                                  | Configuration related to the deployment of the SDP service.                                                                                                    |                                                 |
 | `sdp.deployment.annotations`                                      | Annotations to be added to the deployment.                                                                                                                     | `nil`                                           |
 | `sdp.deployment.podAnnotations`                                   | Annotations specific to the pods.                                                                                                                              | `{}`                                            |

diff --git a/helmchart/sdp/values.yaml b/helmchart/sdp/values.yaml
@@ -111,7 +111,7 @@ sdp:
   image:
     repository: stellar/stellar-disbursement-platform-backend
     pullPolicy: Always
-    tag: "3.5.0"
+    tag: "3.5.1"
 
   ## @extra sdp.deployment Configuration related to the deployment of the SDP service.
   ## @param sdp.deployment.annotations Annotations to be added to the deployment.

diff --git a/internal/serve/httphandler/disbursement_handler_test.go b/internal/serve/httphandler/disbursement_handler_test.go
@@ -571,13 +571,13 @@ func Test_DisbursementHandler_GetDisbursements_Success(t *testing.T) {
 	}
 
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{createdByUser.ID, startedByUser.ID}).
+		On("GetUsersByID", mock.Anything, []string{createdByUser.ID, startedByUser.ID}, false).
 		Return(allUsers, nil)
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{startedByUser.ID, createdByUser.ID}).
+		On("GetUsersByID", mock.Anything, []string{startedByUser.ID, createdByUser.ID}, false).
 		Return(allUsers, nil)
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{createdByUser.ID}).
+		On("GetUsersByID", mock.Anything, []string{createdByUser.ID}, false).
 		Return([]*auth.User{&createdByUser}, nil)
 
 	createdByUserRef := services.UserReference{
@@ -1227,10 +1227,10 @@ func Test_DisbursementHandler_GetDisbursement(t *testing.T) {
 	}
 
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{createdByUser.ID, startedByUser.ID}).
+		On("GetUsersByID", mock.Anything, []string{createdByUser.ID, startedByUser.ID}, false).
 		Return(allUsers, nil)
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{startedByUser.ID, createdByUser.ID}).
+		On("GetUsersByID", mock.Anything, []string{startedByUser.ID, createdByUser.ID}, false).
 		Return(allUsers, nil)
 
 	handler := &DisbursementHandler{

diff --git a/internal/services/disbursement_management_service.go b/internal/services/disbursement_management_service.go
@@ -92,7 +92,7 @@ func (s *DisbursementManagementService) AppendUserMetadata(ctx context.Context,
 		}
 	}
 
-	usersList, err := s.AuthManager.GetUsersByID(ctx, maps.Keys(users))
+	usersList, err := s.AuthManager.GetUsersByID(ctx, maps.Keys(users), false)
 	if err != nil {
 		return nil, fmt.Errorf("error getting user for IDs: %w", err)
 	}

diff --git a/internal/services/disbursement_management_service_test.go b/internal/services/disbursement_management_service_test.go
@@ -72,10 +72,10 @@ func Test_DisbursementManagementService_GetDisbursementsWithCount(t *testing.T)
 
 	authManagerMock := &auth.AuthManagerMock{}
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{users[0].ID, users[1].ID}).
+		On("GetUsersByID", mock.Anything, []string{users[0].ID, users[1].ID}, false).
 		Return(users, nil)
 	authManagerMock.
-		On("GetUsersByID", mock.Anything, []string{users[1].ID, users[0].ID}).
+		On("GetUsersByID", mock.Anything, []string{users[1].ID, users[0].ID}, false).
 		Return(users, nil)
 
 	service := &DisbursementManagementService{

diff --git a/main.go b/main.go
@@ -13,7 +13,7 @@ import (
 
 // Version is the official version of this application. Whenever it's changed
 // here, it also needs to be updated at the `helmchart/Chart.yaml#appVersion“.
-const Version = "3.5.0"
+const Version = "3.5.1"
 
 // GitCommit is populated at build time by
 // go build -ldflags "-X main.GitCommit=$GIT_COMMIT"

diff --git a/stellar-auth/pkg/auth/auth.go b/stellar-auth/pkg/auth/auth.go
@@ -25,7 +25,7 @@ type AuthManager interface {
 	ResetPassword(ctx context.Context, tokenString, password string) error
 	UpdatePassword(ctx context.Context, token, currentPassword, newPassword string) error
 	GetUser(ctx context.Context, tokenString string) (*User, error)
-	GetUsersByID(ctx context.Context, userIDs []string) ([]*User, error)
+	GetUsersByID(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error)
 	GetUserID(ctx context.Context, tokenString string) (string, error)
 	GetTenantID(ctx context.Context, tokenString string) (string, error)
 	GetAllUsers(ctx context.Context, tokenString string) ([]User, error)
@@ -316,8 +316,8 @@ func (am *defaultAuthManager) getUserFromToken(ctx context.Context, tokenString
 	return user, nil
 }
 
-func (am *defaultAuthManager) GetUsersByID(ctx context.Context, userIDs []string) ([]*User, error) {
-	users, err := am.authenticator.GetUsers(ctx, userIDs)
+func (am *defaultAuthManager) GetUsersByID(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error) {
+	users, err := am.authenticator.GetUsers(ctx, userIDs, activeOnly)
 	if err != nil {
 		return nil, fmt.Errorf("getting user with IDs: %w", err)
 	}

diff --git a/stellar-auth/pkg/auth/auth_test.go b/stellar-auth/pkg/auth/auth_test.go
@@ -1379,11 +1379,11 @@ func Test_AuthManager_GetUsersByID(t *testing.T) {
 	t.Run("returns error when aunthenticator fails", func(t *testing.T) {
 		userIDs := []string{"invalid-id"}
 		authenticatorMock.
-			On("GetUsers", ctx, userIDs).
+			On("GetUsers", ctx, userIDs, true).
 			Return(nil, errUnexpectedError).
 			Once()
 
-		_, err := authManager.GetUsersByID(ctx, userIDs)
+		_, err := authManager.GetUsersByID(ctx, userIDs, true)
 		require.Error(t, err)
 	})
 
@@ -1403,11 +1403,11 @@ func Test_AuthManager_GetUsersByID(t *testing.T) {
 
 		userIDs := []string{expectedUsers[0].ID, expectedUsers[1].ID}
 		authenticatorMock.
-			On("GetUsers", ctx, userIDs).
+			On("GetUsers", ctx, userIDs, true).
 			Return(expectedUsers, nil).
 			Once()
 
-		users, err := authManager.GetUsersByID(ctx, userIDs)
+		users, err := authManager.GetUsersByID(ctx, userIDs, true)
 		require.NoError(t, err)
 		assert.Equal(t, expectedUsers, users)
 	})

diff --git a/stellar-auth/pkg/auth/authenticator.go b/stellar-auth/pkg/auth/authenticator.go
@@ -41,7 +41,7 @@ type Authenticator interface {
 	UpdatePassword(ctx context.Context, user *User, currentPassword, newPassword string) error
 	GetAllUsers(ctx context.Context) ([]User, error)
 	GetUser(ctx context.Context, userID string) (*User, error)
-	GetUsers(ctx context.Context, userIDs []string) ([]*User, error)
+	GetUsers(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error)
 }
 
 type defaultAuthenticator struct {
@@ -488,34 +488,29 @@ func (a *defaultAuthenticator) GetUser(ctx context.Context, userID string) (*Use
 }
 
 // GetUsers retrieves the respective users from a list of user IDs.
-func (a *defaultAuthenticator) GetUsers(ctx context.Context, userIDs []string) ([]*User, error) {
+func (a *defaultAuthenticator) GetUsers(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error) {
 	if len(userIDs) == 0 {
 		return nil, nil
 	}
 
-	const query = `
+	query := `
 		SELECT
 			id,
 			first_name,
 			last_name
 		FROM
 			auth_users
 		WHERE
-			id = ANY($1::text[]) AND is_active = true
-	`
+			id = ANY($1::text[])`
+
+	if activeOnly {
+		query += " AND is_active = true"
+	}
 
 	var dbUsers []authUser
 	err := a.dbConnectionPool.SelectContext(ctx, &dbUsers, query, pq.Array(userIDs))
 	if err != nil {
-		return nil, fmt.Errorf("error querying user IDs: %w", err)
-	}
-	if len(dbUsers) != len(userIDs) {
-		return nil,
-			fmt.Errorf(
-				"error querying user IDs: searching for %d users, found %d users",
-				len(userIDs),
-				len(dbUsers),
-			)
+		return nil, fmt.Errorf("querying user IDs: %w", err)
 	}
 
 	users := make([]*User, len(dbUsers))

diff --git a/stellar-auth/pkg/auth/authenticator_test.go b/stellar-auth/pkg/auth/authenticator_test.go
@@ -718,43 +718,75 @@ func Test_DefaultAuthenticator_GetAllUsers(t *testing.T) {
 func Test_DefaultAuthenticator_GetUsers(t *testing.T) {
 	dbt := dbtest.Open(t)
 	defer dbt.Close()
-	dbConnectionPool, err := db.OpenDBConnectionPool(dbt.DSN)
-	require.NoError(t, err)
+	dbConnectionPool, outerErr := db.OpenDBConnectionPool(dbt.DSN)
+	require.NoError(t, outerErr)
 	defer dbConnectionPool.Close()
 
+	ctx := context.Background()
 	passwordEncrypterMock := &PasswordEncrypterMock{}
+	passwordEncrypterMock.
+		On("Encrypt", ctx, mock.AnythingOfType("string")).
+		Return("encryptedPassword", nil).
+		Maybe()
+
 	authenticator := newDefaultAuthenticator(withAuthenticatorDatabaseConnectionPool(dbConnectionPool))
 
-	ctx := context.Background()
+	// create test data
+	randUser1 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role1", "role2")
+	randUser2 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, true, "role1", "role2")
+	randUser3 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role3")
+	deactivatedUser := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role3")
+	outerErr = authenticator.updateIsActive(ctx, deactivatedUser.ID, false)
+	require.NoError(t, outerErr)
 
-	t.Run("returns an error if users for user IDs cannot be found", func(t *testing.T) {
+	t.Run("returns empty array when user ID not found", func(t *testing.T) {
 		userIDs := []string{"invalid-id"}
-		_, err := authenticator.GetUsers(ctx, userIDs)
-		require.EqualError(t, err, "error querying user IDs: searching for 1 users, found 0 users")
+		users, err := authenticator.GetUsers(ctx, userIDs, true)
+		assert.NoError(t, err)
+		assert.Empty(t, users)
 	})
 
 	t.Run("returns nil if called with an empty or nil slice", func(t *testing.T) {
 		userIDs := []string{}
-		users, err := authenticator.GetUsers(ctx, userIDs)
+		users, err := authenticator.GetUsers(ctx, userIDs, true)
 		require.NoError(t, err)
 		assert.Empty(t, users)
 
-		users, err = authenticator.GetUsers(ctx, nil)
+		users, err = authenticator.GetUsers(ctx, nil, true)
 		require.NoError(t, err)
 		assert.Empty(t, users)
 	})
 
-	t.Run("gets users for provided IDs successfully", func(t *testing.T) {
-		passwordEncrypterMock.
-			On("Encrypt", ctx, mock.AnythingOfType("string")).
-			Return("encryptedPassword", nil)
+	t.Run("gets active users for provided IDs successfully", func(t *testing.T) {
+		users, err := authenticator.GetUsers(
+			ctx, []string{randUser1.ID, randUser2.ID, randUser3.ID, deactivatedUser.ID}, true,
+		)
+		require.NoError(t, err)
 
-		randUser1 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role1", "role2")
-		randUser2 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, true, "role1", "role2")
-		randUser3 := CreateRandomAuthUserFixture(t, ctx, dbConnectionPool, passwordEncrypterMock, false, "role3")
+		expectedUsers := []*User{
+			{
+				ID:        randUser1.ID,
+				FirstName: randUser1.FirstName,
+				LastName:  randUser1.LastName,
+			},
+			{
+				ID:        randUser2.ID,
+				FirstName: randUser2.FirstName,
+				LastName:  randUser2.LastName,
+			},
+			{
+				ID:        randUser3.ID,
+				FirstName: randUser3.FirstName,
+				LastName:  randUser3.LastName,
+			},
+		}
 
+		assert.Equal(t, expectedUsers, users)
+	})
+
+	t.Run("gets all users for provided IDs successfully", func(t *testing.T) {
 		users, err := authenticator.GetUsers(
-			ctx, []string{randUser1.ID, randUser2.ID, randUser3.ID},
+			ctx, []string{randUser1.ID, randUser2.ID, randUser3.ID, deactivatedUser.ID}, false,
 		)
 		require.NoError(t, err)
 
@@ -774,6 +806,11 @@ func Test_DefaultAuthenticator_GetUsers(t *testing.T) {
 				FirstName: randUser3.FirstName,
 				LastName:  randUser3.LastName,
 			},
+			{
+				ID:        deactivatedUser.ID,
+				FirstName: deactivatedUser.FirstName,
+				LastName:  deactivatedUser.LastName,
+			},
 		}
 
 		assert.Equal(t, expectedUsers, users)

diff --git a/stellar-auth/pkg/auth/mocks.go b/stellar-auth/pkg/auth/mocks.go
@@ -128,8 +128,8 @@ func (am *AuthenticatorMock) GetUser(ctx context.Context, userID string) (*User,
 	return args.Get(0).(*User), args.Error(1)
 }
 
-func (am *AuthenticatorMock) GetUsers(ctx context.Context, userIDs []string) ([]*User, error) {
-	args := am.Called(ctx, userIDs)
+func (am *AuthenticatorMock) GetUsers(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error) {
+	args := am.Called(ctx, userIDs, activeOnly)
 	if args.Get(0) == nil {
 		return nil, args.Error(1)
 	}
@@ -238,8 +238,8 @@ func (am *AuthManagerMock) GetUser(ctx context.Context, tokenString string) (*Us
 	return args.Get(0).(*User), args.Error(1)
 }
 
-func (am *AuthManagerMock) GetUsersByID(ctx context.Context, tokenString []string) ([]*User, error) {
-	args := am.Called(ctx, tokenString)
+func (am *AuthManagerMock) GetUsersByID(ctx context.Context, userIDs []string, activeOnly bool) ([]*User, error) {
+	args := am.Called(ctx, userIDs, activeOnly)
 	return args.Get(0).([]*User), args.Error(1)
 }