Skip to content

Commit

Permalink
fix(with-incident-details): Ensure the token has the correct scope 'i…
Browse files Browse the repository at this point in the history
…ncident:read'
  • Loading branch information
salome-voltz committed Nov 20, 2024
1 parent b82d7ca commit b3c6e1d
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!--
A new scriv changelog fragment.
Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Removed
- A bullet item for the Removed category.
-->
<!--
### Added
- A bullet item for the Added category.
-->
<!--
### Changed
- A bullet item for the Changed category.
-->
<!--
### Deprecated
- A bullet item for the Deprecated category.
-->

### 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.

<!--
### Security
- A bullet item for the Security category.
-->
32 changes: 30 additions & 2 deletions ggshield/cmd/secret/scan/secret_scan_common_options.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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 (
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)


Expand Down
9 changes: 9 additions & 0 deletions ggshield/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
102 changes: 102 additions & 0 deletions tests/unit/cmd/scan/test_scan_common_options.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit b3c6e1d

Please sign in to comment.