Skip to content

Commit 8d11fb4

Browse files
woodruffwdi
andcommitted
Force user to explicitly select token scope, show warning if user scopes to account (#6274)
* Show warning when user scopes API key to account * warehouse: Toggle token scope warning * warehouse: Handle unspecified scopes * tests: Cover unspecified scope in form * Update API token errors/warning copy * Update warehouse/templates/manage/token.html Co-Authored-By: Dustin Ingram <[email protected]>
1 parent 0ab3d80 commit 8d11fb4

File tree

5 files changed

+52
-6
lines changed

5 files changed

+52
-6
lines changed

tests/unit/manage/test_forms.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def test_validate_description_missing(self):
347347
)
348348

349349
assert not form.validate()
350-
assert form.description.errors.pop() == "Specify a description"
350+
assert form.description.errors.pop() == "Specify a token name"
351351

352352
def test_validate_description_in_use(self):
353353
form = forms.CreateMacaroonForm(
@@ -371,7 +371,18 @@ def test_validate_token_scope_missing(self):
371371
)
372372

373373
assert not form.validate()
374-
assert form.token_scope.errors.pop() == "Specify a token scope"
374+
assert form.token_scope.errors.pop() == "Specify the token scope"
375+
376+
def test_validate_token_scope_unspecified(self):
377+
form = forms.CreateMacaroonForm(
378+
data={"description": "dummy", "token_scope": "scope:unspecified"},
379+
user_id=pretend.stub(),
380+
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
381+
project_names=pretend.stub(),
382+
)
383+
384+
assert not form.validate()
385+
assert form.token_scope.errors.pop() == "Specify the token scope"
375386

376387
@pytest.mark.parametrize(
377388
("scope"), ["not a real scope", "scope:project", "scope:foo:bar"]

warehouse/manage/forms.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,15 @@ def __init__(self, *args, user_id, macaroon_service, project_names, **kwargs):
196196

197197
description = wtforms.StringField(
198198
validators=[
199-
wtforms.validators.DataRequired(message="Specify a description"),
199+
wtforms.validators.DataRequired(message="Specify a token name"),
200200
wtforms.validators.Length(
201201
max=100, message="Description must be 100 characters or less"
202202
),
203203
]
204204
)
205205

206206
token_scope = wtforms.StringField(
207-
validators=[wtforms.validators.DataRequired(message="Specify a token scope")]
207+
validators=[wtforms.validators.DataRequired(message="Specify the token scope")]
208208
)
209209

210210
def validate_description(self, field):
@@ -224,6 +224,9 @@ def validate_token_scope(self, field):
224224
except ValueError:
225225
raise wtforms.ValidationError(f"Unknown token scope: {scope}")
226226

227+
if scope_kind == "unspecified":
228+
raise wtforms.ValidationError(f"Specify the token scope")
229+
227230
if scope_kind == "user":
228231
self.validated_scope = scope_kind
229232
return

warehouse/static/js/warehouse/index.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,24 @@ docReady(ProvisionWebAuthn);
226226
// Get WebAuthn authentication ready
227227
docReady(AuthenticateWebAuthn);
228228

229+
docReady(() => {
230+
const tokenSelect = document.getElementById("token_scope");
231+
232+
if (tokenSelect === null) {
233+
return;
234+
}
235+
236+
tokenSelect.addEventListener("change", () => {
237+
const tokenScopeWarning = document.getElementById("api-token-scope-warning");
238+
if (tokenScopeWarning === null) {
239+
return;
240+
}
241+
242+
const tokenScope = tokenSelect.options[tokenSelect.selectedIndex].value;
243+
tokenScopeWarning.hidden = (tokenScope !== "scope:user");
244+
});
245+
});
246+
229247
// Bind again when client-side includes have been loaded (for the logged-in
230248
// user dropdown)
231249
document.addEventListener("CSILoaded", bindDropdowns);

warehouse/static/sass/blocks/_callout-block.scss

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
2525
Modifiers:
2626
- danger: Makes border red
27+
- warning: Makes border brown
2728
- success: Makes border green
2829
- bottom-margin: Adds extra margin below the callout
2930
*/
@@ -68,6 +69,14 @@
6869
}
6970
}
7071

72+
&--warning {
73+
border-color: $warn-text;
74+
75+
&:before {
76+
background-color: $warn-text;
77+
}
78+
}
79+
7180
&--success {
7281
border-color: $success-color;
7382

warehouse/templates/manage/token.html

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,20 @@ <h2>Token for "{{ macaroon.description }}"</h2>
7070
<p id="description-help-text" class="form-group__help-text">What is this token for?</p>
7171
</div>
7272
<div class="form-group">
73-
<label for="scope" class="form-group__label">Scope</label>
74-
<select name="token_scope" id="scope" class="form-group__input">
73+
<label for="token_scope" class="form-group__label">Scope</label>
74+
<select name="token_scope" id="token_scope" class="form-group__input" aria-describedby="token_scope-errors">
75+
<option disabled selected value="scope:unspecified">Select scope...</option>
7576
<option value="scope:user">Entire account (all projects)</option>
7677
{% for project in project_names %}
7778
<option value="scope:project:{{ project }}">Project: {{ project }}</option>
7879
{% endfor %}
7980
</select>
8081
{{ field_errors(create_macaroon_form.token_scope) }}
8182
</div>
83+
<div id="api-token-scope-warning" class="callout-block callout-block--warning" hidden>
84+
<h3 class="callout-block__heading">Proceed with caution!</h3>
85+
<p>An API token scoped to your entire account will have upload permissions for all of your projects.</p>
86+
</div>
8287
<div>
8388
<input value="Add token" class="button button--primary" type="submit">
8489
</div>

0 commit comments

Comments
 (0)