fix(kv): delete authorization from correct index bucket (#16835)

* fix(kv): delete authorization from correct index bucket * fix(kv): return not found code when user resource mapping indexed by not in source * chore(kv): define failing test for URM on delete
influxdata · Feb 12, 2020 · 7349216 · 7349216
1 parent 03f65cf
commit 7349216
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 7 deletions.
diff --git a/kv/auth.go b/kv/auth.go
@@ -653,7 +653,12 @@ func (s *Service) deleteAuthorization(ctx context.Context, tx Tx, id influxdb.ID
 		}
 	}
 
-	if err := idx.Delete([]byte(authByUserIndexKey(a.UserID, id))); err != nil {
+	uidx, err := authByUserIndexBucket(tx)
+	if err != nil {
+		return &influxdb.Error{Err: err}
+	}
+
+	if err := uidx.Delete([]byte(authByUserIndexKey(a.UserID, id))); err != nil {
 		return err
 	}
 

diff --git a/kv/urm.go b/kv/urm.go
@@ -195,13 +195,16 @@ func (s *Service) findUserResourceMappingsByIndex(ctx context.Context, tx Tx, fi
 	}
 
 	for k, v := cursor.Next(); k != nil && v != nil; k, v = cursor.Next() {
-		v, err := bkt.Get(v)
+		nv, err := bkt.Get(v)
 		if err != nil {
-			return nil, err
+			return nil, &influxdb.Error{
+				Code: influxdb.ENotFound,
+				Err:  err,
+			}
 		}
 
 		m := &influxdb.UserResourceMapping{}
-		if err := json.Unmarshal(v, m); err != nil {
+		if err := json.Unmarshal(nv, m); err != nil {
 			return nil, CorruptURMError(err)
 		}
 
@@ -531,11 +534,13 @@ func (s *Service) deleteOrgDependentMappings(ctx context.Context, tx Tx, m *infl
 		return err
 	}
 	for _, b := range bs {
-		if err := s.deleteUserResourceMapping(ctx, tx, influxdb.UserResourceMappingFilter{
+		filter := influxdb.UserResourceMappingFilter{
 			ResourceType: influxdb.BucketsResourceType,
 			ResourceID:   b.ID,
 			UserID:       m.UserID,
-		}); err != nil {
+		}
+
+		if err := s.deleteUserResourceMapping(ctx, tx, filter); err != nil {
 			if influxdb.ErrorCode(err) == influxdb.ENotFound {
 				s.log.Info("URM bucket is missing", zap.Stringer("orgID", m.ResourceID))
 				continue

diff --git a/kv/urm_test.go b/kv/urm_test.go
@@ -49,9 +49,27 @@ func initUserResourceMappingService(s kv.Store, f influxdbtesting.UserResourceFi
 		t.Fatalf("error initializing urm service: %v", err)
 	}
 
+	for _, o := range f.Organizations {
+		if err := svc.CreateOrganization(ctx, o); err != nil {
+			t.Fatalf("failed to create org %q", err)
+		}
+	}
+
+	for _, u := range f.Users {
+		if err := svc.CreateUser(ctx, u); err != nil {
+			t.Fatalf("failed to create user %q", err)
+		}
+	}
+
+	for _, b := range f.Buckets {
+		if err := svc.PutBucket(ctx, b); err != nil {
+			t.Fatalf("failed to create bucket %q", err)
+		}
+	}
+
 	for _, m := range f.UserResourceMappings {
 		if err := svc.CreateUserResourceMapping(ctx, m); err != nil {
-			t.Fatalf("failed to populate mappings")
+			t.Fatalf("failed to populate mappings %q", err)
 		}
 	}
 
@@ -61,6 +79,24 @@ func initUserResourceMappingService(s kv.Store, f influxdbtesting.UserResourceFi
 				t.Logf("failed to remove user resource mapping: %v", err)
 			}
 		}
+
+		for _, b := range f.Buckets {
+			if err := svc.DeleteBucket(ctx, b.ID); err != nil {
+				t.Logf("failed to delete org", err)
+			}
+		}
+
+		for _, u := range f.Users {
+			if err := svc.DeleteUser(ctx, u.ID); err != nil {
+				t.Fatalf("failed to delete user %q", err)
+			}
+		}
+
+		for _, o := range f.Organizations {
+			if err := svc.DeleteOrganization(ctx, o.ID); err != nil {
+				t.Logf("failed to delete org", err)
+			}
+		}
 	}
 }
 

diff --git a/testing/user_resource_mapping_service.go b/testing/user_resource_mapping_service.go
@@ -39,6 +39,9 @@ var mappingCmpOptions = cmp.Options{
 
 // UserResourceFields includes prepopulated data for mapping tests
 type UserResourceFields struct {
+	Organizations        []*platform.Organization
+	Users                []*platform.User
+	Buckets              []*platform.Bucket
 	UserResourceMappings []*platform.UserResourceMapping
 }
 
@@ -243,6 +246,46 @@ func DeleteUserResourceMapping(
 				err:      fmt.Errorf("user to resource mapping not found"),
 			},
 		},
+		{
+			name: "delete user resource mapping for org",
+			fields: UserResourceFields{
+				Organizations: []*platform.Organization{
+					{
+						ID:   MustIDBase16(orgOneID),
+						Name: "organization1",
+					},
+				},
+				Users: []*platform.User{
+					{
+						ID:   MustIDBase16(userOneID),
+						Name: "user1",
+					},
+				},
+				Buckets: []*platform.Bucket{
+					{
+						ID:    MustIDBase16(bucketOneID),
+						Name:  "bucket1",
+						OrgID: MustIDBase16(orgOneID),
+					},
+				},
+				UserResourceMappings: []*platform.UserResourceMapping{
+					{
+						ResourceID:   MustIDBase16(orgOneID),
+						ResourceType: platform.OrgsResourceType,
+						MappingType:  platform.UserMappingType,
+						UserID:       MustIDBase16(userOneID),
+						UserType:     platform.Member,
+					},
+				},
+			},
+			args: args{
+				resourceID: MustIDBase16(orgOneID),
+				userID:     MustIDBase16(userOneID),
+			},
+			wants: wants{
+				mappings: []*platform.UserResourceMapping{},
+			},
+		},
 	}
 
 	for _, tt := range tests {