diff --git a/src/sentry/identity/gitlab/provider.py b/src/sentry/identity/gitlab/provider.py index ced1d745b2858b..7a3b6dd6fa43a4 100644 --- a/src/sentry/identity/gitlab/provider.py +++ b/src/sentry/identity/gitlab/provider.py @@ -5,6 +5,7 @@ from sentry.http import safe_urlopen, safe_urlread from sentry.identity.oauth2 import OAuth2Provider from sentry.utils import json +from sentry.utils.http import absolute_uri logger = logging.getLogger("sentry.integration.gitlab") @@ -27,7 +28,6 @@ def get_oauth_data(payload): data["token_type"] = payload["token_type"] if "created_at" in payload: data["created_at"] = int(payload["created_at"]) - return data @@ -73,7 +73,17 @@ def build_identity(self, data): } def get_refresh_token_params(self, refresh_token, *args, **kwargs): - return {"grant_type": "refresh_token", "refresh_token": refresh_token} + identity = kwargs.get("identity") + client_id = identity.data.get("client_id") + client_secret = identity.data.get("client_secret") + + return { + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "redirect_uri": absolute_uri("/extensions/gitlab/setup/"), + "client_id": client_id, + "client_secret": client_secret, + } def refresh_identity(self, identity, *args, **kwargs): refresh_token = identity.data.get("refresh_token") @@ -85,6 +95,7 @@ def refresh_identity(self, identity, *args, **kwargs): if not refresh_token_url: raise IdentityNotValid("Missing refresh token url") + kwargs["identity"] = identity data = self.get_refresh_token_params(refresh_token, *args, **kwargs) req = safe_urlopen(url=refresh_token_url, headers={}, data=data) @@ -110,4 +121,5 @@ def refresh_identity(self, identity, *args, **kwargs): self.handle_refresh_error(req, payload) identity.data.update(get_oauth_data(payload)) - return identity.update(data=identity.data) + identity.update(data=identity.data) + return identity diff --git a/src/sentry/integrations/gitlab/client.py b/src/sentry/integrations/gitlab/client.py index c9551a0a59dbf3..4ed2864a4e39bf 100644 --- a/src/sentry/integrations/gitlab/client.py +++ b/src/sentry/integrations/gitlab/client.py @@ -89,9 +89,12 @@ def identity(self): def metadata(self): return self.installation.model.metadata + def request_headers(self, identity): + access_token = identity.data["access_token"] + return {"Authorization": f"Bearer {access_token}"} + def request(self, method, path, data=None, params=None): - access_token = self.identity.data["access_token"] - headers = {"Authorization": f"Bearer {access_token}"} + headers = self.request_headers(self.identity) url = GitLabApiClientPath.build_api_url(self.metadata["base_url"], path) try: return self._request(method, url, headers=headers, data=data, params=params) @@ -99,8 +102,14 @@ def request(self, method, path, data=None, params=None): if self.is_refreshing_token: raise e self.is_refreshing_token = True - self.refresh_auth() - resp = self._request(method, url, headers=headers, data=data, params=params) + new_identity = self.refresh_auth() + resp = self._request( + method, + url, + headers=self.request_headers(new_identity), + data=data, + params=params, + ) self.is_refreshing_token = False return resp @@ -111,7 +120,7 @@ def refresh_auth(self): https://github.com/doorkeeper-gem/doorkeeper/wiki/Enable-Refresh-Token-Credentials#testing-with-oauth2-gem """ - self.identity.get_provider().refresh_identity( + return self.identity.get_provider().refresh_identity( self.identity, refresh_token_url="{}{}".format( self.metadata["base_url"], GitLabApiClientPath.oauth_token diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index a52f5fe1a29d72..6bd824286186c5 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -332,7 +332,15 @@ def get_pipeline_views(self): def build_integration(self, state): data = state["identity"]["data"] - oauth_data = get_oauth_data(data) + + # Gitlab requires the client_id and client_secret for refreshing the access tokens + oauth_config = state.get("oauth_config_information", {}) + oauth_data = { + **get_oauth_data(data), + "client_id": oauth_config.get("client_id"), + "client_secret": oauth_config.get("client_secret"), + } + user = get_user_info(data["access_token"], state["installation_data"]) group = self.get_group_info(data["access_token"], state["installation_data"]) include_subgroups = state["installation_data"]["include_subgroups"] diff --git a/tests/sentry/integrations/gitlab/test_integration.py b/tests/sentry/integrations/gitlab/test_integration.py index a403ae6a737d1e..0fa339e2993f72 100644 --- a/tests/sentry/integrations/gitlab/test_integration.py +++ b/tests/sentry/integrations/gitlab/test_integration.py @@ -58,11 +58,11 @@ def assert_setup_flow(self, user_id="user_id_1"): authorize_params = {k: v[0] for k, v in params.items()} access_token = "xxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx" - + refresh_token = "rrrrr-rrrrrrrrr-rrrrrrrrrr-rrrrrrrrrrrr" responses.add( responses.POST, "https://gitlab.example.com/oauth/token", - json={"access_token": access_token}, + json={"access_token": access_token, "refresh_token": refresh_token}, ) responses.add(responses.GET, "https://gitlab.example.com/api/v4/user", json={"id": user_id}) responses.add( @@ -133,7 +133,12 @@ def test_basic_flow(self, mock_sha): idp=idp, user=self.user, external_id="gitlab.example.com:user_id_1" ) assert identity.status == IdentityStatus.VALID - assert identity.data == {"access_token": "xxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"} + assert identity.data == { + "access_token": "xxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx", + "client_id": "client_id", + "client_secret": "client_secret", + "refresh_token": "rrrrr-rrrrrrrrr-rrrrrrrrrr-rrrrrrrrrrrr", + } def test_goback_to_instructions(self): # Go to instructions @@ -283,8 +288,14 @@ def test_get_stacktrace_link_file_identity_not_valid(self): status=401, json={}, ) - source_url = installation.get_stacktrace_link(repo, "README.md", ref, version) - assert not source_url + + try: + installation.get_stacktrace_link(repo, "README.md", ref, version) + except Exception as e: + assert e.code == 401 + else: + # check that the call throws. + assert False @responses.activate def test_get_stacktrace_link_use_default_if_version_404(self):