Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple chain rights assertions #6725

Merged
merged 1 commit into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ For details about compatibility between different releases, see the **Commitment

### Fixed

- Batch gateway rights assertions when multiple membership chains are available (for example, both via a user and an organization).

### Security

## [3.28.1] - 2023-11-27
Expand Down
29 changes: 29 additions & 0 deletions pkg/identityserver/gateway_access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,21 @@ func TestGatewayBatchAccess(t *testing.T) {
locUserKey, _ := p.NewAPIKey(locUser.GetEntityIdentifiers(), ttnpb.Right_RIGHT_ALL)
locUserCreds := rpcCreds(locUserKey)

dualMembershipUser := p.NewUser()
dualMembershipUserKey, _ := p.NewAPIKey(dualMembershipUser.GetEntityIdentifiers(), ttnpb.Right_RIGHT_ALL)
dualMembershipUserCreds := rpcCreds(dualMembershipUserKey)
p.NewMembership(
dualMembershipUser.GetOrganizationOrUserIdentifiers(),
gtw3.GetEntityIdentifiers(),
ttnpb.Right_RIGHT_GATEWAY_ALL,
)
org1 := p.NewOrganization(dualMembershipUser.GetOrganizationOrUserIdentifiers())
p.NewMembership(
org1.GetOrganizationOrUserIdentifiers(),
gtw3.GetEntityIdentifiers(),
ttnpb.Right_RIGHT_GATEWAY_ALL,
)

testWithIdentityServer(t, func(is *IdentityServer, cc *grpc.ClientConn) {
reg := ttnpb.NewGatewayBatchAccessClient(cc)

Expand Down Expand Up @@ -956,6 +971,20 @@ func TestGatewayBatchAccess(t *testing.T) {
},
Credentials: usr1Creds,
},
{
Name: "Dual membership user",
Request: &ttnpb.AssertGatewayRightsRequest{
GatewayIds: []*ttnpb.GatewayIdentifiers{
gtw3.GetIds(),
},
Required: &ttnpb.Rights{
Rights: []ttnpb.Right{
ttnpb.Right_RIGHT_GATEWAY_ALL,
},
},
},
Credentials: dualMembershipUserCreds,
},
} {
tc := tc
t.Run(tc.Name, func(t *testing.T) {
Expand Down
19 changes: 10 additions & 9 deletions pkg/identityserver/rights.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"go.thethings.network/lorawan-stack/v3/pkg/errors"
"go.thethings.network/lorawan-stack/v3/pkg/identityserver/store"
"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
"go.thethings.network/lorawan-stack/v3/pkg/unique"
)

func allPotentialRights(eIDs *ttnpb.EntityIdentifiers, rights *ttnpb.Rights) *ttnpb.Rights {
Expand Down Expand Up @@ -292,29 +291,29 @@ func (is *IdentityServer) assertGatewayRights( // nolint:gocyclo
if bothStatsAndLocation {
for _, gtw := range gtws {
if gtw.StatusPublic && gtw.LocationPublic {
publicGatewayIds[unique.ID(ctx, gtw.Ids)] = struct{}{}
publicGatewayIds[gtw.IDString()] = struct{}{}
}
}
} else {
if onlyPublicStats {
for _, gtw := range gtws {
if gtw.StatusPublic {
publicGatewayIds[unique.ID(ctx, gtw.Ids)] = struct{}{}
publicGatewayIds[gtw.IDString()] = struct{}{}
}
}
}
if onlyPublicLocation {
for _, gtw := range gtws {
if gtw.LocationPublic {
publicGatewayIds[unique.ID(ctx, gtw.Ids)] = struct{}{}
publicGatewayIds[gtw.IDString()] = struct{}{}
}
}
}
}

entityIDs := make([]string, 0, len(gtwIDs))
for _, gtwID := range gtwIDs {
if _, ok := publicGatewayIds[unique.ID(ctx, gtwID)]; ok {
if _, ok := publicGatewayIds[gtwID.IDString()]; ok {
continue
}
entityIDs = append(entityIDs, gtwID.GetEntityIdentifiers().IDString())
Expand All @@ -336,12 +335,14 @@ func (is *IdentityServer) assertGatewayRights( // nolint:gocyclo
if err != nil {
return err
}
if len(membershipChains) != len(entityIDs) {
return errInsufficientRights.New()
}
entityRights := make(map[string]*ttnpb.Rights, len(membershipChains))
for _, chain := range membershipChains {
id := chain.EntityIdentifiers.IDString()
entityRights[id] = entityRights[id].Union(chain.GetRights())
}
for _, entityID := range entityIDs {
// Make sure that there are no extra rights requested.
if !chain.GetRights().IncludesAll(requiredGatewayRights.GetRights()...) {
if !entityRights[entityID].IncludesAll(requiredGatewayRights.GetRights()...) {
return errInsufficientRights.New()
}
}
Expand Down
Loading