Skip to content

Commit c2e6068

Browse files
committed
Administrators can change the scopes of an existing token
The IDE tokens page has an Edit scopes control under each live token for administrators. The token string stays the same, so whatever holds it keeps working and picks up the new scopes on its next call. Nobody has to make a new token and update the secret it lives in. The scopes are checked against the token's owner, not the administrator making the change, so approve and external can only go on an administrator's token. A token cannot be left with no scopes, revoked and expired tokens cannot be edited, developers cannot change scopes at all, and every change is in the audit log with what it had before.
1 parent 07f041a commit c2e6068

4 files changed

Lines changed: 122 additions & 14 deletions

File tree

‎app/core/api_tokens.py‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,25 +61,61 @@ def _hash(token: str) -> str:
6161
return hashlib.sha256((token or "").encode()).hexdigest()
6262

6363

64+
def allowed_scopes(owner, scopes) -> list:
65+
"""The scopes asked for, checked against the person the token belongs to.
66+
67+
Raises ValueError with a sentence to show. Always the owner's role, never
68+
the role of whoever is editing: an administrator giving a developer's
69+
token the approve scope would hand that developer an administrator's power
70+
through the back door.
71+
"""
72+
from app.core import rbac
73+
74+
wanted = sorted({s.strip() for s in scopes if s.strip() in SCOPES})
75+
if "external" in wanted and not rbac.can(owner, "external.manage"):
76+
raise ValueError("Only an administrator's token can work with external sites.")
77+
if "approve" in wanted and not (rbac.can(owner, "waiver.approve")
78+
and rbac.can(owner, "exclusion.write")):
79+
raise ValueError("Only an administrator's token can approve exceptions.")
80+
if not wanted:
81+
raise ValueError("Pick at least one thing it may do.")
82+
return wanted
83+
84+
85+
def set_scopes(session, token_id: int, scopes, actor):
86+
"""Change what a live token may do. -> (row, the scopes it had before).
87+
88+
Administrators only, on anybody's token. The token itself does not
89+
change, so nothing using it has to be updated: the next call is checked
90+
against the new list. Raises LookupError for a token that is not there,
91+
or is revoked or expired, and ValueError for scopes its owner may not have.
92+
"""
93+
from app.core.models import User
94+
95+
if actor is None or not _sees_everything(actor):
96+
raise PermissionError("Only an administrator can change a token's scopes.")
97+
row = session.query(ApiToken).filter(ApiToken.id == token_id).one_or_none()
98+
if row is None or not row.live():
99+
raise LookupError("No such token, or it no longer works.")
100+
owner = session.query(User).filter(User.id == row.user_id).one_or_none()
101+
if owner is None:
102+
raise LookupError("No such token, or it no longer works.")
103+
wanted = allowed_scopes(owner, scopes)
104+
before = row.scopes or ""
105+
row.scopes = ",".join(wanted)
106+
session.flush()
107+
log.info("api token %s scopes %s -> %s by %s", row.name, before, row.scopes,
108+
actor.username)
109+
return row, before
110+
111+
64112
def create(session, user, name: str, scopes, days: int = DEFAULT_DAYS):
65113
"""-> (row, the token itself). The only time anybody sees the token."""
66114
name = (name or "").strip()
67115
if not name:
68116
raise ValueError("Give it a name, so somebody can tell later what it was for.")
69117

70-
wanted = sorted({s.strip() for s in scopes if s.strip() in SCOPES})
71-
if "external" in wanted:
72-
from app.core import rbac
73-
74-
if not rbac.can(user, "external.manage"):
75-
raise ValueError("Only an administrator's token can work with external sites.")
76-
if "approve" in wanted:
77-
from app.core import rbac
78-
79-
if not (rbac.can(user, "waiver.approve") and rbac.can(user, "exclusion.write")):
80-
raise ValueError("Only an administrator's token can approve exceptions.")
81-
if not wanted:
82-
raise ValueError("Pick at least one thing it may do.")
118+
wanted = allowed_scopes(user, scopes)
83119

84120
try:
85121
days = int(days)

‎app/web/routes_api.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3118,6 +3118,46 @@ async def revoke_token(token_id: int, request: Request,
31183118
return redirect("/tokens?msg=" + quote("That token no longer works."))
31193119

31203120

3121+
@router.post("/tokens/{token_id}/scopes")
3122+
async def change_token_scopes(token_id: int, request: Request,
3123+
user: User = Depends(current_user)):
3124+
"""Change what an existing token may do. Administrators only.
3125+
3126+
The token string stays the same, so the pipeline or editor holding it
3127+
carries on without being touched. The scopes are checked against the
3128+
token's owner, not against the administrator making the change.
3129+
"""
3130+
await check_csrf(request)
3131+
rbac.require(user, "user.write")
3132+
3133+
from urllib.parse import quote
3134+
3135+
from app.core import api_tokens
3136+
3137+
form = await request.form()
3138+
wanted = [scope for scope in api_tokens.SCOPES
3139+
if _checked(form, f"scope:{scope}")]
3140+
with session_scope() as db:
3141+
actor = db.query(User).filter(User.id == user.id).one()
3142+
try:
3143+
row, before = api_tokens.set_scopes(db, token_id, wanted, actor)
3144+
except LookupError:
3145+
raise HTTPException(status_code=404, detail="No such token.")
3146+
except ValueError as exc:
3147+
return redirect(f"/tokens?msg={quote(str(exc))}&kind=error")
3148+
if before == row.scopes:
3149+
return redirect("/tokens?msg=" + quote("Nothing changed."))
3150+
record(db, "api_token.scopes_changed", actor=user.username,
3151+
target=row.name,
3152+
detail=f"{before or 'none'} -> {row.scopes} (owner {row.created_by})",
3153+
source_ip=client_ip(request))
3154+
name = row.name
3155+
3156+
return redirect("/tokens?msg=" + quote(
3157+
f"{name} may now: {', '.join(wanted)}. It works the same as before, "
3158+
"nothing holding it needs to change."))
3159+
3160+
31213161
@router.post("/tokens/{token_id}/delete")
31223162
async def delete_token(token_id: int, request: Request,
31233163
user: User = Depends(current_user)):

‎app/web/routes_pages.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3761,6 +3761,15 @@ def tokens_page(request: Request, msg: str = "", kind: str = "",
37613761

37623762
with session_scope() as db:
37633763
rows = api_tokens.listing(db, user, include_dead=show_dead)
3764+
# Which tokens belong to somebody who may hold the administrator only
3765+
# scopes, so the edit form only offers what would be accepted.
3766+
admin_owned = set()
3767+
if rbac.can(user, "user.write") and rows:
3768+
owners = {u.id: u for u in db.query(User).filter(
3769+
User.id.in_({r.user_id for r in rows})).all()}
3770+
admin_owned = {r.id for r in rows
3771+
if owners.get(r.user_id) is not None
3772+
and rbac.can(owners[r.user_id], "waiver.approve")}
37643773
context = _shared(db, user)
37653774
context.update(
37663775
active_page="tokens",
@@ -3771,6 +3780,7 @@ def tokens_page(request: Request, msg: str = "", kind: str = "",
37713780
default_days=api_tokens.DEFAULT_DAYS,
37723781
max_days=api_tokens.MAX_DAYS,
37733782
sees_all=rbac.can(user, "user.write"),
3783+
admin_owned=admin_owned,
37743784
me=user.username,
37753785
# Shown once, straight after it is made, and never again. What
37763786
# arrives in the address is a one time key, not the token.

‎app/web/templates/tokens.html‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,29 @@
122122
</td>
123123
{% if sees_all %}<td class="small">{{ t.created_by }}</td>{% endif %}
124124
<td class="mono small">{{ t.prefix }}&hellip;</td>
125-
<td class="small">{% for s in t.scopes.split(',') %}<span class="badge">{{ s }}</span> {% endfor %}</td>
125+
<td class="small">{% for s in t.scopes.split(',') %}<span class="badge">{{ s }}</span> {% endfor %}
126+
{% if sees_all and t.live() %}
127+
{# Administrators can change what a token may do without anybody
128+
having to make a new one and update the secret it lives in. #}
129+
<details class="token-scopes">
130+
<summary class="small">Edit scopes</summary>
131+
<form method="post" action="{{ base }}/tokens/{{ t.id }}/scopes"
132+
data-confirm="Change what this token may do? Whatever holds it gets the new scopes on its next call.">
133+
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
134+
{% set have = t.scopes.split(',') %}
135+
{% for scope, what in scopes.items()
136+
if scope not in ('external', 'approve') or t.id in admin_owned %}
137+
<label class="check small" title="{{ what }}">
138+
<input type="checkbox" name="scope:{{ scope }}" value="1"
139+
{{ 'checked' if scope in have }}>
140+
<span class="mono">{{ scope }}</span>
141+
</label>
142+
{% endfor %}
143+
<button class="btn sm" type="submit">Save scopes</button>
144+
</form>
145+
</details>
146+
{% endif %}
147+
</td>
126148
<td class="stack nowrap small">
127149
{% if t.revoked_at %}<span class="badge critical">revoked</span>
128150
{% elif not t.live() %}<span class="badge medium">expired</span>

0 commit comments

Comments
 (0)