From 04516692f4b3e0156ec3d19881e797a6517d8fd4 Mon Sep 17 00:00:00 2001
From: ashmrtn <ashmrtnz@alcion.ai>
Date: Tue, 28 Feb 2023 12:13:34 -0800
Subject: [PATCH] Don't NPE if user isn't in tenant (#2678)

Was causing panics when trying to access kopia stats. Panic was recovered from, but reading the error was difficult.

Add some CLI tests to hopefully stop future regressions for this specific case

---

#### Does this PR need a docs update or release note?

- [x] :white_check_mark: Yes, it's included
- [ ] :clock1: Yes, but in a later PR
- [ ] :no_entry: No

#### Type of change

- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

#### Issue(s)

* closes #2668

#### Test Plan

- [x] :muscle: Manual
- [ ] :zap: Unit test
- [x] :green_heart: E2E
---
 CHANGELOG.md                        |  1 +
 src/cli/backup/exchange_e2e_test.go | 41 +++++++++++++++++++++++++++++
 src/cli/backup/onedrive_e2e_test.go | 35 ++++++++++++++++++++++++
 src/internal/operations/backup.go   |  5 ++++
 4 files changed, 82 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 179a39184..7ba6122fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Corso-generated .meta files and permissions no longer appear in the backup details.
+- Panic and recovery if a user didn't exist in the tenant.
 
 ### Known Issues
 - Folders and Calendars containing zero items or subfolders are not included in the backup.
diff --git a/src/cli/backup/exchange_e2e_test.go b/src/cli/backup/exchange_e2e_test.go
index fccecbbad..642e351cb 100644
--- a/src/cli/backup/exchange_e2e_test.go
+++ b/src/cli/backup/exchange_e2e_test.go
@@ -203,6 +203,47 @@ func (suite *BackupExchangeE2ESuite) TestExchangeBackupCmd() {
 	}
 }
 
+func (suite *BackupExchangeE2ESuite) TestExchangeBackupCmd_UserNotInTenant() {
+	recorder := strings.Builder{}
+
+	for _, set := range backupDataSets {
+		recorder.Reset()
+
+		suite.Run(set.String(), func() {
+			t := suite.T()
+
+			ctx, flush := tester.NewContext()
+			ctx = config.SetViper(ctx, suite.vpr)
+			defer flush()
+
+			cmd := tester.StubRootCmd(
+				"backup", "create", "exchange",
+				"--config-file", suite.cfgFP,
+				"--"+utils.UserFN, "foo@nothere.com",
+				"--"+utils.DataFN, set.String())
+			cli.BuildCommandTree(cmd)
+
+			cmd.SetOut(&recorder)
+
+			ctx = print.SetRootCmd(ctx, cmd)
+
+			// run the command
+			err := cmd.ExecuteContext(ctx)
+			require.Error(t, err)
+			assert.Contains(
+				t,
+				err.Error(),
+				"not found within tenant", "error missing user not found")
+			assert.NotContains(t, err.Error(), "runtime error", "panic happened")
+
+			t.Logf("backup error message: %s", err.Error())
+
+			result := recorder.String()
+			t.Log("backup results", result)
+		})
+	}
+}
+
 // ---------------------------------------------------------------------------
 // tests prepared with a previous backup
 // ---------------------------------------------------------------------------
diff --git a/src/cli/backup/onedrive_e2e_test.go b/src/cli/backup/onedrive_e2e_test.go
index 46a12cdf9..13afa9ffa 100644
--- a/src/cli/backup/onedrive_e2e_test.go
+++ b/src/cli/backup/onedrive_e2e_test.go
@@ -113,6 +113,41 @@ func (suite *NoBackupOneDriveE2ESuite) TestOneDriveBackupListCmd_empty() {
 	assert.Equal(t, "No backups available\n", result)
 }
 
+func (suite *NoBackupOneDriveE2ESuite) TestOneDriveBackupCmd_UserNotInTenant() {
+	recorder := strings.Builder{}
+
+	t := suite.T()
+
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	ctx = config.SetViper(ctx, suite.vpr)
+
+	cmd := tester.StubRootCmd(
+		"backup", "create", "onedrive",
+		"--config-file", suite.cfgFP,
+		"--"+utils.UserFN, "foo@nothere.com")
+	cli.BuildCommandTree(cmd)
+
+	cmd.SetOut(&recorder)
+
+	ctx = print.SetRootCmd(ctx, cmd)
+
+	// run the command
+	err := cmd.ExecuteContext(ctx)
+	require.Error(t, err)
+	assert.Contains(
+		t,
+		err.Error(),
+		"not found within tenant", "error missing user not found")
+	assert.NotContains(t, err.Error(), "runtime error", "panic happened")
+
+	t.Logf("backup error message: %s", err.Error())
+
+	result := recorder.String()
+	t.Log("backup results", result)
+}
+
 // ---------------------------------------------------------------------------
 // tests for deleting backups
 // ---------------------------------------------------------------------------
diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go
index d1b75b603..152b29e20 100644
--- a/src/internal/operations/backup.go
+++ b/src/internal/operations/backup.go
@@ -627,6 +627,11 @@ func (op *BackupOperation) persistResults(
 		op.Status = Failed
 	}
 
+	if opStats.k == nil {
+		op.Status = Failed
+		return errors.New("backup persistence never completed")
+	}
+
 	op.Results.BytesRead = opStats.k.TotalHashedBytes
 	op.Results.BytesUploaded = opStats.k.TotalUploadedBytes
 	op.Results.ItemsWritten = opStats.k.TotalFileCount