Merge pull request #7345 from TheThingsNetwork/fix/default-pagination

Add default limit to IS List RPCs

CHANGELOG.md

@@ -23,6 +23,7 @@ For details about compatibility between different releases, see the **Commitment
 - Fix reversed Join Server dev nonce metrics.
 - Identity Server's store runs each migration within a transaction, so a migration's changes are only applied if all of its queries are successful.
   - Identity Server's store now marks a migration as successful after all its operations are finished. Previously it was possible to have a successful migration which not all of its queries were processed.
+- Enforce default page limit on IS List RPCs if a value is not provided in the request.
 
 ### Security
 

cmd/internal/shared/identityserver/config.go

@@ -79,4 +79,5 @@ func init() {
 	DefaultIdentityServerConfig.LoginTokens.TokenTTL = time.Hour
 	DefaultIdentityServerConfig.Delete.Restore = 24 * time.Hour
 	DefaultIdentityServerConfig.Gateways.TokenValidity = 5 * time.Second
+	DefaultIdentityServerConfig.Pagination.DefaultLimit = 100
 }

pkg/identityserver/bunstore/store_test.go

@@ -72,6 +72,7 @@ func TestApplicationStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestApplicationStoreCRUD(t)
 	st.TestApplicationStorePagination(t)
+	st.TestApplicationStorePaginationDefaults(t)
 }
 
 func TestClientStore(t *testing.T) {
@@ -80,6 +81,7 @@ func TestClientStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestClientStoreCRUD(t)
 	st.TestClientStorePagination(t)
+	st.TestClientStorePaginationDefaults(t)
 }
 
 func TestEndDeviceStore(t *testing.T) {
@@ -108,6 +110,7 @@ func TestOrganizationStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestOrganizationStoreCRUD(t)
 	st.TestOrganizationStorePagination(t)
+	st.TestOrganizationStorePaginationDefaults(t)
 }
 
 func TestUserStore(t *testing.T) {
@@ -116,6 +119,7 @@ func TestUserStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestUserStoreCRUD(t)
 	st.TestUserStorePagination(t)
+	st.TestUserStorePaginationDefaults(t)
 }
 
 func TestUserSessionStore(t *testing.T) {
@@ -124,6 +128,7 @@ func TestUserSessionStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestUserSessionStore(t)
 	st.TestUserSessionStorePagination(t)
+	st.TestUserSessionStorePaginationDefaults(t)
 }
 
 func TestUserBookmarkStore(t *testing.T) {
@@ -140,6 +145,7 @@ func TestAPIKeyStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestAPIKeyStoreCRUD(t)
 	st.TestAPIKeyStorePagination(t)
+	st.TestAPIKeyStorePaginationDefaults(t)
 }
 
 func TestMembershipStore(t *testing.T) {
@@ -148,6 +154,7 @@ func TestMembershipStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestMembershipStoreCRUD(t)
 	st.TestMembershipStorePagination(t)
+	st.TestMembershipStorePaginationDefaults(t)
 }
 
 func TestEmailValidationStore(t *testing.T) {
@@ -163,6 +170,7 @@ func TestInvitationStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestInvitationStore(t)
 	st.TestInvitationStorePagination(t)
+	st.TestInvitationStorePaginationDefaults(t)
 }
 
 func TestLoginTokenStore(t *testing.T) {
@@ -178,6 +186,7 @@ func TestOAuthStore(t *testing.T) {
 	st := storetest.New(t, newTestStore)
 	st.TestOAuthStore(t)
 	st.TestOAuthStorePagination(t)
+	st.TestOAuthStorePaginationDefaults(t)
 }
 
 func TestEUIStore(t *testing.T) {

pkg/identityserver/config.go

@@ -121,6 +121,9 @@ type Config struct {
 		TenantID string          `name:"tenant-id" description:"Tenant ID"`
 	} `name:"network"`
 	TelemetryQueue telemetry.TaskQueue `name:"-"`
+	Pagination     struct {
+		DefaultLimit uint32 `name:"default-limit" description:"The default limit applied to paginated requests if not specified"` // nolint:lll
+	} `name:"pagination" description:"Pagination settings"`
 }
 
 type emailTemplatesConfig struct {

pkg/identityserver/store.go

@@ -18,11 +18,15 @@ import (
 	"github.com/uptrace/bun"
 	"github.com/uptrace/bun/dialect/pgdialect"
 	bunstore "go.thethings.network/lorawan-stack/v3/pkg/identityserver/bunstore"
+	"go.thethings.network/lorawan-stack/v3/pkg/identityserver/store"
 	"go.thethings.network/lorawan-stack/v3/pkg/log"
 	storeutil "go.thethings.network/lorawan-stack/v3/pkg/util/store"
 )
 
 func (is *IdentityServer) setupStore() error {
+	store.SetPaginationDefaults(store.PaginationDefaults{
+		DefaultLimit: is.config.Pagination.DefaultLimit,
+	})
 	return is.setupBunStore()
 }
 

pkg/identityserver/store/pagination.go

@@ -20,6 +20,17 @@ import (
 	"strings"
 )
 
+// PaginationDefaults sets default values for paginations options within the IS store.
+type PaginationDefaults struct {
+	DefaultLimit uint32
+}
+
+var paginationDefaults = PaginationDefaults{}
+
+// SetPaginationDefaults should only be called at the initialization of the server, never on regitries or
+// auxiliary functions.
+func SetPaginationDefaults(d PaginationDefaults) { paginationDefaults = d }
+
 type paginationOptionsKeyType struct{}
 
 var paginationOptionsKey paginationOptionsKeyType
@@ -37,6 +48,9 @@ func WithPagination(ctx context.Context, limit, page uint32, total *uint64) cont
 	if page == 0 {
 		page = 1
 	}
+	if limit == 0 {
+		limit = paginationDefaults.DefaultLimit
+	}
 	return context.WithValue(ctx, paginationOptionsKey, PaginationOptions{
 		limit:  limit,
 		offset: (page - 1) * limit,

pkg/identityserver/storetest/api_key_store.go

@@ -267,3 +267,49 @@ func (st *StoreTest) TestAPIKeyStorePagination(t *T) {
 		}
 	})
 }
+
+// TestAPIKeyStorePaginationDefaults tests the default pagination values.
+func (st *StoreTest) TestAPIKeyStorePaginationDefaults(t *T) {
+	store.SetPaginationDefaults(store.PaginationDefaults{
+		DefaultLimit: 7,
+	})
+
+	app1 := st.population.NewApplication(nil)
+
+	var all []*ttnpb.APIKey
+	for i := 0; i < 15; i++ {
+		_, key := st.population.NewAPIKey(app1.GetEntityIdentifiers(), ttnpb.Right_RIGHT_APPLICATION_ALL)
+		key.Name = fmt.Sprintf("Key %d", i)
+		all = append(all, key)
+	}
+
+	sort.Sort(apiKeysByID(all))
+
+	s, ok := st.PrepareDB(t).(interface {
+		Store
+
+		is.APIKeyStore
+	})
+	defer st.DestroyDB(t, false)
+	if !ok {
+		t.Skip("Store does not implement APIKeyStore")
+	}
+	defer s.Close()
+
+	t.Run("FindAPIKeys_PageLimit", func(t *T) {
+		a, ctx := test.New(t)
+
+		var total uint64
+		paginateCtx := store.WithPagination(ctx, 0, 0, &total)
+		got, err := s.FindAPIKeys(paginateCtx, app1.GetEntityIdentifiers())
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+
+		paginateCtx = store.WithPagination(ctx, 0, 2, &total)
+		got, err = s.FindAPIKeys(paginateCtx, app1.GetEntityIdentifiers())
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+	})
+}

pkg/identityserver/storetest/application_store.go

@@ -360,3 +360,45 @@ func (st *StoreTest) TestApplicationStorePagination(t *T) {
 		}
 	})
 }
+
+// TestApplicationStorePaginationDefaults tests the default pagination values.
+func (st *StoreTest) TestApplicationStorePaginationDefaults(t *T) {
+	store.SetPaginationDefaults(store.PaginationDefaults{
+		DefaultLimit: 7,
+	})
+
+	usr1 := st.population.NewUser()
+
+	for i := 0; i < 15; i++ {
+		st.population.NewApplication(usr1.GetOrganizationOrUserIdentifiers())
+	}
+
+	s, ok := st.PrepareDB(t).(interface {
+		Store
+		is.ApplicationStore
+	})
+	defer st.DestroyDB(t, false)
+	if !ok {
+		t.Skip("Store does not implement ApplicationStore")
+	}
+	defer s.Close()
+
+	mask := ttnpb.ExcludeFields(fieldMask(ttnpb.ApplicationFieldPathsTopLevel...), "contact_info")
+
+	t.Run("FindApplications_PageLimit", func(t *T) {
+		a, ctx := test.New(t)
+
+		var total uint64
+		paginateCtx := store.WithPagination(ctx, 0, 0, &total)
+		got, err := s.FindApplications(paginateCtx, nil, mask)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+
+		paginateCtx = store.WithPagination(ctx, 0, 2, &total)
+		got, err = s.FindApplications(paginateCtx, nil, mask)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+	})
+}

pkg/identityserver/storetest/client_store.go

@@ -348,3 +348,45 @@ func (st *StoreTest) TestClientStorePagination(t *T) {
 		}
 	})
 }
+
+// TestClientStorePaginationDefaults tests the default pagination values.
+func (st *StoreTest) TestClientStorePaginationDefaults(t *T) {
+	store.SetPaginationDefaults(store.PaginationDefaults{
+		DefaultLimit: 7,
+	})
+
+	usr1 := st.population.NewUser()
+
+	for i := 0; i < 15; i++ {
+		st.population.NewClient(usr1.GetOrganizationOrUserIdentifiers())
+	}
+
+	s, ok := st.PrepareDB(t).(interface {
+		Store
+		is.ClientStore
+	})
+	defer st.DestroyDB(t, false)
+	if !ok {
+		t.Skip("Store does not implement ClientStore")
+	}
+	defer s.Close()
+
+	mask := ttnpb.ExcludeFields(fieldMask(ttnpb.ClientFieldPathsTopLevel...), "contact_info")
+
+	t.Run("FindClients_PageLimit", func(t *T) {
+		a, ctx := test.New(t)
+
+		var total uint64
+		paginateCtx := store.WithPagination(ctx, 0, 0, &total)
+		got, err := s.FindClients(paginateCtx, nil, mask)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+
+		paginateCtx = store.WithPagination(ctx, 0, 2, &total)
+		got, err = s.FindClients(paginateCtx, nil, mask)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+	})
+}

pkg/identityserver/storetest/invitation_store.go

@@ -251,3 +251,51 @@ func (st *StoreTest) TestInvitationStorePagination(t *T) {
 		}
 	})
 }
+
+// TestInvitationStorePaginationDefaults tests the default pagination values.
+func (st *StoreTest) TestInvitationStorePaginationDefaults(t *T) {
+	store.SetPaginationDefaults(store.PaginationDefaults{
+		DefaultLimit: 7,
+	})
+
+	a, ctx := test.New(t)
+	start := time.Now().Truncate(time.Second)
+
+	s, ok := st.PrepareDB(t).(interface {
+		Store
+		is.InvitationStore
+	})
+	defer st.DestroyDB(t, false)
+	if !ok {
+		t.Skip("Store does not implement InvitationStore")
+	}
+	defer s.Close()
+
+	for i := 0; i < 15; i++ {
+		_, err := s.CreateInvitation(ctx, &ttnpb.Invitation{
+			Email:     fmt.Sprintf("user%[email protected]", i+1),
+			Token:     fmt.Sprintf("TOKEN%d", i+1),
+			ExpiresAt: timestamppb.New(start.Add(time.Minute)),
+		})
+		if !a.So(err, should.BeNil) {
+			t.FailNow()
+		}
+	}
+
+	t.Run("FindInvitations_PageLimit", func(t *T) {
+		a, ctx := test.New(t)
+
+		var total uint64
+		paginateCtx := store.WithPagination(store.WithOrder(ctx, "email"), 0, 0, &total)
+		got, err := s.FindInvitations(paginateCtx)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+
+		paginateCtx = store.WithPagination(store.WithOrder(ctx, "email"), 0, 2, &total)
+		got, err = s.FindInvitations(paginateCtx)
+		if a.So(err, should.BeNil) && a.So(got, should.NotBeNil) {
+			a.So(got, should.HaveLength, 7)
+		}
+	})
+}