Skip to content

Commit 3b527e0

Browse files
retranretran
committed
refactor: add type-safe BSON helpers in writer_security
1 parent 1d67cb5 commit 3b527e0

2 files changed

Lines changed: 111 additions & 31 deletions

File tree

‎sdk/mpr/writer_security.go‎

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package mpr
44

55
import (
66
"fmt"
7+
"log"
78
"strings"
89

910
"github.com/mendixlabs/mxcli/model"
@@ -12,6 +13,28 @@ import (
1213
"go.mongodb.org/mongo-driver/bson/primitive"
1314
)
1415

16+
// bsonString extracts a string from an interface value, logging unexpected types.
17+
func bsonString(v any, field string) string {
18+
if s, ok := v.(string); ok {
19+
return s
20+
}
21+
if v != nil {
22+
log.Printf("warning: writer_security: expected string for %q, got %T", field, v)
23+
}
24+
return ""
25+
}
26+
27+
// bsonBool extracts a bool from an interface value, logging unexpected types.
28+
func bsonBool(v any, field string) bool {
29+
if b, ok := v.(bool); ok {
30+
return b
31+
}
32+
if v != nil {
33+
log.Printf("warning: writer_security: expected bool for %q, got %T", field, v)
34+
}
35+
return false
36+
}
37+
1538
// GetRawUnitBytes reads the raw BSON bytes for a unit by ID.
1639
// This returns unprocessed bytes suitable for raw BSON patching.
1740
func (r *Reader) GetRawUnitBytes(id model.ID) ([]byte, error) {
@@ -251,7 +274,7 @@ func (w *Writer) RemoveModuleRole(unitID model.ID, roleName string) error {
251274
name := ""
252275
for _, f := range roleDoc {
253276
if f.Key == "Name" {
254-
name, _ = f.Value.(string)
277+
name = bsonString(f.Value, "Name")
255278
break
256279
}
257280
}
@@ -325,7 +348,7 @@ func (w *Writer) AlterUserRoleModuleRoles(unitID model.ID, userRoleName string,
325348
name := ""
326349
for _, f := range roleDoc {
327350
if f.Key == "Name" {
328-
name, _ = f.Value.(string)
351+
name = bsonString(f.Value, "Name")
329352
break
330353
}
331354
}
@@ -476,7 +499,7 @@ func (w *Writer) RemoveUserRole(unitID model.ID, name string) error {
476499
roleName := ""
477500
for _, f := range roleDoc {
478501
if f.Key == "Name" {
479-
roleName, _ = f.Value.(string)
502+
roleName = bsonString(f.Value, "Name")
480503
break
481504
}
482505
}
@@ -530,7 +553,7 @@ func (w *Writer) RemoveDemoUser(unitID model.ID, userName string) error {
530553
name := ""
531554
for _, f := range userDoc {
532555
if f.Key == "UserName" {
533-
name, _ = f.Value.(string)
556+
name = bsonString(f.Value, "UserName")
534557
break
535558
}
536559
}
@@ -582,7 +605,7 @@ func (w *Writer) AddEntityAccessRule(unitID model.ID, entityName string, roleNam
582605
name := ""
583606
for _, f := range entityDoc {
584607
if f.Key == "Name" {
585-
name, _ = f.Value.(string)
608+
name = bsonString(f.Value, "Name")
586609
break
587610
}
588611
}
@@ -767,11 +790,11 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
767790
for _, mf := range maDoc {
768791
switch mf.Key {
769792
case "Attribute":
770-
ref = mf.Value.(string)
793+
ref = bsonString(mf.Value, "Attribute")
771794
case "Association":
772-
ref = mf.Value.(string)
795+
ref = bsonString(mf.Value, "Association")
773796
case "AccessRights":
774-
rights, _ = mf.Value.(string)
797+
rights = bsonString(mf.Value, "AccessRights")
775798
}
776799
}
777800
if ref != "" {
@@ -786,32 +809,32 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
786809
for _, f := range existing {
787810
switch f.Key {
788811
case "AllowCreate":
789-
existCreate, _ = f.Value.(bool)
812+
existCreate = bsonBool(f.Value, "AllowCreate")
790813
case "AllowDelete":
791-
existDelete, _ = f.Value.(bool)
814+
existDelete = bsonBool(f.Value, "AllowDelete")
792815
case "DefaultMemberAccessRights":
793-
existDefault, _ = f.Value.(string)
816+
existDefault = bsonString(f.Value, "DefaultMemberAccessRights")
794817
case "XPathConstraint":
795-
existXPath, _ = f.Value.(string)
818+
existXPath = bsonString(f.Value, "XPathConstraint")
796819
}
797820
}
798821

799822
// Merge into newRule
800823
for i, f := range newRule {
801824
switch f.Key {
802825
case "AllowCreate":
803-
newVal, _ := f.Value.(bool)
826+
newVal := bsonBool(f.Value, "AllowCreate")
804827
newRule[i].Value = newVal || existCreate
805828
case "AllowDelete":
806-
newVal, _ := f.Value.(bool)
829+
newVal := bsonBool(f.Value, "AllowDelete")
807830
newRule[i].Value = newVal || existDelete
808831
case "DefaultMemberAccessRights":
809-
newVal, _ := f.Value.(string)
832+
newVal := bsonString(f.Value, "DefaultMemberAccessRights")
810833
if accessRightsLevel(existDefault) > accessRightsLevel(newVal) {
811834
newRule[i].Value = existDefault
812835
}
813836
case "XPathConstraint":
814-
newVal, _ := f.Value.(string)
837+
newVal := bsonString(f.Value, "XPathConstraint")
815838
if newVal == "" && existXPath != "" {
816839
newRule[i].Value = existXPath
817840
}
@@ -829,11 +852,11 @@ func mergeAccessRule(existing, newRule bson.D) bson.D {
829852
for _, mf := range maDoc {
830853
switch mf.Key {
831854
case "Attribute":
832-
ref = mf.Value.(string)
855+
ref = bsonString(mf.Value, "Attribute")
833856
case "Association":
834-
ref = mf.Value.(string)
857+
ref = bsonString(mf.Value, "Association")
835858
case "AccessRights":
836-
newRights, _ = mf.Value.(string)
859+
newRights = bsonString(mf.Value, "AccessRights")
837860
}
838861
}
839862
if ref == "" {
@@ -885,7 +908,7 @@ func (w *Writer) RemoveEntityAccessRule(unitID model.ID, entityName string, role
885908
name := ""
886909
for _, f := range entityDoc {
887910
if f.Key == "Name" {
888-
name, _ = f.Value.(string)
911+
name = bsonString(f.Value, "Name")
889912
break
890913
}
891914
}
@@ -1016,7 +1039,7 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10161039
name := ""
10171040
for _, f := range entityDoc {
10181041
if f.Key == "Name" {
1019-
name, _ = f.Value.(string)
1042+
name = bsonString(f.Value, "Name")
10201043
break
10211044
}
10221045
}
@@ -1073,7 +1096,7 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10731096
ruleDoc[k].Value = "None"
10741097
ruleModified = true
10751098
} else if revocation.RevokeWriteAll {
1076-
cur, _ := rf.Value.(string)
1099+
cur := bsonString(rf.Value, "DefaultMemberAccessRights")
10771100
if cur == "ReadWrite" {
10781101
ruleDoc[k].Value = "ReadOnly"
10791102
ruleModified = true
@@ -1093,11 +1116,11 @@ func (w *Writer) RevokeEntityMemberAccess(unitID model.ID, entityName string, ro
10931116
for _, mf := range maDoc {
10941117
switch mf.Key {
10951118
case "Attribute":
1096-
ref = mf.Value.(string)
1119+
ref = bsonString(mf.Value, "Attribute")
10971120
case "Association":
1098-
ref = mf.Value.(string)
1121+
ref = bsonString(mf.Value, "Association")
10991122
case "AccessRights":
1100-
rights, _ = mf.Value.(string)
1123+
rights = bsonString(mf.Value, "AccessRights")
11011124
}
11021125
}
11031126
if ref == "" {
@@ -1267,7 +1290,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
12671290
entityName := ""
12681291
for _, f := range entityDoc {
12691292
if f.Key == "Name" {
1270-
entityName, _ = f.Value.(string)
1293+
entityName = bsonString(f.Value, "Name")
12711294
break
12721295
}
12731296
}
@@ -1288,7 +1311,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
12881311
isCalculated := false
12891312
for _, f := range attrDoc {
12901313
if f.Key == "Name" {
1291-
attrName, _ = f.Value.(string)
1314+
attrName = bsonString(f.Value, "Name")
12921315
}
12931316
if f.Key == "Value" {
12941317
if valueDoc, ok := f.Value.(bson.D); ok {
@@ -1363,7 +1386,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
13631386
case "ParentPointer":
13641387
aParentID = extractBsonIDValue(f.Value)
13651388
case "Name":
1366-
aName, _ = f.Value.(string)
1389+
aName = bsonString(f.Value, "Name")
13671390
}
13681391
}
13691392
if aParentID == entityID && aName != "" {
@@ -1382,7 +1405,7 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
13821405
parentID = extractBsonIDValue(f.Value)
13831406
}
13841407
if f.Key == "Name" {
1385-
caName, _ = f.Value.(string)
1408+
caName = bsonString(f.Value, "Name")
13861409
}
13871410
}
13881411
if parentID == entityID && caName != "" {
@@ -1461,10 +1484,10 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in
14611484
assocRef := ""
14621485
for _, mf := range maDoc {
14631486
if mf.Key == "Attribute" {
1464-
attrRef, _ = mf.Value.(string)
1487+
attrRef = bsonString(mf.Value, "Attribute")
14651488
}
14661489
if mf.Key == "Association" {
1467-
assocRef, _ = mf.Value.(string)
1490+
assocRef = bsonString(mf.Value, "Association")
14681491
}
14691492
}
14701493

‎sdk/mpr/writer_security_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
package mpr
44

55
import (
6+
"io"
7+
"log"
68
"testing"
79

810
"go.mongodb.org/mongo-driver/bson"
@@ -124,3 +126,58 @@ func TestRemoveRolesFromAccessRule_ThreeRoles_RemoveMiddle(t *testing.T) {
124126
t.Errorf("expected [Mod.A, Mod.C], got %v", names)
125127
}
126128
}
129+
130+
// =============================================================================
131+
// bsonString / bsonBool — unexpected types must not panic
132+
// =============================================================================
133+
134+
func TestBsonHelpers_UnexpectedTypes_NoPanic(t *testing.T) {
135+
log.SetOutput(io.Discard)
136+
defer log.SetOutput(nil)
137+
138+
// bsonString with non-string values
139+
if s := bsonString(42, "test"); s != "" {
140+
t.Errorf("expected empty string, got %q", s)
141+
}
142+
if s := bsonString(nil, "test"); s != "" {
143+
t.Errorf("expected empty string for nil, got %q", s)
144+
}
145+
if s := bsonString(true, "test"); s != "" {
146+
t.Errorf("expected empty string for bool, got %q", s)
147+
}
148+
149+
// bsonBool with non-bool values
150+
if b := bsonBool("true", "test"); b {
151+
t.Error("expected false for string input")
152+
}
153+
if b := bsonBool(nil, "test"); b {
154+
t.Error("expected false for nil")
155+
}
156+
if b := bsonBool(42, "test"); b {
157+
t.Error("expected false for int input")
158+
}
159+
}
160+
161+
func TestMergeAccessRule_UnexpectedTypes_NoPanic(t *testing.T) {
162+
log.SetOutput(io.Discard)
163+
defer log.SetOutput(nil)
164+
165+
existing := bson.D{
166+
{Key: "$Type", Value: "DomainModels$AccessRule"},
167+
{Key: "AllowCreate", Value: 42}, // wrong type: int instead of bool
168+
{Key: "AllowDelete", Value: "not-a-bool"}, // wrong type: string instead of bool
169+
{Key: "DefaultMemberAccessRights", Value: 99},
170+
}
171+
newRule := bson.D{
172+
{Key: "$Type", Value: "DomainModels$AccessRule"},
173+
{Key: "AllowCreate", Value: true},
174+
{Key: "AllowDelete", Value: false},
175+
{Key: "DefaultMemberAccessRights", Value: "ReadWrite"},
176+
}
177+
178+
// Must not panic
179+
result := mergeAccessRule(existing, newRule)
180+
if result == nil {
181+
t.Error("expected non-nil result")
182+
}
183+
}

0 commit comments

Comments
 (0)