Skip to content

Commit b0f55aa

Browse files
committed
feat(auth): request broader default scopes on login
`ggshield auth login` now requests scan, honeytokens:write, honeytokens:check, and nhi:send-inventory by default. If any of these scopes are not granted, the command fails with a message listing the missing permissions.
1 parent 58dac51 commit b0f55aa

5 files changed

Lines changed: 182 additions & 11 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
### Changed
2+
3+
- `ggshield auth login` now requests broader default scopes (`scan`, `honeytokens:write`, `honeytokens:check`, `nhi:send-inventory`). If any scope is not granted, a warning is printed but login still succeeds.

ggshield/cmd/auth/login.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,36 @@
33

44
import click
55
import requests
6+
from pygitguardian import GGClient
7+
from pygitguardian.models import APITokensResponse
68

79
from ggshield.cmd.utils.common_options import add_common_options
810
from ggshield.cmd.utils.context_obj import ContextObj
9-
from ggshield.core.client import create_client
11+
from ggshield.core.client import create_client, create_client_from_config
1012
from ggshield.core.config import Config
1113
from ggshield.core.constants import DEFAULT_INSTANCE_URL
1214
from ggshield.core.errors import UnexpectedError
1315
from ggshield.core.url_utils import clean_url
14-
from ggshield.verticals.auth import OAuthClient
16+
from ggshield.verticals.auth import DEFAULT_SCOPES, OAuthClient
17+
18+
19+
def _warn_missing_scopes(client: GGClient) -> None:
20+
token_info = client.api_tokens()
21+
granted = (
22+
token_info.scopes
23+
if isinstance(token_info, APITokensResponse) and token_info.scopes
24+
else []
25+
)
26+
missing = [s for s in DEFAULT_SCOPES if s not in granted]
27+
if missing:
28+
click.echo(
29+
"Warning: the following scopes were not granted: "
30+
+ ", ".join(missing)
31+
+ ".\n"
32+
"Some features may require additional permissions at runtime.\n"
33+
"Contact your workspace administrator if you need access.",
34+
err=True,
35+
)
1536

1637

1738
def validate_login_path(
@@ -91,7 +112,7 @@ def print_default_instance_message(config: Config) -> None:
91112
type=str,
92113
help=(
93114
"Space-separated list of extra scopes to request in addition to the default"
94-
" `scan` scope."
115+
" scopes (scan, honeytokens:write, honeytokens:check, nhi:send-inventory)."
95116
),
96117
metavar="SCOPES",
97118
)
@@ -145,9 +166,10 @@ def login_cmd(
145166
Alternatively, you can use `--method token` to authenticate using an already existing token.
146167
The minimum required scope for the token is `scan`.
147168
148-
By default, the created token will have the `scan` scope. Use the `--scopes` option
149-
to grant the token extra scopes. You can find the list of available scopes in
150-
[GitGuardian API documentation][1].
169+
By default, the created token will have the `scan`, `honeytokens:write`,
170+
`honeytokens:check`, and `nhi:send-inventory` scopes.
171+
Use the `--scopes` option to request extra scopes. You can find the list of
172+
available scopes in [GitGuardian API documentation][1].
151173
152174
If a valid personal access token is already configured, this command simply displays
153175
a success message indicating that ggshield is already ready to use.
@@ -232,6 +254,8 @@ def token_login(config: Config, instance: Optional[str]) -> None:
232254
click.echo("Authentication was successful.")
233255
print_default_instance_message(config)
234256

257+
_warn_missing_scopes(client)
258+
235259

236260
def web_login(
237261
config: Config,
@@ -262,4 +286,5 @@ def web_login(
262286
extra_scopes=extra_scopes,
263287
no_browser=no_browser,
264288
)
289+
_warn_missing_scopes(create_client_from_config(config))
265290
print_default_instance_message(config)
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
from .oauth import OAuthClient, OAuthError
1+
from .oauth import DEFAULT_SCOPES, OAuthClient, OAuthError
22

33

44
__all__ = [
5+
"DEFAULT_SCOPES",
56
"OAuthClient",
67
"OAuthError",
78
]

ggshield/verticals/auth/oauth.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@
2727

2828
CLIENT_ID = "ggshield_oauth"
2929
SCAN_SCOPE = "scan"
30+
DEFAULT_SCOPES = [
31+
SCAN_SCOPE,
32+
"honeytokens:write",
33+
"honeytokens:check",
34+
"nhi:send-inventory",
35+
]
3036

3137
# Sentinel `redirect_uri` value used by the out-of-band (browser-less) OAuth
3238
# flow. Mirrors gcloud's `--no-launch-browser` and the historical Google
@@ -239,10 +245,11 @@ def _build_authorize_uri(self) -> str:
239245
"utm_medium": "login",
240246
"utm_campaign": "ggshield",
241247
}
248+
unique_scopes = list(dict.fromkeys([*DEFAULT_SCOPES, *self._extra_scopes]))
242249
return self._oauth_client.prepare_request_uri(
243250
uri=urljoin(self.dashboard_url, self._login_path),
244251
redirect_uri=self.redirect_uri,
245-
scope=[SCAN_SCOPE, *self._extra_scopes],
252+
scope=unique_scopes,
246253
code_challenge=self.code_challenge,
247254
code_challenge_method="S256",
248255
state=self.state,

tests/unit/cmd/auth/test_login.py

Lines changed: 138 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@
4343
"type": "personal_access_token",
4444
"account_id": 17,
4545
"name": "key",
46-
"scope": ["scan"],
46+
"scope": [
47+
"scan",
48+
"honeytokens:write",
49+
"honeytokens:check",
50+
"nhi:send-inventory",
51+
],
4752
"expire_at": None,
4853
}
4954
)
@@ -59,6 +64,25 @@
5964
{"detail": "Invalid GitGuardian API key."}, 401
6065
)
6166

67+
API_TOKENS_ENDPOINT = "/v1/api_tokens/self"
68+
69+
VALID_API_TOKENS_RESPONSE = create_json_response(
70+
{
71+
"id": "00000000-0000-0000-0000-000000000001",
72+
"name": "key",
73+
"workspace_id": 17,
74+
"type": "personal_access_token",
75+
"status": "active",
76+
"created_at": "2021-01-01T00:00:00+00:00",
77+
"scopes": [
78+
"scan",
79+
"honeytokens:write",
80+
"honeytokens:check",
81+
"nhi:send-inventory",
82+
],
83+
}
84+
)
85+
6286
METADATA_ENDPOINT = "/v1/metadata"
6387

6488
VALID_METADATA_RESPONSE = create_json_response(
@@ -92,6 +116,7 @@ def test_auth_login_token(self, monkeypatch, cli_fs_runner, test_case):
92116

93117
if test_case == "valid":
94118
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
119+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
95120
elif test_case == "invalid_scope":
96121
self._request_mock.add_GET(
97122
TOKEN_ENDPOINT, VALID_TOKEN_INVALID_SCOPE_RESPONSE
@@ -119,6 +144,53 @@ def test_auth_login_token(self, monkeypatch, cli_fs_runner, test_case):
119144

120145
self._request_mock.assert_all_requests_happened()
121146

147+
def test_auth_login_token_missing_default_scopes(self, monkeypatch, cli_fs_runner):
148+
"""
149+
GIVEN an API token that has scan but is missing other default scopes
150+
WHEN the auth login command is called with --method=token
151+
THEN login succeed with a message listing the missing permissions
152+
AND the token is saved (scan still works)
153+
AND a warning is displayed
154+
"""
155+
token = "mysupertoken"
156+
instance = "https://dashboard.gitguardian.com"
157+
cmd = ["auth", "login", "--method=token", f"--instance={instance}"]
158+
159+
self._request_mock.add_GET(
160+
TOKEN_ENDPOINT,
161+
create_json_response(
162+
{
163+
"type": "personal_access_token",
164+
"account_id": 17,
165+
"name": "key",
166+
"scope": ["scan"],
167+
"expire_at": None,
168+
}
169+
),
170+
)
171+
self._request_mock.add_GET(
172+
"/v1/api_tokens/self",
173+
create_json_response(
174+
{
175+
**VALID_API_TOKENS_RESPONSE.json(),
176+
"scopes": ["scan"],
177+
}
178+
),
179+
)
180+
181+
result = cli_fs_runner.invoke(cli, cmd, color=False, input=token + "\n")
182+
183+
assert result.exit_code == 0
184+
assert "Warning: the following scopes were not granted:" in result.output
185+
assert "honeytokens:write" in result.output
186+
assert "nhi:send-inventory" in result.output
187+
assert "Some features may require additional permissions" in result.output
188+
189+
config = Config()
190+
assert config.auth_config.get_instance(instance).account.token == token
191+
192+
self._request_mock.assert_all_requests_happened()
193+
122194
def test_auth_login_token_default_instance(self, monkeypatch, cli_fs_runner):
123195
"""
124196
GIVEN a valid API token
@@ -130,6 +202,7 @@ def test_auth_login_token_default_instance(self, monkeypatch, cli_fs_runner):
130202
assert len(config.auth_config.instances) == 0
131203

132204
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
205+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
133206

134207
cmd = ["auth", "login", "--method=token"]
135208

@@ -179,6 +252,7 @@ def test_api_instance_url(
179252
token = "mysupertoken"
180253
cmd = ["auth", "login", "--method=token", f"--instance={cmd_line_instance}"]
181254
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
255+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
182256
result = cli_fs_runner.invoke(cli, cmd, color=False, input=token + "\n")
183257
config = Config()
184258
config_instance_urls = [
@@ -208,6 +282,7 @@ def test_auth_login_token_suggests_set_instance(
208282
assert not Config().auth_config.instances
209283

210284
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
285+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
211286

212287
cmd = ["auth", "login", "--method=token"]
213288
if instance:
@@ -229,8 +304,11 @@ def test_auth_login_token_update_existing_config(self, monkeypatch, cli_fs_runne
229304
THEN the instance configuration is created if it doesn't exist, or updated otherwise
230305
"""
231306
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
307+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
232308
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
309+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
233310
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
311+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
234312

235313
instance = "https://dashboard.gitguardian.com"
236314
cmd = ["auth", "login", "--method=token", f"--instance={instance}"]
@@ -277,6 +355,7 @@ def test_auth_login_token_from_stdin(self, monkeypatch, cli_fs_runner):
277355
assert len(config.auth_config.instances) == 0
278356

279357
self._request_mock.add_GET(TOKEN_ENDPOINT, VALID_TOKEN_RESPONSE)
358+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
280359

281360
token = "mysupertoken"
282361

@@ -619,7 +698,37 @@ def test_scopes(self, cli_fs_runner, monkeypatch):
619698
assert exit_code == ExitCode.SUCCESS, output
620699

621700
self._webbrowser_open_mock.assert_called_once()
622-
self._assert_open_url(scope_set={"scan", "honeytokens:write", "teams:read"})
701+
self._assert_open_url(
702+
scope_set={
703+
"scan",
704+
"honeytokens:write",
705+
"honeytokens:check",
706+
"nhi:send-inventory",
707+
"teams:read",
708+
}
709+
)
710+
711+
def test_missing_default_scopes_fails_with_warning(
712+
self, cli_fs_runner, monkeypatch
713+
):
714+
"""
715+
GIVEN the backend grants only a subset of the default scopes
716+
WHEN the web login flow completes
717+
THEN the command succeed and lists the missing permissions
718+
AND the token is still saved
719+
"""
720+
self.prepare_mocks(
721+
monkeypatch, missing_scopes=["honeytokens:write", "nhi:send-inventory"]
722+
)
723+
exit_code, output = self.run_cmd(cli_fs_runner)
724+
725+
assert exit_code == ExitCode.SUCCESS
726+
assert "Warning: the following scopes were not granted:" in output
727+
assert "honeytokens:write" in output
728+
assert "nhi:send-inventory" in output
729+
assert "Some features may require additional permissions" in output
730+
731+
self._assert_config("mysupertoken")
623732

624733
def prepare_mocks(
625734
self,
@@ -632,6 +741,7 @@ def prepare_mocks(
632741
sso_url=None,
633742
downsized_token: Optional[bool] = False,
634743
scopes: Optional[str] = None,
744+
missing_scopes: Optional[list] = None,
635745
):
636746
"""
637747
Configure self._request_mock to emulate HTTP requests
@@ -707,6 +817,12 @@ def prepare_mocks(
707817
if lifetime is not None:
708818
expire_at = self._get_expiry_date().isoformat()
709819
token_response_payload["expire_at"] = expire_at
820+
if missing_scopes:
821+
token_response_payload["scope"] = [
822+
s
823+
for s in token_response_payload["scope"]
824+
if s not in missing_scopes
825+
]
710826

711827
# mock api call to exchange the code against a valid access token
712828
response = create_json_response({"key": token, **token_response_payload})
@@ -729,6 +845,19 @@ def prepare_mocks(
729845
),
730846
)
731847

848+
if login_result == LoginResult.SUCCESS:
849+
scopes = [
850+
s
851+
for s in VALID_API_TOKENS_RESPONSE.json()["scopes"]
852+
if not missing_scopes or s not in missing_scopes
853+
]
854+
self._request_mock.add_GET(
855+
"/v1/api_tokens/self",
856+
create_json_response(
857+
{**VALID_API_TOKENS_RESPONSE.json(), "scopes": scopes}
858+
),
859+
)
860+
732861
def run_cmd(self, cli_fs_runner, method="web"):
733862
"""
734863
Run the auth login method within a virtual cli.
@@ -814,7 +943,12 @@ def _assert_open_url(
814943
also check if the port of the redirect url is the one expected depending on occupied ports
815944
"""
816945
if scope_set is None:
817-
scope_set = {"scan"}
946+
scope_set = {
947+
"scan",
948+
"honeytokens:write",
949+
"honeytokens:check",
950+
"nhi:send-inventory",
951+
}
818952

819953
(url,), kwargs = self._webbrowser_open_mock.call_args_list[0]
820954
parsed_url = urlparse.urlparse(url)
@@ -1059,6 +1193,7 @@ def _add_token_endpoints(self, post_checker=None):
10591193
post_checker,
10601194
)
10611195
self._request_mock.add_GET(TOKEN_ENDPOINT, create_json_response(token_payload))
1196+
self._request_mock.add_GET("/v1/api_tokens/self", VALID_API_TOKENS_RESPONSE)
10621197
return token
10631198

10641199
def test_oob_happy_path(self, cli_fs_runner, monkeypatch):

0 commit comments

Comments
 (0)