Skip to content

Commit

Permalink
Update account that signed the request
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiasertl committed Jan 1, 2025
1 parent 240f0ff commit 974dd2a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 11 deletions.
16 changes: 8 additions & 8 deletions ca/django_ca/acme/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -679,22 +679,22 @@ def _deactivate_account(self, account: AcmeAccount) -> None:
async def acme_request(
self, message: messages.Registration, slug: Optional[str] = None
) -> AcmeResponseAccount:
# TODO: does this allow updating other peoples accounts!?
account = await AcmeAccount.objects.url().aget(slug=slug)
if slug != self.account.slug:
raise AcmeMalformed(message="Account slug does not match account that signed the request.")

if message.status == AcmeAccount.STATUS_DEACTIVATED:
await sync_to_async(self._deactivate_account)(account)
await sync_to_async(self._deactivate_account)(self.account)
elif message.contact:
self.validate_contacts(message)
account.contact = "\n".join(message.contact)
await account.asave()
self.account.contact = "\n".join(message.contact)
await self.account.asave()
elif message.terms_of_service_agreed is not None:
account.terms_of_service_agreed = message.terms_of_service_agreed
await account.asave()
self.account.terms_of_service_agreed = message.terms_of_service_agreed
await self.account.asave()
else:
raise AcmeMalformed(message="Only contact information can be updated.")

return AcmeResponseAccount(self.request, account)
return AcmeResponseAccount(self.request, self.account)


class AcmeAccountOrdersView(AcmeBaseView):
Expand Down
54 changes: 51 additions & 3 deletions ca/django_ca/tests/acme/views/test_update_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@

from acme.messages import IDENTIFIER_FQDN, Identifier, Registration

from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat

from django.test import Client

import pytest
from pytest_django import DjangoAssertNumQueries

from django_ca.models import AcmeAccount, AcmeAuthorization, AcmeOrder, CertificateAuthority
from django_ca.models import AcmeAccount, AcmeAuthorization, AcmeOrder, CertificateAuthority, acme_slug
from django_ca.tests.acme.views.assertions import assert_malformed
from django_ca.tests.acme.views.base import AcmeWithAccountViewTestCaseMixin
from django_ca.tests.acme.views.utils import acme_request
from django_ca.tests.base.constants import TIMESTAMPS
from django_ca.tests.acme.views.utils import absolute_acme_uri, acme_request
from django_ca.tests.base.constants import CERT_DATA, TIMESTAMPS
from django_ca.tests.base.utils import root_reverse, root_uri

# ACME views require a currently valid certificate authority
Expand Down Expand Up @@ -95,6 +98,51 @@ def test_email(client: Client, url: str, root: CertificateAuthority, account: Ac
assert account.usable is True


def test_update_other_account(
django_assert_num_queries: DjangoAssertNumQueries,
client: Client,
url: str,
root: CertificateAuthority,
account: AcmeAccount,
kid: str,
) -> None:
"""Test that you cannot use this URL to update different accounts."""
slug_two = acme_slug()
kid_two = absolute_acme_uri(":acme-account", serial=root.serial, slug=slug_two)
pem_two = (
CERT_DATA["child-cert"]["key"]["parsed"]
.public_key()
.public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo)
.decode("utf-8")
.strip()
)
account_two = AcmeAccount.objects.create(
ca=root,
contact="mailto:two@example.com",
terms_of_service_agreed=True,
slug=slug_two,
kid=kid_two,
pem=pem_two,
thumbprint="abc",
)
email = "mailto:user.updated@example.com"
message = Registration(contact=(email,))

url = root_reverse("acme-account", slug=slug_two)

with django_assert_num_queries(2): # one for the CA, one for the account
resp = acme_request(client, url, root, message, kid=kid)
assert_malformed(resp, root, "Account slug does not match account that signed the request.")

account.refresh_from_db()
assert account.contact == "mailto:one@example.com"
assert account.usable is True

account_two.refresh_from_db()
assert account_two.contact == "mailto:two@example.com"
assert account_two.usable is True


def test_multiple_emails(
client: Client, url: str, root: CertificateAuthority, account: AcmeAccount, kid: str
) -> None:
Expand Down
7 changes: 7 additions & 0 deletions docs/source/changelog/TBR_2.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
2.2.0 (TBR)
###########


******
ACMEv2
******

* **Security:** No longer allow clients to update other accounts.

***********
Performance
***********
Expand Down

0 comments on commit 974dd2a

Please sign in to comment.