pgsql-jellyfin/docs/syncplay-authorization-error-handling.md

SyncPlay Authorization Error Handling Improvement

Problem

When unauthenticated users accessed SyncPlay endpoints, the application threw an unhandled exception:

System.ArgumentException: Guid can't be empty (Parameter 'id')
at UserManager.GetUserById(Guid id)

This made it appear as a bug to end users, when it's actually expected behavior (authentication required).

Solution

Added proper validation and user-friendly logging to SyncPlayAccessHandler.cs:

Changes Made

1. Added Logger Dependency

   • Injected ILogger<SyncPlayAccessHandler> into the constructor
   • Enables proper logging of authorization failures

2. Early Validation

   • Check for empty GUID before calling GetUserById()
   • Prevents exception from being thrown for expected case

3. User-Friendly Messages

   • Empty user ID: "SyncPlay access denied: User authentication required. Unable to process request with empty user ID"
   • User not found: "SyncPlay access denied: User with ID {UserId} not found"

Before

var userId = context.User.GetUserId();
var user = _userManager.GetUserById(userId);  // ← Throws exception if userId is empty

Log Output:

[ERR] System.ArgumentException: Guid can't be empty (Parameter 'id')

After

var userId = context.User.GetUserId();

// Check if user is authenticated
if (userId.Equals(Guid.Empty))
{
    _logger.LogWarning("SyncPlay access denied: User authentication required. Unable to process request with empty user ID");
    return Task.CompletedTask;  // Results in 403 Forbidden
}

var user = _userManager.GetUserById(userId);
if (user is null)
{
    _logger.LogWarning("SyncPlay access denied: User with ID {UserId} not found", userId);
    throw new ResourceNotFoundException();
}

Log Output:

[WRN] SyncPlay access denied: User authentication required. Unable to process request with empty user ID

Impact

User Experience

Before:

• ❌ Scary error message suggesting a bug
• ❌ Stack trace in logs
• ❌ HTTP 500 Internal Server Error

After:

• ✅ Clear explanation of why access was denied
• ✅ Clean warning message in logs
• ✅ HTTP 403 Forbidden (correct status code)

Administrator Experience

Before:

[ERR] Error processing request. URL GET /SyncPlay/List.
System.ArgumentException: Guid can't be empty (Parameter 'id')
   at UserManager.GetUserById(Guid id)
   [... long stack trace ...]

Looks like a bug that needs fixing

After:

[WRN] SyncPlay access denied: User authentication required. Unable to process request with empty user ID

Clear, expected behavior - user needs to log in

Testing

To verify the fix:

1. Unauthenticated Request:

curl http://localhost:8096/SyncPlay/List

Expected:

• HTTP 403 Forbidden
• Log: [WRN] SyncPlay access denied: User authentication required

2. Invalid User ID:

curl -H "Authorization: MediaBrowser Token=invalid_token" http://localhost:8096/SyncPlay/List

Expected:

• HTTP 404 Not Found
• Log: [WRN] SyncPlay access denied: User with ID {guid} not found

3. Valid Authenticated Request:

curl -H "Authorization: MediaBrowser Token=valid_token" http://localhost:8096/SyncPlay/List

Expected:

• HTTP 200 OK (if user has permissions)
• No error logs

Related Files

• Jellyfin.Api/Auth/SyncPlayAccessPolicy/SyncPlayAccessHandler.cs
  • Added logger dependency
  • Added validation for empty user ID
  • Added user-friendly log messages

Code Style Notes

Guid Comparison

The project has a rule against using Guid.Empty with == operator:

// ❌ Banned
if (userId == Guid.Empty)

// ✅ Correct
if (userId.Equals(Guid.Empty))

This is enforced by analyzer rule RS0030.

Benefits

1. Better Error Messages - Clear explanation of why access was denied
2. Appropriate HTTP Status - 403 Forbidden instead of 500 Internal Server Error
3. Less Noise - Warning instead of error for expected behavior
4. Better Logging - Helps distinguish real bugs from authentication issues
5. User-Friendly - Administrators can quickly understand what happened

Future Considerations

This pattern could be applied to other authorization handlers that might have similar issues with empty user IDs or invalid authentication.

Example Authorization Handlers to Review:

• DefaultAuthorizationHandler
• DownloadPolicy handlers
• FirstTimeSetupOrElevatedPolicy handlers
• LocalAccessPolicy handlers

Consider creating a base authorization handler class that includes this validation pattern.