Change from username/password to token authentication #326
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR replaces basic username/password authentication with token-based authentication across the client, tests, and example, and updates helper logic to handle query-based fixture filenames.
- Replace all username/password parameters with a single
tokenparameter - Refactor
ApiClientto send a Bearer token and adjust the session/subscriber flows - Update tests, fixtures loader, example script, and devcontainer config for token usage
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_subscription.py | Updated expected error messages to match the new generic messages |
| tests/test_client.py | Swapped username/password for token in client init |
| tests/conftest.py | Replaced fixture credentials with token |
| tests/common.py | Made load_response strip query strings before loading JSON |
| pytraccar/client.py | Removed BasicAuth, added Bearer token header, adjusted session calls and WS subscription |
| example.py | Swapped env vars to use TRACCAR_TOKEN |
| .devcontainer.json | Updated Poetry feature reference |
Comments suppressed due to low confidence (2)
tests/test_subscription.py:160
- [nitpick] The new generic message drops the original exception detail, which can hinder debugging. Consider restoring the exception text (e.g.,
Unexpected error - {exception}) or logging it.
"Unexpected error",
tests/test_client.py:14
- There aren’t tests verifying that the
Authorization: Bearer <token>header is sent in API calls—consider adding one to confirm token injection.
"token": "test",
📝 WalkthroughWalkthroughThe changes refactor authentication throughout the codebase from username/password-based to token-based authentication. This involves updating the API client, related test fixtures, and test cases to use a token parameter. Additionally, error messages in subscription tests were simplified, and the devcontainer configuration was updated to use a different Poetry feature source. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApiClient
participant TraccarServer
User->>ApiClient: Instantiate with host, token, session
ApiClient->>TraccarServer: HTTP request with Authorization: Bearer <token>
TraccarServer-->>ApiClient: Responds to API request
ApiClient-->>User: Returns API response
User->>ApiClient: Subscribe to events
ApiClient->>TraccarServer: WebSocket connect with token (query param)
TraccarServer-->>ApiClient: WebSocket event stream
ApiClient-->>User: Forwards events
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.devcontainer.json (1)
44-48: Consider pinning exact digests for deterministic buildsRelying on the floating
:2tag (and likewise the floating Python3.13image) means any rebuild can silently pick up a different Poetry or Python toolchain. This breaks the “works on my machine” guarantee the dev-container is meant to provide.Diff example (replace with the actual digest produced by
docker pull):-"ghcr.io/devcontainers-extra/features/poetry:2": {} +"ghcr.io/devcontainers-extra/features/poetry@sha256:<digest>": {}Same idea for the base image or switch to an LTS Python (3.12) until 3.13 is GA.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
.devcontainer.json(1 hunks)example.py(1 hunks)pytraccar/client.py(5 hunks)tests/common.py(1 hunks)tests/conftest.py(1 hunks)tests/test_client.py(1 hunks)tests/test_subscription.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
pytraccar/client.py (1)
pytraccar/exceptions.py (1)
TraccarException(4-5)
🪛 Ruff (0.11.9)
tests/conftest.py
80-80: Possible hardcoded password assigned to argument: "token"
(S106)
🔇 Additional comments (9)
pytraccar/client.py (4)
42-56: LGTM! Clean refactor to token-based authentication.The constructor change from username/password to token authentication is well-implemented and follows best practices for API authentication.
83-88: Proper implementation of Bearer token authentication.The Authorization header with Bearer token is correctly implemented, and the header merging logic ensures custom headers can still be passed.
226-228: Error message simplification looks good.The simplified error messages provide cleaner output, though consider logging the full exception details for debugging purposes.
205-212: Review session API call implementation
- The
Content-Type: application/x-www-form-urlencodedheader is unnecessary on a GET request (no request body to describe).- The comment above the call still points to the POST
/sessionendpoint, yet the code is issuing a GET. Confirm whether Traccar supports GET for authentication or if this should remain a POST.- Embedding the token in the URL query string risks exposure in server logs. Consider using a POST with the token in the request body or an
Authorizationheader for better security.Please verify the correct HTTP method in Traccar’s API documentation and adjust accordingly.
tests/common.py (1)
28-28: Good fix for handling filenames with query parameters.The logic correctly handles test data filenames that may include query strings, which is necessary for the new token-based authentication tests.
example.py (1)
27-27: Example correctly updated for token authentication.The change properly demonstrates the new token-based authentication usage.
tests/test_client.py (1)
14-14: Test correctly updated for token authentication.The test properly validates the new ApiClient constructor signature.
tests/test_subscription.py (1)
160-160: Test expectations correctly updated.The simplified error message expectations properly match the changes in the client's error handling.
Also applies to: 170-170
tests/conftest.py (1)
80-80: LGTM! Token authentication update is correctly implemented.The change appropriately updates the test fixture to use token-based authentication, aligning with the PR objectives. The hardcoded "test" value is suitable for test fixtures as it's clearly mock data for testing purposes.
Note: The static analysis warning about a "hardcoded password" is a false positive in this context since this is test configuration code with intentionally hardcoded mock values.
📝 WalkthroughWalkthroughThe authentication mechanism for the API client was changed from username/password-based to token-based across the codebase, including the client implementation, example usage, and tests. Related error messages were simplified, and the test utilities were updated to handle filenames with query parameters. The Poetry feature source in the devcontainer configuration was also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApiClient
participant TraccarAPI
User->>ApiClient: Initialize(token, ...)
User->>ApiClient: Make API request
ApiClient->>TraccarAPI: HTTP request with Authorization: Bearer <token>
TraccarAPI-->>ApiClient: API response
ApiClient-->>User: Return data
sequenceDiagram
participant User
participant ApiClient
participant TraccarAPI
User->>ApiClient: Subscribe via WebSocket
ApiClient->>TraccarAPI: GET /api/session?token=<token>
TraccarAPI-->>ApiClient: Session established
ApiClient->>TraccarAPI: WebSocket connection
TraccarAPI-->>ApiClient: WebSocket events
ApiClient-->>User: Deliver events
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🧰 Additional context used🧬 Code Graph Analysis (1)pytraccar/client.py (1)
🪛 Ruff (0.11.9)tests/conftest.py80-80: Possible hardcoded password assigned to argument: "token" (S106) 🔇 Additional comments (12)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
No description provided.