From b443b4f5b9f9af4bbed22e508b226a7c74aac15a Mon Sep 17 00:00:00 2001 From: Adrian-Stefan Mares Date: Wed, 29 Nov 2023 09:44:40 +0100 Subject: [PATCH] is: Fix multiple chain rights assertions --- CHANGELOG.md | 2 ++ pkg/identityserver/gateway_access_test.go | 29 +++++++++++++++++++++++ pkg/identityserver/rights.go | 19 ++++++++------- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f24a52cac..6dd3a99d56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pkg/identityserver/gateway_access_test.go b/pkg/identityserver/gateway_access_test.go index 7c3ccefd18..b666c731e0 100644 --- a/pkg/identityserver/gateway_access_test.go +++ b/pkg/identityserver/gateway_access_test.go @@ -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) @@ -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) { diff --git a/pkg/identityserver/rights.go b/pkg/identityserver/rights.go index acb177a840..b021745dfd 100644 --- a/pkg/identityserver/rights.go +++ b/pkg/identityserver/rights.go @@ -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 { @@ -292,21 +291,21 @@ 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{}{} } } } @@ -314,7 +313,7 @@ func (is *IdentityServer) assertGatewayRights( // nolint:gocyclo 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()) @@ -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() } }