Merge pull request #6725 from TheThingsNetwork/fix/is-batch-rights-union

Fix multiple chain rights assertions

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

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) {

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,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())
@@ -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()
 			}
 		}