Skip to content

Commit 29a24be

Browse files
torcolvinCopilotadamcfraser
authored
CBG-4345 don't panic if role documents can't be read (#7816)
Co-authored-by: Copilot <[email protected]> Co-authored-by: Adam Fraser <[email protected]>
1 parent d7f18c2 commit 29a24be

24 files changed

+526
-240
lines changed

auth/auth_test.go

Lines changed: 96 additions & 45 deletions
Large diffs are not rendered by default.

auth/collection_access.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type CollectionChannelAPI interface {
3838
SetCollectionChannelHistory(scope, collection string, history TimedSetHistory)
3939

4040
// Returns true if the Principal has access to the given channel.
41-
CanSeeCollectionChannel(scope, collection, channel string) bool
41+
CanSeeCollectionChannel(scope, collection, channel string) (bool, error)
4242

4343
// Retrieve invalidation sequence for a collection
4444
getCollectionChannelInvalSeq(scope, collection string) uint64
@@ -52,7 +52,7 @@ type CollectionChannelAPI interface {
5252

5353
// If the Principal has access to the given collection's channel, returns the sequence number at which
5454
// access was granted; else returns zero.
55-
canSeeCollectionChannelSince(scope, collection, channel string) uint64
55+
canSeeCollectionChannelSince(scope, collection, channel string) (uint64, error)
5656

5757
// Returns an error if the Principal does not have access to all the channels in the set, for the specified collection.
5858
authorizeAllCollectionChannels(scope, collection string, channels base.Set) error
@@ -76,23 +76,23 @@ type UserCollectionChannelAPI interface {
7676
SetCollectionJWTChannels(scope, collection string, channels ch.TimedSet, seq uint64)
7777

7878
// Retrieves revoked channels for a collection, based on the given since value
79-
RevokedCollectionChannels(scope, collection string, since uint64, lowSeq uint64, triggeredBy uint64) RevokedChannels
79+
RevokedCollectionChannels(scope, collection string, since uint64, lowSeq uint64, triggeredBy uint64) (RevokedChannels, error)
8080

8181
// Obtains the period over which the user had access to the given collection's channel. Either directly or via a role.
8282
CollectionChannelGrantedPeriods(scope, collection, chanName string) ([]GrantHistorySequencePair, error)
8383

8484
// Every channel the user has access to in the collection, including those inherited from Roles.
85-
InheritedCollectionChannels(scope, collection string) ch.TimedSet
85+
InheritedCollectionChannels(scope, collection string) (ch.TimedSet, error)
8686

8787
// Returns a TimedSet containing only the channels from the input set that the user has access
8888
// to for the collection, annotated with the sequence number at which access was granted.
8989
// Returns a string array containing any channels filtered out due to the user not having access
9090
// to them.
91-
FilterToAvailableCollectionChannels(scope, collection string, channels base.Set) (filtered ch.TimedSet, removed []string)
91+
FilterToAvailableCollectionChannels(scope, collection string, channels base.Set) (filtered ch.TimedSet, removed []string, err error)
9292

9393
// If the input set contains the wildcard "*" channel, returns the user's inheritedChannels for the collection;
9494
// else returns the input channel list unaltered.
95-
expandCollectionWildCardChannel(scope, collection string, channels base.Set) base.Set
95+
expandCollectionWildCardChannel(scope, collection string, channels base.Set) (base.Set, error)
9696
}
9797

9898
// PrincipalCollectionAccess defines a common interface for principal access control. This interface is

auth/collection_access_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,18 @@ import (
2222
// requireCanSeeCollectionChannels asserts that the principal can see all the specified channels in the given collection
2323
func requireCanSeeCollectionChannels(t *testing.T, scope, collection string, princ Principal, channels ...string) {
2424
for _, channel := range channels {
25-
require.True(t, princ.CanSeeCollectionChannel(scope, collection, channel), "Expected %s to be able to see channel %q in %s.%s", princ.Name(), channel, scope, collection)
25+
canSee, err := princ.CanSeeCollectionChannel(scope, collection, channel)
26+
require.NoError(t, err)
27+
require.True(t, canSee, "Expected %s to be able to see channel %q in %s.%s", princ.Name(), channel, scope, collection)
2628
}
2729
}
2830

2931
// requireCannotSeeCollectionChannels asserts that the principal cannot see any of the specified channels in the given collection
3032
func requireCannotSeeCollectionChannels(t *testing.T, scope, collection string, princ Principal, channels ...string) {
3133
for _, channel := range channels {
32-
require.False(t, princ.CanSeeCollectionChannel(scope, collection, channel), "Expected %s to NOT be able to see channel %q in %s.%s", princ.Name(), channel, scope, collection)
34+
canSee, err := princ.CanSeeCollectionChannel(scope, collection, channel)
35+
require.NoError(t, err)
36+
require.False(t, canSee, "Expected %s to NOT be able to see channel %q in %s.%s", princ.Name(), channel, scope, collection)
3337
}
3438
}
3539

@@ -277,5 +281,7 @@ func TestPrincipalConfigSetExplicitChannels(t *testing.T) {
277281

278282
// requireExpandCollectionWildCardChannels asserts that the channels will be expanded to the expected channels for the given collection
279283
func requireExpandCollectionWildCardChannels(t *testing.T, user User, scope, collection string, expectedChannels []string, channelsToExpand []string) {
280-
require.Equal(t, base.SetFromArray(expectedChannels), user.expandCollectionWildCardChannel(scope, collection, base.SetFromArray(channelsToExpand)), "Expected channels %v for %s.%s from %v on user %s", expectedChannels, scope, collection, channelsToExpand, user.Name())
284+
expandedChannels, err := user.expandCollectionWildCardChannel(scope, collection, base.SetFromArray(channelsToExpand))
285+
require.NoError(t, err)
286+
require.Equal(t, base.SetFromArray(expectedChannels), expandedChannels, "Expected channels %v for %s.%s from %v on user %s", expectedChannels, scope, collection, channelsToExpand, user.Name())
281287
}

auth/principal.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ type Principal interface {
2525
SetSequence(sequence uint64)
2626

2727
// Returns true if the Principal has access to the given channel.
28-
canSeeChannel(channel string) bool
28+
canSeeChannel(channel string) (bool, error)
2929

3030
// If the Principal has access to the given channel, returns the sequence number at which
3131
// access was granted; else returns zero.
32-
canSeeChannelSince(channel string) uint64
32+
canSeeChannelSince(channel string) (uint64, error)
3333

3434
// Returns an error if the Principal does not have access to all the channels in the set.
3535
authorizeAllChannels(channels base.Set) error
@@ -104,7 +104,7 @@ type User interface {
104104
SetPassword(password string) error
105105

106106
// GetRoles returns the set of roles the user belongs to, initializing them if necessary.
107-
GetRoles() []Role
107+
GetRoles() ([]Role, error)
108108

109109
// The set of Roles the user belongs to (including ones given to it by the sync function and by OIDC/JWT)
110110
// Returns nil if invalidated
@@ -135,25 +135,25 @@ type User interface {
135135

136136
RoleHistory() TimedSetHistory
137137

138-
InitializeRoles()
138+
InitializeRoles() error
139139

140-
revokedChannels(since uint64, lowSeq uint64, triggeredBy uint64) RevokedChannels
140+
revokedChannels(since uint64, lowSeq uint64, triggeredBy uint64) (RevokedChannels, error)
141141

142142
// Obtains the period over which the user had access to the given channel. Either directly or via a role.
143143
channelGrantedPeriods(chanName string) ([]GrantHistorySequencePair, error)
144144

145145
// Every channel the user has access to, including those inherited from Roles.
146-
inheritedChannels() ch.TimedSet
146+
inheritedChannels() (ch.TimedSet, error)
147147

148148
// If the input set contains the wildcard "*" channel, returns the user's InheritedChannels;
149149
// else returns the input channel list unaltered.
150-
expandWildCardChannel(channels base.Set) base.Set
150+
expandWildCardChannel(channels base.Set) (base.Set, error)
151151

152152
// Returns a TimedSet containing only the channels from the input set that the user has access
153153
// to, annotated with the sequence number at which access was granted.
154154
// Returns a string array containing any channels filtered out due to the user not having access
155155
// to them.
156-
filterToAvailableChannels(channels base.Set) (filtered ch.TimedSet, removed []string)
156+
filterToAvailableChannels(channels base.Set) (filtered ch.TimedSet, removed []string, err error)
157157

158158
setRolesSince(ch.TimedSet)
159159

auth/role.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,17 +373,17 @@ func (role *roleImpl) UnauthError(err error) error {
373373

374374
// Returns true if the Role is allowed to access the channel.
375375
// A nil Role means access control is disabled, so the function will return true.
376-
func (role *roleImpl) canSeeChannel(channel string) bool {
377-
return role == nil || role.Channels().Contains(channel) || role.Channels().Contains(ch.UserStarChannel)
376+
func (role *roleImpl) canSeeChannel(channel string) (bool, error) {
377+
return role == nil || role.Channels().Contains(channel) || role.Channels().Contains(ch.UserStarChannel), nil
378378
}
379379

380380
// Returns the sequence number since which the Role has been able to access the channel, else zero.
381-
func (role *roleImpl) canSeeChannelSince(channel string) uint64 {
381+
func (role *roleImpl) canSeeChannelSince(channel string) (uint64, error) {
382382
seq := role.Channels()[channel]
383383
if seq.Sequence == 0 {
384384
seq = role.Channels()[ch.UserStarChannel]
385385
}
386-
return seq.Sequence
386+
return seq.Sequence, nil
387387
}
388388

389389
func (role *roleImpl) authorizeAllChannels(channels base.Set) error {
@@ -399,7 +399,11 @@ func (role *roleImpl) authorizeAnyChannel(channels base.Set) error {
399399
func authorizeAllChannels(princ Principal, channels base.Set) error {
400400
var forbidden []string
401401
for channel := range channels {
402-
if !princ.canSeeChannel(channel) {
402+
canSee, err := princ.canSeeChannel(channel)
403+
if err != nil {
404+
return err
405+
}
406+
if !canSee {
403407
if forbidden == nil {
404408
forbidden = make([]string, 0, len(channels))
405409
}
@@ -417,7 +421,11 @@ func authorizeAllChannels(princ Principal, channels base.Set) error {
417421
func authorizeAnyChannel(princ Principal, channels base.Set) error {
418422
if len(channels) > 0 {
419423
for channel := range channels {
420-
if princ.canSeeChannel(channel) {
424+
canSee, err := princ.canSeeChannel(channel)
425+
if err != nil {
426+
return err
427+
}
428+
if canSee {
421429
return nil
422430
}
423431
}

auth/role_collection_access.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,22 +147,22 @@ func (role *roleImpl) SetCollectionChannelHistory(scope, collection string, hist
147147

148148
// Returns true if the Role is allowed to access the channel.
149149
// A nil Role means access control is disabled, so the function will return true.
150-
func (role *roleImpl) CanSeeCollectionChannel(scope, collection, channel string) bool {
150+
func (role *roleImpl) CanSeeCollectionChannel(scope, collection, channel string) (bool, error) {
151151
if base.IsDefaultCollection(scope, collection) {
152152
return role.canSeeChannel(channel)
153153
}
154154

155155
if role == nil {
156-
return true
156+
return true, nil
157157
}
158158
if cc, ok := role.getCollectionAccess(scope, collection); ok {
159-
return cc.CanSeeChannel(channel)
159+
return cc.CanSeeChannel(channel), nil
160160
}
161-
return false
161+
return false, nil
162162
}
163163

164164
// Returns the sequence number since which the Role has been able to access the channel, else zero.
165-
func (role *roleImpl) canSeeCollectionChannelSince(scope, collection, channel string) uint64 {
165+
func (role *roleImpl) canSeeCollectionChannelSince(scope, collection, channel string) (uint64, error) {
166166
if base.IsDefaultCollection(scope, collection) {
167167
return role.canSeeChannelSince(channel)
168168
}
@@ -172,9 +172,9 @@ func (role *roleImpl) canSeeCollectionChannelSince(scope, collection, channel st
172172
if seq.Sequence == 0 {
173173
seq = cc.Channels()[ch.UserStarChannel]
174174
}
175-
return seq.Sequence
175+
return seq.Sequence, nil
176176
}
177-
return 0
177+
return 0, nil
178178
}
179179

180180
func (role *roleImpl) authorizeAllCollectionChannels(scope, collection string, channels base.Set) error {

0 commit comments

Comments
 (0)