Revert "feat(bitbucket): Migrate to OAuth 2.0 (#772)"#774
Closed
thomasrockhu-codecov wants to merge 1 commit intomainfrom
Closed
Revert "feat(bitbucket): Migrate to OAuth 2.0 (#772)"#774thomasrockhu-codecov wants to merge 1 commit intomainfrom
thomasrockhu-codecov wants to merge 1 commit intomainfrom
Conversation
This reverts commit ec58642.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+144
to
149
| r = httpx.get(uri, headers=headers) | ||
| resp_args = urllib_parse.parse_qs(r.text) | ||
| return { | ||
| "key": data["access_token"], | ||
| "secret": data.get("refresh_token", ""), | ||
| "key": resp_args["oauth_token"][0], | ||
| "secret": resp_args["oauth_token_secret"][0], | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: The generate_access_token and generate_request_token methods don't handle non-200 HTTP responses from Bitbucket, which will cause an unhandled KeyError during the login flow.
Severity: HIGH
Suggested Fix
Before parsing the response in generate_request_token and generate_access_token, check the r.status_code. If the status is not 200, raise a TorngitServerFailureError or a similar exception that the view's error handling is designed to catch. This will prevent the unhandled KeyError and provide proper error feedback.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: libs/shared/shared/torngit/bitbucket.py#L144-L149
Potential issue: In `generate_request_token()` and `generate_access_token()`, HTTP
requests are made to Bitbucket's OAuth endpoints without checking the response status
code. If Bitbucket returns a non-200 response (e.g., a 4xx or 5xx error), the code
proceeds to parse the response body and access keys like `oauth_token` that will not
exist. This results in an unhandled `KeyError`. Because the calling view does not catch
`KeyError`, the user-facing login flow will fail with an unhandled exception, preventing
users from logging in via Bitbucket when an API error occurs.
Did we get this right? 👍 / 👎 to inform future reviews.
Comment on lines
+68
to
72
| ).decode() | ||
| response.set_signed_cookie( | ||
| "_bb_oauth_state", | ||
| state, | ||
| "_oauth_request_token", | ||
| encryptor.encode(data).decode(), | ||
| domain=settings.COOKIES_DOMAIN, |
Contributor
There was a problem hiding this comment.
Bug: The _oauth_request_token cookie is set without the httponly, secure, and samesite flags, creating a security vulnerability.
Severity: HIGH
Suggested Fix
Update the set_signed_cookie call to include the necessary security attributes. Specifically, set httponly=True, secure=settings.SESSION_COOKIE_SECURE, samesite=settings.COOKIE_SAME_SITE, and a reasonable max_age (e.g., 300 seconds) to mitigate security risks like XSS and CSRF.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/codecov-api/codecov_auth/views/bitbucket.py#L68-L72
Potential issue: When setting the `_oauth_request_token` cookie, the call to
`set_signed_cookie` omits several critical security attributes. The cookie is missing
`httponly=True`, `secure=True`, and `samesite='Lax'`. The absence of `httponly` exposes
the cookie's sensitive OAuth token secret to being stolen via XSS attacks. The lack of
the `secure` flag allows the cookie to be sent over unencrypted HTTP. This is a security
regression, as other OAuth cookies in the application and the previous implementation
correctly set these protective flags.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Cookie missing httponly, secure, samesite, and max_age attributes
- Added httponly=True, secure, samesite, and max_age=300 attributes to the _oauth_request_token cookie.
- ✅ Fixed: Missing error handling causes unhandled KeyError on failures
- Added error handling for network failures, status code checks, and key validation to prevent unhandled KeyError exceptions in both token generation methods.
Or push these changes by commenting:
@cursor push 867718437b
Preview (867718437b)
diff --git a/apps/codecov-api/api/internal/tests/unit/views/test_compare_flags_view.py b/apps/codecov-api/api/internal/tests/unit/views/test_compare_flags_view.py
--- a/apps/codecov-api/api/internal/tests/unit/views/test_compare_flags_view.py
+++ b/apps/codecov-api/api/internal/tests/unit/views/test_compare_flags_view.py
@@ -53,8 +53,8 @@
current_file.parent.parent.parent
/ f"samples/{self.parent_commit.commitid}_chunks.txt",
).read()
- read_chunks_mock.side_effect = (
- lambda x: head_chunks if x == self.commit.commitid else base_chunks
+ read_chunks_mock.side_effect = lambda x: (
+ head_chunks if x == self.commit.commitid else base_chunks
)
diff_totals_mock.return_value = ReportTotals(
files=0,
@@ -225,10 +225,8 @@
current_file.parent.parent.parent
/ f"samples/{self.parent_commit.commitid}_chunks.txt",
).read()
- read_chunks_mock.side_effect = (
- lambda x: head_chunks
- if x == commit_with_custom_reports.commitid
- else base_chunks
+ read_chunks_mock.side_effect = lambda x: (
+ head_chunks if x == commit_with_custom_reports.commitid else base_chunks
)
diff_totals_mock.return_value = ReportTotals(
files=0,
diff --git a/apps/codecov-api/codecov_auth/views/bitbucket.py b/apps/codecov-api/codecov_auth/views/bitbucket.py
--- a/apps/codecov-api/codecov_auth/views/bitbucket.py
+++ b/apps/codecov-api/codecov_auth/views/bitbucket.py
@@ -70,6 +70,10 @@
"_oauth_request_token",
encryptor.encode(data).decode(),
domain=settings.COOKIES_DOMAIN,
+ httponly=True,
+ secure=settings.SESSION_COOKIE_SECURE,
+ samesite=settings.COOKIE_SAME_SITE,
+ max_age=300,
)
self.store_to_cookie_utm_tags(response)
return response
diff --git a/apps/codecov-api/services/bundle_analysis.py b/apps/codecov-api/services/bundle_analysis.py
--- a/apps/codecov-api/services/bundle_analysis.py
+++ b/apps/codecov-api/services/bundle_analysis.py
@@ -50,7 +50,7 @@
Gets the file extension of the file without the dot
"""
# At times file can be something like './index.js + 12 modules', only keep the real filepath
- filename = filename.split(" ")[0]
+ filename = filename.split(" ", maxsplit=1)[0]
# Retrieve the file extension with the dot
_, file_extension = os.path.splitext(filename)
# Return empty string if file has no extension
diff --git a/apps/codecov-api/upload/tokenless/appveyor.py b/apps/codecov-api/upload/tokenless/appveyor.py
--- a/apps/codecov-api/upload/tokenless/appveyor.py
+++ b/apps/codecov-api/upload/tokenless/appveyor.py
@@ -61,8 +61,10 @@
# validate build
if not any(
filter(
- lambda j: j["jobId"] == self.upload_params.get("build", "") # type: ignore
- and j.get("finished") is None,
+ lambda j: (
+ j["jobId"] == self.upload_params.get("build", "") # type: ignore
+ and j.get("finished") is None
+ ),
build["build"]["jobs"],
)
):
diff --git a/apps/codecov-api/webhook_handlers/tests/test_github.py b/apps/codecov-api/webhook_handlers/tests/test_github.py
--- a/apps/codecov-api/webhook_handlers/tests/test_github.py
+++ b/apps/codecov-api/webhook_handlers/tests/test_github.py
@@ -662,13 +662,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_creates_new_owner_if_dne_all_repos_non_default_app(self):
username, service_id = "newuser", 123456
@@ -710,13 +706,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_creates_new_owner_if_dne(self):
username, service_id = "newuser", 123456
@@ -752,13 +744,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_update_repos_existing_ghapp_installation(self):
owner = OwnerFactory(service=Service.GITHUB.value)
@@ -852,13 +840,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_update_existing_ghapp(self):
owner = OwnerFactory(service=Service.GITHUB.value)
@@ -911,13 +895,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- repos_affected,
- using_integration: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, repos_affected, using_integration: (
+ None
+ ),
)
def test_installation_repositories_update_existing_ghapp_all_repos(self):
owner = OwnerFactory(service=Service.GITHUB.value)
@@ -957,13 +937,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_with_other_actions_does_not_set_owner_integration_id_if_none(
self,
@@ -1093,13 +1069,9 @@
)
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_sets_pem_path_for_sentry_app(self):
username, service_id = "sentryuser", 998877
diff --git a/apps/codecov-api/webhook_handlers/tests/test_github_enterprise.py b/apps/codecov-api/webhook_handlers/tests/test_github_enterprise.py
--- a/apps/codecov-api/webhook_handlers/tests/test_github_enterprise.py
+++ b/apps/codecov-api/webhook_handlers/tests/test_github_enterprise.py
@@ -455,13 +455,9 @@
@freeze_time("2024-03-28T00:00:00")
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_creates_new_owner_if_dne(self):
username, service_id = "newuser", 123456
@@ -504,13 +500,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_creates_new_owner_if_dne_all_repos(self):
username, service_id = "newuser", 123456
@@ -553,13 +545,9 @@
@freeze_time("2024-03-28T00:00:00")
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_creates_new_owner_if_dne(self):
username, service_id = "newuser", 123456
@@ -645,13 +633,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_update_existing_ghapp(self):
owner = OwnerFactory(service=Service.GITHUB_ENTERPRISE.value)
@@ -690,13 +674,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_update_existing_ghapp_all_repos(self):
owner = OwnerFactory(service=Service.GITHUB_ENTERPRISE.value)
@@ -733,13 +713,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_with_other_actions_sets_owner_integration_id_if_none(
self,
@@ -782,13 +758,9 @@
@patch(
"services.task.TaskService.refresh",
- lambda self,
- ownerid,
- username,
- sync_teams,
- sync_repos,
- using_integration,
- repos_affected: None,
+ lambda self, ownerid, username, sync_teams, sync_repos, using_integration, repos_affected: (
+ None
+ ),
)
def test_installation_repositories_with_other_actions_sets_owner_itegration_id_if_none(
self,
diff --git a/apps/worker/tasks/notify.py b/apps/worker/tasks/notify.py
--- a/apps/worker/tasks/notify.py
+++ b/apps/worker/tasks/notify.py
@@ -369,8 +369,9 @@
)
ghapp_default_installations = list(
filter(
- lambda obj: obj.name == installation_name_to_use
- and obj.is_configured(),
+ lambda obj: (
+ obj.name == installation_name_to_use and obj.is_configured()
+ ),
commit.repository.author.github_app_installations or [],
)
)
diff --git a/apps/worker/tasks/tests/unit/test_base.py b/apps/worker/tasks/tests/unit/test_base.py
--- a/apps/worker/tasks/tests/unit/test_base.py
+++ b/apps/worker/tasks/tests/unit/test_base.py
@@ -130,8 +130,8 @@
"created_timestamp": "2023-06-13 10:00:00.000000",
"delivery_info": {"routing_key": "my-queue"},
}
- mock_task_request.get.side_effect = (
- lambda key, default: fake_request_values.get(key, default)
+ mock_task_request.get.side_effect = lambda key, default: (
+ fake_request_values.get(key, default)
)
mocked_get_db_session.return_value = dbsession
task_instance = SampleTask()
diff --git a/apps/worker/tasks/tests/unit/test_compute_comparison.py b/apps/worker/tasks/tests/unit/test_compute_comparison.py
--- a/apps/worker/tasks/tests/unit/test_compute_comparison.py
+++ b/apps/worker/tasks/tests/unit/test_compute_comparison.py
@@ -326,11 +326,11 @@
mocker.patch.object(
ReportService,
"get_existing_report_for_commit",
- side_effect=lambda commit,
- *args,
- **kwargs: ReadOnlyReport.create_from_report(sample_report)
- if commit == head_commit
- else None,
+ side_effect=lambda commit, *args, **kwargs: (
+ ReadOnlyReport.create_from_report(sample_report)
+ if commit == head_commit
+ else None
+ ),
)
patch_totals = ReportTotals(
files=3, lines=200, hits=100, misses=100, coverage="10.5"
diff --git a/libs/shared/shared/bots/github_apps.py b/libs/shared/shared/bots/github_apps.py
--- a/libs/shared/shared/bots/github_apps.py
+++ b/libs/shared/shared/bots/github_apps.py
@@ -227,9 +227,13 @@
redis_connection = get_redis_connection()
return list(
filter(
- lambda obj: not determine_if_entity_is_rate_limited(
- redis_connection,
- gh_app_key_name(app_id=obj.app_id, installation_id=obj.installation_id),
+ lambda obj: (
+ not determine_if_entity_is_rate_limited(
+ redis_connection,
+ gh_app_key_name(
+ app_id=obj.app_id, installation_id=obj.installation_id
+ ),
+ )
),
apps_to_consider,
)
diff --git a/libs/shared/shared/bundle_analysis/utils.py b/libs/shared/shared/bundle_analysis/utils.py
--- a/libs/shared/shared/bundle_analysis/utils.py
+++ b/libs/shared/shared/bundle_analysis/utils.py
@@ -278,7 +278,7 @@
Gets the file extension of the file without the dot
"""
# At times file can be something like './index.js + 12 modules', only keep the real filepath
- filename = filename.split(" ")[0]
+ filename = filename.split(" ", maxsplit=1)[0]
# Retrieve the file extension with the dot
_, file_extension = os.path.splitext(filename)
# Return empty string if file has no extension
diff --git a/libs/shared/shared/django_apps/ta_timeseries/tests/factories.py b/libs/shared/shared/django_apps/ta_timeseries/tests/factories.py
--- a/libs/shared/shared/django_apps/ta_timeseries/tests/factories.py
+++ b/libs/shared/shared/django_apps/ta_timeseries/tests/factories.py
@@ -21,9 +21,9 @@
outcome = "pass"
duration_seconds = factory.fuzzy.FuzzyFloat(low=0.0, high=100.0)
failure_message = factory.LazyAttribute(
- lambda obj: f"failure_message_{obj.outcome}"
- if obj.outcome == "failure"
- else None
+ lambda obj: (
+ f"failure_message_{obj.outcome}" if obj.outcome == "failure" else None
+ )
)
framework = "Pytest"
filename = factory.Sequence(lambda n: f"test_{n}.py")
diff --git a/libs/shared/shared/torngit/bitbucket.py b/libs/shared/shared/torngit/bitbucket.py
--- a/libs/shared/shared/torngit/bitbucket.py
+++ b/libs/shared/shared/torngit/bitbucket.py
@@ -125,9 +125,30 @@
callback_uri=redirect_url,
)
uri, headers, body = client.sign(self._OAUTH_REQUEST_TOKEN_URL)
- r = httpx.get(uri, headers=headers)
- oauth_token = urllib_parse.parse_qs(r.text)["oauth_token"][0]
- oauth_token_secret = urllib_parse.parse_qs(r.text)["oauth_token_secret"][0]
+ try:
+ r = httpx.get(uri, headers=headers)
+ except (httpx.NetworkError, httpx.TimeoutException):
+ raise TorngitServerUnreachableError("Bitbucket was not able to be reached.")
+
+ if r.status_code >= 500:
+ raise TorngitServer5xxCodeError("Bitbucket is having 5xx issues")
+ elif r.status_code >= 400:
+ raise TorngitClientGeneralError(
+ r.status_code,
+ response_data={"content": r.content},
+ message=f"Bitbucket API: {r.reason_phrase}",
+ )
+
+ parsed = urllib_parse.parse_qs(r.text)
+ if "oauth_token" not in parsed or "oauth_token_secret" not in parsed:
+ raise TorngitClientGeneralError(
+ r.status_code,
+ response_data={"content": r.content},
+ message="Invalid OAuth response from Bitbucket",
+ )
+
+ oauth_token = parsed["oauth_token"][0]
+ oauth_token_secret = parsed["oauth_token_secret"][0]
return {"oauth_token": oauth_token, "oauth_token_secret": oauth_token_secret}
def generate_access_token(
@@ -141,8 +162,28 @@
verifier=verifier,
)
uri, headers, body = client.sign(self._OAUTH_ACCESS_TOKEN_URL)
- r = httpx.get(uri, headers=headers)
+ try:
+ r = httpx.get(uri, headers=headers)
+ except (httpx.NetworkError, httpx.TimeoutException):
+ raise TorngitServerUnreachableError("Bitbucket was not able to be reached.")
+
+ if r.status_code >= 500:
+ raise TorngitServer5xxCodeError("Bitbucket is having 5xx issues")
+ elif r.status_code >= 400:
+ raise TorngitClientGeneralError(
+ r.status_code,
+ response_data={"content": r.content},
+ message=f"Bitbucket API: {r.reason_phrase}",
+ )
+
resp_args = urllib_parse.parse_qs(r.text)
+ if "oauth_token" not in resp_args or "oauth_token_secret" not in resp_args:
+ raise TorngitClientGeneralError(
+ r.status_code,
+ response_data={"content": r.content},
+ message="Invalid OAuth response from Bitbucket",
+ )
+
return {
"key": resp_args["oauth_token"][0],
"secret": resp_args["oauth_token_secret"][0],
diff --git a/libs/shared/tests/integration/test_report.py b/libs/shared/tests/integration/test_report.py
--- a/libs/shared/tests/integration/test_report.py
+++ b/libs/shared/tests/integration/test_report.py
@@ -579,13 +579,15 @@
chunks="null\n[1]\n[1]\n[1]\n<<<<< end_of_chunk >>>>>\nnull\n[1]\n[1]\n[1]",
),
{
- "color": lambda cov: "purple"
- if cov is None
- else "#e1e1e1"
- if cov == 0
- else "green"
- if cov > 0
- else "red"
+ "color": lambda cov: (
+ "purple"
+ if cov is None
+ else "#e1e1e1"
+ if cov == 0
+ else "green"
+ if cov > 0
+ else "red"
+ )
},
[
{
diff --git a/libs/shared/tests/unit/encryption/test_selector.py b/libs/shared/tests/unit/encryption/test_selector.py
--- a/libs/shared/tests/unit/encryption/test_selector.py
+++ b/libs/shared/tests/unit/encryption/test_selector.py
@@ -101,7 +101,10 @@
different_enc = EncryptorDivider(enc_dict, "abd")
encoded = different_enc.encode(value)
res = different_enc.decrypt_token(encoded)
- assert res == {"key": value.split(":")[0], "secret": value.split(":")[1]}
+ assert res == {
+ "key": value.split(":", maxsplit=1)[0],
+ "secret": value.split(":")[1],
+ }
def test_decrypt_token_key_normal_generated_with_secret_pair_refresh():This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| "_bb_oauth_state", | ||
| state, | ||
| "_oauth_request_token", | ||
| encryptor.encode(data).decode(), |
There was a problem hiding this comment.
Cookie missing httponly, secure, samesite, and max_age attributes
Medium Severity
The _oauth_request_token cookie storing encrypted OAuth credentials is set without httponly=True, secure, samesite, or max_age attributes. The OAuth 2.0 version being reverted had all four protections. Without httponly, JavaScript can read the cookie (XSS risk). Without secure, it can be transmitted over plain HTTP. Without samesite, CSRF protections are weakened. Without max_age, the cookie persists for the entire browser session instead of expiring after 5 minutes.
| data={ | ||
| "grant_type": "refresh_token", | ||
| "refresh_token": current_token["secret"], | ||
| }, |
There was a problem hiding this comment.
Missing error handling causes unhandled KeyError on failures
Medium Severity
generate_request_token and generate_access_token call httpx.get without checking the response status code. If Bitbucket returns a 4xx/5xx error, urllib_parse.parse_qs(r.text) won't contain the expected oauth_token or oauth_token_secret keys, raising a KeyError. This exception is not a subclass of TorngitServerFailureError, so the view's except TorngitServerFailureError handler won't catch it, resulting in a 500 error for users instead of a graceful redirect to the login page.
Additional Locations (1)
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #774 +/- ##
==========================================
+ Coverage 92.25% 92.27% +0.01%
==========================================
Files 1305 1305
Lines 47957 47938 -19
Branches 1636 1628 -8
==========================================
- Hits 44245 44233 -12
+ Misses 3401 3396 -5
+ Partials 311 309 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit ec58642.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Note
High Risk
High risk because it changes Bitbucket login/authentication flow and request signing, impacting how user tokens are issued/stored and how all Bitbucket API calls are authenticated.
Overview
Reverts Bitbucket OAuth 2.0 migration back to OAuth 1.0a. The Bitbucket login flow now requests a temporary OAuth1 request token, stores the token+secret in a signed
_oauth_request_tokencookie, and exchanges it usingoauth_verifier(removing the OAuth2code/stateCSRF flow).Updates Bitbucket API authentication to OAuth1-signed requests.
shared/torngit/bitbucket.pynow signs requests viaoauthlib(query-param signature), adds newgenerate_request_token/updatedgenerate_access_token, and drops the prior OAuth2 bearer-token + refresh-token logic.Disables token refresh callbacks for Bitbucket. Both API and worker-side
get_token_refresh_callbacknow returnNoneforbitbucket(andbitbucket_server), with tests adjusted; integration VCR cassettes are updated to reflect OAuth parameters in Bitbucket API URLs.Written by Cursor Bugbot for commit 063a508. This will update automatically on new commits. Configure here.