From b3c6e1d73739c632a47b9fedd9ea70c7bd1b017b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Salom=C3=A9=20Voltz?= Date: Wed, 20 Nov 2024 16:11:45 +0100 Subject: [PATCH] fix(with-incident-details): Ensure the token has the correct scope 'incident:read' --- ...nt_details_does_not_tell_about_required.md | 41 +++++++ .../secret/scan/secret_scan_common_options.py | 32 +++++- ggshield/core/errors.py | 9 ++ pyproject.toml | 2 +- .../unit/cmd/scan/test_scan_common_options.py | 102 ++++++++++++++++++ 5 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 changelog.d/20241120_160018_salome.voltz_scrt_4913_ggshield_with_incident_details_does_not_tell_about_required.md create mode 100644 tests/unit/cmd/scan/test_scan_common_options.py diff --git a/changelog.d/20241120_160018_salome.voltz_scrt_4913_ggshield_with_incident_details_does_not_tell_about_required.md b/changelog.d/20241120_160018_salome.voltz_scrt_4913_ggshield_with_incident_details_does_not_tell_about_required.md new file mode 100644 index 0000000000..62d5a98edb --- /dev/null +++ b/changelog.d/20241120_160018_salome.voltz_scrt_4913_ggshield_with_incident_details_does_not_tell_about_required.md @@ -0,0 +1,41 @@ + + + + + + + +### Fixed + +- When `ggshield secret scan` is called with `--with-incident-details` and the token does not have the required scopes, the command fails and an error message is printed. + + diff --git a/ggshield/cmd/secret/scan/secret_scan_common_options.py b/ggshield/cmd/secret/scan/secret_scan_common_options.py index 5bcd3693e4..ce34960107 100644 --- a/ggshield/cmd/secret/scan/secret_scan_common_options.py +++ b/ggshield/cmd/secret/scan/secret_scan_common_options.py @@ -1,6 +1,7 @@ from typing import Callable, List, Optional import click +from pygitguardian.models import ApiTokensResponse, Detail, TokenScopes from ggshield.cmd.utils.common_options import ( AnyFunction, @@ -16,7 +17,9 @@ from ggshield.cmd.utils.context_obj import ContextObj from ggshield.cmd.utils.output_format import OutputFormat from ggshield.core import ui +from ggshield.core.client import create_client_from_config from ggshield.core.config.user_config import SecretConfig +from ggshield.core.errors import APIKeyCheckError, MissingScopeError, UnexpectedError from ggshield.core.filter import init_exclusion_regexes from ggshield.utils.click import RealPath from ggshield.verticals.secret.output import ( @@ -126,6 +129,31 @@ def _banlist_detectors_callback( metavar="DETECTOR", ) + +def _with_incident_details_callback( + ctx: click.Context, param: click.Parameter, value: Optional[bool] +) -> Optional[bool]: + if value is not None: + ctx_obj = ContextObj.get(ctx) + ctx_obj.client = create_client_from_config(ctx_obj.config) + response = ctx_obj.client.api_tokens() + + if not isinstance(response, (Detail, ApiTokensResponse)): + raise UnexpectedError("Unexpected api_tokens response") + elif isinstance(response, Detail) and response.status_code == 401: + raise APIKeyCheckError(ctx_obj.client.base_uri, "Invalid API key.") + elif isinstance(response, Detail): + raise UnexpectedError(response.detail) + + if TokenScopes.INCIDENTS_READ not in response.scopes: + raise MissingScopeError( + "Token is missing the scope: 'incidents:read' to retrieve incident details." + ) + + ctx_obj.config.user_config.secret.with_incident_details = value + return value + + _with_incident_details_option = click.option( "--with-incident-details", is_flag=True, @@ -134,8 +162,8 @@ def _banlist_detectors_callback( # If the option is placed early in the command line, the value may be overridden # later on with False if no default is defined. default=None, - help="Display full details about the dashboard incident if one is found (JSON and SARIF formats only).", - callback=create_config_callback("secret", "with_incident_details"), + help="""Display full details about the dashboard incident if one is found (JSON and SARIF formats only). Requires the 'incidents:read' scope.""", # noqa + callback=_with_incident_details_callback, ) diff --git a/ggshield/core/errors.py b/ggshield/core/errors.py index aafd92eafb..822a314bad 100644 --- a/ggshield/core/errors.py +++ b/ggshield/core/errors.py @@ -66,6 +66,15 @@ def __init__(self, message: str): super().__init__(ExitCode.UNEXPECTED_ERROR, message) +class MissingScopeError(_ExitError): + """ + Token does not have the required scope + """ + + def __init__(self, message: str): + super().__init__(ExitCode.UNEXPECTED_ERROR, message) + + class AuthError(_ExitError): """ Base exception for Auth-related configuration error diff --git a/pyproject.toml b/pyproject.toml index 27e8fe796c..9cb7167138 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,7 @@ dependencies = [ "marshmallow~=3.18.0", "marshmallow-dataclass~=8.5.8", "oauthlib~=3.2.1", - "pygitguardian~=1.17.0", + "pygitguardian @ git+https://github.com/GitGuardian/py-gitguardian.git@5085a84f921f7c0eb81bfdcd4b0b5c8f7fefa6b9", "pyjwt~=2.6.0", "python-dotenv~=0.21.0", "pyyaml~=6.0.1", diff --git a/tests/unit/cmd/scan/test_scan_common_options.py b/tests/unit/cmd/scan/test_scan_common_options.py new file mode 100644 index 0000000000..45846bf8a7 --- /dev/null +++ b/tests/unit/cmd/scan/test_scan_common_options.py @@ -0,0 +1,102 @@ +from unittest.mock import Mock + +import click +import pytest +from pygitguardian import GGClient +from pygitguardian.models import ApiTokensResponse, Detail + +from ggshield.cmd.secret.scan.secret_scan_common_options import ( + _with_incident_details_callback, +) +from ggshield.cmd.utils.context_obj import ContextObj +from ggshield.core.config.config import Config +from ggshield.core.errors import APIKeyCheckError, MissingScopeError, UnexpectedError + + +API_TOKENS_RESPONSE_SCAN_SCOPES = { + "id": "5ddaad0c-5a0c-4674-beb5-1cd198d13360", + "name": "myTokenName", + "workspace_id": 42, + "type": "personal_access_token", + "status": "revoked", + "created_at": "2023-05-20T12:40:55.662949Z", + "last_used_at": "2023-05-24T12:40:55.662949Z", + "expire_at": None, + "revoked_at": "2023-05-27T12:40:55.662949Z", + "member_id": 22015, + "creator_id": 22015, + "scopes": ["scan"], +} + +API_TOKENS_RESPONSE_INCIDENT_READ_SCOPES = { + "id": "5ddaad0c-5a0c-4674-beb5-1cd198d13360", + "name": "myTokenName", + "workspace_id": 42, + "type": "personal_access_token", + "status": "revoked", + "created_at": "2023-05-20T12:40:55.662949Z", + "last_used_at": "2023-05-24T12:40:55.662949Z", + "expire_at": None, + "revoked_at": "2023-05-27T12:40:55.662949Z", + "member_id": 22015, + "creator_id": 22015, + "scopes": ["scan", "incidents:read"], +} + + +@pytest.fixture +def mock_context(): + """Fixture for creating the mock context and config.""" + config = Config() + ctx_obj = ContextObj() + ctx_obj.config = config + return click.Context(click.Command(""), obj=ctx_obj) + + +@pytest.mark.parametrize("with_incident_details", [True, False]) +def test_with_incident_details_callback( + mock_context, monkeypatch, with_incident_details +): + monkeypatch.setattr( + GGClient, + "api_tokens", + Mock( + return_value=ApiTokensResponse.from_dict( + API_TOKENS_RESPONSE_INCIDENT_READ_SCOPES + ) + ), + ) + + assert ( + _with_incident_details_callback(mock_context, Mock(), with_incident_details) + is with_incident_details + ) + + +@pytest.mark.parametrize( + "api_response, expected_error", + [ + ( + Detail(status_code=401, detail="Unauthorized"), + pytest.raises(APIKeyCheckError, match="Invalid API key."), + ), + ( + Detail(detail="Unexpected response"), + pytest.raises(UnexpectedError, match="Unexpected response"), + ), + ( + ApiTokensResponse.from_dict(API_TOKENS_RESPONSE_SCAN_SCOPES), + pytest.raises( + MissingScopeError, + match="Token is missing the scope: 'incidents:read' to retrieve incident details.", + ), + ), + ], +) +def test_with_incident_details_callback_error( + mock_context, monkeypatch, api_response, expected_error +): + monkeypatch.setattr(GGClient, "api_tokens", Mock(return_value=api_response)) + + with expected_error: + _with_incident_details_callback(mock_context, Mock(), True)