service: consolidate join grant-limit checks on the validate path (#5066)

* service: enforce participant name length on join

StartSession checked the token's metadata and attributes against the configured limits but not its name, so a participant could join with a name longer than limits.max_participant_name_length, which UpdateParticipant and the signal UpdateMetadata path reject.

* service: consolidate join grant-limit checks on the validate path

Extract checkGrantLimits and run it from ValidateConnectRequest and
WHIPService.validateCreate so the /rtc/validate endpoint and the WHIP
ingest path reject over-limit name, metadata and attributes the same way
a real join does. StartSession keeps its own checks as the backstop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* service: run the full join limit check in StartSession

StartSession now calls checkConnectLimits, so the RTC node checks
identity, room name, participant name, metadata and attributes itself.
ValidateConnectRequest runs the check after the publish suffix, so it
measures the same identity StartSession sees. checkConnectLimits returns
a plain error, and the HTTP callers map it to 400. The name-only
StartSession test is dropped for TestConnectLimits, which gains a
publish-suffix case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* service: test the StartSession limit check directly

TestStartSessionConnectLimits runs the same over-limit cases as
TestConnectLimits through StartSession on the RTC node. The cases move
into an exported ConnectLimitCases table so both test packages share it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: ANDROIDHASSAN <84630851+ANDROIDHASSAN@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

pkg/service/grantlimits_test.go

@@ -0,0 +1,138 @@
+// Copyright 2024 LiveKit, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package service
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/livekit/livekit-server/pkg/config"
+	"github.com/livekit/protocol/auth"
+	"github.com/livekit/protocol/livekit"
+	"github.com/livekit/protocol/logger"
+)
+
+// stubRoomAllocator is a RoomAllocator whose ValidateCreateRoom always succeeds. The embedded
+// interface is nil, so any other method panics, which keeps the tests honest about what the
+// join path actually calls.
+type stubRoomAllocator struct{ RoomAllocator }
+
+func (stubRoomAllocator) ValidateCreateRoom(context.Context, livekit.RoomName) error { return nil }
+
+func grantRequest(grant *auth.ClaimGrants) *http.Request {
+	r := httptest.NewRequest(http.MethodPost, "/", nil)
+	return r.WithContext(WithGrants(r.Context(), grant, "apikey"))
+}
+
+// ConnectLimitCase is a join token that is over one limit. Every join path must reject it.
+// It is exported so that TestStartSessionConnectLimits in package service_test runs the same cases.
+type ConnectLimitCase struct {
+	Name  string
+	Limit config.LimitConfig
+	Grant func() *auth.ClaimGrants // fresh grant per call, since a join path may change it
+	Err   error
+}
+
+func ConnectLimitCases() []ConnectLimitCase {
+	return []ConnectLimitCase{
+		{
+			Name:  "identity exceeds",
+			Limit: config.LimitConfig{MaxParticipantIdentityLength: 5},
+			Grant: grantWith(func(g *auth.ClaimGrants) { g.Identity = "toolongidentity" }),
+			Err:   ErrParticipantIdentityExceedsLimits,
+		},
+		{
+			Name:  "room name exceeds",
+			Limit: config.LimitConfig{MaxRoomNameLength: 3},
+			Grant: grantWith(func(g *auth.ClaimGrants) { g.Video.Room = "toolongroom" }),
+			Err:   ErrRoomNameExceedsLimits,
+		},
+		{
+			Name:  "name exceeds",
+			Limit: config.LimitConfig{MaxParticipantNameLength: 5},
+			Grant: grantWith(func(g *auth.ClaimGrants) { g.Name = "toolongname" }),
+			Err:   ErrNameExceedsLimits,
+		},
+		{
+			Name:  "metadata exceeds",
+			Limit: config.LimitConfig{MaxMetadataSize: 5},
+			Grant: grantWith(func(g *auth.ClaimGrants) { g.Metadata = "abcdefg" }),
+			Err:   ErrMetadataExceedsLimits,
+		},
+		{
+			Name:  "attributes exceeds",
+			Limit: config.LimitConfig{MaxAttributesSize: 5},
+			Grant: grantWith(func(g *auth.ClaimGrants) { g.Attributes = map[string]string{"k": "abcdefg"} }),
+			Err:   ErrAttributeExceedsLimits,
+		},
+	}
+}
+
+func baseGrant() *auth.ClaimGrants {
+	return &auth.ClaimGrants{
+		Identity: "id",
+		Name:     "name",
+		Video:    &auth.VideoGrant{RoomJoin: true, Room: "room"},
+	}
+}
+
+func grantWith(mutate func(*auth.ClaimGrants)) func() *auth.ClaimGrants {
+	return func() *auth.ClaimGrants {
+		g := baseGrant()
+		mutate(g)
+		return g
+	}
+}
+
+// TestConnectLimits checks that both HTTP join paths (WHIP ingest and the signal connection via
+// ValidateConnectRequest) reject every ConnectLimitCase.
+func TestConnectLimits(t *testing.T) {
+	for _, tc := range ConnectLimitCases() {
+		t.Run("WHIP/"+tc.Name, func(t *testing.T) {
+			svc := &WHIPService{config: &config.Config{Limit: tc.Limit}, roomAllocator: stubRoomAllocator{}}
+			_, code, err := svc.validateCreate(httptest.NewRecorder(), grantRequest(tc.Grant()))
+			require.ErrorIs(t, err, tc.Err)
+			require.Equal(t, http.StatusBadRequest, code)
+		})
+
+		t.Run("signal/"+tc.Name, func(t *testing.T) {
+			// nil router skips the node-limit check, reaching the grant checks
+			_, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(tc.Grant()), tc.Limit, ValidateConnectRequestParams{}, nil, stubRoomAllocator{})
+			require.ErrorIs(t, err, tc.Err)
+			require.Equal(t, http.StatusBadRequest, code)
+		})
+	}
+
+	t.Run("signal/identity exceeds after publish suffix", func(t *testing.T) {
+		g := baseGrant()
+		g.Video.SetCanPublish(true)
+		// "id" fits a limit of 5, but the final identity "id#abcd" does not
+		params := ValidateConnectRequestParams{publish: "abcd"}
+		_, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(g), config.LimitConfig{MaxParticipantIdentityLength: 5}, params, nil, stubRoomAllocator{})
+		require.ErrorIs(t, err, ErrParticipantIdentityExceedsLimits)
+		require.Equal(t, http.StatusBadRequest, code)
+	})
+
+	t.Run("within limits", func(t *testing.T) {
+		limit := config.LimitConfig{MaxParticipantIdentityLength: 5, MaxRoomNameLength: 10, MaxParticipantNameLength: 5, MaxMetadataSize: 5, MaxAttributesSize: 5}
+		_, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(baseGrant()), limit, ValidateConnectRequestParams{}, nil, stubRoomAllocator{})
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, code)
+	})
+}

pkg/service/roommanager.go

@@ -297,11 +297,8 @@ func (r *RoomManager) StartSession(
 	sessionStartTime := time.Now()
 
 	if pi.Identity != "" && pi.Grants != nil {
-		if !r.config.Limit.CheckMetadataSize(pi.Grants.Metadata) {
-			return ErrMetadataExceedsLimits
-		}
-		if !r.config.Limit.CheckAttributesSize(pi.Grants.Attributes) {
-			return ErrAttributeExceedsLimits
+		if err := checkConnectLimits(r.config.Limit, string(pi.Identity), livekit.RoomName(pi.CreateRoom.GetName()), pi.Grants); err != nil {
+			return err
 		}
 	}
 

pkg/service/roommanager_test.go

@@ -169,6 +169,28 @@ func TestRoomCompositeEgressOnCreateRoom(t *testing.T) {
 	})
 }
 
+// TestStartSessionConnectLimits checks that StartSession on the RTC node rejects the same
+// over-limit joins as the HTTP join paths, without relying on the caller's validation.
+func TestStartSessionConnectLimits(t *testing.T) {
+	for _, tc := range service.ConnectLimitCases() {
+		t.Run(tc.Name, func(t *testing.T) {
+			conf, err := config.NewConfig("", true, nil, nil)
+			require.NoError(t, err)
+			conf.Limit = tc.Limit
+			rm := newTestRoomManager(t, conf, nil)
+
+			g := tc.Grant()
+			err = rm.StartSession(context.Background(), routing.ParticipantInit{
+				Identity:   livekit.ParticipantIdentity(g.Identity),
+				Grants:     g,
+				Client:     &livekit.ClientInfo{},
+				CreateRoom: &livekit.CreateRoomRequest{Name: g.Video.Room},
+			}, &routingfakes.FakeMessageSource{}, &routingfakes.FakeMessageSink{}, false)
+			require.ErrorIs(t, err, tc.Err)
+		})
+	}
+}
+
 const roomPresetConfig = `
 room:
   room_configurations:

pkg/service/utils.go

@@ -366,9 +366,6 @@ func ValidateConnectRequest(
 	if claims.Identity == "" {
 		return res, http.StatusBadRequest, ErrIdentityEmpty
 	}
-	if !limitConfig.CheckParticipantIdentityLength(claims.Identity) {
-		return res, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, limitConfig.MaxParticipantIdentityLength)
-	}
 
 	if claims.RoomConfig != nil {
 		if err := claims.RoomConfig.CheckCredentials(); err != nil {
@@ -384,8 +381,30 @@ func ValidateConnectRequest(
 	if res.roomName == "" {
 		return res, http.StatusBadRequest, ErrNoRoomName
 	}
-	if !limitConfig.CheckRoomNameLength(string(res.roomName)) {
-		return res, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, limitConfig.MaxRoomNameLength)
+
+	if len(params.metadata) != 0 {
+		// Make sure grant has GetCanUpdateOwnMetadata set
+		if !claims.Video.GetCanUpdateOwnMetadata() {
+			return res, http.StatusUnauthorized, rtc.ErrPermissionDenied
+		}
+		claims.Metadata = params.metadata
+	}
+
+	// Add extra attributes to the participant
+	if len(params.attributes) != 0 {
+		// Make sure grant has GetCanUpdateOwnMetadata set
+		if !claims.Video.GetCanUpdateOwnMetadata() {
+			return res, http.StatusUnauthorized, rtc.ErrPermissionDenied
+		}
+		if claims.Attributes == nil {
+			claims.Attributes = make(map[string]string, len(params.attributes))
+		}
+		for k, v := range params.attributes {
+			if v == "" {
+				continue // do not allow deleting existing attributes
+			}
+			claims.Attributes[k] = v
+		}
 	}
 
 	// this is new connection for existing participant -  with publish only permissions
@@ -399,6 +418,11 @@ func ValidateConnectRequest(
 		claims.Identity += "#" + params.publish
 	}
 
+	// after the publish suffix, so the identity checked is the one StartSession sees
+	if err := checkConnectLimits(limitConfig, claims.Identity, res.roomName, claims); err != nil {
+		return res, http.StatusBadRequest, err
+	}
+
 	// room allocator validations
 	err = roomAllocator.ValidateCreateRoom(r.Context(), res.roomName)
 	if err != nil {
@@ -425,36 +449,33 @@ func ValidateConnectRequest(
 	SetRoomConfiguration(createRequest, claims.GetRoomConfiguration())
 	res.createRoomRequest = createRequest
 
-	if len(params.metadata) != 0 {
-		// Make sure grant has GetCanUpdateOwnMetadata set
-		if !claims.Video.GetCanUpdateOwnMetadata() {
-			return res, http.StatusUnauthorized, rtc.ErrPermissionDenied
-		}
-		claims.Metadata = params.metadata
-	}
-
-	// Add extra attributes to the participant
-	if len(params.attributes) != 0 {
-		// Make sure grant has GetCanUpdateOwnMetadata set
-		if !claims.Video.GetCanUpdateOwnMetadata() {
-			return res, http.StatusUnauthorized, rtc.ErrPermissionDenied
-		}
-		if claims.Attributes == nil {
-			claims.Attributes = make(map[string]string, len(params.attributes))
-		}
-		for k, v := range params.attributes {
-			if v == "" {
-				continue // do not allow deleting existing attributes
-			}
-			claims.Attributes[k] = v
-		}
-	}
-
 	res.grants = claims
 	res.tokenExpiresAt = GetTokenExpiresAt(r.Context())
 	return res, http.StatusOK, nil
 }
 
+// checkConnectLimits checks a join's identity, room name, participant name, metadata and
+// attributes against the configured limits. Every join path runs it: the signal connection,
+// WHIP, and StartSession on the RTC node, which does not trust the caller's validation.
+func checkConnectLimits(limitConfig config.LimitConfig, identity string, roomName livekit.RoomName, grants *auth.ClaimGrants) error {
+	if !limitConfig.CheckParticipantIdentityLength(identity) {
+		return fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, limitConfig.MaxParticipantIdentityLength)
+	}
+	if !limitConfig.CheckRoomNameLength(string(roomName)) {
+		return fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, limitConfig.MaxRoomNameLength)
+	}
+	if !limitConfig.CheckParticipantNameLength(grants.Name) {
+		return fmt.Errorf("%w: max length %d", ErrNameExceedsLimits, limitConfig.MaxParticipantNameLength)
+	}
+	if !limitConfig.CheckMetadataSize(grants.Metadata) {
+		return fmt.Errorf("%w: max size %d", ErrMetadataExceedsLimits, limitConfig.MaxMetadataSize)
+	}
+	if !limitConfig.CheckAttributesSize(grants.Attributes) {
+		return fmt.Errorf("%w: max size %d", ErrAttributeExceedsLimits, limitConfig.MaxAttributesSize)
+	}
+	return nil
+}
+
 func IsRTCPath(path string) bool {
 	return path == "/rtc" || path == "/rtc/v1"
 }

pkg/service/whipservice.go

@@ -131,15 +131,12 @@ func (s *WHIPService) validateCreate(w http.ResponseWriter, r *http.Request) (*c
 	if roomName == "" {
 		return nil, http.StatusUnauthorized, errors.New("room name cannot be empty")
 	}
-	if !s.config.Limit.CheckRoomNameLength(string(roomName)) {
-		return nil, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, s.config.Limit.MaxRoomNameLength)
-	}
-
 	if claims.Identity == "" {
 		return nil, http.StatusBadRequest, ErrIdentityEmpty
 	}
-	if !s.config.Limit.CheckParticipantIdentityLength(claims.Identity) {
-		return nil, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, s.config.Limit.MaxParticipantIdentityLength)
+
+	if err := checkConnectLimits(s.config.Limit, claims.Identity, roomName, claims); err != nil {
+		return nil, http.StatusBadRequest, err
 	}
 
 	if err := s.roomAllocator.ValidateCreateRoom(r.Context(), roomName); err != nil {