Skip to content

Commit c2e5074

Browse files
authored
confdb, overlord/confdbstate: add visibility-based pruning to view.Get and authorization checks to view.Set and view.Unset (#16400)
* confdb: add pre-get pruning * confdb, o/confdbstate: add visibility check during set * confdb, overlord/confdbstate: add secrets check to unset * confdb: minor - return nil instead of empty slice * confdb: add comment in set explaining approach
1 parent 400bea6 commit c2e5074

File tree

3 files changed

+755
-254
lines changed

3 files changed

+755
-254
lines changed

confdb/confdb.go

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,9 @@ func validateSetValue(initial any) error {
11051105

11061106
// Set sets the named view to a specified non-nil value. Matches that cannot
11071107
// extract their data from the provided value will be considered as being unset.
1108-
func (v *View) Set(databag Databag, request string, value any) error {
1108+
// If the userID does not have sufficient permissions to set the data indicated
1109+
// by the request, then an UnauthorizedAccessError is returned.
1110+
func (v *View) Set(databag Databag, request string, value any, userID int) error {
11091111
if request == "" {
11101112
return badRequestErrorFrom(v, "set", request, "")
11111113
}
@@ -1177,10 +1179,28 @@ func (v *View) Set(databag Databag, request string, value any) error {
11771179
getAccs = func(i int) []Accessor { return expandedMatches[i].storagePath }
11781180
sort.Slice(expandedMatches, byAccessor(getAccs))
11791181

1182+
visibilities := getVisibilitiesToPrune(userID)
1183+
11801184
for _, match := range expandedMatches {
11811185
if err := databag.Set(match.storagePath, match.value); err != nil {
11821186
return err
11831187
}
1188+
data, err := databag.Data()
1189+
if err != nil {
1190+
return err
1191+
}
1192+
// we can safely check visibility after setting data in the databag since the
1193+
// data is only persisted if this Set method returns a non-error
1194+
pruned, err := v.schema.DatabagSchema.PruneByVisibility(match.storagePath, visibilities, data)
1195+
if err != nil {
1196+
if errors.Is(err, &UnauthorizedAccessError{}) {
1197+
return &UnauthorizedAccessError{viewID: v.ID(), operation: "set", request: request}
1198+
}
1199+
return err
1200+
}
1201+
if len(pruned) != len(data) {
1202+
return &UnauthorizedAccessError{viewID: v.ID(), operation: "set", request: request}
1203+
}
11841204
}
11851205

11861206
data, err := databag.Data()
@@ -1220,7 +1240,10 @@ func byAccessor(getAccs accGetter) func(x, y int) bool {
12201240
}
12211241
}
12221242

1223-
func (v *View) Unset(databag Databag, request string) error {
1243+
// Unset unsets the value at request in the named view.
1244+
// If the userID does not have sufficient permissions to unset the data
1245+
// indicated by the request, then an UnauthorizedAccessError is returned
1246+
func (v *View) Unset(databag Databag, request string, userID int) error {
12241247
opts := ParseOptions{AllowPlaceholders: false}
12251248
accessors, err := ParsePathIntoAccessors(request, opts)
12261249
if err != nil {
@@ -1236,7 +1259,25 @@ func (v *View) Unset(databag Databag, request string) error {
12361259
return NewNoMatchError(v, "unset", []string{request})
12371260
}
12381261

1262+
visibilities := getVisibilitiesToPrune(userID)
12391263
for _, match := range matches {
1264+
if len(visibilities) > 0 {
1265+
data, err := databag.Data()
1266+
if err != nil {
1267+
return err
1268+
}
1269+
pruned, err := v.schema.DatabagSchema.PruneByVisibility(match.storagePath, visibilities, data)
1270+
if err != nil {
1271+
if errors.Is(err, &UnauthorizedAccessError{}) {
1272+
return &UnauthorizedAccessError{viewID: v.ID(), operation: "unset", request: request}
1273+
}
1274+
return err
1275+
}
1276+
if len(pruned) != len(data) {
1277+
return &UnauthorizedAccessError{viewID: v.ID(), operation: "unset", request: request}
1278+
}
1279+
}
1280+
12401281
if err := databag.Unset(match.storagePath); err != nil {
12411282
return err
12421283
}
@@ -1828,11 +1869,21 @@ func (v *View) CheckAllConstraintsAreUsed(requests []string, constraints map[str
18281869
return NewUnmatchedConstraintsError(v, requests, unusedConstraints)
18291870
}
18301871

1872+
func getVisibilitiesToPrune(userID int) []Visibility {
1873+
if userID == 0 {
1874+
return nil
1875+
}
1876+
return []Visibility{SecretVisibility}
1877+
}
1878+
18311879
// Get returns the view value identified by the request after the constraints
1832-
// have been applied to any matching filter in the storage path. If the request
1833-
// cannot be matched against any rule, it returns a NoMatchError, and if no data
1834-
// is stored for the request, a NoDataError is returned.
1835-
func (v *View) Get(databag Databag, request string, constraints map[string]any) (any, error) {
1880+
// have been applied to any matching filter in the storage path. The userID
1881+
// determines whether data with restrictive visibility levels are returned.
1882+
// If the request cannot be matched against any rule, it returns a NoMatchError,
1883+
// and if no data is stored for the request, a NoDataError is returned. If data
1884+
// would have been returned but was removed due to the visibility level, then a
1885+
// UnauthorizedAccessError is returned.
1886+
func (v *View) Get(databag Databag, request string, constraints map[string]any, userID int) (any, error) {
18361887
var accessors []Accessor
18371888
if request != "" {
18381889
var err error
@@ -1852,9 +1903,35 @@ func (v *View) Get(databag Databag, request string, constraints map[string]any)
18521903
return nil, err
18531904
}
18541905

1906+
visibilities := getVisibilitiesToPrune(userID)
1907+
allUnauthorized := len(visibilities) > 0
18551908
var merged any
18561909
for _, match := range matches {
1857-
val, err := databag.Get(match.storagePath, constraints)
1910+
bag := databag
1911+
if len(visibilities) > 0 {
1912+
data, err := databag.Data()
1913+
if err != nil {
1914+
return nil, err
1915+
}
1916+
data, err = v.schema.DatabagSchema.PruneByVisibility(match.storagePath, visibilities, data)
1917+
if err != nil {
1918+
if errors.Is(err, &UnauthorizedAccessError{}) || errors.Is(err, &NoDataError{}) {
1919+
continue
1920+
}
1921+
return nil, err
1922+
}
1923+
allUnauthorized = false
1924+
var decoded map[string]json.RawMessage
1925+
if err := jsonutil.DecodeWithNumber(bytes.NewReader(data), &decoded); err != nil {
1926+
_, ok := err.(*json.UnmarshalTypeError)
1927+
if !ok {
1928+
return nil, err
1929+
}
1930+
}
1931+
bag = JSONDatabag(decoded)
1932+
}
1933+
1934+
val, err := bag.Get(match.storagePath, constraints)
18581935
if err != nil {
18591936
if errors.Is(err, &NoDataError{}) {
18601937
continue
@@ -1880,6 +1957,9 @@ func (v *View) Get(databag Databag, request string, constraints map[string]any)
18801957
if request != "" {
18811958
requests = []string{request}
18821959
}
1960+
if allUnauthorized {
1961+
return nil, &UnauthorizedAccessError{request: request, operation: "get", viewID: v.ID()}
1962+
}
18831963
return nil, NewNoDataError(v, requests)
18841964
}
18851965

0 commit comments

Comments
 (0)