CloudMigrations: improve nil handling (#93257)

* CloudMigrations: fail token decryption if session is not found or without a token

* CloudMigrations: do not report event if session is nil

pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go

@@ -351,10 +351,10 @@ func (s *Service) GetSession(ctx context.Context, uid string) (*cloudmigration.C
 func (s *Service) GetSessionList(ctx context.Context) (*cloudmigration.CloudMigrationSessionListResponse, error) {
 	values, err := s.store.GetCloudMigrationSessionList(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("retrieving session list from store: %w", err)
 	}
 
-	migrations := make([]cloudmigration.CloudMigrationSessionResponse, 0)
+	migrations := make([]cloudmigration.CloudMigrationSessionResponse, 0, len(values))
 	for _, v := range values {
 		migrations = append(migrations, cloudmigration.CloudMigrationSessionResponse{
 			UID:     v.UID,
@@ -473,7 +473,7 @@ func (s *Service) DeleteSession(ctx context.Context, sessionUID string) (*cloudm
 	session, snapshots, err := s.store.DeleteMigrationSessionByUID(ctx, sessionUID)
 	if err != nil {
 		s.report(ctx, session, gmsclient.EventDisconnect, 0, err)
-		return nil, fmt.Errorf("deleting migration from db: %w", err)
+		return nil, fmt.Errorf("deleting migration from db for session %v: %w", sessionUID, err)
 	}
 
 	err = s.deleteLocalFiles(snapshots)
@@ -755,6 +755,17 @@ func (s *Service) report(
 		return
 	}
 
+	if sess == nil {
+		errMessage := "session not found"
+		if evtErr != nil {
+			errMessage = evtErr.Error()
+		}
+
+		s.log.Error("failed to report event", "type", t, "error", errMessage)
+
+		return
+	}
+
 	e := gmsclient.EventRequestDTO{
 		Event:   t,
 		LocalID: id,

pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go

@@ -137,8 +137,15 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) {
 	s.gmsClient = gmsClientMock
 
 	// Insert a session and snapshot into the database before we start
-	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{})
+	createTokenResp, err := s.CreateToken(context.Background())
+	assert.NoError(t, err)
+	assert.NotEmpty(t, createTokenResp.Token)
+
+	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{
+		AuthToken: createTokenResp.Token,
+	})
 	require.NoError(t, err)
+
 	uid, err := s.store.CreateSnapshot(context.Background(), cloudmigration.CloudMigrationSnapshot{
 		UID:            "test uid",
 		SessionUID:     sess.UID,
@@ -306,8 +313,15 @@ func Test_OnlyQueriesStatusFromGMSWhenRequired(t *testing.T) {
 	s.gmsClient = gmsClientMock
 
 	// Insert a snapshot into the database before we start
-	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{})
+	createTokenResp, err := s.CreateToken(context.Background())
+	assert.NoError(t, err)
+	assert.NotEmpty(t, createTokenResp.Token)
+
+	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{
+		AuthToken: createTokenResp.Token,
+	})
 	require.NoError(t, err)
+
 	uid, err := s.store.CreateSnapshot(context.Background(), cloudmigration.CloudMigrationSnapshot{
 		UID:            uuid.NewString(),
 		SessionUID:     sess.UID,
@@ -416,7 +430,13 @@ func Test_NonCoreDataSourcesHaveWarning(t *testing.T) {
 	s := setUpServiceTest(t, false).(*Service)
 
 	// Insert a processing snapshot into the database before we start so we query GMS
-	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{})
+	createTokenResp, err := s.CreateToken(context.Background())
+	assert.NoError(t, err)
+	assert.NotEmpty(t, createTokenResp.Token)
+
+	sess, err := s.store.CreateMigrationSession(context.Background(), cloudmigration.CloudMigrationSession{
+		AuthToken: createTokenResp.Token,
+	})
 	require.NoError(t, err)
 	snapshotUid, err := s.store.CreateSnapshot(context.Background(), cloudmigration.CloudMigrationSnapshot{
 		UID:            uuid.NewString(),
@@ -528,6 +548,80 @@ func Test_NonCoreDataSourcesHaveWarning(t *testing.T) {
 	assert.Equal(t, uninstalledAltered.Error, "Only core data sources are supported. Please ensure the plugin is installed on the cloud stack.")
 }
 
+func TestDeleteSession(t *testing.T) {
+	s := setUpServiceTest(t, false).(*Service)
+
+	t.Run("when deleting a session that does not exist in the database, it returns an error", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		t.Cleanup(cancel)
+
+		session, err := s.DeleteSession(ctx, "invalid-session-uid")
+		require.Nil(t, session)
+		require.Error(t, err)
+	})
+
+	t.Run("when deleting an existing session, it returns the deleted session and no error", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		t.Cleanup(cancel)
+
+		createTokenResp, err := s.CreateToken(ctx)
+		require.NoError(t, err)
+		require.NotEmpty(t, createTokenResp.Token)
+
+		cmd := cloudmigration.CloudMigrationSessionRequest{
+			AuthToken: createTokenResp.Token,
+		}
+
+		createResp, err := s.CreateSession(ctx, cmd)
+		require.NoError(t, err)
+		require.NotEmpty(t, createResp.UID)
+		require.NotEmpty(t, createResp.Slug)
+
+		deletedSession, err := s.DeleteSession(ctx, createResp.UID)
+		require.NoError(t, err)
+		require.NotNil(t, deletedSession)
+		require.Equal(t, deletedSession.UID, createResp.UID)
+
+		notFoundSession, err := s.GetSession(ctx, deletedSession.UID)
+		require.ErrorIs(t, err, cloudmigration.ErrMigrationNotFound)
+		require.Nil(t, notFoundSession)
+	})
+}
+
+func TestReportEvent(t *testing.T) {
+	t.Run("when the session is nil, it does not report the event", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		t.Cleanup(cancel)
+
+		gmsMock := &gmsClientMock{}
+
+		s := setUpServiceTest(t, false).(*Service)
+		s.gmsClient = gmsMock
+
+		require.NotPanics(t, func() {
+			s.report(ctx, nil, gmsclient.EventConnect, time.Minute, nil)
+		})
+
+		require.Zero(t, gmsMock.reportEventCalled)
+	})
+
+	t.Run("when the session is not nil, it reports the event", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		t.Cleanup(cancel)
+
+		gmsMock := &gmsClientMock{}
+
+		s := setUpServiceTest(t, false).(*Service)
+		s.gmsClient = gmsMock
+
+		require.NotPanics(t, func() {
+			s.report(ctx, &cloudmigration.CloudMigrationSession{}, gmsclient.EventConnect, time.Minute, nil)
+		})
+
+		require.Equal(t, 1, gmsMock.reportEventCalled)
+	})
+}
+
 func ctxWithSignedInUser() context.Context {
 	c := &contextmodel.ReqContext{
 		SignedInUser: &user.SignedInUser{OrgID: 1},

pkg/services/cloudmigration/cloudmigrationimpl/xorm_store.go

@@ -45,7 +45,7 @@ func (ss *sqlStore) GetMigrationSessionByUID(ctx context.Context, uid string) (*
 	}
 
 	if err := ss.decryptToken(ctx, &cm); err != nil {
-		return &cm, err
+		return nil, fmt.Errorf("decrypting token: %w", err)
 	}
 
 	return &cm, err
@@ -69,7 +69,7 @@ func (ss *sqlStore) CreateMigrationRun(ctx context.Context, cmr cloudmigration.C
 
 func (ss *sqlStore) CreateMigrationSession(ctx context.Context, migration cloudmigration.CloudMigrationSession) (*cloudmigration.CloudMigrationSession, error) {
 	if err := ss.encryptToken(ctx, &migration); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("encrypting token: %w", err)
 	}
 
 	err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
@@ -101,8 +101,9 @@ func (ss *sqlStore) GetCloudMigrationSessionList(ctx context.Context) ([]*cloudm
 
 	for i := 0; i < len(migrations); i++ {
 		m := migrations[i]
+
 		if err := ss.decryptToken(ctx, m); err != nil {
-			return migrations, err
+			return nil, fmt.Errorf("decrypting token: %w", err)
 		}
 	}
 
@@ -169,7 +170,7 @@ func (ss *sqlStore) DeleteMigrationSessionByUID(ctx context.Context, uid string)
 	}
 
 	if err := ss.decryptToken(ctx, &c); err != nil {
-		return &c, snapshots, err
+		return nil, nil, fmt.Errorf("decrypting token: %w", err)
 	}
 
 	return &c, snapshots, nil
@@ -474,6 +475,14 @@ func (ss *sqlStore) encryptToken(ctx context.Context, cm *cloudmigration.CloudMi
 }
 
 func (ss *sqlStore) decryptToken(ctx context.Context, cm *cloudmigration.CloudMigrationSession) error {
+	if cm == nil {
+		return cloudmigration.ErrMigrationNotFound
+	}
+
+	if len(cm.AuthToken) == 0 {
+		return cloudmigration.ErrTokenNotFound
+	}
+
 	decoded, err := base64.StdEncoding.DecodeString(cm.AuthToken)
 	if err != nil {
 		return fmt.Errorf("token could not be decoded")
@@ -483,6 +492,7 @@ func (ss *sqlStore) decryptToken(ctx context.Context, cm *cloudmigration.CloudMi
 	if err != nil {
 		return fmt.Errorf("decrypting auth token: %w", err)
 	}
+
 	cm.AuthToken = string(t)
 
 	return nil

pkg/services/cloudmigration/cloudmigrationimpl/xorm_store_test.go

@@ -325,6 +325,53 @@ func TestGetSnapshotList(t *testing.T) {
 	})
 }
 
+func TestDecryptToken(t *testing.T) {
+	t.Parallel()
+
+	_, s := setUpTest(t)
+	ctx := context.Background()
+
+	t.Run("with an nil session, it returns a `migration not found` error", func(t *testing.T) {
+		t.Parallel()
+
+		var cm *cloudmigration.CloudMigrationSession
+
+		require.ErrorIs(t, s.decryptToken(ctx, cm), cloudmigration.ErrMigrationNotFound)
+	})
+
+	t.Run("with an empty auth token, it returns a `token not found` error", func(t *testing.T) {
+		t.Parallel()
+
+		var cm cloudmigration.CloudMigrationSession
+
+		require.ErrorIs(t, s.decryptToken(ctx, &cm), cloudmigration.ErrTokenNotFound)
+	})
+
+	t.Run("with an invalid base64 auth token, it returns an error", func(t *testing.T) {
+		t.Parallel()
+
+		cm := cloudmigration.CloudMigrationSession{
+			AuthToken: "invalid-base64-",
+		}
+
+		require.Error(t, s.decryptToken(ctx, &cm))
+	})
+
+	t.Run("with a valid base64 auth token, it decrypts it and overrides the auth token field", func(t *testing.T) {
+		t.Parallel()
+
+		rawAuthToken := "raw-and-fake"
+		encodedAuthToken := base64.StdEncoding.EncodeToString([]byte(rawAuthToken))
+
+		cm := cloudmigration.CloudMigrationSession{
+			AuthToken: encodedAuthToken,
+		}
+
+		require.NoError(t, s.decryptToken(ctx, &cm))
+		require.Equal(t, rawAuthToken, cm.AuthToken)
+	})
+}
+
 func setUpTest(t *testing.T) (*sqlstore.SQLStore, *sqlStore) {
 	testDB := db.InitTestDB(t)
 	s := &sqlStore{