diff --git a/CHANGES.rst b/CHANGES.rst index b4d8d81c..ce1c61b7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,6 +8,7 @@ Added - Display TOS and policy URI on the consent list page. :pr:`102` - Admin token deletion :pr:`100` :pr:`101` +- Revoked consents can be restored. :pr:`103` Fixed ***** diff --git a/canaille/oidc/consents.py b/canaille/oidc/consents.py index 3981a5bd..c44a9a4c 100644 --- a/canaille/oidc/consents.py +++ b/canaille/oidc/consents.py @@ -5,7 +5,7 @@ from flask import Blueprint from flask import flash from flask import redirect from flask import url_for -from flask_babel import gettext +from flask_babel import _ as _ from flask_themer import render_template from .utils import SCOPE_DETAILS @@ -18,7 +18,6 @@ bp = Blueprint("consents", __name__, url_prefix="/consent") @user_needed() def consents(user): consents = Consent.filter(subject=user.dn) - consents = [c for c in consents if not c.revokation_date] client_dns = list({t.client for t in consents}) clients = {dn: Client.get(dn) for dn in client_dns} return render_template( @@ -31,16 +30,37 @@ def consents(user): ) -@bp.route("/delete/") +@bp.route("/revoke/") @user_needed() -def delete(user, consent_id): +def revoke(user, consent_id): consent = Consent.get(consent_id) if not consent or consent.subject != user.dn: - flash(gettext("Could not delete this access"), "error") + flash(_("Could not revoke this access"), "error") + + elif consent.revokation_date: + flash(_("The access is already revoked"), "error") else: consent.revoke() - flash(gettext("The access has been revoked"), "success") + flash(_("The access has been revoked"), "success") + + return redirect(url_for("oidc.consents.consents")) + + +@bp.route("/restore/") +@user_needed() +def restore(user, consent_id): + consent = Consent.get(consent_id) + + if not consent or consent.subject != user.dn: + flash(_("Could not restore this access"), "error") + + elif not consent.revokation_date: + flash(_("The access is not revoked"), "error") + + else: + consent.restore() + flash(_("The access has been restored"), "success") return redirect(url_for("oidc.consents.consents")) diff --git a/canaille/oidc/endpoints.py b/canaille/oidc/endpoints.py index 395ac612..650e2e4c 100644 --- a/canaille/oidc/endpoints.py +++ b/canaille/oidc/endpoints.py @@ -93,12 +93,13 @@ def authorize(): client=client.dn, subject=user.dn, ) - consents = [c for c in consents if not c.revokation_date] consent = consents[0] if consents else None if request.method == "GET": - if client.preconsent or ( - consent and all(scope in set(consent.scope) for scope in scopes) + if ( + (client.preconsent and (not consent or not consent.revoked)) + or (consent and all(scope in set(consent.scope) for scope in scopes)) + and not consent.revoked ): return authorization.create_authorization_response(grant_user=user.dn) @@ -137,6 +138,8 @@ def authorize(): grant_user = user.dn if consent: + if consent.revoked: + consent.restore() consent.scope = client.get_allowed_scope( list(set(scopes + consents[0].scope)) ).split(" ") diff --git a/canaille/oidc/models.py b/canaille/oidc/models.py index 50a7a310..841006a2 100644 --- a/canaille/oidc/models.py +++ b/canaille/oidc/models.py @@ -222,6 +222,10 @@ class Consent(LDAPObject): "revokation_date": "oauthRevokationDate", } + @property + def revoked(self): + return bool(self.revokation_date) + def revoke(self): self.revokation_date = datetime.datetime.now() self.save() @@ -230,10 +234,11 @@ class Consent(LDAPObject): oauthClient=self.client, oauthSubject=self.subject, ) + tokens = [token for token in tokens if not token.revoked] for t in tokens: - different_scope = any(scope not in t.scope[0] for scope in self.scope) - if t.revoked or different_scope: - continue - t.revokation_date = self.revokation_date t.save() + + def restore(self): + self.revokation_date = None + self.save() diff --git a/canaille/templates/oidc/user/consent_list.html b/canaille/templates/oidc/user/consent_list.html index a628c46b..8b1aecc0 100644 --- a/canaille/templates/oidc/user/consent_list.html +++ b/canaille/templates/oidc/user/consent_list.html @@ -41,7 +41,13 @@
{% trans %}Revoked:{% endtrans %} {{ consent.revokation_date.strftime("%d/%m/%Y %H:%M:%S") }}
{% endif %}
-

{% trans %}Has access to:{% endtrans %}

+

+ {% if consent.revokation_date %} + {% trans %}Had access to:{% endtrans %} + {% else %} + {% trans %}Has access to:{% endtrans %} + {% endif %} +

{% for scope in consent.scope %} {% if scope not in ignored_scopes %} @@ -74,10 +80,17 @@ {% endif %}
{% endif %} - - - {% trans %}Remove access{% endtrans %} - + {% if consent.revokation_date %} + + + {% trans %}Restore access{% endtrans %} + + {% else %} + + + {% trans %}Revoke access{% endtrans %} + + {% endif %}
{% endfor %} diff --git a/canaille/translations/messages.pot b/canaille/translations/messages.pot index f0911148..cbb68a30 100644 --- a/canaille/translations/messages.pot +++ b/canaille/translations/messages.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2023-02-10 09:52+0100\n" +"POT-Creation-Date: 2023-02-14 18:43+0100\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -388,14 +388,30 @@ msgstr "" msgid "The client has been deleted." msgstr "" -#: canaille/oidc/consents.py:40 -msgid "Could not delete this access" +#: canaille/oidc/consents.py:39 +msgid "Could not revoke this access" msgstr "" -#: canaille/oidc/consents.py:44 +#: canaille/oidc/consents.py:42 +msgid "The access is already revoked" +msgstr "" + +#: canaille/oidc/consents.py:46 msgid "The access has been revoked" msgstr "" +#: canaille/oidc/consents.py:57 +msgid "Could not restore this access" +msgstr "" + +#: canaille/oidc/consents.py:60 +msgid "The access is not revoked" +msgstr "" + +#: canaille/oidc/consents.py:64 +msgid "The access has been restored" +msgstr "" + #: canaille/oidc/endpoints.py:130 msgid "You have been successfully logged out." msgstr "" @@ -1208,15 +1224,31 @@ msgstr "" msgid "Revoked:" msgstr "" -#: canaille/templates/oidc/user/consent_list.html:44 +#: canaille/templates/oidc/user/consent_list.html:46 +msgid "Had access to:" +msgstr "" + +#: canaille/templates/oidc/user/consent_list.html:48 msgid "Has access to:" msgstr "" -#: canaille/templates/oidc/user/consent_list.html:63 -msgid "Remove access" +#: canaille/templates/oidc/user/consent_list.html:72 +msgid "Policy" msgstr "" -#: canaille/templates/oidc/user/consent_list.html:72 +#: canaille/templates/oidc/user/consent_list.html:78 +msgid "Terms of service" +msgstr "" + +#: canaille/templates/oidc/user/consent_list.html:86 +msgid "Restore access" +msgstr "" + +#: canaille/templates/oidc/user/consent_list.html:91 +msgid "Revoke access" +msgstr "" + +#: canaille/templates/oidc/user/consent_list.html:101 msgid "You did not authorize applications yet." msgstr "" diff --git a/tests/oidc/test_consent.py b/tests/oidc/test_consent.py index 3785ab6a..886cc495 100644 --- a/tests/oidc/test_consent.py +++ b/tests/oidc/test_consent.py @@ -1,52 +1,144 @@ -import datetime +from urllib.parse import parse_qs +from urllib.parse import urlsplit + +from canaille.oidc.models import Consent +from canaille.oidc.models import Token + +from . import client_credentials def test_no_logged_no_access(testclient): testclient.get("/consent", status=403) -def test_delete(testclient, client, consent, logged_user, token): +def test_revokation(testclient, client, consent, logged_user, token): res = testclient.get("/consent", status=200) assert client.client_name in res.text + assert "Revoke access" in res.text + assert "Restore access" not in res.text + assert not consent.revoked assert not token.revoked - res = testclient.get(f"/consent/delete/{consent.cn[0]}", status=302) + res = testclient.get(f"/consent/revoke/{consent.cn[0]}", status=302) assert ("success", "The access has been revoked") in res.flashes res = res.follow(status=200) - assert client.client_name not in res.text + assert "Revoke access" not in res.text + assert "Restore access" in res.text + consent.reload() + assert consent.revoked token.reload() assert token.revoked -def test_delete_token_already_revoked(testclient, client, consent, logged_user, token): - revokation_date = datetime.datetime.utcnow().replace( - microsecond=0 - ) - datetime.timedelta(days=7) - token.revokation_date = revokation_date - token.save() +def test_revokation_already_revoked(testclient, client, consent, logged_user): + consent.revoke() - token.reload() - assert token.revoked - assert token.revokation_date == revokation_date + consent.reload() + assert consent.revoked - res = testclient.get(f"/consent/delete/{consent.cn[0]}", status=302) - assert ("success", "The access has been revoked") in res.flashes + res = testclient.get(f"/consent/revoke/{consent.cn[0]}", status=302) + assert ("error", "The access is already revoked") in res.flashes res = res.follow(status=200) - assert client.client_name not in res.text + consent.reload() + assert consent.revoked + + +def test_restoration(testclient, client, consent, logged_user, token): + consent.revoke() + + consent.reload() + assert consent.revoked + token.reload() + assert token.revoked + + res = testclient.get(f"/consent/restore/{consent.cn[0]}", status=302) + assert ("success", "The access has been restored") in res.flashes + res = res.follow(status=200) + + consent.reload() + assert not consent.revoked token.reload() assert token.revoked - assert token.revokation_date == revokation_date -def test_invalid_consent_delete(testclient, client, logged_user): - res = testclient.get(f"/consent/delete/invalid", status=302) +def test_restoration_already_restored(testclient, client, consent, logged_user, token): + assert not consent.revoked + + res = testclient.get(f"/consent/restore/{consent.cn[0]}", status=302) + assert ("error", "The access is not revoked") in res.flashes + res = res.follow(status=200) + + +def test_invalid_consent_revokation(testclient, client, logged_user): + res = testclient.get(f"/consent/revoke/invalid", status=302) assert ("success", "The access has been revoked") not in res.flashes - assert ("error", "Could not delete this access") in res.flashes + assert ("error", "Could not revoke this access") in res.flashes -def test_someone_else_consent_delete(testclient, client, consent, logged_moderator): - res = testclient.get(f"/consent/delete/{consent.cn[0]}", status=302) +def test_someone_else_consent_revokation(testclient, client, consent, logged_moderator): + res = testclient.get(f"/consent/revoke/{consent.cn[0]}", status=302) assert ("success", "The access has been revoked") not in res.flashes - assert ("error", "Could not delete this access") in res.flashes + assert ("error", "Could not revoke this access") in res.flashes + + +def test_invalid_consent_restoration(testclient, client, logged_user): + res = testclient.get(f"/consent/restore/invalid", status=302) + assert ("success", "The access has been restored") not in res.flashes + assert ("error", "Could not restore this access") in res.flashes + + +def test_someone_else_consent_restoration( + testclient, client, consent, logged_moderator +): + res = testclient.get(f"/consent/restore/{consent.cn[0]}", status=302) + assert ("success", "The access has been restore") not in res.flashes + assert ("error", "Could not restore this access") in res.flashes + + +def test_oidc_authorization_after_revokation( + testclient, logged_user, client, keypair, consent +): + consent.revoke() + + consent.reload() + assert consent.revoked + + res = testclient.get( + "/oauth/authorize", + params=dict( + response_type="code", + client_id=client.client_id, + scope="openid profile", + nonce="somenonce", + ), + status=200, + ) + + res = res.form.submit(name="answer", value="accept", status=302) + + Consent.all() + consents = Consent.filter(client=client.dn, subject=logged_user.dn) + assert consents[0].dn == consent.dn + consent.reload() + assert not consent.revoked + + params = parse_qs(urlsplit(res.location).query) + code = params["code"][0] + res = testclient.post( + "/oauth/token", + params=dict( + grant_type="authorization_code", + code=code, + scope="openid profile", + redirect_uri=client.redirect_uris[0], + ), + headers={"Authorization": f"Basic {client_credentials(client)}"}, + status=200, + ) + + access_token = res.json["access_token"] + token = Token.get(access_token=access_token) + assert token.client == client.dn + assert token.subject == logged_user.dn