Skip to content

Feat/variables #279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Conversation

spinecow
Copy link

Context

This PR enhances the authentication system by adding comprehensive logging to help diagnose authentication issues. The primary goal is to provide better visibility into the credential parsing and authentication process without changing the core functionality. This will help administrators quickly identify and resolve authentication problems in production environments.

The PR also maintains full compatibility with the new JSON format for credentials that was recently implemented, ensuring that existing deployments continue to work correctly.

Choices

I solved this by adding detailed logging at key points in the authentication flow:

  1. Credential Loading: Added logging when credentials are loaded from the environment variable, including the number of credentials parsed and details about each credential (without exposing sensitive information).

  2. Authentication Attempts: Added logging for each authentication attempt, including the username (but not password) and the result of the authentication.

  3. Authorization Results: Added logging for successful authorizations and various failure modes (missing credentials, invalid username, invalid password, expired credentials).

  4. File Serving: Added logging when the M3U file is served, which helps confirm successful authentication.

This approach was chosen because:

  • It provides comprehensive visibility into the authentication process without changing the core logic
  • It maintains security by not logging sensitive information like passwords
  • It helps administrators quickly diagnose authentication issues in production
  • It has minimal performance impact as the logging is only enabled when DEBUG is true
  • It maintains full backward compatibility with existing deployments

Test instructions

  1. Environment Setup:

    • Set up the application with the new JSON format for credentials:
      CREDENTIALS='[{"username":"user1","password":"pass1","expiration":"2026-02-01T00:00:00Z"},{"username":"user2","password":"pass2"}]'
    • Enable debug logging with DEBUG=true
  2. Authentication Testing:

    • Test with valid credentials: curl "http://localhost:PORT/playlist.m3u?username=user2&password=pass2"
    • Test with invalid credentials: curl "http://localhost:PORT/playlist.m3u?username=user2&password=wrongpass"
    • Test with missing credentials: curl "http://localhost:PORT/playlist.m3u"
    • Test with expired credentials: curl "http://localhost:PORT/playlist.m3u?username=user1&password=pass1" (if the expiration date has passed)
  3. Log Verification:

    • Check that the logs show credential loading with the correct number of credentials
    • Verify that authentication attempts are logged with appropriate information
    • Confirm that successful authentications and failures are clearly distinguishable in the logs
    • Ensure that sensitive information like passwords is not exposed in the logs

Checklist before requesting a review

  • I have performed a self-review of my code
  • I've added documentation about this change to the README (the README already had documentation about the JSON format)
  • I've not introduced breaking changes (this change is backward compatible)

GittyUp-beep and others added 7 commits July 25, 2025 18:52
- Refactor DataPath and TempPath to be read from DATA_PATH and TEMP_PATH environment variables respectively.
- Add sensible fallbacks to ./data and ./temp if environment variables are not set.
- Update config package to handle environment variable loading.
- Add a new Credential struct with Username, Password, and Expiration fields.
- Update parseCredentials to parse the CREDENTIALS environment variable as a JSON array of Credential objects.
- The authentication logic now uses the new struct, but is not yet fully updated.
- Update M3UHTTPHandler to parse and cache credentials at startup.
- Implement constant-time comparison for passwords to prevent timing attacks.
- Add check for credential expiration.
- The authentication logic now uses the in-memory cache.
- Update m3u_http_test.go to use JSON for credentials.
- Add test cases for credential expiration.
- Ensure existing tests pass with the new credential format.
- Update the  environment variable description to reflect the new JSON array format.
- Add  and  environment variables to the documentation.
…unctions

- Add comprehensive logging to M3UHTTPHandler for better debugging
- Log credential loading, authentication attempts, and authorization results
- Include information about parsed credentials without exposing sensitive data
- Add logging for file serving operations
@sonroyaalmerol
Copy link
Owner

Thanks for this! Do you mind resolving the failing test cases for me? I'll see if I can merge this over the weekend.

@spinecow spinecow closed this by deleting the head repository Jul 29, 2025
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.

3 participants