Feature/auth tests api (CORE-13484) #1
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds token-aware HTTP client features and a new AuthAPIClient; expands fixtures and token-fuzzing utilities; introduces Pydantic auth schemas and a large OpenAPI 3.0.3 contract; and adds extensive end-to-end authentication test suites (login, refresh, logout, profile, password, username, audit-log, authorization). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthAPI as AuthAPIClient
participant Server
participant DB
Client->>AuthAPI: login(username, password)
AuthAPI->>Server: POST /v1/auth/login (credentials)
Server->>DB: validate credentials
DB-->>Server: user record / auth decision
Server-->>AuthAPI: 200 + access_token + refresh_token + user
AuthAPI-->>Client: response (tokens, profile)
sequenceDiagram
participant Client
participant AuthAPI as AuthAPIClient
participant Server
participant TokenStore
Client->>AuthAPI: post_refresh(refresh_token)
AuthAPI->>Server: POST /v1/auth/refresh (refresh_token)
Server->>TokenStore: verify/rotate refresh token
TokenStore-->>Server: validation result + new tokens
Server-->>AuthAPI: 200 + new access_token (+/- refresh_token)
AuthAPI-->>Client: response (new tokens)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Hard-Coded Secrets (1)
More info on how to fix Hard-Coded Secrets in Python. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @tests/api/test_auth_audit_log.py:
- Around line 199-217: Import the missing base64 module and ensure the modified
headers are applied to the client before calling the API: in the tests
test_get_audit_log_with_wrong_auth_format and
test_get_audit_log_with_too_long_access_token, add "import base64" if absent,
set authenticated_auth_client.headers = headers immediately after creating the
headers variable (before calling authenticated_auth_client.get_audit_log()),
then perform the assertions and ErrorResponse(**response.json()), and finally
restore the original headers if needed.
- Around line 189-197: Test test_get_audit_log_with_wrong_auth_type is missing
the base64 import and never applies the modified headers before calling
get_audit_log; add "import base64" at the top of the test module and update
test_get_audit_log_with_wrong_auth_type to assign the modified headers to
authenticated_auth_client.headers before invoking
authenticated_auth_client.get_audit_log() (or pass the headers into
get_audit_log if that method accepts them), and keep or restore the original
headers afterwards as needed.
- Around line 135-145: The test file defines test_audit_log_entry_structure
twice which causes the first definition to be shadowed and some assertions to
never run; rename one of the duplicates (e.g., to
test_audit_log_entry_structure_with_results) so both tests run, and fix the
inconsistent indentation in the first test body (the block under if
len(audit_log.results) > 0 should align with the if) — update the method name
and adjust indentation around test_audit_log_entry_structure and its use of
authenticated_auth_client and AuditLogResponse to restore correct structure and
execution.
In @tests/api/test_auth_login.py:
- Around line 157-163: The test test_login_missing_password is calling
auth_client.login with password=valid_credentials["password"] (a copy-paste
error) instead of omitting the password; update the call in
test_login_missing_password to call
auth_client.login(username=valid_credentials["username"]) so the request
contains only the username and validates the missing-password 400 response,
leaving the assertions and ErrorResponse parsing intact.
- Around line 237-246: In test_login_with_wrong_method, the test calls
send_custom_request on auth_client.client but send_custom_request is defined on
the API client itself; update the call to use auth_client.send_custom_request
(keep the same parameters and assertions) so the test invokes the correct method
(refer to test_login_with_wrong_method and send_custom_request).
- Around line 32-47: The test function duplicate named test_login_success causes
the second definition to shadow the first; rename this duplicate to a unique,
descriptive name (e.g., test_login_success_recurrent or
test_recurrent_login_with_valid_credentials) and update any references or
markers if needed; ensure the function signature using auth_client,
valid_credentials, authenticated_auth_client and assertions remain the same so
pytest runs both tests independently.
- Around line 294-305: The test uses ProfileResponse but that name isn't
imported/defined; replace its usage with the correct schema UserProfile and add
an import for UserProfile from the project schemas (or tests' schema module) so
the lines constructing the model become
UserProfile(**get_profile_response.json()); update both occurrences around
authenticated_auth_client.get_profile() and ensure the import is added to the
top of tests/api/test_auth_login.py alongside other test schema imports.
In @tests/api/test_auth_logout.py:
- Around line 217-226: The test test_logout_with_wrong_content_type incorrectly
reads/writes auth_client.headers (which doesn't exist); update it to call the
logout method (or underlying client request) with the explicit headers variable
instead of mutating auth_client.headers — e.g., build headers =
auth_client.headers.copy() should be replaced with a base headers dict from the
fixture or auth_client.get_default_headers(), then call
auth_client.logout(headers=headers) or auth_client.client.post("/logout",
headers=headers) and remove the final auth_client.headers assignment; ensure the
response assertion and ErrorResponse(**response.json()) remain unchanged.
- Around line 207-216: The test is incorrectly accessing a non-existent
auth_client.headers on APIClient; replace the header-manipulation and
auth_client.logout() call with a direct HTTP POST via auth_client.client to the
logout endpoint. Specifically, in test_logout_without_content_type use
auth_client.client.post to POST to f"{auth_client.base_url}/v1/auth/logout" with
content set to "{}" (or empty payload) and headers={} to omit Content-Type, then
assert response.status_code == 400 and validate the body with
ErrorResponse(**response.json()); remove any reads/writes to auth_client.headers
and the auth_client.logout() invocation.
- Around line 1-7: The tests in test_auth_logout.py call base64.b64encode() but
the module isn't imported; add "import base64" to the top of the file alongside
the other imports so the calls to base64.b64encode() in the logout tests execute
without NameError (no code change elsewhere; just add the base64 import).
- Around line 131-139: The test test_logout_with_too_long_access_token
constructs a modified headers dict but never uses it when calling
authenticated_auth_client.logout(), so the long Authorization token is not sent;
update the test to pass the modified headers into
authenticated_auth_client.logout(headers=...) (or set
authenticated_auth_client.headers = headers before the call) and then assert the
431 response, but account for the possibility of no JSON body by only calling
ErrorResponse(**response.json()) when response has a JSON content-type or
non-empty body.
In @tests/api/test_auth_password.py:
- Around line 229-241: The test test_change_password_extra_fields is calling
send_custom_request with the wrong kwarg name (using url=) which doesn't match
the client signature; update the call in test_change_password_extra_fields to
use endpoint= instead of url= when invoking
authenticated_auth_client.send_custom_request, keeping the same method and json
payload so the request is sent to the correct endpoint.
- Around line 1-6: The test file is missing imports for WEAK_PASSWORD_CASES,
generate_invalid_refresh_tokens, and the base64 module; add an import for base64
and import WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens from the same
test_cases module where EMPTY_STRING_CASES and NONSTRING_CASES come from so
references to WEAK_PASSWORD_CASES, generate_invalid_refresh_tokens, and
base64.b64encode/b64decode resolve at runtime.
In @tests/api/test_auth_profile.py:
- Around line 1-6: The test file is missing the base64 import and references a
non-existent helper; add "import base64" at the top of
tests/api/test_auth_profile.py and replace usages of generate_invalid_tokens
with the correct helper(s) from utils/token_generator.py (e.g.,
generate_invalid_bearer_tokens or generate_invalid_refresh_tokens) or create a
thin wrapper named generate_invalid_tokens that calls the appropriate functions;
ensure any tests that expect both types call the right helper so there are no
NameError/ImportError at runtime.
- Around line 154-172: The tests test_get_profile_with_wrong_auth_type and
test_get_profile_with_wrong_auth_format fail to import base64 and also never
apply the modified headers when calling get_profile; add "import base64" to the
tests module and either set authenticated_auth_client.headers = headers before
calling authenticated_auth_client.get_profile() or pass headers into get_profile
(whichever matches the client API), then validate response and restore headers
if needed.
In @tests/api/test_auth_refresh.py:
- Around line 187-215: Tests modify an Authorization header but never apply it
to the client and they also use base64 without importing it; add "import base64"
at the module top and, in each test (test_refresh_with_wrong_auth_type,
test_refresh_with_wrong_auth_format, test_refresh_with_too_long_access_token),
assign the modified headers back to the client before making the request
(authenticated_auth_client.headers = headers) so the request uses the altered
Authorization value, then leave the existing restoration line after the request.
- Around line 1-6: The import and usage mismatch: replace the imported
generate_invalid_refresh_tokens with generate_invalid_bearer_tokens from
utils.token_generator, and update the call site that currently invokes
generate_invalid_tokens() (around the test using generate_invalid_tokens on line
163) to call generate_invalid_bearer_tokens() so the imported symbol matches the
function actually used; verify the test refers to generate_invalid_bearer_tokens
in tests/api/test_auth_refresh.py and remove or rename any incorrect references.
In @tests/api/test_auth_username.py:
- Around line 193-221: The tests test_change_username_with_wrong_auth_type /
test_change_username_with_wrong_auth_format /
test_change_username_with_too_long_access_token fail to import base64 and never
apply the modified headers to the client before making the request; add "import
base64" to the test module and, inside each test, assign the modified headers
back to authenticated_auth_client.headers before calling
authenticated_auth_client.change_username(...), then restore the original
headers afterward (or use a try/finally) so the request uses the altered
Authorization value.
- Around line 1-6: The test references two missing imports: add "import base64"
for the stdlib base64 usage and import the helper that produces bad tokens (e.g.
"from tests.api.utils import generate_invalid_tokens" or from whichever test
helper module houses that function in this repo) so generate_invalid_tokens and
base64 are available where EMPTY_STRING_CASES and NONSTRING_CASES are used in
tests/api/test_auth_username.py.
🟠 Major comments (4)
tests/api/test_auth_logout.py-228-235 (1)
228-235: Inconsistent Cache-Control expectations and authentication.
- This test uses
auth_client(unauthenticated) but expects 200 - logout requires authentication per the OpenAPI spec.- Cache-Control expectation (
no-cache, no-store, must-revalidate) differs fromtest_logout_check_response_headersline 146 (no-store).Clarify expected behavior and use consistent assertions.
clients/api_client.py-171-172 (1)
171-172: Methodrefresh_tokenshadows instance attribute of same name.The
APIClient.__init__storesself.refresh_token(line 12), but this method has the same name. Accessingclient.refresh_tokenreturns the method, not the stored token value.Proposed fix: rename method or attribute
- def refresh_token(self, refresh_token: str) -> httpx.Response: + def refresh_access_token(self, refresh_token: str) -> httpx.Response: return self.post("/v1/auth/refresh", json={"refresh_token": refresh_token})Or rename the attribute in
APIClient.__init__:- self.refresh_token = refresh_token + self.stored_refresh_token = refresh_tokentests/api/test_auth_password.py-156-168 (1)
156-168: Wrong token generator function for access token testing.Using
generate_invalid_refresh_tokens()to test invalid access tokens is incorrect. These generators produce different token structures. Usegenerate_invalid_bearer_tokens()instead.Proposed fix
@allure.title("Change password with invalid authentication token") @allure.severity(allure.severity_level.CRITICAL) - @pytest.mark.parametrize("invalid_token", generate_invalid_refresh_tokens()) + @pytest.mark.parametrize("invalid_token", generate_invalid_bearer_tokens()) def test_change_password_with_invalid_token(self, authenticated_auth_client, invalid_token, valid_credentials, faker):tests/api/test_auth_refresh.py-231-239 (1)
231-239: Test modifies headers but doesn't apply them before request.
headers.pop("Content-Type")modifies a copy, but the actual client headers remain unchanged when the request is made.Proposed fix
def test_refresh_without_content_type(self, authenticated_auth_client): - headers = authenticated_auth_client.headers.copy() - headers.pop("Content-Type") - response = authenticated_auth_client.refresh_token(authenticated_auth_client.refresh_token) + original_headers = authenticated_auth_client.headers.copy() + authenticated_auth_client.headers.pop("Content-Type", None) + response = authenticated_auth_client.refresh_token(authenticated_auth_client.refresh_token) assert response.status_code == 400, f"Expected 400, got {response.status_code}" ErrorResponse(**response.json()) - authenticated_auth_client.headers = headers + authenticated_auth_client.headers = original_headers
🟡 Minor comments (7)
tests/api/cases/const.py-1-1 (1)
1-1: Clarify the constant's intent: max valid value vs. overflow value.
2**63equals 9,223,372,036,854,775,808, which actually exceeds the maximum positive value of a signed 64-bit integer (2**63 - 1 = 9,223,372,036,854,775,807).If this constant is meant to represent the maximum valid 64-bit signed integer for boundary testing, it should be:
MAX_64_BIT_INT = 2**63 - 1If it's intentionally an overflow value for negative testing, consider renaming it to something like
OVERFLOW_64_BIT_INTfor clarity.tests/api/cases/test_cases.py-2-3 (1)
2-3: Sets and tuples are not JSON-serializable.Both
NONSTRING_CASESandNONINTEGER_CASESinclude(1,2,3)(tuple) and{1,2,3}(set), which are not natively JSON-serializable. If these test cases are used directly in API request bodies, they will causeTypeErrorduring JSON serialization.Consider either:
- Removing sets and tuples if they're not needed for the test scenarios
- Converting them to lists
[1,2,3]if the intent is to test array-like inputs- Ensuring the test code handles serialization appropriately before sending
Suggested fix if lists are acceptable substitutes
-NONSTRING_CASES = [123, True, None, [123], {"key": "value"}, (1,2,3), {1,2,3}, 123.45] -NONINTEGER_CASES = [True, None, [123], {"key": "value"}, (1,2,3), {1,2,3}, 123.45, "123", "null", "true", "false"] +NONSTRING_CASES = [123, True, None, [123], {"key": "value"}, [1,2,3], 123.45] +NONINTEGER_CASES = [True, None, [123], {"key": "value"}, [1,2,3], 123.45, "123", "null", "true", "false"]tests/api/test_auth_logout.py-85-88 (1)
85-88: Typo in class name:TestLogoutAcess→TestLogoutAccess.Proposed fix
@allure.feature("Authentication") @allure.story("Logout") @pytest.mark.api -class TestLogoutAcess: +class TestLogoutAccess:tests/api/test_auth_authorization.py-158-172 (1)
158-172: Incomplete test: only/healthzis verified.The test defines three public endpoints but the loop only executes logic for
/healthz. The login and refresh endpoints aren't tested for accessibility.Proposed fix to test all endpoints
def test_public_endpoints_no_auth(self, auth_client): public_endpoints = [ "/healthz", "/v1/auth/login", "/v1/auth/refresh", ] for endpoint in public_endpoints: with allure.step(f"Testing public endpoint: {endpoint}"): if endpoint == "/healthz": response = auth_client.get(endpoint) assert response.status_code in [200, 404], \ f"{endpoint} should be accessible without auth" + elif endpoint == "/v1/auth/login": + response = auth_client.post(endpoint, json={"username": "", "password": ""}) + # Login fails but endpoint is accessible (not 401 for missing auth) + assert response.status_code in [400, 401], \ + f"{endpoint} should be accessible without auth" + elif endpoint == "/v1/auth/refresh": + response = auth_client.post(endpoint, json={"refresh_token": ""}) + assert response.status_code in [400, 401], \ + f"{endpoint} should be accessible without auth"fixtures/api_fixtures.py-38-49 (1)
38-49: Silent failure when login fails inauthenticated_auth_client.If login returns non-200, the fixture silently yields a client without valid tokens. Tests will fail with confusing 401 errors rather than a clear setup failure.
Proposed fix to raise on login failure
@pytest.fixture def authenticated_auth_client(config: Settings): client = AuthAPIClient(config) if config.user_log and config.user_pass: response = client.login(config.user_log, config.user_pass) if response.status_code == 200: token = response.json().get("access_token") refresh_token = response.json().get("refresh_token") client.token = token client.refresh_token = refresh_token + else: + raise RuntimeError(f"Login failed with status {response.status_code}: {response.text}") + else: + raise RuntimeError("Missing USER_LOG or USER_PASS in configuration") yield client client.close()clients/api_client.py-168-169 (1)
168-169: Invalid type annotation:anyshould beAnyorOptional[str].Python's
anyis a built-in function, not a type. UseAnyfromtypingor preferablyOptional[str]for explicit typing.Proposed fix
- def login(self, username: any = None, password: any = None) -> httpx.Response: + def login(self, username: Optional[str] = None, password: Optional[str] = None) -> httpx.Response: return self.post("/v1/auth/login", json={"username": username, "password": password})tests/api/test_auth_profile.py-100-111 (1)
100-111: Unusedvalid_rolesvariable - role validation is incomplete.The
valid_roleslist is defined but never used. It appears the intent was to validate thattenant_roleis one of the allowed values.Proposed fix
def test_profile_tenant_role(self, authenticated_auth_client): response = authenticated_auth_client.get_profile() assert response.status_code == 200 profile = UserProfile(**response.json()) valid_roles = ["owner", "admin", "editor", "viewer"] assert profile.tenant_role, "Tenant role should not be empty" + assert profile.tenant_role in valid_roles, f"Tenant role '{profile.tenant_role}' should be one of {valid_roles}"
🧹 Nitpick comments (7)
utils/token_generator.py (1)
9-84: Good coverage of invalid token scenarios.The function provides comprehensive test cases covering malformed structure, encoding issues, tampered signatures, and injection attempts.
Nit: Lines 51, 52, and 64 have f-string prefixes without any placeholders. While harmless, they can be simplified.
Remove unnecessary f-string prefixes
# Malformed structure - f"...", - f"....", + "...", + "....", f"{valid_header}...{valid_signature}", # Invalid JSON in parts "bm90X2pzb24.bm90X2pzb24.signature", # "not_json" base64 encoded # Tampered signature f"{valid_header}.{valid_payload}.tampered_signature", f"{valid_header}.{valid_payload}.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", # Special characters f"{valid_header}.{valid_payload}.sig@#$%", - f"header!@#.payload$%^.signature&*()", + "header!@#.payload$%^.signature&*()",tests/api/schemas/__init__.py (1)
5-22: LGTM!Clean utility function that centralizes schema validation logic for tests. The error message provides good context by including the schema class name and validation error details.
Optional: Consider adding a type hint for
schema_classfor better IDE support:from typing import Type, TypeVar from pydantic import BaseModel T = TypeVar('T', bound=BaseModel) def validate_schema(response_data: dict, schema_class: Type[T]) -> T:openapi.yaml (3)
1-12: Consider adding global security with explicit overrides for public endpoints.The spec defines
BearerAuthbut doesn't set it globally. While login/refresh correctly don't require auth, the deployment and preset endpoints appear unprotected. If these should require authentication, apply security globally and override withsecurity: []for public endpoints.# Add after servers section security: - BearerAuth: []Then for public endpoints like
/v1/auth/login:security: [] # Public endpoint
1157-1177: Consider addingmaxItemsconstraints to array schemas.List endpoints like
AuditLogResponse.resultslackmaxItemsbounds. Whilepage_sizehas a maximum of 100, explicitly constraining the array provides defense-in-depth and clearer API contracts.results: type: array maxItems: 100 items: $ref: '#/components/schemas/AuditLogEntry'
1186-1196: Unused "Internal" tag defined.The
Internaltag (line 1193-1194) is defined but no endpoints in thepathssection use it. Consider removing it or documenting its intended future use.clients/api_client.py (1)
187-189: Audit log params only sent when bothpageandpage_sizeare provided.The condition
if page and page_sizerequires both values. Users expecting to set onlypage_sizewill get no params. Consider sending non-None values independently.Proposed fix
def get_audit_log(self, page: Optional[int] = None, page_size: Optional[int] = None) -> httpx.Response: - params = {"page": page, "page_size": page_size} if page and page_size else None + params = {} + if page is not None: + params["page"] = page + if page_size is not None: + params["page_size"] = page_size return self.get("/v1/auth/audit-log", params=params)tests/api/test_auth_profile.py (1)
193-223:TestProfileHeadersclass is missing allure feature/story decorators.Unlike other test classes in this file,
TestProfileHeaderslacks the@allure.featureand@allure.storydecorators, which will cause inconsistent test reporting.Proposed fix
+@allure.feature("Authentication") +@allure.story("User Profile") +@pytest.mark.api class TestProfileHeaders:
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
clients/api_client.pyclients/eth_client.pyfixtures/api_fixtures.pyfixtures/eth_fixtures.pyopenapi.yamltests/api/README.mdtests/api/__init__.pytests/api/cases/const.pytests/api/cases/test_cases.pytests/api/schemas/__init__.pytests/api/schemas/auth_schemas.pytests/api/test_auth_audit_log.pytests/api/test_auth_authorization.pytests/api/test_auth_login.pytests/api/test_auth_logout.pytests/api/test_auth_password.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.pytests/api/test_auth_username.pyutils/token_generator.py
🧰 Additional context used
🧬 Code graph analysis (9)
tests/api/test_auth_logout.py (5)
tests/api/schemas/auth_schemas.py (3)
LogoutResponse(29-30)ErrorResponse(33-34)UserProfile(6-14)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-49)auth_client(32-35)clients/api_client.py (4)
logout(174-176)refresh_token(171-172)get_profile(178-179)send_custom_request(85-99)fixtures/playwright_fixtures.py (1)
base_url(38-39)
utils/token_generator.py (1)
fixtures/api_fixtures.py (1)
faker(7-8)
tests/api/test_auth_authorization.py (3)
utils/token_generator.py (1)
generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (3)
auth_client(32-35)valid_credentials(53-57)invalid_credentials(79-83)clients/api_client.py (5)
send_custom_request(85-99)get_profile(178-179)get(31-42)refresh_token(171-172)logout(174-176)
tests/api/schemas/auth_schemas.py (1)
clients/api_client.py (1)
refresh_token(171-172)
clients/api_client.py (1)
config/settings.py (1)
Settings(6-78)
fixtures/eth_fixtures.py (1)
clients/eth_client.py (1)
EthereumClient(8-153)
tests/api/test_auth_refresh.py (4)
tests/api/schemas/auth_schemas.py (4)
LoginResponse(17-21)RefreshTokenResponse(24-26)ErrorResponse(33-34)UserProfile(6-14)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)fixtures/api_fixtures.py (3)
auth_client(32-35)valid_credentials(53-57)authenticated_auth_client(39-49)clients/api_client.py (4)
refresh_token(171-172)post(45-56)get_profile(178-179)logout(174-176)
tests/api/test_auth_profile.py (3)
tests/api/schemas/auth_schemas.py (3)
UserProfile(6-14)ErrorResponse(33-34)LoginResponse(17-21)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-49)valid_credentials(53-57)clients/api_client.py (3)
get_profile(178-179)login(168-169)logout(174-176)
tests/api/test_auth_username.py (3)
tests/api/schemas/auth_schemas.py (2)
UserProfile(6-14)ErrorResponse(33-34)fixtures/api_fixtures.py (3)
authenticated_auth_client(39-49)valid_username(65-66)valid_credentials(53-57)clients/api_client.py (3)
change_username(184-185)send_custom_request(85-99)logout(174-176)
🪛 Checkov (3.2.334)
openapi.yaml
[high] 1-1198: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-1198: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 48-52: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 Gitleaks (8.30.0)
tests/api/test_auth_username.py
[high] 184-184: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 markdownlint-cli2 (0.18.1)
tests/api/README.md
18-18: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
30-30: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
42-42: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
56-56: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
72-72: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
82-82: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.10)
tests/api/test_auth_logout.py
115-115: Undefined name base64
(F821)
125-125: Undefined name base64
(F821)
utils/token_generator.py
26-26: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
51-51: f-string without any placeholders
Remove extraneous f prefix
(F541)
52-52: f-string without any placeholders
Remove extraneous f prefix
(F541)
64-64: f-string without any placeholders
Remove extraneous f prefix
(F541)
108-108: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
tests/api/test_auth_authorization.py
46-46: Possible hardcoded password assigned to: "expired_token"
(S105)
clients/api_client.py
99-99: Avoid specifying long messages outside the exception class
(TRY003)
168-168: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
168-168: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
tests/api/test_auth_password.py
106-106: Undefined name WEAK_PASSWORD_CASES
(F821)
158-158: Undefined name generate_invalid_refresh_tokens
(F821)
174-174: Undefined name base64
(F821)
187-187: Undefined name base64
(F821)
tests/api/test_auth_login.py
35-35: Redefinition of unused test_login_success from line 16
(F811)
93-93: Undefined name valid_credentials
(F821)
93-93: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
93-93: Undefined name valid_credentials
(F821)
94-94: Undefined name valid_credentials
(F821)
94-94: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
94-94: Undefined name valid_credentials
(F821)
95-95: Undefined name valid_credentials
(F821)
95-95: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
95-95: Undefined name valid_credentials
(F821)
96-96: Undefined name valid_credentials
(F821)
96-96: Undefined name valid_credentials
(F821)
96-96: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
97-97: Undefined name valid_credentials
(F821)
97-97: Undefined name valid_credentials
(F821)
97-97: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
98-98: Undefined name valid_credentials
(F821)
98-98: Undefined name valid_credentials
(F821)
98-98: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
99-99: Undefined name valid_credentials
(F821)
99-99: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
99-99: Undefined name valid_credentials
(F821)
99-99: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
100-100: Undefined name valid_credentials
(F821)
100-100: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
100-100: Undefined name valid_credentials
(F821)
100-100: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
101-101: Undefined name valid_credentials
(F821)
101-101: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
101-101: Undefined name valid_credentials
(F821)
101-101: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
112-112: Undefined name valid_credentials
(F821)
112-112: Undefined name valid_credentials
(F821)
113-113: Undefined name valid_credentials
(F821)
113-113: Undefined name valid_credentials
(F821)
114-114: Undefined name valid_credentials
(F821)
114-114: Undefined name valid_credentials
(F821)
115-115: Undefined name valid_credentials
(F821)
115-115: Undefined name valid_credentials
(F821)
116-116: Undefined name valid_credentials
(F821)
116-116: Undefined name valid_credentials
(F821)
117-117: Undefined name valid_credentials
(F821)
117-117: Undefined name valid_credentials
(F821)
118-118: Undefined name valid_credentials
(F821)
118-118: Undefined name valid_credentials
(F821)
119-119: Undefined name valid_credentials
(F821)
119-119: Undefined name valid_credentials
(F821)
120-120: Undefined name valid_credentials
(F821)
120-120: Undefined name valid_credentials
(F821)
121-121: Undefined name valid_credentials
(F821)
121-121: Undefined name valid_credentials
(F821)
122-122: Undefined name valid_credentials
(F821)
122-122: Undefined name valid_credentials
(F821)
298-298: Undefined name ProfileResponse
(F821)
304-304: Undefined name ProfileResponse
(F821)
tests/api/test_auth_refresh.py
71-71: Possible hardcoded password assigned to: "expired_token"
(S105)
163-163: Undefined name generate_invalid_tokens
(F821)
178-178: Possible hardcoded password assigned to: "expired_token"
(S105)
191-191: Undefined name base64
(F821)
201-201: Undefined name base64
(F821)
tests/api/test_auth_profile.py
108-108: Local variable valid_roles is assigned to but never used
Remove assignment to unused variable valid_roles
(F841)
145-145: Possible hardcoded password assigned to: "expired_token"
(S105)
158-158: Undefined name base64
(F821)
168-168: Undefined name base64
(F821)
tests/api/test_auth_audit_log.py
149-149: Redefinition of unused test_audit_log_entry_structure from line 135
(F811)
193-193: Undefined name base64
(F821)
203-203: Undefined name base64
(F821)
tests/api/test_auth_username.py
169-169: Undefined name generate_invalid_tokens
(F821)
184-184: Possible hardcoded password assigned to: "expired_token"
(S105)
197-197: Undefined name base64
(F821)
207-207: Undefined name base64
(F821)
🔇 Additional comments (9)
fixtures/eth_fixtures.py (1)
2-2: LGTM!The import path update aligns with the broader repository refactoring to centralize client modules under the
clientspackage. TheEthereumClientclass exists at the new location as confirmed by the relevant code snippets.utils/token_generator.py (1)
86-167: LGTM!Good implementation including the
wrong_type_tokencase to test token type confusion attacks (using an access token where a refresh token is expected). The test cases mirror the bearer token generator appropriately while including refresh-specific scenarios.tests/api/README.md (1)
1-202: LGTM! Comprehensive test documentation.The README provides clear guidance on test coverage, running instructions, configuration, and CI/CD integration. The structure aligns well with the test suite being introduced.
fixtures/api_fixtures.py (1)
6-83: LGTM! Well-structured test fixtures.The fixture hierarchy is clean with appropriate scoping. The faker-based fixtures provide good test data isolation.
tests/api/test_auth_logout.py (1)
12-75: LGTM! Core logout tests are well-structured.The test coverage for logout success, token invalidation, and refresh token revocation is comprehensive with proper schema validation.
tests/api/test_auth_authorization.py (1)
1-55: LGTM! Comprehensive authorization test coverage.The test suite covers critical auth scenarios: protected endpoints, invalid tokens, expiration, token isolation, and scheme validation. The TODO comments appropriately flag areas needing clarification.
tests/api/schemas/auth_schemas.py (1)
1-55: LGTM! Clean Pydantic schema definitions.The models correctly mirror the OpenAPI spec with appropriate field constraints (
expires_in > 0) and optional fields. The nestedUserProfileinLoginResponseenables comprehensive response validation.clients/api_client.py (1)
159-180: LGTM! AuthAPIClient provides clean abstraction for auth endpoints.The client methods align well with the OpenAPI spec. Cookie management methods add useful functionality for session-based testing.
tests/api/test_auth_username.py (1)
96-109: Verify: Should common username patterns likeuser_nameanduser.namebe rejected?The test expects HTTP 400 for usernames containing dots, underscores, hyphens, and digits. These are typically valid username characters in most systems. Please verify if this aligns with your API's username requirements.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @clients/api_client.py:
- Around line 84-99: send_custom_request calls self.patch but APIClient lacks a
patch method, causing an AttributeError; add a patch method to APIClient
(similar to existing get/post/put) that accepts endpoint, optional json and
headers, builds url from self.base_url and request headers via
self._get_headers, makes the request via self.client.patch(url, json=json,
headers=request_headers), and wraps request/response logging with the same
allure.step/attachments pattern used by get/post/put so PATCH requests via
send_custom_request succeed.
- Around line 187-188: The current falsy check in get_audit_log uses "if page
and page_size" which treats page=0 as False and omits valid pagination; change
the condition to explicitly check for None (e.g., use "if page is not None and
page_size is not None") or build params by including keys when their values are
not None (e.g., params = {k: v for k, v in (("page", page), ("page_size",
page_size)) if v is not None}) so page=0 is preserved.
- Around line 168-169: The login method uses the builtin function name "any" as
a type hint and includes None values in the request body; update the signature
of login (function name: login) to use proper types (e.g., Optional[str] or str
| None) for username and password, and build the JSON payload so that keys with
None values are omitted before calling self.post (i.e., only include "username"
and "password" in the JSON when they are not None).
🧹 Nitpick comments (1)
clients/api_client.py (1)
171-172: Method namerefresh_tokenshadows the instance attribute.The method
refresh_tokenshadowsself.refresh_tokenset in__init__. This can cause confusion and makes the instance attribute inaccessible viaself.refresh_tokenafter class definition.♻️ Proposed rename
- def refresh_token(self, refresh_token: str) -> httpx.Response: + def refresh_access_token(self, refresh_token: str) -> httpx.Response: return self.post("/v1/auth/refresh", json={"refresh_token": refresh_token})
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
clients/api_client.py
🧰 Additional context used
🧬 Code graph analysis (1)
clients/api_client.py (1)
config/settings.py (1)
Settings(6-78)
🪛 Ruff (0.14.10)
clients/api_client.py
99-99: Avoid specifying long messages outside the exception class
(TRY003)
168-168: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
168-168: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (2)
clients/api_client.py (2)
9-14: LGTM!The addition of
refresh_tokenparameter to the baseAPIClientfollows the existing pattern and properly supports the newAuthAPIClientsubclass.
101-109: LGTM!Cookie management methods are cleanly implemented using the underlying httpx client's cookie jar.
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @clients/api_client.py:
- Around line 94-96: send_custom_request delegates PATCH to self.patch but
APIClient has no patch method, causing AttributeError; add a patch method on
APIClient (mirror the existing post/put methods) that accepts endpoint, json,
params, headers (or same signature as put/post) and forwards to the shared
request implementation (e.g., call the internal _request or session.request with
method "PATCH") so send_custom_request can call self.patch without error.
In @tests/api/test_auth_audit_log.py:
- Around line 190-198: The test copies and mutates a headers dict but never
assigns it back to the client, so the modified Authorization is not sent; update
the test_get_audit_log_with_wrong_auth_type test to set
authenticated_auth_client.headers = headers before calling
authenticated_auth_client.get_audit_log(), and apply the same change to the
other affected tests (lines referenced 200-208 and 210-218) so the mutated
headers are actually used for the request.
- Around line 182-185: The test calls
auth_client.get_audit_log(token=invalid_token) but get_audit_log(page=None,
page_size=None) doesn't accept a token and will raise TypeError; fix by removing
the token kwarg and setting the invalid token on the client before invoking
get_audit_log (e.g., use the client's token setter or assign auth_client.token /
call auth_client.set_token(invalid_token) depending on the client API), then
call auth_client.get_audit_log() and assert 401 as before; update
test_get_audit_log_with_invalid_access_token accordingly.
In @tests/api/test_auth_login.py:
- Around line 202-210: In test_login_without_content_type update the modified
headers dict back onto the client before making the request: after popping
"Content-Type" assign headers to auth_client.headers so auth_client.login uses
the altered headers, and restore the original headers after the assertion; apply
the same fix to the other test referenced (the block covering 212-221) so both
tests actually send the modified headers when calling auth_client.login.
- Around line 107-114: The parametrize call uses
randomize_valid_credentials(valid_credentials) at import time but
valid_credentials is a fixture and not available yet; update
test_login_wrong_case to obtain valid_credentials via the fixture and call
randomize_valid_credentials inside the test (or change to a fixture with params)
instead of in the @pytest.mark.parametrize decorator so that randomization
happens at runtime; specifically modify the usage around test_login_wrong_case
and randomize_valid_credentials (and/or convert valid_credentials to a
parametrized fixture) rather than referencing valid_credentials at module load.
- Around line 88-105: The parametrize currently references the fixture
valid_credentials at collection time and uses nested f-string quoting; instead
parametrize only the whitespace variants (e.g. "prefix", "suffix", "both") or
simple templates like (" {u}", "{u} ", " {u} ") for username and similarly for
password, and then inside test_login_stripcases accept the valid_credentials
fixture and build username/password by formatting those templates with
valid_credentials["username"]/["password"] (avoid nested quotes by using
.format() or concatenation). Update the pytest.mark.parametrize decorator to
pass plain templates/flags (no fixture access) and adjust
test_login_stripcases(auth_client, valid_credentials, username_template,
password_template) to construct the final values before calling
auth_client.login.
In @tests/api/test_auth_logout.py:
- Around line 68-75: Summary: The test method
test_refresh_token_invalidated_after_logout uses
authenticated_auth_client.refresh_token both as the stored token attribute and
as a method, causing a str-to-call TypeError; rename the attribute and update
calls. Rename the stored token attribute on the test client (e.g., refresh_token
-> stored_refresh_token or saved_refresh_token) and change the test to pass
authenticated_auth_client.stored_refresh_token into the client methods (e.g.,
authenticated_auth_client.logout(authenticated_auth_client.stored_refresh_token)
and
authenticated_auth_client.refresh_token(authenticated_auth_client.stored_refresh_token));
update any other usages of the old attribute name in tests/helpers to the new
name to avoid shadowing the refresh_token method.
- Around line 164-172: In test_logout_and_refresh_token_at_the_same_time, avoid
shadowing the client's refresh_token attribute by first storing it in a local
variable (e.g., token = authenticated_auth_client.refresh_token) and use that
variable for both calls; then change the sequential calls to run concurrently
(use concurrent.futures.ThreadPoolExecutor to submit
authenticated_auth_client.logout(refresh_token=token) and
authenticated_auth_client.refresh_token(refresh_token=token) in parallel),
collect the two futures, then assert logout_response.status_code == 200 and
refresh_response.status_code == 401 and validate payloads with
LogoutResponse(...) and ErrorResponse(...).
In @tests/api/test_auth_password.py:
- Around line 196-207: The test modifies a local headers copy but never assigns
it back to the client, so the request still uses the original headers; update
the test_change_password_with_too_long_access_token to set
authenticated_auth_client.headers = headers before calling change_password and
then restore the original headers afterwards (save original =
authenticated_auth_client.headers.copy(), assign modified headers, run
change_password, assert and validate ErrorResponse, then restore
authenticated_auth_client.headers = original); apply the same pattern to the
other similar tests referenced (lines 243-254 and 256-268) that modify a headers
copy instead of the client’s headers.
- Around line 1-6: The test file imports are missing WEAK_PASSWORD_CASES,
generate_invalid_refresh_tokens, and base64 which causes NameError/undefined
symbol failures; add imports at the top of the file: import base64, import
WEAK_PASSWORD_CASES from its test data module (same place as
EMPTY_STRING_CASES/NONSTRING_CASES), and import generate_invalid_refresh_tokens
from the helper/case utilities where other generator helpers live so the symbols
WEAK_PASSWORD_CASES, generate_invalid_refresh_tokens, and base64 are defined
before they are used in the tests.
In @tests/api/test_auth_profile.py:
- Around line 196-204: The tests modify a local headers copy but never apply it
to the client, so the request still uses the original headers; update both
test_get_profile_without_content_type and
test_get_profile_with_wrong_content_type to apply the modified headers before
making the request by assigning authenticated_auth_client.headers = headers (or
passing headers into authenticated_auth_client.get_profile(...) if that helper
accepts a headers param) so the request actually uses the removed/incorrect
Content-Type and then assert the 400 response and validate ErrorResponse as
before.
- Around line 155-163: The modified headers aren’t used because you build a new
headers dict but call authenticated_auth_client.get_profile() before applying
it; update the test_get_profile_with_wrong_auth_type to assign the modified
headers to authenticated_auth_client.headers (or pass headers into get_profile
if that method accepts it) before calling get_profile so the request uses the
"Basic ..." Authorization value, then perform the assertions and restore any
original headers if needed.
In @tests/api/test_auth_refresh.py:
- Around line 231-239: The test creates a copy of headers but pops
"Content-Type" only from the copy and then calls
authenticated_auth_client.refresh_token which still uses the original client
headers; update test_refresh_without_content_type to either assign the modified
headers back to authenticated_auth_client.headers before calling refresh_token
(and restore the original after) or call refresh_token with the modified headers
if the method accepts headers; apply the same change to the similar test block
at lines 241-250 so the request actually omits Content-Type.
- Around line 1-6: The test file is missing the base64 module import used by
base64.b64encode in the refresh token tests; add "import base64" to the top
import block alongside pytest/allure/time so calls to base64.b64encode succeed;
update the imports in tests/api/test_auth_refresh.py (near the existing imports
for pytest, allure, time, and pydantic.ValidationError) to include base64.
🟠 Major comments (7)
clients/api_client.py-172-173 (1)
172-173: Methodrefresh_tokenshadows instance attributeself.refresh_token.This causes issues in tests where
authenticated_auth_client.refresh_tokenis used both as an attribute (the stored token) and as a method call. Consider renaming the method to avoid confusion.Proposed fix - rename method
- def refresh_token(self, refresh_token: str) -> httpx.Response: + def refresh_access_token(self, refresh_token: str) -> httpx.Response: return self.post("/v1/auth/refresh", json={"refresh_token": refresh_token})This requires updating all call sites in tests accordingly.
tests/api/test_auth_login.py-143-149 (1)
143-149: Test name and implementation mismatch.Test is titled "Login fails with missing password" but line 146 passes
password=valid_credentials["username"]instead ofusername=valid_credentials["username"]. This tests a scenario with missing username, not missing password.Proposed fix
@allure.title("Login fails with missing password") @allure.severity(allure.severity_level.NORMAL) def test_login_missing_password(self, auth_client, valid_credentials): - response = auth_client.login(password=valid_credentials["username"]) + response = auth_client.login(username=valid_credentials["username"]) assert response.status_code == 400, f"Expected 400, got {response.status_code}"clients/api_client.py-188-189 (1)
188-189: Params only sent when bothpageandpage_sizeare provided.The condition
if page and page_sizemeans single-parameter requests (page-only or page_size-only) won't include any query params. Tests liketest_get_audit_log_page_onlyexpect this to work.Proposed fix
def get_audit_log(self, page: Optional[int] = None, page_size: Optional[int] = None) -> httpx.Response: - params = {"page": page, "page_size": page_size} if page and page_size else None + params = {} + if page is not None: + params["page"] = page + if page_size is not None: + params["page_size"] = page_size return self.get("/v1/auth/audit-log", params=params if params else None)clients/api_client.py-169-170 (1)
169-170: UseAnyfrom typing instead of lowercaseany.
anyis Python's built-in function, not a type hint. This should beAnyfrom thetypingmodule, or more appropriatelyOptional[str]since credentials are strings.Proposed fix
- def login(self, username: any = None, password: any = None) -> httpx.Response: + def login(self, username: Optional[str] = None, password: Optional[str] = None) -> httpx.Response: return self.post("/v1/auth/login", json={"username": username, "password": password})tests/api/test_auth_profile.py-185-192 (1)
185-192:login()called without credentials will fail — pass username and password explicitly.On line 192,
authenticated_auth_client.login()is called without arguments, which sends{"username": null, "password": null}to the authentication API. TheAuthAPIClientdoes not cache credentials internally, so there is no mechanism to retrieve stored credentials for re-authentication. This will result in an authentication failure.Pass the configured credentials explicitly:
authenticated_auth_client.login(config.user_log, config.user_pass)Or store credentials in the client during initialization for reuse.
tests/api/test_auth_username.py-195-203 (1)
195-203: Bug: Modified headers not used in request, then restoration overwrites valid headers.Line 198 copies headers, line 199 sets
Authorizationon the copy but never applies it to the client. The request on line 200 uses the original (unchanged)client.headers. Then line 203 restoresheaders(with the wrong auth type) as the client headers, corrupting state for subsequent tests.If the intent is to test with
Basicauth, you need to modifyauthenticated_auth_client.headersdirectly before the request and restore afterward.Proposed fix
def test_change_username_with_wrong_auth_type(self, authenticated_auth_client, valid_username): - headers = authenticated_auth_client.headers.copy() - headers["Authorization"] = "Basic " + base64.b64encode(authenticated_auth_client.token.encode()).decode() - response = authenticated_auth_client.change_username(valid_username) + original_headers = authenticated_auth_client.headers.copy() + authenticated_auth_client.headers["Authorization"] = "Basic " + base64.b64encode(authenticated_auth_client.token.encode()).decode() + response = authenticated_auth_client.change_username(valid_username) assert response.status_code == 401, f"Expected 401, got {response.status_code}" ErrorResponse(**response.json()) - authenticated_auth_client.headers = headers + authenticated_auth_client.headers = original_headerstests/api/test_auth_username.py-239-258 (1)
239-258: Bug: Header modifications not applied to client before request.Both tests copy headers, modify the copy, but never apply the changes to the client before making the request:
- Line 243:
headers.pop("Content-Type")modifies the copy, notauthenticated_auth_client.headers- Line 254:
headers["Content-Type"] = content_typemodifies the copy, not the clientThe requests on lines 244 and 255 use the original unmodified client headers.
Proposed fix for test_change_username_without_content_type
def test_change_username_without_content_type(self, authenticated_auth_client, valid_username): - headers = authenticated_auth_client.headers.copy() - headers.pop("Content-Type") - response = authenticated_auth_client.change_username(valid_username) + original_headers = authenticated_auth_client.headers.copy() + authenticated_auth_client.headers.pop("Content-Type", None) + response = authenticated_auth_client.change_username(valid_username) assert response.status_code == 400, f"Expected 400, got {response.status_code}" ErrorResponse(**response.json()) - authenticated_auth_client.headers = headers + authenticated_auth_client.headers = original_headersProposed fix for test_change_username_with_wrong_content_type
def test_change_username_with_wrong_content_type(self, authenticated_auth_client, content_type, valid_username): - headers = authenticated_auth_client.headers.copy() - headers["Content-Type"] = content_type - response = authenticated_auth_client.change_username(valid_username) + original_headers = authenticated_auth_client.headers.copy() + authenticated_auth_client.headers["Content-Type"] = content_type + response = authenticated_auth_client.change_username(valid_username) assert response.status_code == 400, f"Expected 400, got {response.status_code}" ErrorResponse(**response.json()) - authenticated_auth_client.headers = headers + authenticated_auth_client.headers = original_headers
🟡 Minor comments (4)
tests/api/test_auth_logout.py-86-89 (1)
86-89: Typo in class name:TestLogoutAcessshould beTestLogoutAccess.Proposed fix
-class TestLogoutAcess: +class TestLogoutAccess:tests/api/test_auth_profile.py-109-111 (1)
109-111: Unused variablevalid_roles.The
valid_roleslist is defined but never used. The assertion only checks iftenant_roleis truthy, not whether it's one of the valid roles.Proposed fix to validate against allowed roles
profile = UserProfile(**response.json()) valid_roles = ["owner", "admin", "editor", "viewer"] - assert profile.tenant_role, "Tenant role should not be empty" + assert profile.tenant_role in valid_roles, f"Tenant role should be one of {valid_roles}, got {profile.tenant_role}"tests/api/test_auth_username.py-34-40 (1)
34-40: Test does not match its description: sendsnullinstead of omitting field.The test is named
test_change_username_missing_field, but passingNonetochange_username()sends{"new_username": null}(perapi_client.py:184-185), not a request with the field missing. To test a truly missing field, usesend_custom_requestwith an empty JSON body.Proposed fix
@allure.title("Change username with missing new_username") @allure.severity(allure.severity_level.NORMAL) def test_change_username_missing_field(self, authenticated_auth_client): - response = authenticated_auth_client.change_username(None) + response = authenticated_auth_client.send_custom_request( + "PUT", + "/v1/auth/username", + json={} + ) assert response.status_code == 400, f"Expected 400, got {response.status_code}" ErrorResponse(**response.json())tests/api/test_auth_username.py-225-232 (1)
225-232: Remove thelogin()call or pass credentials explicitly.Line 232 calls
authenticated_auth_client.login()without arguments. While thelogin()method has optional parameters (username: any = None, password: any = None), calling it without credentials will sendNonevalues to the API endpoint, which will likely fail. Either remove this line if it's unnecessary for the test, or passconfig.user_logandconfig.user_passas arguments.
🧹 Nitpick comments (6)
tests/api/test_auth_logout.py (2)
4-4: Unused imports.
LoginResponseandUserProfileare imported but never used in this file.Suggested fix
-from tests.api.schemas.auth_schemas import LogoutResponse, ErrorResponse, LoginResponse, UserProfile +from tests.api.schemas.auth_schemas import LogoutResponse, ErrorResponse
29-29: Missing Allure decorators on some test classes.
TestLogoutRefreshToken(line 29),TestLogoutRaceConditions(line 151), andTestLogoutQueryManipulation(lines 175-178) are missing@allure.featureand@allure.storydecorators, unlike other classes in this file.Also applies to: 151-151, 175-178
tests/api/test_auth_password.py (1)
51-54: Redundant status code check.Line 52 asserts that status code is 400, then line 53 checks
if response.status_code == 400before validating. The conditional is unnecessary since the assertion would fail first.Proposed fix
assert response.status_code == 400, f"Expected 400, got {response.status_code}" - if response.status_code == 400: - ErrorResponse(**response.json()) + ErrorResponse(**response.json())tests/api/test_auth_username.py (3)
205-213: Same bug: Headers modified but not applied correctly.The pattern here has the same issue as the previous test. Line 208 copies headers for restoration, but line 209 modifies the client headers directly. However, line 213 then restores the copy (which has original values), which is correct—but this is inconsistent with the pattern in the previous test.
For consistency and clarity, use the same save/modify/restore pattern across all tests.
215-223: Same pattern issue with header manipulation.Line 218 copies headers, line 219 modifies client headers, but line 223 restores the copy. This is actually correct behavior but inconsistent with line 197-203. Consider extracting a context manager or fixture for header manipulation to ensure consistent save/restore.
181-193: Address TODO: Generate expired token dynamically.The hardcoded
expired_token(flagged by static analysis as S105) is brittle and could become invalid if JWT format requirements change. Consider extendingutils/token_generator.pyto generate expired tokens with valid structure but past expiration timestamps.Would you like me to help implement an
generate_expired_token()function in the token generator?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
clients/api_client.pytests/api/test_auth_audit_log.pytests/api/test_auth_login.pytests/api/test_auth_logout.pytests/api/test_auth_password.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.pytests/api/test_auth_username.py
🧰 Additional context used
🧬 Code graph analysis (5)
tests/api/test_auth_username.py (4)
tests/api/schemas/auth_schemas.py (2)
UserProfile(6-14)ErrorResponse(33-34)utils/token_generator.py (1)
generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (3)
authenticated_auth_client(39-49)valid_username(65-66)valid_credentials(53-57)clients/api_client.py (3)
change_username(185-186)send_custom_request(86-100)login(169-170)
tests/api/test_auth_password.py (4)
tests/api/schemas/auth_schemas.py (2)
ChangePasswordResponse(37-38)ErrorResponse(33-34)fixtures/api_fixtures.py (5)
authenticated_auth_client(39-49)valid_credentials(53-57)valid_password(69-70)invalid_password(74-75)faker(7-8)clients/api_client.py (2)
change_password(182-183)send_custom_request(86-100)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)
tests/api/test_auth_audit_log.py (4)
tests/api/schemas/auth_schemas.py (2)
AuditLogResponse(51-55)ErrorResponse(33-34)utils/token_generator.py (1)
generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (4)
authenticated_auth_client(39-49)valid_credentials(53-57)valid_username(65-66)auth_client(32-35)clients/api_client.py (5)
get_audit_log(188-190)send_custom_request(86-100)logout(175-177)login(169-170)change_username(185-186)
tests/api/test_auth_login.py (3)
tests/api/schemas/auth_schemas.py (3)
LoginResponse(17-21)ErrorResponse(33-34)UserProfile(6-14)fixtures/api_fixtures.py (5)
valid_credentials(53-57)auth_client(32-35)invalid_username(61-62)invalid_password(74-75)invalid_credentials(79-83)clients/api_client.py (4)
login(169-170)refresh_token(172-173)post(46-57)get_profile(179-180)
clients/api_client.py (1)
config/settings.py (1)
Settings(6-78)
🪛 Gitleaks (8.30.0)
tests/api/test_auth_refresh.py
[high] 71-71: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 178-178: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 Ruff (0.14.10)
tests/api/test_auth_profile.py
109-109: Local variable valid_roles is assigned to but never used
Remove assignment to unused variable valid_roles
(F841)
146-146: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_username.py
186-186: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_password.py
106-106: Undefined name WEAK_PASSWORD_CASES
(F821)
158-158: Undefined name generate_invalid_refresh_tokens
(F821)
174-174: Undefined name base64
(F821)
187-187: Undefined name base64
(F821)
tests/api/test_auth_refresh.py
71-71: Possible hardcoded password assigned to: "expired_token"
(S105)
178-178: Possible hardcoded password assigned to: "expired_token"
(S105)
191-191: Undefined name base64
(F821)
201-201: Undefined name base64
(F821)
tests/api/test_auth_login.py
91-91: Undefined name valid_credentials
(F821)
91-91: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
91-91: Undefined name valid_credentials
(F821)
92-92: Undefined name valid_credentials
(F821)
92-92: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
92-92: Undefined name valid_credentials
(F821)
93-93: Undefined name valid_credentials
(F821)
93-93: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
93-93: Undefined name valid_credentials
(F821)
94-94: Undefined name valid_credentials
(F821)
94-94: Undefined name valid_credentials
(F821)
94-94: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
95-95: Undefined name valid_credentials
(F821)
95-95: Undefined name valid_credentials
(F821)
95-95: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
96-96: Undefined name valid_credentials
(F821)
96-96: Undefined name valid_credentials
(F821)
96-96: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
97-97: Undefined name valid_credentials
(F821)
97-97: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
97-97: Undefined name valid_credentials
(F821)
97-97: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
98-98: Undefined name valid_credentials
(F821)
98-98: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
98-98: Undefined name valid_credentials
(F821)
98-98: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
99-99: Undefined name valid_credentials
(F821)
99-99: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
99-99: Undefined name valid_credentials
(F821)
99-99: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
109-109: Undefined name valid_credentials
(F821)
clients/api_client.py
100-100: Avoid specifying long messages outside the exception class
(TRY003)
169-169: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
169-169: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (7)
tests/api/test_auth_refresh.py (1)
70-76: Hardcoded JWT is acceptable for expired token testing.The Gitleaks warning about the JWT is a false positive - this is an intentionally expired test token with dummy data (
"sub": "1234567890","name": "John Doe"). However, consider adding a comment explaining this is a crafted test token.tests/api/test_auth_logout.py (1)
147-149: Remove this comment—the tests are validating different scenarios.
test_logout_check_response_headersuses an authenticated client (logged-in state) whiletest_logout_check_cacheuses an unauthenticated client (no token). These are testing different conditions, not the same endpoint behavior. Different Cache-Control values for different authentication states is expected and correct.Likely an incorrect or invalid review comment.
tests/api/test_auth_password.py (1)
12-28: No action required - fixture mutation does not affect test isolation.The
valid_credentialsfixture is function-scoped (default scope when not explicitly specified), meaning each test function receives its own independent dictionary instance. The mutation ofvalid_credentials["password"]intest_change_password_successonly affects that specific test's copy and does not impact subsequent tests, as they receive fresh fixture instances. Test isolation is preserved.Likely an incorrect or invalid review comment.
tests/api/test_auth_username.py (4)
98-111: Verify whether these special characters should truly be invalid.Characters like
.,_,-, and digits are commonly allowed in usernames. Expecting400foruser.name,user_name,user-name,user123, and123userseems overly restrictive. This ties back to the TODO on line 14—clarify the actual username requirements with the API specification before finalizing these test expectations.
251-251:application/json; charset=utf-8may be a valid Content-Type.Including
application/json; charset=utf-8in the "wrong content type" list might be incorrect—many APIs accept this as valid JSON content type. Verify whether the API under test specifically rejects this format.
260-266: LGTM: Cache header validation.This test correctly validates that sensitive authentication endpoints return proper cache-control headers to prevent caching of responses.
18-27: Address the TODO comment on line 14 to clarify username validation requirements.The TODO on line 14 should be resolved before finalizing these tests to ensure username validation requirements are clearly documented.
Note: The mutation of
valid_credentialson line 25 (valid_credentials["username"] = valid_username) is not a test isolation issue since the fixture uses the default function scope, ensuring each test receives a fresh instance.Likely an incorrect or invalid review comment.
| import pytest | ||
| import allure | ||
| from pydantic import ValidationError | ||
| from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse | ||
| from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES | ||
|
|
There was a problem hiding this comment.
Missing imports: WEAK_PASSWORD_CASES, generate_invalid_refresh_tokens, and base64.
Static analysis confirms these names are undefined. The file needs additional imports.
Proposed fix
import pytest
import allure
+import base64
from pydantic import ValidationError
from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse
-from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES
+from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES, WEAK_PASSWORD_CASES
+from utils.token_generator import generate_invalid_refresh_tokens🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 1 - 6, The test file imports
are missing WEAK_PASSWORD_CASES, generate_invalid_refresh_tokens, and base64
which causes NameError/undefined symbol failures; add imports at the top of the
file: import base64, import WEAK_PASSWORD_CASES from its test data module (same
place as EMPTY_STRING_CASES/NONSTRING_CASES), and import
generate_invalid_refresh_tokens from the helper/case utilities where other
generator helpers live so the symbols WEAK_PASSWORD_CASES,
generate_invalid_refresh_tokens, and base64 are defined before they are used in
the tests.
| return response | ||
|
|
||
| @allure.step("Custom Request {method} {endpoint}") | ||
| def send_custom_request(self, method: str, endpoint: str, |
There was a problem hiding this comment.
methods like get(), post() accept optional headers, but send_custom_request() doesn't.
| @pytest.fixture | ||
| def invalid_username(): | ||
| return fake.email() | ||
| def authenticated_auth_client(config: Settings): |
There was a problem hiding this comment.
Multiple tests modify client.token, client.headers, call logout(), etc. With session scope, one test's modifications affect all subsequent tests.
There was a problem hiding this comment.
Basically after check with modified token/headers it returns to valid state, but in any case i'll add func scope.
|
|
||
| assert response.status_code == 200 | ||
| #TODO:Clarify expected cors headers | ||
| assert response.headers["Access-Control-Allow-Origin"] == "*" |
There was a problem hiding this comment.
Access-Control-Allow-Origin: * with Access-Control-Allow-Credentials: true is invalid
There was a problem hiding this comment.
It's just a stab, not real check. Thats why i left todo:
#TODO:Clarify expected cors headers
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @clients/api_client.py:
- Around line 186-187: The instance attribute self.refresh_token defined in
APIClient.__init__ is being shadowed by the method def refresh_token(self,
refresh_token: str) -> httpx.Response; rename the method (e.g., to
refresh_access_token or post_refresh_token) or rename the stored attribute
(e.g., to self._refresh_token or self.stored_refresh_token) so they do not
collide; update all internal references to the renamed symbol (e.g., calls to
client.refresh_token(...) -> client.refresh_access_token(...), or usages of the
stored token to self._refresh_token) and ensure the method still calls
self.post("/v1/auth/refresh", json={"refresh_token": refresh_token}).
In @tests/api/test_auth_audit_log.py:
- Around line 150-163: The test test_audit_log_entry_structure_with_user_actions
mutates the shared fixture valid_credentials by assigning
valid_credentials["username"] = valid_username; avoid mutating the fixture by
creating and using a local copy (e.g., creds = valid_credentials.copy()) and
update creds["username"] instead, then use creds for login; alternatively ensure
the valid_credentials fixture is function-scoped so each test gets a fresh
dict—pick one approach and apply it in the test function to prevent cross-test
pollution.
- Around line 190-198: The test modifies a local headers copy but calls
authenticated_auth_client.get_audit_log() before applying them; update
test_get_audit_log_with_wrong_auth_type to save the original =
authenticated_auth_client.headers, create the wrong-auth headers, assign
authenticated_auth_client.headers = headers before calling
authenticated_auth_client.get_audit_log(), perform the assertions and
ErrorResponse check, then restore authenticated_auth_client.headers = original
at the end so the client is returned to its prior state.
In @tests/api/test_auth_password.py:
- Around line 197-208: In test_change_password_with_too_long_access_token the
code mutates a local headers copy but never applies it to the client, so the
request uses the original headers; fix by saving the original_headers =
authenticated_auth_client.headers.copy(), then set
authenticated_auth_client.headers = headers (the modified copy with the long
Authorization) before calling authenticated_auth_client.change_password(...),
assert the response and parse ErrorResponse, and finally restore
authenticated_auth_client.headers = original_headers after the test.
- Around line 244-255: The test test_change_password_without_content_type
creates a local copy of headers and pops "Content-Type" but never applies it to
authenticated_auth_client, so the request still sends the original headers;
update the code to set authenticated_auth_client.headers = headers (or pass
headers=headers into authenticated_auth_client.change_password) before calling
change_password so the modified headers without "Content-Type" are actually
used, and ensure you restore the original headers after the request if needed.
- Around line 257-269: The test test_change_password_with_wrong_content_type
creates a copy of authenticated_auth_client.headers into headers and modifies
Content-Type but never applies it to authenticated_auth_client before calling
change_password, so the request uses the original headers; before calling
authenticated_auth_client.change_password set authenticated_auth_client.headers
= headers (and restore original headers afterwards if needed) so the modified
Content-Type is actually sent with the request.
- Around line 1-6: The test module is missing imports for WEAK_PASSWORD_CASES
and generate_invalid_refresh_tokens which causes undefined name errors; add
imports for WEAK_PASSWORD_CASES from the module that defines weak password test
cases (the same module that defines EMPTY_STRING_CASES / NONSTRING_CASES) and
import generate_invalid_refresh_tokens from its helper module (the module that
provides invalid token generators), then update the import block at the top of
the file to include these two names so the parametrize decorators referencing
WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens() resolve.
In @tests/api/test_auth_profile.py:
- Around line 206-215: The test test_get_profile_with_wrong_content_type
modifies a local headers copy but calls authenticated_auth_client.get_profile()
before applying them; assign the modified headers to
authenticated_auth_client.headers before calling
authenticated_auth_client.get_profile() so the request uses the intended
Content-Type, and (optionally) restore the original headers after the request to
avoid side effects.
- Around line 101-111: The test test_profile_tenant_role defines valid_roles but
never uses it; update the assertions in that test to validate tenant_role
membership instead of only non-empty: keep or replace the current assert
profile.tenant_role check with an assertion like assert profile.tenant_role in
valid_roles (and include a helpful message), ensuring you reference the
valid_roles list and the UserProfile instance (profile.tenant_role) when making
the assertion.
- Around line 130-139: test_get_profile_invalid_token mutates
authenticated_auth_client.token without restoring it which can leak state across
parametrized iterations; capture the original token before setting
authenticated_auth_client.token = invalid_token and ensure you restore it after
the request (e.g., in a finally block) so authenticated_auth_client.token is
reset for subsequent iterations and cleanup; reference the test function
test_get_profile_invalid_token and the authenticated_auth_client.token attribute
when implementing the fix.
In @tests/api/test_auth_refresh.py:
- Around line 242-251: The test test_refresh_with_wrong_content_type copies and
mutates headers but never applies them to the client before calling
authenticated_auth_client.refresh_token, so the request uses the original
headers; fix by assigning the mutated headers back to
authenticated_auth_client.headers before invoking refresh_token on
authenticated_auth_client.refresh_token, then (as the test already does) restore
the original headers afterward to avoid side effects.
- Around line 198-206: The test test_refresh_with_wrong_auth_format modifies a
local headers copy but never applies it to the client before calling
authenticated_auth_client.refresh_token, so the request still uses the original
headers; fix by assigning the modified headers back to
authenticated_auth_client.headers (or passing headers into refresh_token if that
method accepts a headers param) before invoking
authenticated_auth_client.refresh_token(authenticated_auth_client.refresh_token)
so the "Authorization" header with the base64 token is actually used.
🧹 Nitpick comments (3)
clients/api_client.py (1)
99-114: Missingheadersparameter forwarding insend_custom_request.The method accepts
jsonandparamsbut doesn't accept or forward theheadersparameter to the delegated methods. This limits flexibility when callers need custom headers.♻️ Suggested fix
@allure.step("Custom Request {method} {endpoint}") def send_custom_request(self, method: str, endpoint: str, json: Optional[Dict[str, Any]] = None, - params: Optional[Dict[str, Any]] = None) -> httpx.Response: + params: Optional[Dict[str, Any]] = None, + headers: Optional[Dict[str, str]] = None) -> httpx.Response: if method.upper() == "GET": - return self.get(endpoint, params=params) + return self.get(endpoint, params=params, headers=headers) elif method.upper() == "POST": - return self.post(endpoint, json=json) + return self.post(endpoint, json=json, headers=headers) elif method.upper() == "PUT": - return self.put(endpoint, json=json) + return self.put(endpoint, json=json, headers=headers) elif method.upper() == "PATCH": - return self.patch(endpoint, json=json) + return self.patch(endpoint, json=json, headers=headers) elif method.upper() == "DELETE": - return self.delete(endpoint) + return self.delete(endpoint, headers=headers) else: raise ValueError(f"Unsupported HTTP method: {method}")tests/api/test_auth_password.py (1)
53-55: Redundant conditional check after assertion.The
if response.status_code == 400check on line 54 is redundant because the assertion on line 53 already guarantees the status code is 400.♻️ Suggested fix
assert response.status_code == 400, f"Expected 400, got {response.status_code}" - if response.status_code == 400: - ErrorResponse(**response.json()) + ErrorResponse(**response.json())tests/api/test_auth_refresh.py (1)
79-100: Consider using token-based differentiation instead oftime.sleep.The
time.sleep(1)on line 91 assumes tokens will differ due to timestamp-based claims. This is fragile and slows down tests. If tokens include ajti(JWT ID) claim, they should already be unique without needing a delay.If time-based differentiation is required by the token implementation, consider mocking time instead of sleeping in tests.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
clients/api_client.pytests/api/test_auth_audit_log.pytests/api/test_auth_password.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T13:38:40.404Z
Learnt from: 0xc7c5
Repo: chainstack/cp-autotests PR: 1
File: tests/api/test_auth_refresh.py:187-195
Timestamp: 2026-01-12T13:38:40.404Z
Learning: In tests/api/test_auth_refresh.py, the header manipulation pattern `headers = client.headers.copy()` followed by `client.headers["Authorization"] = ...` and later `client.headers = headers` is correct: it backs up, modifies the client's headers directly, then restores them.
Applied to files:
clients/api_client.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.pytests/api/test_auth_audit_log.pytests/api/test_auth_password.py
🧬 Code graph analysis (4)
clients/api_client.py (1)
config/settings.py (1)
Settings(6-78)
tests/api/test_auth_profile.py (3)
tests/api/schemas/auth_schemas.py (3)
UserProfile(6-14)ErrorResponse(33-34)LoginResponse(17-21)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-49)valid_credentials(53-57)clients/api_client.py (3)
get_profile(193-194)login(183-184)logout(189-191)
tests/api/test_auth_refresh.py (4)
tests/api/schemas/auth_schemas.py (4)
LoginResponse(17-21)RefreshTokenResponse(24-26)ErrorResponse(33-34)UserProfile(6-14)utils/token_generator.py (2)
generate_invalid_refresh_tokens(86-167)generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (3)
auth_client(32-35)valid_credentials(53-57)authenticated_auth_client(39-49)clients/api_client.py (5)
login(183-184)refresh_token(186-187)post(46-57)get_profile(193-194)logout(189-191)
tests/api/test_auth_password.py (4)
tests/api/schemas/auth_schemas.py (2)
ChangePasswordResponse(37-38)ErrorResponse(33-34)fixtures/api_fixtures.py (4)
authenticated_auth_client(39-49)valid_credentials(53-57)valid_password(69-70)faker(7-8)clients/api_client.py (2)
change_password(196-197)send_custom_request(100-114)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)
🪛 Gitleaks (8.30.0)
tests/api/test_auth_profile.py
[high] 146-146: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
tests/api/test_auth_refresh.py
[high] 72-72: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 179-179: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 Ruff (0.14.10)
clients/api_client.py
114-114: Avoid specifying long messages outside the exception class
(TRY003)
tests/api/test_auth_profile.py
109-109: Local variable valid_roles is assigned to but never used
Remove assignment to unused variable valid_roles
(F841)
146-146: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_refresh.py
72-72: Possible hardcoded password assigned to: "expired_token"
(S105)
179-179: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_password.py
107-107: Undefined name WEAK_PASSWORD_CASES
(F821)
159-159: Undefined name generate_invalid_refresh_tokens
(F821)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (13)
clients/api_client.py (3)
85-97: LGTM!The
patchmethod is consistent with the other HTTP method implementations, properly using Allure steps for reporting and following the same header handling pattern.
116-124: LGTM!Cookie management methods are cleanly implemented and provide useful utilities for test scenarios requiring cookie manipulation.
174-204: All endpoint paths and HTTP methods inAuthAPIClientcorrectly match the OpenAPI specification—no changes needed.tests/api/test_auth_audit_log.py (4)
14-30: LGTM!The general audit log test properly validates response schema and pagination defaults. Good use of Pydantic for schema validation.
49-58: Verify expected behavior for values exceeding 64-bit integer range.Testing
MAX_64_BIT_INT+1may cause integer overflow or API rejection depending on the backend. The assertionaudit_log.page == pageassumes the API accepts and echoes back overflow values, which seems unlikely.Consider whether the expected behavior is a 400 error for out-of-range values rather than a successful 200.
229-236: LGTM!Good test coverage for response header validation ensuring sensitive data isn't cached.
200-208: Bug: Modified headers not applied to client before request.Same issue as
test_get_audit_log_with_wrong_auth_type. The headers dict copy is modified but the client's actual headers remain unchanged during the request.🐛 Proposed fix
def test_get_audit_log_with_wrong_auth_format(self, authenticated_auth_client): headers = authenticated_auth_client.headers.copy() authenticated_auth_client.headers["Authorization"] = "Bearer " + base64.b64encode(authenticated_auth_client.token.encode()).decode() response = authenticated_auth_client.get_audit_log() assert response.status_code == 401, f"Expected 401, got {response.status_code}" ErrorResponse(**response.json()) authenticated_auth_client.headers = headersThis one is already correct per the learned pattern - disregard this comment if the code at line 204 shows
authenticated_auth_client.headers["Authorization"]being modified directly.⛔ Skipped due to learnings
Learnt from: 0xc7c5 Repo: chainstack/cp-autotests PR: 1 File: tests/api/test_auth_refresh.py:187-195 Timestamp: 2026-01-12T13:38:40.404Z Learning: In tests/api/test_auth_refresh.py, the header manipulation pattern `headers = client.headers.copy()` followed by `client.headers["Authorization"] = ...` and later `client.headers = headers` is correct: it backs up, modifies the client's headers directly, then restores them.tests/api/test_auth_profile.py (2)
141-153: Hardcoded JWT is acceptable for expired token testing.The Gitleaks warning is a false positive in this context—this is an intentionally expired/invalid JWT for testing purposes. The TODO comment indicates plans to generate these dynamically.
196-204: Verify that GET requests require Content-Type header.It's unusual for a server to require
Content-Typeon GET requests since they typically don't have a body. This test expects a 400 when Content-Type is missing.Confirm this is the intended API behavior. If the API doesn't actually require Content-Type for GET requests, this test will fail or test incorrect behavior.
tests/api/test_auth_refresh.py (4)
14-35: LGTM!Comprehensive test for successful token refresh with proper schema validation and verification that the new access token differs from the original.
67-77: Hardcoded expired JWT is acceptable for testing.The Gitleaks warning is a false positive—this is an intentionally expired JWT for testing the expired token scenario. The TODO comment indicates plans to generate these dynamically.
124-142: Verify expected behavior: old access tokens invalidated after refresh.This test assumes the API invalidates old access tokens when a new token is issued via refresh. This is not universal behavior—some APIs allow both old and new tokens to remain valid until expiration.
Confirm with the API specification whether token refresh should invalidate the previous access token. If not, this test's expectation of 401 would be incorrect.
253-259: LGTM!Good test for verifying cache-control headers on token refresh responses to prevent sensitive token data from being cached.
| import pytest | ||
| import allure | ||
| from pydantic import ValidationError | ||
| from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse | ||
| from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES | ||
| import base64 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if WEAK_PASSWORD_CASES exists in test_cases
echo "=== Searching for WEAK_PASSWORD_CASES ==="
rg "WEAK_PASSWORD_CASES" tests/api/cases/ 2>/dev/null | head -20
echo ""
echo "=== Searching for generate_invalid_refresh_tokens ==="
rg "generate_invalid_refresh_tokens" --glob="*.py" 2>/dev/null | head -20
echo ""
echo "=== Checking test_auth_password.py line 107 context ==="
sed -n '100,115p' tests/api/test_auth_password.py 2>/dev/null
echo ""
echo "=== Checking test_auth_password.py line 159 context ==="
sed -n '155,165p' tests/api/test_auth_password.py 2>/dev/null
echo ""
echo "=== Checking imports in test_auth_password.py ==="
head -20 tests/api/test_auth_password.py 2>/dev/nullRepository: chainstack/cp-autotests
Length of output: 3322
Add missing imports WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens.
Static analysis correctly identifies undefined names causing test collection failure:
- Line 107 uses
WEAK_PASSWORD_CASESin parametrize decorator - Line 159 uses
generate_invalid_refresh_tokens()in parametrize decorator
Both are defined in their respective modules but not imported.
🐛 Proposed fix
import pytest
import allure
from pydantic import ValidationError
from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse
-from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES
+from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES, WEAK_PASSWORD_CASES
+from utils.token_generator import generate_invalid_refresh_tokens
import base64🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 1 - 6, The test module is
missing imports for WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens
which causes undefined name errors; add imports for WEAK_PASSWORD_CASES from the
module that defines weak password test cases (the same module that defines
EMPTY_STRING_CASES / NONSTRING_CASES) and import generate_invalid_refresh_tokens
from its helper module (the module that provides invalid token generators), then
update the import block at the top of the file to include these two names so the
parametrize decorators referencing WEAK_PASSWORD_CASES and
generate_invalid_refresh_tokens() resolve.
There was a problem hiding this comment.
Thats because such entities will be implemented when credentials requirements be clarified.
There was a problem hiding this comment.
@0xc7c5, understood! Since these will be implemented once the credentials requirements are clarified, you might want to consider:
- Adding TODO comments near lines 107 and 159 to document this intention
- Potentially skipping or marking these specific test methods with
@pytest.mark.skip(reason="Pending credentials requirements")until the implementations are ready
This will prevent test collection failures in the meantime while preserving the test structure for future implementation.
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In @fixtures/api_fixtures.py:
- Around line 38-49: The fixture authenticated_auth_client silently continues
when client.login fails, leaving tests to hit confusing 401s; change it to fail
fast by checking the login response (from client.login called in
authenticated_auth_client) and raise an explicit error (e.g., pytest.fail or
RuntimeError) or log and raise if response.status_code != 200 or
token/refresh_token are missing, ensuring the client is closed before raising so
resources are cleaned up.
In @tests/api/test_auth_audit_log.py:
- Around line 220-227: The test calls authenticated_auth_client.login() with no
credentials after logout which will fail; update the
test_get_audit_log_with_revoked_access_token to restore authentication properly
by calling authenticated_auth_client.login(...) with valid credentials (e.g.,
pass the same username/password used to create the fixture) or by using a helper
on the client that reuses stored credentials (e.g.,
authenticated_auth_client.reauthenticate() or
authenticated_auth_client.login(saved_username, saved_password)), so the client
is properly authenticated again for subsequent tests.
- Around line 150-163: The test mutates the shared valid_credentials fixture and
calls authenticated_auth_client.login() without explicit credentials; instead,
avoid mutating the fixture by creating a local copy (e.g., creds =
dict(valid_credentials)) and set creds["username"] = valid_username, use creds
when calling authenticated_auth_client.login(creds["username"],
creds["password"]) after logout, and keep change_username(valid_username) as-is;
update any subsequent references to use the local creds rather than modifying
valid_credentials.
In @tests/api/test_auth_password.py:
- Line 171: Fix the typo in the test's Allure title decorator: update the
@allure.title string on the test in tests/api/test_auth_password.py from "Change
password with with wrong auth type" to "Change password with wrong auth type"
(locate the @allure.title decorator for the test and remove the duplicate
"with").
- Around line 114-115: The assertion message in tests/api/test_auth_password.py
is inconsistent: it claims "Expected 400 or 501" but the assertion only checks
for 400; update the test to either (a) change the assertion message to "Expected
400" to match the current check, or (b) expand the assertion to allow both
status codes (e.g., assert response.status_code in (400, 501)) and keep the
existing message; ensure you still call ErrorResponse(**response.json()) against
the response object so the error payload is validated.
- Around line 43-44: The assertion in the test uses "response.status_code== 400"
with no space before the equality operator; update the assertion to use proper
spacing around the == operator for readability (modify the assertion referencing
response.status_code in tests/api/test_auth_password.py) while leaving the
subsequent ErrorResponse(**response.json()) line unchanged.
- Around line 246-255: The test_change_password_without_content_type test copies
headers to a local variable and pops Content-Type from the copy but never
applies it to authenticated_auth_client, so change_password() still sends the
original headers; fix by saving the original headers (e.g., orig =
authenticated_auth_client.headers.copy()), then remove "Content-Type" directly
from authenticated_auth_client.headers before calling
authenticated_auth_client.change_password(...), assert the response, and finally
restore the original headers by assigning orig back to
authenticated_auth_client.headers.
- Around line 1-6: The test file is missing two imports which cause NameError at
runtime: import WEAK_PASSWORD_CASES from the test cases module (where
EMPTY_STRING_CASES and NONSTRING_CASES are imported) and import
generate_invalid_refresh_tokens from the token generator module (where the
refresh token helpers live); add these two imports at the top of
tests/api/test_auth_password.py and verify WEAK_PASSWORD_CASES is defined in the
test_cases module (if not, define it there) so references to WEAK_PASSWORD_CASES
and generate_invalid_refresh_tokens used in the tests resolve.
- Around line 260-269: The test_change_password_with_wrong_content_type test
modifies a local headers copy but never applies it to the client or the request,
so the wrong Content-Type is not actually sent; update the test to either set
authenticated_auth_client.headers = headers before calling
authenticated_auth_client.change_password or pass headers=headers into
authenticated_auth_client.change_password so the modified headers
(headers["Content-Type"] = content_type) are used for the request and then
restore the original headers after the call.
In @tests/api/test_auth_profile.py:
- Around line 185-192: The test calls authenticated_auth_client.login() with no
arguments which sends None credentials and fails; update
test_get_profile_with_revoked_access_token so the re-login supplies valid
credentials (e.g., call authenticated_auth_client.login(username, password) with
the fixture or module test credentials or use the fixture's provided reauth
helper if available) after ErrorResponse(**response.json()) to restore a valid
session instead of calling login() with no args.
- Line 5: The test imports a non-existent symbol generate_invalid_tokens;
replace that import with the actual function name generate_invalid_bearer_tokens
from utils.token_generator and update any call sites in
tests/api/test_auth_profile.py that reference generate_invalid_tokens to call
generate_invalid_bearer_tokens instead so the test imports and uses the correct
function name.
In @tests/api/test_auth_refresh.py:
- Around line 218-225: The test test_refresh_with_revoked_refresh_token calls
authenticated_auth_client.login() with no args after logout which will fail;
update the teardown to re-authenticate by calling login with credentials (e.g.,
authenticated_auth_client.login(authenticated_auth_client.username,
authenticated_auth_client.password)) or recreate the fixture before finishing
the test so post-logout re-login supplies the required credentials instead of a
no-arg call.
- Around line 198-206: The test modifies a local headers copy but never applies
it before calling post_refresh, so it doesn't exercise the wrong auth format;
update authenticated_auth_client.headers with the modified headers (or pass
headers into authenticated_auth_client.post_refresh if that method accepts a
headers param) before calling post_refresh in
test_refresh_with_wrong_auth_format, and restore the original headers after the
request to avoid side effects on other tests.
🧹 Nitpick comments (4)
fixtures/api_fixtures.py (1)
68-75:valid_passwordandinvalid_passworduse identical generation logic.Both fixtures call
faker.password(), making the naming distinction misleading. If these are intended to represent passwords that pass/fail validation rules, consider differentiating them (e.g., by length, character requirements).tests/api/test_auth_audit_log.py (1)
49-58: 64-bit integer overflow test may have unexpected behavior.
MAX_64_BIT_INT+1exceeds the maximum signed 64-bit integer. The test asserts that the response echoes these values back (audit_log.page == page), but integer overflow handling varies by backend. If the server clamps or rejects overflow values, this assertion will fail.Consider separating overflow cases into a distinct test expecting 400/422 errors.
tests/api/test_auth_refresh.py (1)
79-100:time.sleep(1)may be insufficient or unnecessary.The sleep ensures tokens have different timestamps, but:
- If
iathas second precision, 1 second should suffice- If the API uses sub-second precision or includes randomness, sleep is unnecessary
- This adds test latency
Consider verifying if the sleep is actually needed based on token generation behavior.
tests/api/test_auth_password.py (1)
53-55: Redundant conditional check after assertion.The
if response.status_code == 400:guard is unnecessary—if the assertion on line 53 passes, the condition is always true; if it fails, this code is never reached.Proposed fix
assert response.status_code == 400, f"Expected 400, got {response.status_code}" - if response.status_code == 400: - ErrorResponse(**response.json()) + ErrorResponse(**response.json())
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
clients/api_client.pyfixtures/api_fixtures.pytests/api/test_auth_audit_log.pytests/api/test_auth_password.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T13:38:40.404Z
Learnt from: 0xc7c5
Repo: chainstack/cp-autotests PR: 1
File: tests/api/test_auth_refresh.py:187-195
Timestamp: 2026-01-12T13:38:40.404Z
Learning: In tests/api/test_auth_refresh.py, the header manipulation pattern `headers = client.headers.copy()` followed by `client.headers["Authorization"] = ...` and later `client.headers = headers` is correct: it backs up, modifies the client's headers directly, then restores them.
Applied to files:
fixtures/api_fixtures.pytests/api/test_auth_profile.pyclients/api_client.pytests/api/test_auth_audit_log.pytests/api/test_auth_refresh.pytests/api/test_auth_password.py
🧬 Code graph analysis (5)
fixtures/api_fixtures.py (2)
clients/api_client.py (3)
AuthAPIClient(175-205)login(184-185)get(32-43)config/settings.py (1)
Settings(6-78)
tests/api/test_auth_profile.py (3)
tests/api/schemas/auth_schemas.py (3)
UserProfile(6-14)ErrorResponse(33-34)LoginResponse(17-21)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-49)valid_credentials(53-57)clients/api_client.py (2)
get_profile(194-195)login(184-185)
tests/api/test_auth_audit_log.py (4)
tests/api/schemas/auth_schemas.py (2)
AuditLogResponse(51-55)ErrorResponse(33-34)utils/token_generator.py (1)
generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (3)
authenticated_auth_client(39-49)valid_credentials(53-57)auth_client(32-35)clients/api_client.py (6)
get_audit_log(203-205)send_custom_request(100-115)logout(190-192)login(184-185)change_username(200-201)get_profile(194-195)
tests/api/test_auth_refresh.py (4)
tests/api/schemas/auth_schemas.py (4)
LoginResponse(17-21)RefreshTokenResponse(24-26)ErrorResponse(33-34)UserProfile(6-14)utils/token_generator.py (2)
generate_invalid_refresh_tokens(86-167)generate_invalid_bearer_tokens(9-84)fixtures/api_fixtures.py (3)
auth_client(32-35)valid_credentials(53-57)authenticated_auth_client(39-49)clients/api_client.py (5)
login(184-185)post_refresh(187-188)post(46-57)get_profile(194-195)logout(190-192)
tests/api/test_auth_password.py (4)
tests/api/schemas/auth_schemas.py (2)
ChangePasswordResponse(37-38)ErrorResponse(33-34)fixtures/api_fixtures.py (5)
authenticated_auth_client(39-49)valid_credentials(53-57)valid_password(69-70)invalid_password(74-75)faker(7-8)clients/api_client.py (2)
change_password(197-198)send_custom_request(100-115)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)
🪛 Gitleaks (8.30.0)
tests/api/test_auth_refresh.py
[high] 72-72: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 179-179: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 Ruff (0.14.10)
tests/api/test_auth_profile.py
146-146: Possible hardcoded password assigned to: "expired_token"
(S105)
clients/api_client.py
115-115: Avoid specifying long messages outside the exception class
(TRY003)
tests/api/test_auth_refresh.py
72-72: Possible hardcoded password assigned to: "expired_token"
(S105)
179-179: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_password.py
107-107: Undefined name WEAK_PASSWORD_CASES
(F821)
159-159: Undefined name generate_invalid_refresh_tokens
(F821)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (17)
fixtures/api_fixtures.py (2)
52-57: LGTM!The
valid_credentialsfixture correctly sources credentials from config.
6-8: LGTM!Session-scoped faker fixture is appropriate for consistent test data generation.
clients/api_client.py (4)
85-97: LGTM!The
patchmethod follows the same pattern as other HTTP methods with proper Allure integration.
99-115: LGTM with minor note.The
send_custom_requestmethod provides a clean dispatch mechanism. The static analysis hint (TRY003) about the long exception message is a style nit that can be safely ignored here since the message is contextually appropriate for debugging.
175-205: LGTM!
AuthAPIClientis well-structured with clear endpoint mappings. The methods appropriately delegate to the base class HTTP methods.
9-15: This concern is unfounded —self.headerscaching does not cause authentication issues.All HTTP methods (
get,post,put,delete,patch) call_get_headers()fresh on each request, not the cachedself.headers. Theself.headersattribute exists solely for tests to directly manipulate headers via the backup/restore pattern (as correctly shown intest_auth_refresh.py), and does not affect actual authentication or API calls. No changes needed.Likely an incorrect or invalid review comment.
tests/api/test_auth_audit_log.py (2)
14-30: LGTM!Well-structured smoke test with proper schema validation and meaningful assertions.
168-186: LGTM!Good coverage of authentication edge cases. The header backup/restore pattern is correct per retrieved learnings.
tests/api/test_auth_profile.py (3)
141-153: Hardcoded expired JWT is acceptable for test purposes.The static analysis flag (S105) about hardcoded password is a false positive — this is an intentionally invalid/expired test token, not a real credential. The TODO comment indicates plans to generate these dynamically.
12-31: LGTM!Good smoke test coverage with schema validation. The commented-out tenant assertions (lines 27-29) suggest these fields may be optional or not yet implemented.
196-215: Verify API design: GET requests require Content-Type header.These tests assert that GET
/v1/auth/profilereturns 400 whenContent-Typeis missing or incorrect. This is unusual—GET requests typically don't requireContent-Typesince they have no body (per RFC 7231). However, the API client and multiple endpoints in the suite (POST /v1/auth/login,POST /v1/auth/logout,POST /v1/auth/refresh,PUT /v1/auth/password) all enforce this validation universally. If this is intentional API design, consider documenting it in the OpenAPI spec. If not, reconsider whetherContent-Typevalidation is necessary for GET requests.tests/api/test_auth_refresh.py (3)
67-77: Hardcoded expired JWT is acceptable for tests.The static analysis (S105) and Gitleaks flags are false positives — these are intentionally invalid test tokens with a TODO to generate them dynamically. Same applies to line 179.
124-142: Verify that old access tokens are invalidated after refresh.This test assumes the API invalidates old access tokens upon refresh (token rotation). This is a security best practice but not universal. If the API allows multiple valid access tokens concurrently, this test will fail.
14-35: LGTM!Solid smoke test for token refresh flow with proper schema validation.
tests/api/test_auth_password.py (3)
17-29: LGTM with a note on credential state.The test correctly validates the password change flow and updates
valid_credentials["password"]to track the new password. Be aware that ifvalid_credentialsfixture has session/module scope, this mutation could affect subsequent tests.
206-208: Potential failure when parsing 431 response body.HTTP 431 (Request Header Fields Too Large) responses often don't include a JSON body—the server may close the connection immediately. The
ErrorResponse(**response.json())call could fail with a JSON decode error.Consider guarding the schema validation:
assert response.status_code == 431, f"Expected 431, got {response.status_code}" - ErrorResponse(**response.json()) + if response.content: + ErrorResponse(**response.json())
271-279: Inconsistent Cache-Control header expectations.
test_change_password_check_response_headers(line 218) expectsCache-Control: "no-store", while this test expectsCache-Control: "no-cache, no-store, must-revalidate". Both test the same successful password change endpoint. Verify the actual API response and align the assertions.
| assert response.status_code == 400, f"Expected 400 or 501, got {response.status_code}" | ||
| ErrorResponse(**response.json()) |
There was a problem hiding this comment.
Assertion message inconsistent with actual check.
The message says "Expected 400 or 501" but the assertion only checks for 400. Update either the message or the assertion logic.
Option 1: Fix message
- assert response.status_code == 400, f"Expected 400 or 501, got {response.status_code}"
+ assert response.status_code == 400, f"Expected 400, got {response.status_code}"Option 2: Allow both status codes
- assert response.status_code == 400, f"Expected 400 or 501, got {response.status_code}"
+ assert response.status_code in (400, 501), f"Expected 400 or 501, got {response.status_code}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert response.status_code == 400, f"Expected 400 or 501, got {response.status_code}" | |
| ErrorResponse(**response.json()) | |
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | |
| ErrorResponse(**response.json()) |
| assert response.status_code == 400, f"Expected 400 or 501, got {response.status_code}" | |
| ErrorResponse(**response.json()) | |
| assert response.status_code in (400, 501), f"Expected 400 or 501, got {response.status_code}" | |
| ErrorResponse(**response.json()) |
🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 114 - 115, The assertion
message in tests/api/test_auth_password.py is inconsistent: it claims "Expected
400 or 501" but the assertion only checks for 400; update the test to either (a)
change the assertion message to "Expected 400" to match the current check, or
(b) expand the assertion to allow both status codes (e.g., assert
response.status_code in (400, 501)) and keep the existing message; ensure you
still call ErrorResponse(**response.json()) against the response object so the
error payload is validated.
| ErrorResponse(**logout_response.json()) | ||
| authenticated_auth_client.token = token | ||
|
|
||
| @allure.title("Change password with with wrong auth type") |
There was a problem hiding this comment.
Typo: Double "with" in title.
- @allure.title("Change password with with wrong auth type")
+ @allure.title("Change password with wrong auth type")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @allure.title("Change password with with wrong auth type") | |
| @allure.title("Change password with wrong auth type") |
🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py at line 171, Fix the typo in the test's
Allure title decorator: update the @allure.title string on the test in
tests/api/test_auth_password.py from "Change password with with wrong auth type"
to "Change password with wrong auth type" (locate the @allure.title decorator
for the test and remove the duplicate "with").
| def test_change_password_without_content_type(self, authenticated_auth_client, valid_credentials, valid_password): | ||
| headers = authenticated_auth_client.headers.copy() | ||
| headers.pop("Content-Type") | ||
| response = authenticated_auth_client.change_password( | ||
| old_password=valid_credentials["password"], | ||
| new_password=valid_password | ||
| ) | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.headers = headers |
There was a problem hiding this comment.
Header modification is never applied to the request.
The test copies headers to a local variable, modifies the copy, but never applies it. The change_password() call still uses the client's original headers. The header restoration at line 255 also restores headers that were never changed on the client.
Proposed fix: Modify client headers directly
def test_change_password_without_content_type(self, authenticated_auth_client, valid_credentials, valid_password):
headers = authenticated_auth_client.headers.copy()
- headers.pop("Content-Type")
+ authenticated_auth_client.headers.pop("Content-Type", None)
response = authenticated_auth_client.change_password(
old_password=valid_credentials["password"],
new_password=valid_password
)
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
authenticated_auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 246 - 255, The
test_change_password_without_content_type test copies headers to a local
variable and pops Content-Type from the copy but never applies it to
authenticated_auth_client, so change_password() still sends the original
headers; fix by saving the original headers (e.g., orig =
authenticated_auth_client.headers.copy()), then remove "Content-Type" directly
from authenticated_auth_client.headers before calling
authenticated_auth_client.change_password(...), assert the response, and finally
restore the original headers by assigning orig back to
authenticated_auth_client.headers.
| import allure | ||
| from pydantic import ValidationError | ||
| from tests.api.schemas.auth_schemas import UserProfile, ErrorResponse | ||
| from utils.token_generator import generate_invalid_tokens |
There was a problem hiding this comment.
Import error: generate_invalid_tokens does not exist.
The import references generate_invalid_tokens, but per utils/token_generator.py (lines 8-83 in relevant snippets), the actual function is named generate_invalid_bearer_tokens. This will cause an ImportError at test collection time.
Proposed fix
-from utils.token_generator import generate_invalid_tokens
+from utils.token_generator import generate_invalid_bearer_tokensAlso update line 132:
- @pytest.mark.parametrize("invalid_token", generate_invalid_tokens())
+ @pytest.mark.parametrize("invalid_token", generate_invalid_bearer_tokens())🤖 Prompt for AI Agents
In @tests/api/test_auth_profile.py at line 5, The test imports a non-existent
symbol generate_invalid_tokens; replace that import with the actual function
name generate_invalid_bearer_tokens from utils.token_generator and update any
call sites in tests/api/test_auth_profile.py that reference
generate_invalid_tokens to call generate_invalid_bearer_tokens instead so the
test imports and uses the correct function name.
There was a problem hiding this comment.
Will be implemented later.
There was a problem hiding this comment.
@0xc7c5, understood. Since this import error will prevent the tests from running, would you like me to create an issue to track this fix for a follow-up PR?
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In @tests/api/test_auth_login.py:
- Around line 146-152: The test test_login_missing_password is passing
password=valid_credentials["username"] to auth_client.login (testing missing
username) instead of omitting the password; update the call in
test_login_missing_password to call auth_client.login without the password
argument (or explicitly with password=None/omit) while still providing the
username from valid_credentials so the request simulates a missing password,
then keep the existing assertions (status_code == 400 and
ErrorResponse(**response.json())).
- Around line 22-33: The f-strings in stripcase_valid_credentials reuse double
quotes for both the f-string and the dict key access, which is a Python <3.12
syntax error; fix by changing the inner key access to use single quotes (e.g.,
valid_credentials['username'] and valid_credentials['password']) or assign
username/password to local variables at the top of stripcase_valid_credentials
and use those in the f-strings; update every tuple entry in
stripcase_valid_credentials to use the corrected references.
- Around line 205-213: In test_login_without_content_type the local headers copy
is modified but not applied to the client, so test_login_without_content_type
should either set auth_client.headers = headers before calling auth_client.login
or pass the modified headers into auth_client.login, then perform the request
and finally restore the original headers (e.g., reassign the saved original
headers back to auth_client.headers) to avoid side effects; update references in
the test to use auth_client.headers (or the explicit headers arg) so the
Content-Type is actually removed during the login call.
- Around line 215-224: In test_login_with_wrong_content_type the local headers
copy is modified but never applied to the request; before calling
auth_client.login in test_login_with_wrong_content_type either set
auth_client.headers = headers so the client uses the modified Content-Type or
(if auth_client.login accepts per-request headers) pass headers=headers into
auth_client.login(username, password, headers=headers); ensure you still restore
auth_client.headers to its original value afterward if you mutate the client.
In @tests/api/test_auth_password.py:
- Around line 1-6: The test file references WEAK_PASSWORD_CASES and
generate_invalid_refresh_tokens but never imports them, causing NameError; add
an import for WEAK_PASSWORD_CASES (same place as
EMPTY_STRING_CASES/NONSTRING_CASES — e.g., from tests.api.cases.test_cases
import WEAK_PASSWORD_CASES) and add an import for
generate_invalid_refresh_tokens from the module that defines token helpers
(where helper functions live in the test suite, e.g.,
tests.api.utils.token_helpers or the appropriate tests.api.cases helper module)
so both symbols are available to the tests.
- Around line 244-255: The test test_change_password_without_content_type
currently pops "Content-Type" from a local headers copy but never applies it to
authenticated_auth_client, so the request still includes the header; update the
test to remove the header directly on the client (e.g.,
authenticated_auth_client.headers.pop("Content-Type") before calling
change_password) and then restore the original headers afterward (or assign
authenticated_auth_client.headers = headers after popping) so the client
actually sends the request without Content-Type.
- Around line 257-269: The test modifies a local copy of headers but never
applies it to the client, so authenticated_auth_client.change_password still
uses the original headers; set authenticated_auth_client.headers = headers
before calling change_password (or alternatively pass headers into
change_password if that helper accepts a headers param) in
test_change_password_with_wrong_content_type so the modified Content-Type is
actually sent with the request.
- Around line 271-280: The two tests for the change-password endpoint have
mismatched expectations for the Cache-Control header
(test_change_password_check_cache expects "no-cache, no-store, must-revalidate"
while test_change_password_check_response_headers expects "no-store"); confirm
the API's actual Cache-Control value by inspecting the response from
authenticated_auth_client.change_password and then update both tests to assert
the same, correct header value (and keep the Pragma assertion as-is if it is
correct); locate the assertions in test_change_password_check_cache and
test_change_password_check_response_headers and replace the hardcoded expected
Cache-Control string in both with the verified canonical value.
In @tests/api/test_auth_profile.py:
- Around line 206-215: The test creates a modified headers dict but never
applies it to the client before calling get_profile, so the request still uses
the original headers; set authenticated_auth_client.headers = headers before
calling authenticated_auth_client.get_profile() and then restore the original
headers after the assertion (or assign headers back only for the request and
revert to previous headers) to ensure test_get_profile_with_wrong_content_type
actually sends the wrong Content-Type to get_profile.
- Around line 185-192: The test leaves authenticated_auth_client in an
inconsistent state because after calling logout() and then login(...) the
client's token and refresh_token are not updated; modify the
test_get_profile_with_revoked_access_token to capture the login response (e.g.,
resp = authenticated_auth_client.login(valid_credentials["username"],
valid_credentials["password"])), parse the returned tokens from resp.json()
(access_token / refresh_token or the actual keys your API returns) and assign
them back to authenticated_auth_client.token and
authenticated_auth_client.refresh_token (or the equivalent attributes on the
client) so the client state is fully restored after re-login.
- Around line 194-204: TestProfileHeaders is missing the same decorators used on
the other test classes; add the decorators above the class definition:
@allure.feature with the appropriate feature name, @allure.story with the
appropriate story name, and @pytest.mark.api so the class has the same metadata
as TestProfileGeneral/TestProfileAccess; place these decorators immediately
above class TestProfileHeaders to ensure the test class inherits the same
markers.
In @tests/api/test_auth_refresh.py:
- Around line 218-225: The test leaves authenticated_auth_client in an
inconsistent state because authenticated_auth_client.login() is called but its
returned tokens aren't assigned; update test_refresh_with_revoked_refresh_token
to capture the result of authenticated_auth_client.login() (or the returned
access/refresh tokens) and set them back on the authenticated_auth_client
instance (the same pattern used elsewhere where login returns tokens used to
populate authenticated_auth_client.refresh_token / access_token) so the client
is fully restored after the test; ensure you preserve the logout(),
post_refresh(...), and ErrorResponse(**response.json()) checks.
- Around line 242-251: In test_refresh_with_wrong_content_type the local headers
copy is modified but never applied to the request; set the client's headers
before calling authenticated_auth_client.post_refresh (e.g., assign
authenticated_auth_client.headers = headers) or pass headers to
authenticated_auth_client.post_refresh(headers=headers) so the modified
Content-Type is sent, and if needed restore the original headers after the
request.
🧹 Nitpick comments (5)
fixtures/api_fixtures.py (2)
38-51: Consider handling missing credentials more explicitly.When
config.user_logorconfig.user_passisNone, the fixture silently yields an unauthenticated client. This could lead to confusing test failures if credentials are missing from the configuration. Consider raising an explicit error or logging a warning.Also, as flagged by static analysis, consider using a custom exception class instead of the generic
Exception.♻️ Suggested improvement
+class LoginFailedError(Exception): + """Raised when authentication fails during fixture setup.""" + pass + + @pytest.fixture(scope="function") def authenticated_auth_client(config: Settings): client = AuthAPIClient(config) - if config.user_log and config.user_pass: - response = client.login(config.user_log, config.user_pass) - if response.status_code == 200: - token = response.json().get("access_token") - refresh_token = response.json().get("refresh_token") - client.token = token - client.refresh_token = refresh_token - else: - raise Exception("Login failed") + if not config.user_log or not config.user_pass: + raise LoginFailedError("Missing user_log or user_pass in config") + + response = client.login(config.user_log, config.user_pass) + if response.status_code != 200: + raise LoginFailedError(f"Login failed with status {response.status_code}") + + data = response.json() + client.token = data.get("access_token") + client.refresh_token = data.get("refresh_token") yield client client.close()
70-77:valid_passwordandinvalid_passwordhave identical implementations.Both fixtures use
faker.password(), making them semantically identical. Ifinvalid_passwordshould represent a non-existent user's password (for failed login tests), this is fine. However, if it should represent passwords that fail validation (e.g., too short, no special characters), the implementation should differ.Consider renaming to clarify intent (e.g.,
random_passwordandnonexistent_user_password) or implementing actual validation-failing passwords forinvalid_password.tests/api/test_auth_profile.py (2)
141-153: Hardcoded JWT token is acceptable for testing purposes.The static analysis flagged this as a potential secret, but this is an intentionally crafted expired test token with fake data. The TODO comment indicates plans to generate these dynamically, which would be cleaner.
Would you like me to help implement a utility function to dynamically generate expired JWT tokens for testing?
130-139: Missing token restoration after test.Unlike
test_get_profile_without_tokenandtest_get_profile_expired_token, this test modifies the client token but doesn't restore it afterward. Whileauthenticated_auth_clientis function-scoped (so isolation should be maintained), the pattern is inconsistent with sibling tests.♻️ Suggested fix for consistency
@pytest.mark.parametrize("invalid_token", generate_invalid_tokens()) def test_get_profile_invalid_token(self, authenticated_auth_client, invalid_token): + original_token = authenticated_auth_client.token authenticated_auth_client.token = invalid_token response = authenticated_auth_client.get_profile() assert response.status_code == 401, f"Expected 401, got {response.status_code}" ErrorResponse(**response.json()) + authenticated_auth_client.token = original_tokentests/api/test_auth_password.py (1)
17-29: Avoid mutating fixture return values.Line 27 mutates
valid_credentials["password"]which modifies the fixture's returned dictionary. Whilevalid_credentialsis function-scoped and should return a fresh dict per test, mutating fixture values is a test smell that can lead to hard-to-debug order dependencies.If you need to track the new password for subsequent operations, use a local variable instead.
♻️ Suggested approach
def test_change_password_success(self, authenticated_auth_client, valid_credentials, valid_password): response = authenticated_auth_client.change_password( old_password=valid_credentials["password"], new_password=valid_password ) assert response.status_code == 200, f"Expected 200, got {response.status_code}" try: change_response = ChangePasswordResponse(**response.json()) assert change_response.message, "Response should contain success message" - valid_credentials["password"] = valid_password + # Note: Password changed to valid_password for this session except ValidationError as e: pytest.fail(f"Password change response schema validation failed: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
fixtures/api_fixtures.pytests/api/test_auth_audit_log.pytests/api/test_auth_login.pytests/api/test_auth_password.pytests/api/test_auth_profile.pytests/api/test_auth_refresh.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/api/test_auth_audit_log.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T13:38:40.404Z
Learnt from: 0xc7c5
Repo: chainstack/cp-autotests PR: 1
File: tests/api/test_auth_refresh.py:187-195
Timestamp: 2026-01-12T13:38:40.404Z
Learning: In tests/api/test_auth_refresh.py, the header manipulation pattern `headers = client.headers.copy()` followed by `client.headers["Authorization"] = ...` and later `client.headers = headers` is correct: it backs up, modifies the client's headers directly, then restores them.
Applied to files:
tests/api/test_auth_profile.pytests/api/test_auth_refresh.pytests/api/test_auth_login.pytests/api/test_auth_password.pyfixtures/api_fixtures.py
🧬 Code graph analysis (4)
tests/api/test_auth_profile.py (3)
tests/api/schemas/auth_schemas.py (3)
UserProfile(6-14)ErrorResponse(33-34)LoginResponse(17-21)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-51)valid_credentials(55-59)clients/api_client.py (2)
get_profile(194-195)login(184-185)
tests/api/test_auth_login.py (3)
tests/api/schemas/auth_schemas.py (3)
LoginResponse(17-21)ErrorResponse(33-34)UserProfile(6-14)fixtures/api_fixtures.py (6)
valid_credentials(55-59)auth_client(32-35)invalid_username(63-64)invalid_password(76-77)invalid_credentials(81-85)authenticated_auth_client(39-51)clients/api_client.py (4)
login(184-185)post(46-57)send_custom_request(100-115)get_profile(194-195)
tests/api/test_auth_password.py (4)
tests/api/schemas/auth_schemas.py (2)
ChangePasswordResponse(37-38)ErrorResponse(33-34)fixtures/api_fixtures.py (5)
authenticated_auth_client(39-51)valid_credentials(55-59)valid_password(71-72)invalid_password(76-77)faker(7-8)clients/api_client.py (2)
change_password(197-198)send_custom_request(100-115)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)
fixtures/api_fixtures.py (2)
clients/api_client.py (2)
AuthAPIClient(175-205)login(184-185)config/settings.py (1)
Settings(6-78)
🪛 Gitleaks (8.30.0)
tests/api/test_auth_profile.py
[high] 146-146: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 Ruff (0.14.10)
tests/api/test_auth_profile.py
146-146: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_refresh.py
72-72: Possible hardcoded password assigned to: "expired_token"
(S105)
179-179: Possible hardcoded password assigned to: "expired_token"
(S105)
tests/api/test_auth_login.py
24-24: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
25-25: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
26-26: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
27-27: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
28-28: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
29-29: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
30-30: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
30-30: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
31-31: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
31-31: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
32-32: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
32-32: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12)
(invalid-syntax)
103-103: Undefined name valid_credentials
(F821)
112-112: Undefined name valid_credentials
(F821)
tests/api/test_auth_password.py
107-107: Undefined name WEAK_PASSWORD_CASES
(F821)
159-159: Undefined name generate_invalid_refresh_tokens
(F821)
fixtures/api_fixtures.py
49-49: Create your own exception
(TRY002)
49-49: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (4)
fixtures/api_fixtures.py (1)
66-68: Verifyvalid_usernamegenerates appropriate format.
faker.name()generates full names (e.g., "John Smith"), which may not match typical username formats. If the system expects specific username patterns, consider usingfaker.user_name()instead.tests/api/test_auth_refresh.py (3)
1-7: LGTM on imports and structure.The imports are appropriate for the test file's needs. The organization with allure decorators and pytest markers is consistent with the test suite patterns.
79-100: Verify refresh token reuse policy.This test assumes a single refresh token can be used multiple times to generate different access tokens. If the API implements refresh token rotation (where each refresh invalidates the old refresh token), the second
post_refreshcall would fail.
124-142: Verify old token invalidation behavior.This test assumes the old access token becomes immediately invalid after a refresh. Many JWT-based APIs don't implement token blacklisting—old tokens remain valid until they expire naturally. Confirm the API's token invalidation strategy matches this expectation.
| @allure.title("Login fails with missing password") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_login_missing_password(self, auth_client, valid_credentials): | ||
| response = auth_client.login(password=valid_credentials["username"]) | ||
|
|
||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) |
There was a problem hiding this comment.
Incorrect test: passes username as password instead of omitting password.
The test title says "Login fails with missing password" but line 149 passes password=valid_credentials["username"] instead of omitting the password argument. This tests logging in with only a password (no username), not with a missing password.
🐛 Proposed fix
@allure.title("Login fails with missing password")
@allure.severity(allure.severity_level.NORMAL)
def test_login_missing_password(self, auth_client, valid_credentials):
- response = auth_client.login(password=valid_credentials["username"])
+ response = auth_client.login(username=valid_credentials["username"])
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())🤖 Prompt for AI Agents
In @tests/api/test_auth_login.py around lines 146 - 152, The test
test_login_missing_password is passing password=valid_credentials["username"] to
auth_client.login (testing missing username) instead of omitting the password;
update the call in test_login_missing_password to call auth_client.login without
the password argument (or explicitly with password=None/omit) while still
providing the username from valid_credentials so the request simulates a missing
password, then keep the existing assertions (status_code == 400 and
ErrorResponse(**response.json())).
| @allure.title("Login without content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_login_without_content_type(self, auth_client, valid_credentials): | ||
| headers = auth_client.headers.copy() | ||
| headers.pop("Content-Type") | ||
| response = auth_client.login(valid_credentials["username"], valid_credentials["password"]) | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| auth_client.headers = headers |
There was a problem hiding this comment.
Test doesn't remove Content-Type from client headers.
The test pops Content-Type from the local headers copy but never applies this change to auth_client.headers. The login request uses the original headers with Content-Type intact.
🐛 Proposed fix
def test_login_without_content_type(self, auth_client, valid_credentials):
headers = auth_client.headers.copy()
- headers.pop("Content-Type")
- response = auth_client.login(valid_credentials["username"], valid_credentials["password"])
+ auth_client.headers.pop("Content-Type")
+ response = auth_client.login(valid_credentials["username"], valid_credentials["password"])
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_login.py around lines 205 - 213, In
test_login_without_content_type the local headers copy is modified but not
applied to the client, so test_login_without_content_type should either set
auth_client.headers = headers before calling auth_client.login or pass the
modified headers into auth_client.login, then perform the request and finally
restore the original headers (e.g., reassign the saved original headers back to
auth_client.headers) to avoid side effects; update references in the test to use
auth_client.headers (or the explicit headers arg) so the Content-Type is
actually removed during the login call.
| @allure.title("Login with wrong content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| @pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"]) | ||
| def test_login_with_wrong_content_type(self, auth_client, valid_credentials, content_type): | ||
| headers = auth_client.headers.copy() | ||
| headers["Content-Type"] = content_type | ||
| response = auth_client.login(valid_credentials["username"], valid_credentials["password"]) | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| auth_client.headers = headers |
There was a problem hiding this comment.
Test doesn't apply modified Content-Type to client.
Same issue: the local headers copy is modified but the client's headers remain unchanged during the request.
🐛 Proposed fix
@pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"])
def test_login_with_wrong_content_type(self, auth_client, valid_credentials, content_type):
headers = auth_client.headers.copy()
- headers["Content-Type"] = content_type
- response = auth_client.login(valid_credentials["username"], valid_credentials["password"])
+ auth_client.headers["Content-Type"] = content_type
+ response = auth_client.login(valid_credentials["username"], valid_credentials["password"])
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_login.py around lines 215 - 224, In
test_login_with_wrong_content_type the local headers copy is modified but never
applied to the request; before calling auth_client.login in
test_login_with_wrong_content_type either set auth_client.headers = headers so
the client uses the modified Content-Type or (if auth_client.login accepts
per-request headers) pass headers=headers into auth_client.login(username,
password, headers=headers); ensure you still restore auth_client.headers to its
original value afterward if you mutate the client.
| import pytest | ||
| import allure | ||
| from pydantic import ValidationError | ||
| from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse | ||
| from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES | ||
| import base64 |
There was a problem hiding this comment.
Missing imports for WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens.
The file uses WEAK_PASSWORD_CASES (line 107) and generate_invalid_refresh_tokens (line 159) but neither is imported. This will cause NameError at runtime.
🐛 Add missing imports
import pytest
import allure
from pydantic import ValidationError
from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse
-from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES
+from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES, WEAK_PASSWORD_CASES
+from utils.token_generator import generate_invalid_refresh_tokens
import base64📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import pytest | |
| import allure | |
| from pydantic import ValidationError | |
| from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse | |
| from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES | |
| import base64 | |
| import pytest | |
| import allure | |
| from pydantic import ValidationError | |
| from tests.api.schemas.auth_schemas import ChangePasswordResponse, ErrorResponse | |
| from tests.api.cases.test_cases import EMPTY_STRING_CASES, NONSTRING_CASES, WEAK_PASSWORD_CASES | |
| from utils.token_generator import generate_invalid_refresh_tokens | |
| import base64 |
🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 1 - 6, The test file references
WEAK_PASSWORD_CASES and generate_invalid_refresh_tokens but never imports them,
causing NameError; add an import for WEAK_PASSWORD_CASES (same place as
EMPTY_STRING_CASES/NONSTRING_CASES — e.g., from tests.api.cases.test_cases
import WEAK_PASSWORD_CASES) and add an import for
generate_invalid_refresh_tokens from the module that defines token helpers
(where helper functions live in the test suite, e.g.,
tests.api.utils.token_helpers or the appropriate tests.api.cases helper module)
so both symbols are available to the tests.
| @allure.title("Change password without content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_change_password_without_content_type(self, authenticated_auth_client, valid_credentials, valid_password): | ||
| headers = authenticated_auth_client.headers.copy() | ||
| headers.pop("Content-Type") | ||
| response = authenticated_auth_client.change_password( | ||
| old_password=valid_credentials["password"], | ||
| new_password=valid_password | ||
| ) | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.headers = headers |
There was a problem hiding this comment.
Test doesn't remove Content-Type from client headers.
Same pattern issue: pops from local headers copy but doesn't apply to authenticated_auth_client.headers.
🐛 Proposed fix
def test_change_password_without_content_type(self, authenticated_auth_client, valid_credentials, valid_password):
headers = authenticated_auth_client.headers.copy()
- headers.pop("Content-Type")
- response = authenticated_auth_client.change_password(
+ authenticated_auth_client.headers.pop("Content-Type")
+ response = authenticated_auth_client.change_password(
old_password=valid_credentials["password"],
new_password=valid_password
)
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
authenticated_auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_password.py around lines 244 - 255, The test
test_change_password_without_content_type currently pops "Content-Type" from a
local headers copy but never applies it to authenticated_auth_client, so the
request still includes the header; update the test to remove the header directly
on the client (e.g., authenticated_auth_client.headers.pop("Content-Type")
before calling change_password) and then restore the original headers afterward
(or assign authenticated_auth_client.headers = headers after popping) so the
client actually sends the request without Content-Type.
| @allure.title("Get profile with revoked access token") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_get_profile_with_revoked_access_token(self, authenticated_auth_client, valid_credentials): | ||
| authenticated_auth_client.logout() | ||
| response = authenticated_auth_client.get_profile() | ||
| assert response.status_code == 401, f"Expected 401, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.login(valid_credentials["username"], valid_credentials["password"]) |
There was a problem hiding this comment.
Incomplete state restoration after login.
After calling logout() and login(), the client's token and refresh_token attributes are not updated with the new values from the login response. This leaves the client in an inconsistent state.
🐛 Proposed fix
def test_get_profile_with_revoked_access_token(self, authenticated_auth_client, valid_credentials):
authenticated_auth_client.logout()
response = authenticated_auth_client.get_profile()
assert response.status_code == 401, f"Expected 401, got {response.status_code}"
ErrorResponse(**response.json())
- authenticated_auth_client.login(valid_credentials["username"], valid_credentials["password"])
+ login_response = authenticated_auth_client.login(valid_credentials["username"], valid_credentials["password"])
+ if login_response.status_code == 200:
+ data = login_response.json()
+ authenticated_auth_client.token = data.get("access_token")
+ authenticated_auth_client.refresh_token = data.get("refresh_token")🤖 Prompt for AI Agents
In @tests/api/test_auth_profile.py around lines 185 - 192, The test leaves
authenticated_auth_client in an inconsistent state because after calling
logout() and then login(...) the client's token and refresh_token are not
updated; modify the test_get_profile_with_revoked_access_token to capture the
login response (e.g., resp =
authenticated_auth_client.login(valid_credentials["username"],
valid_credentials["password"])), parse the returned tokens from resp.json()
(access_token / refresh_token or the actual keys your API returns) and assign
them back to authenticated_auth_client.token and
authenticated_auth_client.refresh_token (or the equivalent attributes on the
client) so the client state is fully restored after re-login.
| class TestProfileHeaders: | ||
|
|
||
| @allure.title("Get profile without content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_get_profile_without_content_type(self, authenticated_auth_client): | ||
| headers = authenticated_auth_client.headers.copy() | ||
| authenticated_auth_client.headers.pop("Content-Type") | ||
| response = authenticated_auth_client.get_profile() | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.headers = headers |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the test file
git ls-files tests/api/test_auth_profile.pyRepository: chainstack/cp-autotests
Length of output: 97
🏁 Script executed:
# Check file size
wc -l tests/api/test_auth_profile.pyRepository: chainstack/cp-autotests
Length of output: 101
🏁 Script executed:
cat -n tests/api/test_auth_profile.pyRepository: chainstack/cp-autotests
Length of output: 12757
Add missing class decorators to TestProfileHeaders.
TestProfileHeaders is missing @allure.feature, @allure.story, and @pytest.mark.api decorators that are consistently applied to the other test classes (TestProfileGeneral and TestProfileAccess).
♻️ Add missing decorators
+@allure.feature("Authentication")
+@allure.story("User Profile")
+@pytest.mark.api
class TestProfileHeaders:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class TestProfileHeaders: | |
| @allure.title("Get profile without content type") | |
| @allure.severity(allure.severity_level.NORMAL) | |
| def test_get_profile_without_content_type(self, authenticated_auth_client): | |
| headers = authenticated_auth_client.headers.copy() | |
| authenticated_auth_client.headers.pop("Content-Type") | |
| response = authenticated_auth_client.get_profile() | |
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | |
| ErrorResponse(**response.json()) | |
| authenticated_auth_client.headers = headers | |
| @allure.feature("Authentication") | |
| @allure.story("User Profile") | |
| @pytest.mark.api | |
| class TestProfileHeaders: | |
| @allure.title("Get profile without content type") | |
| @allure.severity(allure.severity_level.NORMAL) | |
| def test_get_profile_without_content_type(self, authenticated_auth_client): | |
| headers = authenticated_auth_client.headers.copy() | |
| authenticated_auth_client.headers.pop("Content-Type") | |
| response = authenticated_auth_client.get_profile() | |
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | |
| ErrorResponse(**response.json()) | |
| authenticated_auth_client.headers = headers |
🤖 Prompt for AI Agents
In @tests/api/test_auth_profile.py around lines 194 - 204, TestProfileHeaders is
missing the same decorators used on the other test classes; add the decorators
above the class definition: @allure.feature with the appropriate feature name,
@allure.story with the appropriate story name, and @pytest.mark.api so the class
has the same metadata as TestProfileGeneral/TestProfileAccess; place these
decorators immediately above class TestProfileHeaders to ensure the test class
inherits the same markers.
| @allure.title("Get profile with wrong content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| @pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"]) | ||
| def test_get_profile_with_wrong_content_type(self, authenticated_auth_client, content_type): | ||
| headers = authenticated_auth_client.headers.copy() | ||
| headers["Content-Type"] = content_type | ||
| response = authenticated_auth_client.get_profile() | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.headers = headers |
There was a problem hiding this comment.
Test doesn't apply modified headers to the request.
The test modifies the local headers copy but never assigns it back to authenticated_auth_client.headers before making the request. The client uses its original headers, so this test doesn't actually verify wrong content-type behavior.
🐛 Proposed fix
@pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"])
def test_get_profile_with_wrong_content_type(self, authenticated_auth_client, content_type):
headers = authenticated_auth_client.headers.copy()
- headers["Content-Type"] = content_type
- response = authenticated_auth_client.get_profile()
+ authenticated_auth_client.headers["Content-Type"] = content_type
+ response = authenticated_auth_client.get_profile()
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
authenticated_auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_profile.py around lines 206 - 215, The test creates a
modified headers dict but never applies it to the client before calling
get_profile, so the request still uses the original headers; set
authenticated_auth_client.headers = headers before calling
authenticated_auth_client.get_profile() and then restore the original headers
after the assertion (or assign headers back only for the request and revert to
previous headers) to ensure test_get_profile_with_wrong_content_type actually
sends the wrong Content-Type to get_profile.
| @allure.title("Refresh with revoked refresh token") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_refresh_with_revoked_refresh_token(self, authenticated_auth_client): | ||
| authenticated_auth_client.logout() | ||
| response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token) | ||
| assert response.status_code == 401, f"Expected 401, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.login() |
There was a problem hiding this comment.
Incomplete state restoration after login.
Same issue as in profile tests: login() is called but the returned tokens are not assigned to the client, leaving it in an inconsistent state.
🐛 Proposed fix
def test_refresh_with_revoked_refresh_token(self, authenticated_auth_client):
authenticated_auth_client.logout()
response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token)
assert response.status_code == 401, f"Expected 401, got {response.status_code}"
ErrorResponse(**response.json())
- authenticated_auth_client.login()
+ login_response = authenticated_auth_client.login()
+ if login_response.status_code == 200:
+ data = login_response.json()
+ authenticated_auth_client.token = data.get("access_token")
+ authenticated_auth_client.refresh_token = data.get("refresh_token")🤖 Prompt for AI Agents
In @tests/api/test_auth_refresh.py around lines 218 - 225, The test leaves
authenticated_auth_client in an inconsistent state because
authenticated_auth_client.login() is called but its returned tokens aren't
assigned; update test_refresh_with_revoked_refresh_token to capture the result
of authenticated_auth_client.login() (or the returned access/refresh tokens) and
set them back on the authenticated_auth_client instance (the same pattern used
elsewhere where login returns tokens used to populate
authenticated_auth_client.refresh_token / access_token) so the client is fully
restored after the test; ensure you preserve the logout(), post_refresh(...),
and ErrorResponse(**response.json()) checks.
| @allure.title("Refresh with wrong content type") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| @pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"]) | ||
| def test_refresh_with_wrong_content_type(self, authenticated_auth_client, content_type): | ||
| headers = authenticated_auth_client.headers.copy() | ||
| headers["Content-Type"] = content_type | ||
| response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token) | ||
| assert response.status_code == 400, f"Expected 400, got {response.status_code}" | ||
| ErrorResponse(**response.json()) | ||
| authenticated_auth_client.headers = headers |
There was a problem hiding this comment.
Test doesn't apply modified headers to the request.
Same issue as in profile tests: the local headers copy is modified but never assigned back to the client before the request.
🐛 Proposed fix
@pytest.mark.parametrize("content_type", ["text/plain", "application/xml", "application/json; charset=utf-8"])
def test_refresh_with_wrong_content_type(self, authenticated_auth_client, content_type):
headers = authenticated_auth_client.headers.copy()
- headers["Content-Type"] = content_type
- response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token)
+ authenticated_auth_client.headers["Content-Type"] = content_type
+ response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token)
assert response.status_code == 400, f"Expected 400, got {response.status_code}"
ErrorResponse(**response.json())
authenticated_auth_client.headers = headers🤖 Prompt for AI Agents
In @tests/api/test_auth_refresh.py around lines 242 - 251, In
test_refresh_with_wrong_content_type the local headers copy is modified but
never applied to the request; set the client's headers before calling
authenticated_auth_client.post_refresh (e.g., assign
authenticated_auth_client.headers = headers) or pass headers to
authenticated_auth_client.post_refresh(headers=headers) so the modified
Content-Type is sent, and if needed restore the original headers after the
request.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @tests/api/test_auth_logout.py:
- Around line 164-172: The test calls the string attribute
authenticated_auth_client.refresh_token as a method which raises a TypeError;
replace calls to authenticated_auth_client.refresh_token(...) with the actual
client method
authenticated_auth_client.post_refresh(refresh_token=authenticated_auth_client.refresh_token)
so refresh_response is obtained correctly, and keep the logout call using
authenticated_auth_client.logout(refresh_token=...). Also update the test name
or implement concurrency (e.g., spawn threads/tasks to call logout and
post_refresh concurrently) if you truly intend to test "at the same time" race
conditions.
- Around line 144-149: The two tests (test_logout_check_response_headers and
test_logout_check_cache) assert different Cache-Control values for the same
endpoint (/v1/auth/logout) depending only on the fixture
(authenticated_auth_client vs auth_client); confirm the actual behavior in the
logout handler and make both tests assert the correct, consistent Cache-Control
header value returned by that handler. Update either
test_logout_check_response_headers or test_logout_check_cache to match the real
response (or adjust the logout implementation if it should vary by auth state),
ensuring both tests reference the same expected Cache-Control string and keep
the other header assertions (Expires, Pragma) consistent.
- Around line 229-235: The tests are inconsistent: test_logout_check_cache uses
the unauthenticated fixture auth_client but expects 200 while
test_logout_without_auth_token expects 401, and the Cache-Control expectation
differs from test_logout_check_response_headers; fix by making logout
authentication expectations consistent (either switch test_logout_check_cache to
use an authenticated fixture or change its expected status to 401 to match
test_logout_without_auth_token) and unify the Cache-Control assertion in
test_logout_check_cache with the actual endpoint behavior (update the expected
value to match the implementation or assert a subset/contains check like
"no-store" if multiple directives are returned) so both tests reference the same
header semantics.
🧹 Nitpick comments (5)
tests/api/test_auth_logout.py (5)
1-7: Remove unused imports.
LoginResponseandUserProfileare imported but never used in this file.Suggested fix
import pytest import allure from pydantic import ValidationError -from tests.api.schemas.auth_schemas import LogoutResponse, ErrorResponse, LoginResponse, UserProfile +from tests.api.schemas.auth_schemas import LogoutResponse, ErrorResponse from tests.api.cases.test_cases import EMPTY_STRING_CASES from utils.token_generator import generate_invalid_refresh_tokens, generate_expired_token import base64
29-84: Missing class-level allure decorators for consistency.
TestLogoutRefreshTokenlacks the@allure.feature,@allure.story, and@pytest.mark.apidecorators that are applied to other test classes in this file (e.g.,TestLogoutGeneral,TestLogoutAcess).Suggested fix
+@allure.feature("Authentication") +@allure.story("Logout") +@pytest.mark.api class TestLogoutRefreshToken:
89-89: Typo in class name: "Acess" should be "Access".Suggested fix
-class TestLogoutAcess: +class TestLogoutAccess:
112-112: Typo in allure title: duplicate "with".Suggested fix
- @allure.title("Logout with with wrong auth type") + @allure.title("Logout with wrong auth type")
151-162: Missing class-level allure decorators and test is sequential, not concurrent.
TestLogoutRaceConditionslacks class-level decorators for consistency with other classes.- The test
test_logout_multiple_timesexecutes requests sequentially. While this tests idempotency behavior correctly, the class name "RaceConditions" may be misleading since no actual concurrent execution occurs.Suggested fix for decorators
+@allure.feature("Authentication") +@allure.story("Logout") +@pytest.mark.api class TestLogoutRaceConditions:
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/api/test_auth_logout.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T13:38:40.404Z
Learnt from: 0xc7c5
Repo: chainstack/cp-autotests PR: 1
File: tests/api/test_auth_refresh.py:187-195
Timestamp: 2026-01-12T13:38:40.404Z
Learning: In tests/api/test_auth_refresh.py, the header manipulation pattern `headers = client.headers.copy()` followed by `client.headers["Authorization"] = ...` and later `client.headers = headers` is correct: it backs up, modifies the client's headers directly, then restores them.
Applied to files:
tests/api/test_auth_logout.py
🧬 Code graph analysis (1)
tests/api/test_auth_logout.py (4)
tests/api/schemas/auth_schemas.py (2)
LogoutResponse(29-30)ErrorResponse(33-34)utils/token_generator.py (1)
generate_invalid_refresh_tokens(86-167)fixtures/api_fixtures.py (2)
authenticated_auth_client(39-51)auth_client(32-35)clients/api_client.py (5)
logout(190-192)get_profile(194-195)post_refresh(187-188)send_custom_request(100-115)post(46-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: guardrails/scan
🔇 Additional comments (1)
tests/api/test_auth_logout.py (1)
220-220: Verify ifapplication/json; charset=utf-8should be rejected.
application/json; charset=utf-8is a valid and common Content-Type header. Many APIs accept it as equivalent toapplication/json. Consider verifying whether the API intentionally rejects charset parameters or if this test case expectation is incorrect.
| def test_logout_check_response_headers(self, authenticated_auth_client): | ||
| response = authenticated_auth_client.logout() | ||
| assert response.status_code == 200, f"Expected 200, got {response.status_code}" | ||
| assert response.headers["Cache-Control"] == "no-store", "Cache-Control should be no-store" | ||
| assert response.headers["Expires"] == "0", "Expires should be 0" | ||
| assert response.headers["Pragma"] == "no-cache", "Pragma should be no-cache" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the test file
find . -name "test_auth_logout.py" -type fRepository: chainstack/cp-autotests
Length of output: 98
🏁 Script executed:
# Read the test file around the mentioned lines
wc -l tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 100
🏁 Script executed:
# Read the test around lines 144-149
sed -n '140,150p' tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 707
🏁 Script executed:
# Read the test around line 234
sed -n '230,240p' tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 571
🏁 Script executed:
# Get a broader view of the test structure around line 234
sed -n '220,250p' tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 1124
🏁 Script executed:
# Find the logout endpoint implementation
find . -type f -name "*.py" | xargs grep -l "def logout" | head -5Repository: chainstack/cp-autotests
Length of output: 90
🏁 Script executed:
# Inspect the api_client implementation
sed -n '1,50p' ./clients/api_client.py | head -30Repository: chainstack/cp-autotests
Length of output: 1014
🏁 Script executed:
# Search for the logout method implementation
grep -n "def logout" ./clients/api_client.py -A 10Repository: chainstack/cp-autotests
Length of output: 731
🏁 Script executed:
# Also check if there's any middleware or endpoint decorator that sets headers based on auth
find . -type f -name "*.py" -path "*/api/*" | xargs grep -l "logout" | grep -v test | head -10Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
# Search for fixture definitions
find . -type f -name "conftest.py" | xargs grep -l "authenticated_auth_client\|auth_client"Repository: chainstack/cp-autotests
Length of output: 51
🏁 Script executed:
# Check the fixture definitions
grep -n "authenticated_auth_client\|auth_client" tests/ -r --include="*.py" | grep "def\|@fixture" | head -20Repository: chainstack/cp-autotests
Length of output: 2571
🏁 Script executed:
# Find conftest files
find . -name "conftest.py" -type fRepository: chainstack/cp-autotests
Length of output: 80
🏁 Script executed:
# Read the conftest to see fixture definitions
cat tests/conftest.py | head -100Repository: chainstack/cp-autotests
Length of output: 116
🏁 Script executed:
# Also look for authenticated_auth_client and auth_client fixtures specifically
grep -n "authenticated_auth_client\|def auth_client" tests/conftest.py -A 5Repository: chainstack/cp-autotests
Length of output: 117
🏁 Script executed:
# Search for logout endpoint in backend code
find . -type f \( -name "*.py" -o -name "*.js" -o -name "*.ts" \) ! -path "*/tests/*" ! -path "*/node_modules/*" | xargs grep -l "logout" 2>/dev/null | head -10Repository: chainstack/cp-autotests
Length of output: 90
🏁 Script executed:
# Read the root conftest.py
cat ./conftest.pyRepository: chainstack/cp-autotests
Length of output: 629
🏁 Script executed:
# Also check if there's any documentation or backend code showing expected headers
find . -type f \( -name "*.md" -o -name "*.yaml" -o -name "*.yml" \) | xargs grep -l "Cache-Control\|logout" 2>/dev/null | head -5Repository: chainstack/cp-autotests
Length of output: 93
🏁 Script executed:
# Search for where authenticated_auth_client and auth_client are defined
grep -r "def authenticated_auth_client\|def auth_client" . --include="*.py"Repository: chainstack/cp-autotests
Length of output: 204
🏁 Script executed:
# Check the test_auth_logout.py file more comprehensively to see fixture usage
head -50 tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 2410
🏁 Script executed:
# Check the openapi.yaml for logout endpoint documentation
grep -A 20 "logout" ./openapi.yamlRepository: chainstack/cp-autotests
Length of output: 1386
🏁 Script executed:
# Read the api_fixtures.py to see the difference between auth_client and authenticated_auth_client
cat ./fixtures/api_fixtures.pyRepository: chainstack/cp-autotests
Length of output: 2156
Cache-Control header values are inconsistent across logout endpoint tests.
test_logout_check_response_headers expects "no-store", while test_logout_check_cache expects "no-cache, no-store, must-revalidate" for the same endpoint. Both tests call /v1/auth/logout and expect a 200 response, but with different authentication states (authenticated_auth_client vs auth_client).
Verify whether the endpoint intentionally returns different Cache-Control headers based on authentication state, or if one test's expectation is incorrect. Align both tests to match the actual endpoint behavior.
🤖 Prompt for AI Agents
In @tests/api/test_auth_logout.py around lines 144 - 149, The two tests
(test_logout_check_response_headers and test_logout_check_cache) assert
different Cache-Control values for the same endpoint (/v1/auth/logout) depending
only on the fixture (authenticated_auth_client vs auth_client); confirm the
actual behavior in the logout handler and make both tests assert the correct,
consistent Cache-Control header value returned by that handler. Update either
test_logout_check_response_headers or test_logout_check_cache to match the real
response (or adjust the logout implementation if it should vary by auth state),
ensuring both tests reference the same expected Cache-Control string and keep
the other header assertions (Expires, Pragma) consistent.
| @allure.title("Logout and refresh token at the same time") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_logout_and_refresh_token_at_the_same_time(self, authenticated_auth_client): | ||
| logout_response = authenticated_auth_client.logout(refresh_token=authenticated_auth_client.refresh_token) | ||
| refresh_response = authenticated_auth_client.refresh_token(refresh_token=authenticated_auth_client.refresh_token) | ||
| assert logout_response.status_code == 200, f"Expected 200, got {logout_response.status_code}" | ||
| LogoutResponse(**logout_response.json()) | ||
| assert refresh_response.status_code == 401, f"Expected 401, got {refresh_response.status_code}" | ||
| ErrorResponse(**refresh_response.json()) |
There was a problem hiding this comment.
Critical bug: calling string attribute as a method.
Line 168 calls authenticated_auth_client.refresh_token(...), but refresh_token is a string attribute, not a method. This will raise TypeError: 'str' object is not callable. Based on the API client code, the correct method is post_refresh().
Additionally, this test claims to execute "at the same time" but runs sequentially, so it doesn't actually test race conditions.
Proposed fix
@allure.title("Logout and refresh token at the same time")
@allure.severity(allure.severity_level.NORMAL)
def test_logout_and_refresh_token_at_the_same_time(self, authenticated_auth_client):
logout_response = authenticated_auth_client.logout(refresh_token=authenticated_auth_client.refresh_token)
- refresh_response = authenticated_auth_client.refresh_token(refresh_token=authenticated_auth_client.refresh_token)
+ refresh_response = authenticated_auth_client.post_refresh(authenticated_auth_client.refresh_token)
assert logout_response.status_code == 200, f"Expected 200, got {logout_response.status_code}"
LogoutResponse(**logout_response.json())
assert refresh_response.status_code == 401, f"Expected 401, got {refresh_response.status_code}"
ErrorResponse(**refresh_response.json())🤖 Prompt for AI Agents
In @tests/api/test_auth_logout.py around lines 164 - 172, The test calls the
string attribute authenticated_auth_client.refresh_token as a method which
raises a TypeError; replace calls to
authenticated_auth_client.refresh_token(...) with the actual client method
authenticated_auth_client.post_refresh(refresh_token=authenticated_auth_client.refresh_token)
so refresh_response is obtained correctly, and keep the logout call using
authenticated_auth_client.logout(refresh_token=...). Also update the test name
or implement concurrency (e.g., spawn threads/tasks to call logout and
post_refresh concurrently) if you truly intend to test "at the same time" race
conditions.
| @allure.title("Logout check cache") | ||
| @allure.severity(allure.severity_level.NORMAL) | ||
| def test_logout_check_cache(self, auth_client): | ||
| response = auth_client.logout() | ||
| assert response.status_code == 200, f"Expected 200, got {response.status_code}" | ||
| assert response.headers["Cache-Control"] == "no-cache, no-store, must-revalidate", "Cache-Control header should be set to no-cache, no-store, must-revalidate" | ||
| assert response.headers["Pragma"] == "no-cache", "Pragma header should be set to no-cache" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find auth_client fixture definition
rg -n "def auth_client" --type py -A 15 tests/Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check test_logout_without_auth_token and test_logout_check_response_headers
rg -n "test_logout_without_auth_token|test_logout_check_response_headers" --type py -A 8 tests/Repository: chainstack/cp-autotests
Length of output: 1685
🏁 Script executed:
#!/bin/bash
# Get context on the test file structure
head -50 tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 2410
🏁 Script executed:
#!/bin/bash
# Get the exact test at lines 229-235
sed -n '229,235p' tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 610
🏁 Script executed:
#!/bin/bash
# Find conftest.py files to understand fixture definitions
fd conftest.py tests/ -x cat {}Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Find authenticated_auth_client fixture
rg -n "def authenticated_auth_client" --type py -A 10 tests/Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Find all conftest files
find tests/ -name "conftest.py" -type fRepository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for auth_client fixture definition
rg -n "auth_client" --type py -B 2 -A 10 tests/ | head -100Repository: chainstack/cp-autotests
Length of output: 8347
🏁 Script executed:
#!/bin/bash
# Check for fixture decorators in the test file itself
rg -n "@pytest.fixture" --type py -A 3 tests/api/test_auth_logout.pyRepository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Find all conftest files in the repo
find . -name conftest.py -type f 2>/dev/nullRepository: chainstack/cp-autotests
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Search for auth_client fixture at root level
rg -n "def auth_client|auth_client.*=" --type py -B 2 -A 8 | head -80Repository: chainstack/cp-autotests
Length of output: 7615
🏁 Script executed:
#!/bin/bash
# Look at broader pytest fixture definitions
rg -n "@pytest.fixture.*auth" --type py -A 15 | head -150Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Read the conftest.py file to find fixtures
cat conftest.pyRepository: chainstack/cp-autotests
Length of output: 629
🏁 Script executed:
#!/bin/bash
# Check for fixture definitions in conftest.py
rg -n "def.*auth.*client|@pytest.fixture" conftest.py -A 10Repository: chainstack/cp-autotests
Length of output: 345
🏁 Script executed:
#!/bin/bash
# Find conftest.py in tests/api directory
cat tests/api/conftest.py 2>/dev/null || echo "File not found"Repository: chainstack/cp-autotests
Length of output: 81
🏁 Script executed:
#!/bin/bash
# Search for conftest files in subdirectories
find tests -name "conftest.py" -type f -exec cat {} \;Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for auth_client and authenticated_auth_client fixture definitions broadly
rg -n "@pytest.fixture" --type py -A 10 | grep -A 10 "auth_client"Repository: chainstack/cp-autotests
Length of output: 1140
🏁 Script executed:
#!/bin/bash
# Search for fixture definitions in all Python files
fd -e py -x rg -l "def auth_client|def authenticated_auth_client" {}Repository: chainstack/cp-autotests
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Look in tests directory structure
ls -la tests/Repository: chainstack/cp-autotests
Length of output: 394
🏁 Script executed:
#!/bin/bash
# Read the full api_fixtures.py to understand fixture definitions
cat fixtures/api_fixtures.pyRepository: chainstack/cp-autotests
Length of output: 2156
🏁 Script executed:
#!/bin/bash
# Find logout endpoint implementation
find . -name "*.py" -path "*/routes/*" -o -path "*/endpoints/*" | head -20Repository: chainstack/cp-autotests
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for logout route/endpoint implementation
rg -n "def logout" --type py -B 2 -A 15 | grep -v test | head -100Repository: chainstack/cp-autotests
Length of output: 1548
🏁 Script executed:
#!/bin/bash
# Check the auth client implementation to understand logout method
rg -n "def logout|class AuthAPIClient" --type py -A 20 clients/Repository: chainstack/cp-autotests
Length of output: 2451
Verify authentication requirement and Cache-Control header inconsistencies for logout endpoint.
-
The test uses
auth_client(unauthenticated fixture) expecting 200, buttest_logout_without_auth_tokenexpects 401 for unauthenticated logout requests. Either the logout endpoint should reject unauthenticated requests consistently, or the test expectation needs clarification. -
Cache-Control header value differs between tests:
test_logout_check_response_headersexpects"no-store"(line 147), while this test expects"no-cache, no-store, must-revalidate"(line 235). Verify which value is correct per the actual endpoint implementation.
🤖 Prompt for AI Agents
In @tests/api/test_auth_logout.py around lines 229 - 235, The tests are
inconsistent: test_logout_check_cache uses the unauthenticated fixture
auth_client but expects 200 while test_logout_without_auth_token expects 401,
and the Cache-Control expectation differs from
test_logout_check_response_headers; fix by making logout authentication
expectations consistent (either switch test_logout_check_cache to use an
authenticated fixture or change its expected status to 401 to match
test_logout_without_auth_token) and unify the Cache-Control assertion in
test_logout_check_cache with the actual endpoint behavior (update the expected
value to match the implementation or assert a subset/contains check like
"no-store" if multiple directives are returned) so both tests reference the same
header semantics.
0xc7c5
force-pushed
the
feature/auth-tests-api
branch
from
8b1922d to
3a5d867
Compare
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.