fix: fixed account revocation list bug during user deletion when acco…

…unt... doesn't exist.
zerosubnet · Mar 2, 2023 · 8da3850 · 8da3850
1 parent 4f30fde
commit 8da3850
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 16 deletions.
diff --git a/build/vault/Dockerfile b/build/vault/Dockerfile
@@ -15,5 +15,6 @@ FROM vault:1.12.3
 RUN mkdir -p /etc/vault/vault_plugins
 RUN mkdir -p /etc/vault/vault_plugins_checksums
 COPY --from=builder /vault-plugin-secrets-nats.sha256 /etc/vault/vault_plugins_checksums
+RUN cat /etc/vault/vault_plugins_checksums/*
 COPY --from=builder /go/src/app/build/vault/plugins/vault-plugin-secrets-nats /etc/vault/vault_plugins
 
diff --git a/paths_issue_user.go b/paths_issue_user.go
@@ -218,7 +218,6 @@ func addUserIssue(ctx context.Context, storage logical.Storage, params IssueUser
 }
 
 func refreshUser(ctx context.Context, storage logical.Storage, issue *IssueUserStorage) error {
-
 	// create nkey and signing nkeys
 	err := issueUserNKeys(ctx, storage, *issue)
 	if err != nil {
@@ -277,7 +276,6 @@ func listUserIssues(ctx context.Context, storage logical.Storage, params IssueUs
 }
 
 func deleteUserIssue(ctx context.Context, storage logical.Storage, params IssueUserParameters) error {
-
 	// get stored signing keys
 	issue, err := readUserIssue(ctx, storage, params)
 	if err != nil {
@@ -288,23 +286,20 @@ func deleteUserIssue(ctx context.Context, storage logical.Storage, params IssueU
 		return nil
 	}
 
-	// get stored signing keys
+	// account revocation list handling for deleted user
 	account, err := readAccountIssue(ctx, storage, IssueAccountParameters{
 		Operator: issue.Operator,
 		Account:  issue.Account,
 	})
 	if err != nil {
 		return err
 	}
-	if account == nil {
-		// nothing to delete
-		return nil
-	}
-
-	// add deleted user to revocation list and update the account JWT
-	err = addUserToRevocationList(ctx, storage, account, issue)
-	if err != nil {
-		return err
+	if account != nil {
+		// add deleted user to revocation list and update the account JWT
+		err = addUserToRevocationList(ctx, storage, account, issue)
+		if err != nil {
+			return err
+		}
 	}
 
 	// delete user nkey
@@ -402,7 +397,6 @@ func issueUserNKeys(ctx context.Context, storage logical.Storage, issue IssueUse
 }
 
 func issueUserJWT(ctx context.Context, storage logical.Storage, issue IssueUserStorage) error {
-
 	// use either operator nkey or signing nkey
 	// to sign jwt and add issuer claim
 	useSigningKey := issue.UseSigningKey
@@ -513,7 +507,6 @@ func issueUserJWT(ctx context.Context, storage logical.Storage, issue IssueUserS
 }
 
 func issueUserCreds(ctx context.Context, storage logical.Storage, issue IssueUserStorage) error {
-
 	// receive user nkey seed
 	// to add to creds file
 	userNkey, err := readUserNkey(ctx, storage, NkeyParameters{
@@ -578,7 +571,6 @@ func getUserIssuePath(operator string, account string, user string) string {
 }
 
 func createResponseIssueUserData(issue *IssueUserStorage) (*logical.Response, error) {
-
 	data := &IssueUserData{
 		Operator:      issue.Operator,
 		Account:       issue.Account,
@@ -601,7 +593,6 @@ func createResponseIssueUserData(issue *IssueUserStorage) (*logical.Response, er
 }
 
 func updateUserStatus(ctx context.Context, storage logical.Storage, issue *IssueUserStorage) {
-
 	// account status
 	nkey, err := readUserNkey(ctx, storage, NkeyParameters{
 		Operator: issue.Operator,

diff --git a/paths_issue_user_test.go b/paths_issue_user_test.go
@@ -895,3 +895,59 @@ func Test_UnmarshalIssueUserParameters(t *testing.T) {
 	assert.Nil(err)
 	fmt.Printf("%+v\n", claims)
 }
+
+func Test_EverythingBeforeUser(t *testing.T) {
+	b, reqStorage := getTestBackend(t)
+	t.Run("Test delete everything before user", func(t *testing.T) {
+
+		resp, err := b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.CreateOperation,
+			Path:      "issue/operator/op1",
+			Storage:   reqStorage,
+			Data:      map[string]interface{}{},
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.CreateOperation,
+			Path:      "issue/operator/op1/account/ac1",
+			Storage:   reqStorage,
+			Data:      map[string]interface{}{},
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.CreateOperation,
+			Path:      "issue/operator/op1/account/ac1/user/us1",
+			Storage:   reqStorage,
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.DeleteOperation,
+			Path:      "issue/operator/op1",
+			Storage:   reqStorage,
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.DeleteOperation,
+			Path:      "issue/operator/op1/account/ac1",
+			Storage:   reqStorage,
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.DeleteOperation,
+			Path:      "issue/operator/op1/account/ac1/user/us1",
+			Storage:   reqStorage,
+		})
+		assert.NoError(t, err)
+		assert.False(t, resp.IsError())
+	})
+}