client session updation#433
client session updation#433Bala-Sakabattula merged 2 commits intorelease-engineering:uat-instancefrom
Conversation
Review Summary by QodoReplace deprecated session() with server_info() for auth validation
WalkthroughsDescription• Replace deprecated session() call with server_info() for authentication validation • Update authentication check to use non-deprecated Jira REST endpoint • Align with Atlassian's deprecation notice for cookie-based authentication • Update corresponding unit tests to mock new server_info() method Diagramflowchart LR
A["Deprecated session() endpoint"] -->|"Replace with"| B["server_info() endpoint"]
B -->|"Validates authentication"| C["Non-deprecated REST API"]
D["Unit tests"] -->|"Update mocks"| E["server_info() method"]
File Changes1. sync2jira/downstream_issue.py
|
Code Review by Qodo
1.
|
sync2jira/downstream_issue.py
Outdated
| # Validate authentication (server_info uses a non-deprecated endpoint; /rest/auth/1/session is deprecated) | ||
| client.server_info() |
There was a problem hiding this comment.
1. Server_info() bypasses auth validation 🐞 Bug ✓ Correctness
client.server_info() calls /rest/api/2/serverInfo, a publicly accessible Jira Cloud endpoint that returns HTTP 200 regardless of credential validity. This means misconfigured or invalid credentials will silently pass the auth check in get_jira_client(), and all downstream authenticated API calls will fail with confusing, context-free errors instead of a clear auth failure at client initialization time.
Agent Prompt
## Issue description
`client.server_info()` was introduced as a replacement for the deprecated `client.session()` to validate authentication after creating a JIRA client. However, `/rest/api/2/serverInfo` is a publicly accessible endpoint that returns HTTP 200 regardless of credential validity. This silently allows misconfigured clients to pass the auth check, causing all subsequent authenticated operations to fail with confusing errors.
## Issue Context
The original `client.session()` called `/rest/auth/1/session` which explicitly required valid credentials and raised `JIRAError` on failure. The replacement must also call an **authenticated** endpoint. The existing `check_jira_status()` function already uses `server_info()` as a server-reachability probe — using it again in `get_jira_client()` for auth validation is both semantically wrong and redundant.
## Fix Focus Areas
- `sync2jira/downstream_issue.py[297-297]` — replace `client.server_info()` with `client.myself()` and update the comment to reflect that `myself()` calls `/rest/api/2/myself`, which requires valid credentials and raises `JIRAError` on auth failure
- `tests/test_downstream_issue.py[161-161]` — update mock: `mock_jira_instance.myself.return_value = None`
- `tests/test_downstream_issue.py[172-172]` — update assertion: `mock_jira_instance.myself.assert_called_once()`
- `tests/test_downstream_issue.py[184-184]` — update side_effect: `mock_jira_instance.myself.side_effect = JIRAError("Authentication failed")`
- `tests/test_downstream_issue.py[195-195]` — update assertion: `mock_jira_instance.myself.assert_called_once()`
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
@Bala-Sakabattula, are we really trying to validate our credentials here, or are we just trying to establish that the Jira service is available?
If it is the latter, then you should update your code comment to reflect that. If it is the former, then I guess we need to pick some other API method.
Regardless, we shouldn't refer to /rest/auth/1/session in the code (comment) going forward, since we no longer make that call. (When we look at this code next year or in the years after that, we won't care that, back in 2025, we used to call the session endpoint here!)
There was a problem hiding this comment.
Code Implementation 🛠️
Implementation: Replace the post-client-construction auth validation call from server_info() (public endpoint) to myself() (authenticated endpoint), and update the accompanying comment to avoid referencing deprecated endpoints and to reflect the new intent.
| # Validate authentication (server_info uses a non-deprecated endpoint; /rest/auth/1/session is deprecated) | |
| client.server_info() | |
| # Validate authentication using an authenticated endpoint. | |
| # NOTE: server_info() may succeed even when credentials are invalid (e.g., Jira Cloud). | |
| client.myself() |
See review comment here
There was a problem hiding this comment.
But client.server_info() fails if we have valid invalid creds as you can see in the doc as well it will return 401 unauthorized if creds are invalid.
@webbnh I will change the comment part.
There was a problem hiding this comment.
Question:
But client.server_info() fails if we have valid invalid creds as you can see in the doc as well it will return 401 unauthorized if creds are invalid. https://developer.atlassian.com/cloud/jira/platform/rest/v2/api-group-server-info/#api-rest-api-2-serverinfo-get @webbnh I will change the comment part.
Answer:
""
relevant_sections: []
question_is_relevant: 0
|
/agentic_review |
|
Persistent review updated to latest commit 0aae3c2 |
9abae4a
into
release-engineering:uat-instance
I updated the Jira client call used for checking service availability from
session()toserver_info()as thesession()function is depreciated and will be removed soon.