Skip to content

Add WebSocket with Authentication Middleware example (Fixes issue#1699) #ieeesoc #1715

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

TJ456
Copy link

@TJ456 TJ456 commented May 15, 2025

This PR addresses issue #1699 by adding a new example that demonstrates how to use WebSocket connections with authentication middleware in GoFr.

The example shows:

  • How to set up Basic Authentication for a GoFr application
  • How to use WebSockets with authenticated connections
  • How to extract username from authentication credentials
  • How to track active WebSocket connections
  • How to handle messages from authenticated clients

This example will help developers implement secure WebSocket applications with proper authentication.

@TJ456 TJ456 changed the title Add WebSocket with Authentication Middleware example (Fixes issue#1699) Add WebSocket with Authentication Middleware example (Fixes issue#1699) #ieeesoc May 15, 2025
Copy link

@pankaj-bind pankaj-bind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @TJ456

To align this implementation with the issue’s requirements and GoFr’s contribution guidelines, please make the following changes. Each resolution includes the reason for the change to clarify its necessity.

1. Switch to API Key Authentication

  • Change: Replace app.EnableBasicAuthWithValidator with middleware.APIKeyAuth to use the required API key authentication.
    • Reason: The issue explicitly requires API key authentication using gofr.dev/pkg/gofr/http/middleware/apikey_auth with an X-API-KEY header. Using Basic Authentication deviates from the core requirement, making the implementation incorrect.
  • Change: Remove the validateCredentials and extractUsernameFromAuthHeader functions, as they are specific to Basic Authentication.
    • Reason: These functions are tailored to Basic Authentication and extract usernames, which is irrelevant for API key authentication, as API keys typically don’t encode user identities.
  • Change: Load the API key from the .env file using app.Config.Get("API_KEY").
    • Reason: GoFr examples (e.g., using-web-socket) use environment variables for configuration, and the issue requires storing the API key in configs/.env for consistency and security.
  • Change: Replace username-based tracking with connection IDs (e.g., conn-123), as API keys do not provide user identities.
    • Reason: The issue’s chat-like functionality doesn’t require user-specific identifiers. Connection IDs maintain simplicity and align with WebSocket connection tracking without assuming user data.

2. Fix Directory Name

  • Change: Rename the directory from examples/using-web-socket-with-auth/ to examples/using-websocket-auth/ to match GoFr’s naming conventions.
    • Reason: GoFr’s .ls-lint.yml enforces consistent naming (e.g., using-web-socket, using-http-service). The extra hyphens in using-web-socket-with-auth will cause CI failures, and the issue specifies using-websocket-auth.

3. Add .env File

  • Change: Create a configs/.env file in the example directory with the following content:
    API_KEY=secret-key
    HTTP_PORT=8000
    • Reason: The issue requires a configs/.env file to store the API_KEY, following GoFr’s convention for configuration (e.g., using-web-socket/configs/.env). Its absence breaks the example’s setup and violates the issue’s requirements.

4. Improve Tests

  • Change: Update test cases to use X-API-KEY headers, e.g., header.Add("X-API-KEY", "secret-key"), instead of Basic Authentication credentials.
    • Reason: Tests must reflect the issue’s API key authentication requirement. Current tests are written for Basic Authentication, making them irrelevant to the intended functionality.
  • Change: Strengthen Test_UsersEndpoint by adding assertions for ActiveUsers and Count fields in the response. Alternatively, remove the /users endpoint if it’s not required by the issue.
    • Reason: The current Test_UsersEndpoint only logs results without verifying the response, reducing test effectiveness. Assertions ensure the endpoint works as expected, and removing it may be appropriate since the issue doesn’t require it.
  • Change: Replace time.Sleep(100 * time.Millisecond) with a polling mechanism to check server readiness, such as retrying HTTP requests to /health-check.
    • Reason: time.Sleep is fragile, as server startup time varies. A polling mechanism (e.g., checking /health-check) ensures tests wait reliably, improving robustness, as seen in GoFr’s testing practices.
  • Change: Add a test case for concurrent WebSocket connections to ensure thread safety.
    • Reason: The issue implies a production-ready example, and WebSocket servers must handle multiple clients. Testing concurrent connections verifies thread safety, aligning with GoFr’s websocket.New() usage.

5. Update README

  • Change: Rewrite the README to describe API key authentication using the X-API-KEY header.
    • Reason: The README currently describes Basic Authentication, which misaligns with the issue’s requirements and confuses users expecting API key instructions.
  • Change: Update testing instructions to reflect API key usage, e.g.:
    wscat -c ws://localhost:8000/ws -H "X-API-KEY: secret-key"
    • Reason: Testing instructions must match the authentication method (API key) to guide users correctly, ensuring they can replicate the example.
  • Change: Clarify that browser WebSocket clients can pass API keys via query parameters (e.g., ws://localhost:8000/ws?api_key=secret-key) or subprotocols, correcting the misleading note about header limitations.
    • Reason: The current README incorrectly states that browser WebSocket clients can’t set headers. Query parameters or subprotocols are common workarounds, and correcting this improves accuracy.
  • Change: Remove all references to Basic Authentication.
    • Reason: Basic Authentication is irrelevant to the issue and including it in the README causes confusion.

6. Add go.mod File

  • Change: Create a go.mod file in the example directory, specifying only the necessary dependencies, e.g.:
    module github.com/gofr-dev/examples/using-websocket-auth
    
    go 1.24
    
    require (
        gofr.dev/pkg/gofr v0.24.0
        github.com/gorilla/websocket v1.5.3
        github.com/stretchr/testify v1.9.0
    )
    • Reason: GoFr examples typically include a go.mod file to define dependencies explicitly. The provided go.work.sum is from a workspace and includes irrelevant dependencies (e.g., cloud.google.com/go/*), which could cause confusion or bloat.
  • Change: Ensure unrelated dependencies from go.work.sum (e.g., cloud.google.com/go/*) are excluded.
    • Reason: Including unrelated dependencies suggests they are required for the example, which is misleading and violates GoFr’s practice of minimal dependencies.

7. Ensure Compliance

  • Change: Run goimports -w . to format the code according to GoFr’s standards.
    • Reason: GoFr’s CONTRIBUTING.md requires goimports for consistent formatting, ensuring the code passes CI checks.
  • Change: Run golangci-lint run to verify that the code passes all linting checks (e.g., errcheck, staticcheck).
    • Reason: golangci-lint is mandated by GoFr’s CI to catch issues like unhandled errors or dead code, ensuring high code quality.
  • Change: Run go test -cover to confirm test coverage remains high and no coverage is lost.
    • Reason: GoFr requires new code to be fully tested without reducing overall coverage, as per CONTRIBUTING.md.
  • Change: Verify that the directory name (using-websocket-auth) complies with .ls-lint.yml to pass CI checks.
    • Reason: .ls-lint.yml enforces consistent naming, and the current using-web-socket-with-auth will fail CI, blocking the PR.

@coolwednesday @Umang01-hash Please let me know if I’ve missed anything or if there are additional points to address.
@TJ456 I’ve provided a detailed description of the suggested changes. Feel free to proceed with the updates or reach out if you need further clarification.

@TJ456
Copy link
Author

TJ456 commented May 20, 2025

working on the
points

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