Skip to content

SP4: Server-derived member_id for SyncPlay group operations#342

Merged
detain merged 1 commit into
masterfrom
fix/server-sp4-ws-jwt-auth
Jun 29, 2026
Merged

SP4: Server-derived member_id for SyncPlay group operations#342
detain merged 1 commit into
masterfrom
fix/server-sp4-ws-jwt-auth

Conversation

@detain

@detain detain commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Implements SP4 specification requirement that member_id is server-derived from the authenticated connection's userId, not from the client wire payload.

Changes

Core SP4 Logic

  1. handleGroupCreate(): Now uses $connection->getUserId() as the member_id for the creating member
  2. handleGroupJoin(): Now uses $connection->getUserId() as the member_id for the joining member
  3. Host authorization (handlePlaybackPlay, handlePlaybackPause, handlePlaybackSeek, handlePlaybackQueue): Now checks $connection->getUserId() === $group['host_member_id'] instead of trusting client-supplied member_id in payload
  4. Authentication guards: Added NOT_AUTHENTICATED error when connection's getUserId() returns null

GroupState::getState()

Returns members as an associative array keyed by member_id (instead of a numeric list) to support server-derived member_id lookup.

Test Infrastructure

  • Extracted TestConnection class to separate PSR-12 compliant file tests/Unit/Server/WebSocket/TestConnection.php
  • WsAuthenticationTest.php updated to use the extracted class

Security Fix

This prevents spoofed member_id attacks where a malicious client claims to be a different member. The server now always uses the authenticated connection's userId for authorization decisions, ensuring only the actual group host can control playback.

Test Results

✔ testUnauthenticatedConnectionCannotCreateGroup
✔ testUnauthenticatedConnectionCannotJoinGroup  
✔ testUnauthenticatedConnectionCannotControlPlayback
✔ testServerDerivedMemberIdIsUsedInsteadOfClientSupplied
✔ testHostAuthorizationUsesServerDerivedMemberId
✔ testJoinGroupUsesServerDerivedMemberId

PHPStan level 9: ✅ PASS
PHPCS PSR-12: ✅ PASS

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codacy-production

codacy-production Bot commented Jun 29, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 high · 19 medium · 79 minor

Alerts:
⚠ 100 issues (≤ 0 issues of at least minor severity)

Results:
100 new issues

Category Results
UnusedCode 1 medium
BestPractice 5 medium
Documentation 47 minor
ErrorProne 1 medium
Security 2 high
CodeStyle 30 minor
Complexity 12 medium
Comprehensibility 2 minor

View in Codacy

🟢 Metrics 64 complexity · 3 duplication

Metric Results
Complexity 64
Duplication 3

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

This implements SP4 spec requirement that member_id is server-derived from
the authenticated connection's userId, not from the client wire payload.

Changes:
- handleGroupCreate(): Use $connection->getUserId() as member_id
- handleGroupJoin(): Use $connection->getUserId() as member_id
- handlePlaybackPlay/Pause/Seek/Queue(): Use $connection->getUserId()
  for host authorization (check if connection's userId is the host)
- Added NOT_AUTHENTICATED error when connection has no userId
- GroupState::getState() now returns members as keyed dict (not list)
- Extracted TestConnection to separate PSR-12 compliant file

The host authorization fix prevents spoofed member_id attacks where
a client claims to be a different member. The server now always uses
the authenticated connection's userId for authorization decisions.
@detain detain force-pushed the fix/server-sp4-ws-jwt-auth branch from bbb320b to 1271134 Compare June 29, 2026 17:47
@detain detain merged commit e6c05d1 into master Jun 29, 2026
11 of 14 checks passed
@detain detain deleted the fix/server-sp4-ws-jwt-auth branch June 29, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant