Skip to content

Commit 650ba6d

Browse files
committed
Merge branch 'secret-expiration' into 'main'
Handle secret expiration in the OIDC client interface See merge request yaal/canaille!344
2 parents 3b21dbc + a821c83 commit 650ba6d

9 files changed

Lines changed: 250 additions & 13 deletions

File tree

CHANGES.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
[0.3.6] - Unreleased
22
--------------------
33

4+
Added
5+
^^^^^
6+
- Client secret expiration dates are displayed and editable in the client administration page. Clients cannot authenticate with an expired secret anymore.
7+
- Client secrets can be renewed from the client administration page.
8+
49
Fixed
510
^^^^^
611
- Dynamically registered clients had their ``client_secret_expires_at`` set to 1970-01-01 instead of being left empty.

canaille/oidc/basemodels.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ def trusted(self) -> bool:
7171
expiration.
7272
"""
7373

74+
@property
75+
def secret_expired(self) -> bool:
76+
"""Whether the client secret has expired.
77+
78+
Clients with an expired secret cannot authenticate anymore with the
79+
``client_secret_basic`` and ``client_secret_post`` methods.
80+
"""
81+
return (
82+
self.client_secret_expires_at is not None
83+
and self.client_secret_expires_at
84+
< datetime.datetime.now(datetime.timezone.utc)
85+
)
86+
7487
redirect_uris: list[str] = []
7588
"""Array of redirection URI strings for use in redirect-based flows such as
7689
the authorization code and implicit flows.

canaille/oidc/endpoints/clients.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ def edit(user, client):
118118
if request.form and request.form.get("action") == "new-access-token":
119119
return client_new_token(user, client)
120120

121+
if request.form and request.form.get("action") == "new-client-secret":
122+
return client_new_secret(user, client)
123+
121124
return client_edit(user, client)
122125

123126

@@ -169,6 +172,7 @@ def client_edit(user, client):
169172
contacts=form["contacts"].data,
170173
client_uri=form["client_uri"].data,
171174
redirect_uris=form["redirect_uris"].data,
175+
client_secret_expires_at=form["client_secret_expires_at"].data,
172176
post_logout_redirect_uris=form["post_logout_redirect_uris"].data,
173177
grant_types=form["grant_types"].data,
174178
scope=form["scope"].data.split(" "),
@@ -216,6 +220,23 @@ def client_delete(user, client):
216220
return redirect(url_for("oidc.clients.index"))
217221

218222

223+
def client_new_secret(user, client):
224+
Backend.instance.update(
225+
client,
226+
client_secret=gen_salt(48),
227+
client_secret_expires_at=None,
228+
)
229+
Backend.instance.save(client)
230+
current_app.logger.security(
231+
f"Renewed the secret of client {client.client_id} by {user.id}"
232+
)
233+
flash(
234+
_("The client secret has been renewed. The new secret does not expire."),
235+
"success",
236+
)
237+
return redirect(url_for("oidc.clients.edit", client=client))
238+
239+
219240
def client_new_token(user, client):
220241
flash(
221242
_("A token has been created for the client {client_name}.").format(

canaille/oidc/endpoints/forms.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from joserfc.jwk import KeySet
55

66
from canaille.app import models
7+
from canaille.app.forms import DateTimeUTCField
78
from canaille.app.forms import Form
89
from canaille.app.forms import IDToModel
910
from canaille.app.forms import email_validator
@@ -113,6 +114,20 @@ class ClientAddForm(Form):
113114
class ClientEditForm(ClientAddForm):
114115
"""Complete form for editing a client with all metadata fields."""
115116

117+
client_secret_expires_at = DateTimeUTCField(
118+
_("Secret expiration"),
119+
validators=[wtforms.validators.Optional()],
120+
format=[
121+
"%Y-%m-%d %H:%M",
122+
"%Y-%m-%dT%H:%M",
123+
"%Y-%m-%d %H:%M:%S",
124+
"%Y-%m-%dT%H:%M:%S",
125+
],
126+
description=_(
127+
"Date after which the client secret cannot be used to authenticate anymore. Leave this empty so the secret never expires."
128+
),
129+
)
130+
116131
post_logout_redirect_uris = wtforms.FieldList(
117132
wtforms.URLField(
118133
_("Post logout redirect URIs"),

canaille/oidc/models.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from authlib.oauth2.rfc6749 import util
77
from authlib.oidc.core import AuthorizationCodeMixin
88
from blinker import signal
9+
from flask import current_app
910

1011
from canaille.app import models
1112
from canaille.backends import Backend
@@ -90,6 +91,13 @@ def check_redirect_uri(self, redirect_uri: str) -> bool:
9091
return redirect_uri in self.redirect_uris
9192

9293
def check_client_secret(self, client_secret: str) -> bool:
94+
if self.secret_expired:
95+
current_app.logger.security(
96+
f"Client {self.client_id} authentication attempt with a secret "
97+
f"expired since {self.client_secret_expires_at}"
98+
)
99+
return False
100+
93101
return client_secret == self.client_secret
94102

95103
def check_endpoint_auth_method(self, method, endpoint):

canaille/templates/macro/form.html

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,11 @@
179179

180180
{%- endmacro %}
181181

182-
{% macro render_fields(form) %}
182+
{% macro render_fields(form, exclude=[]) %}
183183
{% for field in form %}
184-
{{ render_field(field) }}
184+
{% if field.name not in exclude %}
185+
{{ render_field(field) }}
186+
{% endif %}
185187
{% endfor %}
186188
{% endmacro %}
187189

canaille/templates/oidc/client_edit.html

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ <h2 class="ui center aligned header">
7171
{% trans %}Trusted{% endtrans %}
7272
</span>
7373
{% endif %}
74+
{% if client.secret_expired %}
75+
<span class="ui red label" title="{% trans %}This application cannot authenticate anymore until its secret is renewed{% endtrans %}">
76+
<i class="exclamation triangle icon"></i>
77+
{% trans %}Expired secret{% endtrans %}
78+
</span>
79+
{% endif %}
7480
</h2>
7581

7682
<div class="ui form">
@@ -85,16 +91,6 @@ <h2 class="ui center aligned header">
8591
</button>
8692
</div>
8793
</div>
88-
<div class="field">
89-
<label>{% trans %}Secret{% endtrans %}</label>
90-
<div class="ui fluid action input">
91-
<input type="text" value="{{ client.client_secret }}" readonly class="copy-text" id="client-secret" data-copy="client-secret" name="client_secret">
92-
<button type="button" class="ui primary right labeled icon button copy-button" data-copy="client-secret">
93-
<i class="copy icon"></i>
94-
{% trans %}Copy{% endtrans %}
95-
</button>
96-
</div>
97-
</div>
9894
<div class="field">
9995
<label>{% trans %}Issued at{% endtrans %}</label>
10096
<div class="ui cornor labeled input">
@@ -108,7 +104,22 @@ <h2 class="ui center aligned header">
108104
</div>
109105

110106
{% call fui.render_form(form) %}
111-
{{ fui.render_fields(form) }}
107+
<div class="field">
108+
<label>{% trans %}Secret{% endtrans %}</label>
109+
<div class="ui fluid action input">
110+
<input type="text" value="{{ client.client_secret }}" readonly class="copy-text" id="client-secret" data-copy="client-secret">
111+
<button type="submit" class="ui teal right labeled icon button" name="action" value="new-client-secret" id="new-client-secret" formnovalidate>
112+
<i class="redo icon"></i>
113+
{% trans %}Renew{% endtrans %}
114+
</button>
115+
<button type="button" class="ui primary right labeled icon button copy-button" data-copy="client-secret">
116+
<i class="copy icon"></i>
117+
{% trans %}Copy{% endtrans %}
118+
</button>
119+
</div>
120+
</div>
121+
{{ fui.render_field(form.client_secret_expires_at) }}
122+
{{ fui.render_fields(form, exclude=["client_secret_expires_at"]) }}
112123

113124
<div class="ui right aligned container">
114125
<div class="ui stackable buttons">

canaille/templates/oidc/partial/client_list.html

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
{% if client.trusted %}
2626
<i class="small teal shield alternate icon" title="{% trans %}This application is trusted{% endtrans %}"></i>
2727
{% endif %}
28+
{% if client.secret_expired %}
29+
<i class="small red exclamation triangle icon" title="{% trans %}The secret of this application has expired{% endtrans %}"></i>
30+
{% endif %}
2831
</td>
2932
<td>
3033
{% if client.client_uri %}
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import datetime
2+
import logging
3+
4+
from . import client_credentials
5+
6+
7+
def test_expired_secret_cannot_authenticate(testclient, client, backend, caplog):
8+
"""Clients cannot use an expired secret at the token endpoint."""
9+
client.client_secret_expires_at = datetime.datetime.now(
10+
datetime.timezone.utc
11+
) - datetime.timedelta(days=1)
12+
backend.save(client)
13+
assert client.secret_expired
14+
15+
res = testclient.post(
16+
"/oauth/token",
17+
params=dict(grant_type="client_credentials"),
18+
headers={"Authorization": f"Basic {client_credentials(client)}"},
19+
status=401,
20+
)
21+
22+
assert res.json["error"] == "invalid_client"
23+
assert (
24+
"canaille",
25+
logging.SECURITY,
26+
f"Client {client.client_id} authentication attempt with a secret "
27+
f"expired since {client.client_secret_expires_at}",
28+
) in caplog.record_tuples
29+
30+
31+
def test_expired_secret_cannot_authenticate_with_client_secret_post(
32+
testclient, client, backend
33+
):
34+
"""The client_secret_post method is guarded the same way as client_secret_basic."""
35+
client.token_endpoint_auth_method = "client_secret_post"
36+
client.client_secret_expires_at = datetime.datetime.now(
37+
datetime.timezone.utc
38+
) - datetime.timedelta(days=1)
39+
backend.save(client)
40+
41+
res = testclient.post(
42+
"/oauth/token",
43+
params=dict(
44+
grant_type="client_credentials",
45+
client_id=client.client_id,
46+
client_secret=client.client_secret,
47+
),
48+
status=401,
49+
)
50+
51+
assert res.json["error"] == "invalid_client"
52+
53+
54+
def test_future_expiration_can_authenticate(testclient, client, backend):
55+
"""A secret expiring in the future is still valid."""
56+
client.client_secret_expires_at = datetime.datetime.now(
57+
datetime.timezone.utc
58+
) + datetime.timedelta(days=1)
59+
backend.save(client)
60+
assert not client.secret_expired
61+
62+
testclient.post(
63+
"/oauth/token",
64+
params=dict(grant_type="client_credentials"),
65+
headers={"Authorization": f"Basic {client_credentials(client)}"},
66+
status=200,
67+
)
68+
69+
70+
def test_edit_secret_expiration(testclient, client, logged_admin, backend):
71+
"""Administrators can set and unset the secret expiration date."""
72+
assert not client.client_secret_expires_at
73+
74+
expiration = datetime.datetime.now(datetime.timezone.utc).replace(
75+
second=0, microsecond=0
76+
) + datetime.timedelta(days=30)
77+
78+
res = testclient.get("/admin/client/edit/" + client.client_id)
79+
res.forms["clienteditform"]["client_secret_expires_at"] = expiration.strftime(
80+
"%Y-%m-%d %H:%M"
81+
)
82+
res = res.forms["clienteditform"].submit(status=302, name="action", value="edit")
83+
assert ("success", "The client has been edited.") in res.flashes
84+
85+
backend.reload(client)
86+
assert client.client_secret_expires_at == expiration
87+
assert not client.secret_expired
88+
89+
res = res.follow()
90+
res.forms["clienteditform"]["client_secret_expires_at"] = ""
91+
res = res.forms["clienteditform"].submit(status=302, name="action", value="edit")
92+
93+
backend.reload(client)
94+
assert client.client_secret_expires_at is None
95+
96+
97+
def test_new_client_secret(testclient, client, logged_admin, backend, caplog):
98+
"""Renewing the secret of a client clears its expiration date."""
99+
old_secret = client.client_secret
100+
client.client_secret_expires_at = datetime.datetime.now(
101+
datetime.timezone.utc
102+
) - datetime.timedelta(days=1)
103+
backend.save(client)
104+
105+
res = testclient.get("/admin/client/edit/" + client.client_id)
106+
res = res.forms["clienteditform"].submit(
107+
status=302, name="action", value="new-client-secret"
108+
)
109+
assert (
110+
"success",
111+
"The client secret has been renewed. The new secret does not expire.",
112+
) in res.flashes
113+
assert (
114+
"canaille",
115+
logging.SECURITY,
116+
f"Renewed the secret of client {client.client_id} by {logged_admin.id}",
117+
) in caplog.record_tuples
118+
119+
backend.reload(client)
120+
assert client.client_secret != old_secret
121+
assert client.client_secret_expires_at is None
122+
assert not client.secret_expired
123+
124+
testclient.post(
125+
"/oauth/token",
126+
params=dict(grant_type="client_credentials"),
127+
headers={"Authorization": f"Basic {client_credentials(client)}"},
128+
status=200,
129+
)
130+
131+
132+
def test_expired_secret_is_displayed(testclient, client, logged_admin, backend):
133+
"""Expired secrets are reported on the client page and in the client list."""
134+
res = testclient.get("/admin/client/edit/" + client.client_id)
135+
res.mustcontain(no="Expired secret")
136+
res = testclient.get("/admin/client")
137+
res.mustcontain(no="The secret of this application has expired")
138+
139+
client.client_secret_expires_at = datetime.datetime.now(
140+
datetime.timezone.utc
141+
) - datetime.timedelta(days=1)
142+
backend.save(client)
143+
144+
res = testclient.get("/admin/client/edit/" + client.client_id)
145+
res.mustcontain("Expired secret")
146+
res = testclient.get("/admin/client")
147+
res.mustcontain("The secret of this application has expired")
148+
149+
150+
def test_secret_expiration_is_serialized_as_a_timestamp(testclient, client, backend):
151+
"""The RFC7591 client information uses timestamps, and 0 when there is no expiration."""
152+
assert client.client_info["client_secret_expires_at"] == 0
153+
154+
expiration = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(
155+
days=1
156+
)
157+
client.client_secret_expires_at = expiration
158+
backend.save(client)
159+
assert client.client_info["client_secret_expires_at"] == int(expiration.timestamp())

0 commit comments

Comments
 (0)