Restrict multiple devices per Nakama account#169
Restrict multiple devices per Nakama account#169Germ-99 wants to merge 1 commit intoEchoTools:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements restrictions on multiple device connections per Nakama account to prevent users from simultaneously joining matches as players from different devices while allowing spectator access.
Key Changes:
- Added
CountSessionsForUser()method to track active sessions per user - Implemented restrictions for secondary connections: spectator-only access for matches, no session creation, and no social lobby access
- Added new error code
SecondaryConnectionRestrictedfor handling violations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/session_registry.go | Added CountSessionsForUser() interface method and implementation to count active sessions per user |
| server/match_common_test.go | Added mock implementation of CountSessionsForUser() for test registry |
| server/evr_lobby_session.go | Added multi-session checks to restrict secondary connections to spectator role and prevent session creation |
| server/evr_lobby_join.go | Added check to prevent secondary connections from accessing social lobbies |
| server/evr_lobby_find.go | Added check to prevent secondary connections from finding social lobbies |
| server/evr_lobby_errors.go | Added new error code SecondaryConnectionRestricted for multi-session violations |
| // Check if this is a secondary connection and restrict it | ||
| sessionCount := p.sessionRegistry.CountSessionsForUser(session.UserID()) | ||
| if sessionCount > 1 { | ||
| // This user has multiple sessions open | ||
| switch in.(type) { | ||
| case *evr.LobbyFindSessionRequest: | ||
| // For find requests, only allow spectator role | ||
| if lobbyParams.Role != evr.TeamSpectator { | ||
| logger.Warn("Secondary connection attempted to join as player", | ||
| zap.String("user_id", session.UserID().String()), | ||
| zap.String("session_id", session.ID().String()), | ||
| zap.String("role", TeamIndex(lobbyParams.Role).String())) | ||
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections can only spectate") | ||
| } | ||
|
|
||
| case *evr.LobbyJoinSessionRequest: | ||
| // Join requests also need spectator check if non-spectator role requested | ||
| if lobbyParams.Role != evr.TeamSpectator { | ||
| logger.Warn("Secondary connection attempted to join as player", | ||
| zap.String("user_id", session.UserID().String()), | ||
| zap.String("session_id", session.ID().String()), | ||
| zap.String("role", TeamIndex(lobbyParams.Role).String())) | ||
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections can only spectate") | ||
| } | ||
|
|
||
| case *evr.LobbyCreateSessionRequest: | ||
| // Secondary connections cannot create sessions | ||
| logger.Warn("Secondary connection attempted to create session", | ||
| zap.String("user_id", session.UserID().String()), | ||
| zap.String("session_id", session.ID().String())) | ||
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections cannot create sessions") | ||
| } | ||
| } |
There was a problem hiding this comment.
The new multi-session restriction logic introduced in this function lacks test coverage. Consider adding unit tests to verify:
- Secondary connections (sessionCount > 1) are restricted to spectator role for LobbyFindSessionRequest
- Secondary connections are restricted to spectator role for LobbyJoinSessionRequest
- Secondary connections cannot create sessions (LobbyCreateSessionRequest)
- Primary connections (sessionCount == 1) are not affected by these restrictions
Tests should be added to an appropriate EVR test file (e.g., evr_lobby_session_test.go).
| // Check if this is a secondary connection and prevent social lobby access | ||
| sessionCount := p.sessionRegistry.CountSessionsForUser(session.UserID()) | ||
| if sessionCount > 1 && (label.Mode == evr.ModeSocialPublic || label.Mode == evr.ModeSocialPrivate) { | ||
| logger.Warn("Secondary connection attempted to join social lobby", | ||
| zap.String("user_id", session.UserID().String()), | ||
| zap.String("session_id", session.ID().String()), | ||
| zap.String("mode", label.Mode.String())) | ||
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections cannot access social lobbies") | ||
| } |
There was a problem hiding this comment.
The social lobby restriction for secondary connections lacks test coverage. Consider adding unit tests to verify:
- Secondary connections are blocked from joining social lobbies (both ModeSocialPublic and ModeSocialPrivate)
- Primary connections can join social lobbies normally
- The correct error is returned when restrictions apply
Tests should be added to an appropriate EVR test file (e.g., evr_lobby_join_test.go).
| // Check if this is a secondary connection and prevent social lobby access | ||
| sessionCount := p.sessionRegistry.CountSessionsForUser(session.UserID()) | ||
| if sessionCount > 1 && lobbyParams.Mode == evr.ModeSocialPublic { | ||
| logger.Warn("Secondary connection attempted to access social lobby", | ||
| zap.String("user_id", session.UserID().String()), | ||
| zap.String("session_id", session.ID().String())) | ||
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections cannot access social lobbies") | ||
| } |
There was a problem hiding this comment.
The social lobby restriction for secondary connections lacks test coverage. Consider adding unit tests to verify:
- Secondary connections are blocked from finding social lobbies
- Primary connections can find social lobbies normally
- The correct error is returned when restrictions apply
Tests should be added to an appropriate EVR test file (e.g., evr_lobby_find_test.go).
|
|
||
| // Check if this is a secondary connection and prevent social lobby access | ||
| sessionCount := p.sessionRegistry.CountSessionsForUser(session.UserID()) | ||
| if sessionCount > 1 && lobbyParams.Mode == evr.ModeSocialPublic { |
There was a problem hiding this comment.
Inconsistent restriction logic: In evr_lobby_join.go (line 38), the check includes both ModeSocialPublic and ModeSocialPrivate, but here only ModeSocialPublic is checked. Secondary connections should be restricted from ModeSocialPrivate lobbies as well for consistency.
Consider changing this to:
if sessionCount > 1 && (lobbyParams.Mode == evr.ModeSocialPublic || lobbyParams.Mode == evr.ModeSocialPrivate) {| if sessionCount > 1 && lobbyParams.Mode == evr.ModeSocialPublic { | |
| if sessionCount > 1 && (lobbyParams.Mode == evr.ModeSocialPublic || lobbyParams.Mode == evr.ModeSocialPrivate) { |
| var err error | ||
| var matchID MatchID | ||
|
|
||
| // Check if this is a secondary connection and restrict it |
There was a problem hiding this comment.
Trailing whitespace at end of line
| // Check if this is a secondary connection and restrict it | |
| // Check if this is a secondary connection and restrict it |
| func (r *LocalSessionRegistry) CountSessionsForUser(userID uuid.UUID) int { | ||
| count := 0 | ||
| r.sessions.Range(func(id uuid.UUID, session Session) bool { | ||
| if session.UserID() == userID { | ||
| count++ | ||
| } | ||
| return true | ||
| }) | ||
| return count | ||
| } |
There was a problem hiding this comment.
The CountSessionsForUser function iterates through all active sessions on every call. This could become a performance bottleneck with many concurrent sessions, especially since this method is called for every lobby request.
Consider:
- Maintaining a separate index map from userID to session count that's updated in
Add()andRemove()methods for O(1) lookups - Or caching the result with appropriate invalidation when sessions are added/removed
thesprockee
left a comment
There was a problem hiding this comment.
@copilot implement changes based on the review. use the stream count for the user.
use something akin to nk.StreamCount(StreamModeService, <user_id>, "", StreamLabelMatchService) to see if the user has any other match connections active.
| SuspendedFromLobbyGroup | ||
| KickedFromLobbyGroup | ||
| NotALobbyGroupMod | ||
| SecondaryConnectionRestricted |
There was a problem hiding this comment.
Do not add to the LobbyErrorCodeValue's, as they are hard coded in the EVR client.
| SuspendedFromLobbyGroup: "suspended_from_lobby_group", | ||
| KickedFromLobbyGroup: "kicked_from_lobby_group", | ||
| NotALobbyGroupMod: "not_a_lobby_group_mod", | ||
| SecondaryConnectionRestricted: "secondary_connection_restricted", |
There was a problem hiding this comment.
Do not add to the LobbyErrorCodeValue's, as they are hard coded in the EVR client.
There was a problem hiding this comment.
this is an "upstream" file, do not modify it.
There was a problem hiding this comment.
this is an "upstream" file, do not modify it.
| return NewLobbyError(SecondaryConnectionRestricted, "secondary connections can only spectate") | ||
| } | ||
|
|
||
| case *evr.LobbyCreateSessionRequest: |
There was a problem hiding this comment.
allow secondary connections to create sessions, as long as they are spectators.
|
|
||
| startTime := time.Now() | ||
|
|
||
| // Check if this is a secondary connection and prevent social lobby access |
| lobbyParams.GroupID = label.GetGroupID() | ||
| lobbyParams.Mode = label.Mode | ||
|
|
||
| // Check if this is a secondary connection and prevent social lobby access |
Fixes: #168