Skip to content

feat(bitbucket): Migrate to OAuth 2.0 #772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
drazisil-codecov merged 10 commits into from
Mar 17, 2026
Merged

feat(bitbucket): Migrate to OAuth 2.0 #772

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d45d8bb
feat(bitbucket): Migrate to OAuth 2.0
sentry[bot] Mar 16, 2026
63d11ff
fix(bitbucket): add OAuth 2.0 state param to prevent CSRF/account tak…
drazisil-codecov Mar 16, 2026
b879dc9
test(bitbucket): update VCR cassettes for OAuth 2.0 Bearer token auth
drazisil-codecov Mar 16, 2026
1d8cf9f
test(bitbucket): add missing VCR cassette for test_list_repos_generator
drazisil-codecov Mar 16, 2026
68e59cf
fix(bitbucket): add OAuth 2.0 token refresh on 401
drazisil-codecov Mar 16, 2026
6e40505
fix(bitbucket): restore BITBUCKET_SERVER refresh guard; handle refres…
drazisil-codecov Mar 16, 2026
ef14c54
test(bitbucket): update tests for enabled token refresh callback
drazisil-codecov Mar 16, 2026
308161b
fix(bitbucket): add secure flag to state cookie; remove unused param
drazisil-codecov Mar 16, 2026
433f3a9
fix(bitbucket): add samesite and max_age to state cookie
drazisil-codecov Mar 16, 2026
7c1c15f
refactor(bitbucket): replace while loop with explicit two-attempt retry
drazisil-codecov Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 63 additions & 39 deletions apps/codecov-api/codecov_auth/tests/unit/views/test_bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,16 @@
from codecov_auth.models import Owner
from codecov_auth.views.bitbucket import BitbucketLoginView
from shared.torngit.bitbucket import Bitbucket
from shared.torngit.exceptions import TorngitServer5xxCodeError
from utils.encryption import encryptor
from shared.torngit.exceptions import (
TorngitClientGeneralError,
)


def test_get_bitbucket_redirect(client, settings, mocker):
mocked_get = mocker.patch.object(
mocked_generate = mocker.patch.object(
Bitbucket,
"generate_request_token",
return_value={
"oauth_token": "testy6r2of6ajkmrub",
"oauth_token_secret": "testzibw5q01scpl8qeeupzh8u9yu8hz",
},
"generate_redirect_url",
return_value="https://bitbucket.org/site/oauth2/authorize?client_id=testqmo19ebdkseoby&response_type=code&redirect_uri=http%3A%2F%2Flocalhost&state=teststate",
)
settings.BITBUCKET_REDIRECT_URI = "http://localhost"
settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
Expand All @@ -28,30 +26,30 @@ def test_get_bitbucket_redirect(client, settings, mocker):
res = client.get(url, SERVER_NAME="localhost:8000")
assert res.status_code == 302

assert "_oauth_request_token" in res.cookies
cookie = res.cookies["_oauth_request_token"]
assert "_bb_oauth_state" in res.cookies
cookie = res.cookies["_bb_oauth_state"]
assert cookie.value
assert cookie.get("domain") == settings.COOKIES_DOMAIN
assert (
res.url
== "https://bitbucket.org/api/1.0/oauth/authenticate?oauth_token=testy6r2of6ajkmrub"
)
mocked_get.assert_called_with(settings.BITBUCKET_REDIRECT_URI)
assert mocked_generate.call_count == 1
# state kwarg was passed through
_, kwargs = mocked_generate.call_args
assert kwargs.get("state") is not None


def test_get_bitbucket_redirect_bitbucket_unavailable(client, settings, mocker):
mocked_get = mocker.patch.object(
Bitbucket, "generate_request_token", side_effect=TorngitServer5xxCodeError()
def test_get_bitbucket_redirect_bitbucket_error(client, settings, mocker):
mocker.patch.object(
Bitbucket,
"generate_redirect_url",
side_effect=TorngitClientGeneralError(400, {}, "bad request"),
)
settings.BITBUCKET_REDIRECT_URI = "http://localhost"
settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
url = reverse("bitbucket-login")
res = client.get(url, SERVER_NAME="localhost:8000")
assert res.status_code == 302
assert "_oauth_request_token" not in res.cookies
assert "_bb_oauth_state" not in res.cookies
assert res.url == url
mocked_get.assert_called_with(settings.BITBUCKET_REDIRECT_URI)


async def fake_get_authenticated_user():
Expand Down Expand Up @@ -110,23 +108,22 @@ async def fake_list_teams():
"generate_access_token",
return_value={
"key": "test6tl3evq7c8vuyn",
"secret": "testdm61tppb5x0tam7nae3qajhcepzz",
"secret": "testrefreshtoken",
},
)
settings.BITBUCKET_REDIRECT_URI = "http://localhost"
settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
settings.CODECOV_DASHBOARD_URL = "dashboard.value"
settings.COOKIE_SECRET = "aaaaa"

state = "test_state_value_abc123"
url = reverse("bitbucket-login")
oauth_request_token = (
"dGVzdDZ0bDNldnE3Yzh2dXlu|dGVzdGRtNjF0cHBiNXgwdGFtN25hZTNxYWpoY2Vweno="
)
client.cookies = SimpleCookie(
{
"_oauth_request_token": signing.get_cookie_signer(
salt="_oauth_request_token"
).sign(encryptor.encode(oauth_request_token).decode())
"_bb_oauth_state": signing.get_cookie_signer(salt="_bb_oauth_state").sign(
state
)
}
)
mock_create_user_onboarding_metric = mocker.patch(
Expand All @@ -135,17 +132,17 @@ async def fake_list_teams():

res = client.get(
url,
{"oauth_verifier": 8519288973, "oauth_token": "test1daxl4jnhegoh4"},
{"code": "auth_code_from_bitbucket", "state": state},
SERVER_NAME="localhost:8000",
)
assert res.status_code == 302
assert res.url == "dashboard.value/bb"
assert "_oauth_request_token" in res.cookies
cookie = res.cookies["_oauth_request_token"]
assert "_bb_oauth_state" in res.cookies
cookie = res.cookies["_bb_oauth_state"]
assert cookie.value == ""
assert cookie.get("domain") == settings.COOKIES_DOMAIN
mocked_get.assert_called_with(
"test6tl3evq7c8vuyn", "testdm61tppb5x0tam7nae3qajhcepzz", "8519288973"
"auth_code_from_bitbucket", settings.BITBUCKET_REDIRECT_URI
)
owner = Owner.objects.get(username="ThiagoCodecov", service="bitbucket")
expected_call = call(
Expand All @@ -155,13 +152,8 @@ async def fake_list_teams():
)
assert mock_create_user_onboarding_metric.call_args_list == [expected_call]

assert (
encryptor.decode(owner.oauth_token)
== "test6tl3evq7c8vuyn:testdm61tppb5x0tam7nae3qajhcepzz"
)


def test_get_bitbucket_already_token_no_cookie(
def test_get_bitbucket_already_token_no_state_cookie(
client, settings, mocker, db, mock_redis
):
mocker.patch(
Expand All @@ -175,7 +167,7 @@ def test_get_bitbucket_already_token_no_cookie(
"generate_access_token",
return_value={
"key": "test6tl3evq7c8vuyn",
"secret": "testdm61tppb5x0tam7nae3qajhcepzz",
"secret": "testrefreshtoken",
},
)
settings.BITBUCKET_REDIRECT_URI = "http://localhost"
Expand All @@ -184,7 +176,39 @@ def test_get_bitbucket_already_token_no_cookie(
url = reverse("bitbucket-login")
res = client.get(
url,
{"oauth_verifier": 8519288973, "oauth_token": "test1daxl4jnhegoh4"},
{"code": "auth_code_from_bitbucket", "state": "some_state"},
SERVER_NAME="localhost:8000",
)
assert res.status_code == 302
assert res.url == "/login/bitbucket"
assert not mocked_get.called


def test_get_bitbucket_state_mismatch(client, settings, mocker, db, mock_redis):
mocked_get = mocker.patch.object(
Bitbucket,
"generate_access_token",
return_value={
"key": "test6tl3evq7c8vuyn",
"secret": "testrefreshtoken",
},
)
settings.BITBUCKET_REDIRECT_URI = "http://localhost"
settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
settings.COOKIE_SECRET = "aaaaa"

url = reverse("bitbucket-login")
client.cookies = SimpleCookie(
{
"_bb_oauth_state": signing.get_cookie_signer(salt="_bb_oauth_state").sign(
"legit_state"
)
}
)
res = client.get(
url,
{"code": "auth_code_from_bitbucket", "state": "attacker_injected_state"},
SERVER_NAME="localhost:8000",
)
assert res.status_code == 302
Expand Down
51 changes: 21 additions & 30 deletions apps/codecov-api/codecov_auth/views/bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import base64
import logging
from urllib.parse import urlencode
import secrets

from asgiref.sync import async_to_sync
from django.conf import settings
Expand All @@ -13,8 +12,10 @@
UserOnboardingMetricsService,
)
from shared.torngit import Bitbucket
from shared.torngit.exceptions import TorngitServerFailureError
from utils.encryption import encryptor
from shared.torngit.exceptions import (
TorngitClientGeneralError,
TorngitServerFailureError,
)

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -53,23 +54,16 @@ def redirect_to_bitbucket_step(self, request):
"secret": settings.BITBUCKET_CLIENT_SECRET,
}
)
oauth_token_pair = repo_service.generate_request_token(
settings.BITBUCKET_REDIRECT_URI
state = secrets.token_urlsafe(32)
url_to_redirect = repo_service.generate_redirect_url(
settings.BITBUCKET_REDIRECT_URI, state=state
)
oauth_token = oauth_token_pair["oauth_token"]
oauth_token_secret = oauth_token_pair["oauth_token_secret"]
url_params = urlencode({"oauth_token": oauth_token})
url_to_redirect = f"{Bitbucket._OAUTH_AUTHORIZE_URL}?{url_params}"
response = redirect(url_to_redirect)
data = (
base64.b64encode(oauth_token.encode())
+ b"|"
+ base64.b64encode(oauth_token_secret.encode())
).decode()
response.set_signed_cookie(
"_oauth_request_token",
encryptor.encode(data).decode(),
"_bb_oauth_state",
state,
domain=settings.COOKIES_DOMAIN,
httponly=True,
)
self.store_to_cookie_utm_tags(response)
return response
Expand All @@ -81,19 +75,13 @@ def actual_login_step(self, request):
"secret": settings.BITBUCKET_CLIENT_SECRET,
}
)
oauth_verifier = request.GET.get("oauth_verifier")
request_cookie = request.get_signed_cookie("_oauth_request_token", default=None)
if not request_cookie:
log.warning(
"Request arrived with proper url params but not the proper cookies"
)
expected_state = request.get_signed_cookie("_bb_oauth_state", default=None)
if not expected_state or request.GET.get("state") != expected_state:
log.warning("Bitbucket OAuth state mismatch — possible CSRF attempt")
Copy link
Copy Markdown
Contributor

@ajay-sentry ajay-sentry Mar 16, 2026

Choose a reason for hiding this comment

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

nit: would it be worth it to add exc_info to this log? Or maybe some other identifying things

return redirect(reverse("bitbucket-login"))
request_cookie = encryptor.decode(request_cookie)
cookie_key, cookie_secret = [
base64.b64decode(i).decode() for i in request_cookie.split("|")
]
code = request.GET.get("code")
token = repo_service.generate_access_token(
cookie_key, cookie_secret, oauth_verifier
code, settings.BITBUCKET_REDIRECT_URI
)
user_dict = self.fetch_user_data(token)
user = self.get_and_modify_owner(user_dict, request)
Expand All @@ -102,7 +90,7 @@ def actual_login_step(self, request):
redirection_url, user
)
response = redirect(redirection_url)
response.delete_cookie("_oauth_request_token", domain=settings.COOKIES_DOMAIN)
response.delete_cookie("_bb_oauth_state", domain=settings.COOKIES_DOMAIN)
self.login_owner(user, request, response)
log.info("User successfully logged in", extra={"ownerid": user.ownerid})
UserOnboardingMetricsService.create_user_onboarding_metric(
Expand All @@ -115,7 +103,7 @@ def get(self, request):
return redirect(f"{settings.CODECOV_DASHBOARD_URL}/login")

try:
if request.GET.get("oauth_verifier"):
if request.GET.get("code"):
log.info("Logging into bitbucket after authorization")
return self.actual_login_step(request)
else:
Expand All @@ -124,3 +112,6 @@ def get(self, request):
except TorngitServerFailureError:
log.warning("Bitbucket not available for login")
return redirect(reverse("bitbucket-login"))
except TorngitClientGeneralError:
log.warning("Bitbucket OAuth error during login")
return redirect(reverse("bitbucket-login"))
2 changes: 0 additions & 2 deletions apps/codecov-api/services/repo_providers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ def get_token_refresh_callback(
"""
if owner is None:
return None
if service == Service.BITBUCKET or service == Service.BITBUCKET_SERVER:
return None

@sync_to_async
def callback(new_token: OauthConsumerToken) -> None:
Expand Down
4 changes: 0 additions & 4 deletions apps/worker/helpers/token_refresh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ def get_token_refresh_callback(owner: Owner) -> Callable[[dict], None]:
if owner is None:
return None

service = owner.service
if service == "bitbucket" or service == "bitbucket_server":
return None

async def callback(new_token: dict) -> None:
Comment on lines +22 to 25
Copy link
Copy Markdown
Contributor Author

@sentry sentry bot Mar 16, 2026

Choose a reason for hiding this comment

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

Bug: The worker's token refresh callback for Bitbucket updates owner.oauth_token in memory but doesn't commit the change to the database, causing the new token to be lost.
Severity: HIGH

Suggested Fix

After updating the owner.oauth_token in the callback within apps/worker/helpers/token_refresh.py, ensure the change is persisted to the database. This can be done by calling owner.get_db_session().commit() or an equivalent persistence method on the SQLAlchemy session.

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/worker/helpers/token_refresh.py#L22-L25

Potential issue: The pull request enables a token refresh callback for Bitbucket in the
worker context. When this callback is invoked, it successfully refreshes the token and
updates the `owner.oauth_token` attribute on the SQLAlchemy `Owner` model instance.
However, it fails to call `owner.save()` or `db_session.commit()` to persist this change
to the database. As a result, the refreshed token is lost after the task completes.
Subsequent tasks will use the old, expired token, triggering an unnecessary and
continuous refresh cycle without ever saving the valid new token.

log.info(
"Saving new token after refresh",
Expand Down
Loading
Loading