💄

Author: jeanp413

components/gitpod-db/go/personal_access_token.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/google/uuid"
 	"gorm.io/gorm"
-	"gorm.io/gorm/clause"
 )
 
 type PersonalAccessToken struct {
@@ -105,14 +104,24 @@ func UpdatePersonalAccessToken(ctx context.Context, conn *gorm.DB, tokenID uuid.
 	db := conn.WithContext(ctx)
 
 	var token PersonalAccessToken
-	db = db.
-		Model(&token).
-		Clauses(clause.Returning{}).
-		Where("id = ?", tokenID).
-		Where("userId = ?", userID).
-		Where("deleted = ?", 0).
-		Select("hash", "expirationTime").Updates(PersonalAccessToken{Hash: hash, ExpirationTime: expirationTime})
-	if db.Error != nil {
+	err := db.Transaction(func(tx *gorm.DB) error {
+		txErr := tx.
+			Where("id = ?", tokenID).
+			Where("userId = ?", userID).
+			Where("deleted = ?", 0).
+			Select("hash", "expirationTime").Updates(PersonalAccessToken{Hash: hash, ExpirationTime: expirationTime}).Error
+		if txErr != nil {
+			return txErr
+		}
+
+		txErr = tx.Where("id = ?", tokenID).Where("userId = ?", userID).Where("deleted = ?", 0).First(&token).Error
+		if txErr != nil {
+			return txErr
+		}
+
+		return nil
+	})
+	if err != nil {
 		if errors.Is(db.Error, gorm.ErrRecordNotFound) {
 			return PersonalAccessToken{}, fmt.Errorf("Token with ID %s does not exist: %w", tokenID, ErrorNotFound)
 		}

components/public-api-server/pkg/apiv1/tokens_test.go

@@ -22,10 +22,8 @@ import (
 	protocol "github.com/gitpod-io/gitpod/gitpod-protocol"
 	"github.com/gitpod-io/gitpod/public-api-server/pkg/auth"
 	"github.com/golang/mock/gomock"
-	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
-	"google.golang.org/protobuf/testing/protocmp"
 	"google.golang.org/protobuf/types/known/timestamppb"
 	"gorm.io/gorm"
 )
@@ -368,6 +366,33 @@ func TestTokensService_RegeneratePersonalAccessToken(t *testing.T) {
 		require.Equal(t, connect.CodeInvalidArgument, connect.CodeOf(err))
 	})
 
+	t.Run("responds with not found when token is not found", func(t *testing.T) {
+		serverMock, dbConn, client := setupTokensService(t, withTokenFeatureEnabled)
+
+		someTokenId := uuid.New().String()
+
+		dbtest.CreatePersonalAccessTokenRecords(t, dbConn,
+			dbtest.NewPersonalAccessToken(t, db.PersonalAccessToken{
+				UserID: uuid.MustParse(user.ID),
+			}),
+			dbtest.NewPersonalAccessToken(t, db.PersonalAccessToken{
+				UserID: uuid.MustParse(user2.ID),
+			}),
+		)
+
+		serverMock.EXPECT().GetLoggedInUser(gomock.Any()).Return(user, nil)
+
+		newTime, err := time.Parse(time.RFC3339, "2023-01-02T15:04:05Z")
+		require.NoError(t, err)
+
+		newTimestamp := timestamppb.New(newTime)
+		_, err = client.RegeneratePersonalAccessToken(context.Background(), connect.NewRequest(&v1.RegeneratePersonalAccessTokenRequest{
+			Id:             someTokenId,
+			ExpirationTime: newTimestamp,
+		}))
+		require.Error(t, err, fmt.Errorf("Token with ID %s does not exist: not found", someTokenId))
+	})
+
 	t.Run("regenerate correct token", func(t *testing.T) {
 		serverMock, dbConn, client := setupTokensService(t, withTokenFeatureEnabled)
 
@@ -380,36 +405,30 @@ func TestTokensService_RegeneratePersonalAccessToken(t *testing.T) {
 			}),
 		)
 
-		serverMock.EXPECT().GetLoggedInUser(gomock.Any()).Return(user, nil)
+		serverMock.EXPECT().GetLoggedInUser(gomock.Any()).Return(user, nil).MaxTimes(2)
 
 		origResponse, err := client.GetPersonalAccessToken(context.Background(), connect.NewRequest(&v1.GetPersonalAccessTokenRequest{
 			Id: tokens[0].ID.String(),
 		}))
+		require.NoError(t, err)
 
+		newTime, err := time.Parse(time.RFC3339, "2023-01-02T15:04:05Z")
 		require.NoError(t, err)
 
-		newTimestamp := timestamppb.Now()
+		newTimestamp := timestamppb.New(newTime)
 		response, err := client.RegeneratePersonalAccessToken(context.Background(), connect.NewRequest(&v1.RegeneratePersonalAccessTokenRequest{
 			Id:             tokens[0].ID.String(),
 			ExpirationTime: newTimestamp,
 		}))
-
 		require.NoError(t, err)
 
-		// require.Equal(t, response.Msg.Token.ExpirationTime, newTimestamp)
-		// require.NotEqual(t, response.Msg.Token.Value, origResponse.Msg.Token.Value)
-
-		requireNotEqualProto(t, &v1.RegeneratePersonalAccessTokenResponse{
-			Token: personalAccessTokenToAPI(tokens[0], origResponse.Msg.Token.Value),
-		}, response.Msg)
-
-		// fake code
-		tokens[0].Hash = response.Msg.Token.Value
-		tokens[0].ExpirationTime = response.Msg.Token.GetExpirationTime().AsTime()
-
-		requireEqualProto(t, &v1.RegeneratePersonalAccessTokenResponse{
-			Token: personalAccessTokenToAPI(tokens[0], ""),
-		}, response.Msg)
+		require.Equal(t, origResponse.Msg.Token.Id, response.Msg.Token.Id)
+		require.NotEqual(t, "", response.Msg.Token.Value)
+		require.Equal(t, origResponse.Msg.Token.Name, response.Msg.Token.Name)
+		require.Equal(t, origResponse.Msg.Token.Description, response.Msg.Token.Description)
+		require.Equal(t, origResponse.Msg.Token.Scopes, response.Msg.Token.Scopes)
+		require.Equal(t, newTimestamp.AsTime(), response.Msg.Token.ExpirationTime.AsTime())
+		require.Equal(t, origResponse.Msg.Token.CreatedAt, response.Msg.Token.CreatedAt)
 	})
 }
 
@@ -536,12 +555,3 @@ func setupTokensService(t *testing.T, expClient experiments.Client) (*protocol.M
 
 	return serverMock, dbConn, client
 }
-
-func requireNotEqualProto(t *testing.T, expected interface{}, actual interface{}) {
-	t.Helper()
-
-	diff := cmp.Diff(expected, actual, protocmp.Transform())
-	if diff == "" {
-		require.Fail(t, diff, "they should not equal")
-	}
-}