Skip to content

Add PATCH endpoints so objects can be updated without being deleted - #604

Open
JakeHuneau wants to merge 1 commit into
OpenAS2:masterfrom
JakeHuneau:feature/patch-endpoints
Open

Add PATCH endpoints so objects can be updated without being deleted#604
JakeHuneau wants to merge 1 commit into
OpenAS2:masterfrom
JakeHuneau:feature/patch-endpoints

Conversation

@JakeHuneau

Copy link
Copy Markdown
Contributor

There is no update operation anywhere in the codebase: add refuses to overwrite an existing entry, so the only way to change a partner or a partnership is to delete it and recreate it. That loses the definition outright if the recreate then fails, and it is what makes an editing UI destructive by construction.

What this adds

New update commands for partner and partnership that merge only what is supplied and leave everything else alone, implemented on both the XML and the database partnership store, plus a PATCH endpoint that exposes them:

PATCH /api/partner/<name>
PATCH /api/partnership/<name>
PATCH /api/cert/<alias>

The attributes to change go in as form fields. A partnership also accepts pollerConfig.<attr> to change one poller attribute, and sender.name / receiver.name to point it at a different partner.

A certificate PATCH takes either a base64 certificate in data, which replaces a partner certificate, or a base64 PKCS12 in data plus the password that opens it in password, which replaces a certificate and its private key together.

Replacing a key pair did not work before

importPrivateKey staged the certificate with setCertificateEntry, which a keystore refuses on an alias that already holds a private key because it would orphan the key. The import therefore failed for exactly the alias anyone would want to rotate, through the console cert import <alias> <file.p12> <password> path as well as the API.

The key entry is now written in a single operation, which replaces whatever the alias held, so there is no window where the alias has a certificate but no key. Three things this is careful about, since the alias in question is usually the server's own identity:

  • The previous key pair is only replaced by a successful write, and is put back if the write fails, so a failed rotation leaves the running server able to keep signing with the identity it already had.
  • The entry is stored under the keystore password rather than the password of the file the key came from, because that is what getPrivateKey reads it back with. The old code stored an unreadable key whenever the two differed.
  • The certificate chain is taken from the source keystore so intermediates survive; the old path could only carry the leaf. The existing self-signed fallback shape is preserved so nothing about existing keystores changes.

Other behaviour

  • Neither object can be renamed by an update. Other records reference them by name, so a rename would silently orphan them; the error says to add the replacement and delete the old one instead.
  • An unknown partner or partnership comes back as a command error rather than being raised, so an API caller gets the reason instead of a server error. This matches how the delete command already behaves.

Neighbouring fixes

Two verbs in ApiResource were unusable and are fixed here since a client adopting PATCH would hit them immediately:

  • PUT and DELETE dropped the first character of the item name (DELETE /api/partner/PartnerA called delete on artnerA), because they hand the ID to a method that expects it to still carry the leading path separator.
  • PUT and HEAD bound @PathParam("param"), which is not in their path template, so the resource name always arrived null.

Happy to split these into a separate PR if you would rather keep them apart.

Testing

162 tests pass, 26 new.

  • UpdateCommandsTest covers both stores directly: only the supplied attributes change, an attribute that was not there is added rather than duplicated, poller config merges, a partnership can be repointed, a partner change propagates to partnerships that inherit from it, and an unknown entry is refused rather than created.
  • PatchApiTest covers the routing over real HTTP, including that the whole item name reaches the command, that an unauthenticated PATCH is not applied, and two certificate cases: rotating an alias that holds a private key and then reading the key back through the server's own factory, and a rotation with the wrong password leaving the original certificate and private key intact.

There was no update operation anywhere: "add" refuses to overwrite an
existing entry, so the only way to change a partner or a partnership was to
delete it and recreate it. That loses the definition outright if the recreate
then fails, and it is what makes an editing UI destructive by construction.

Adds "update" commands for partner and partnership that merge only what is
supplied and leave everything else alone, implemented on both the XML and the
database partnership store, and a PATCH API endpoint that exposes them:

    PATCH /api/partner/<name>
    PATCH /api/partnership/<name>
    PATCH /api/cert/<alias>

with the attributes to change as form fields. A partnership also accepts
pollerConfig.<attr> to change one poller attribute, and sender.name or
receiver.name to point it at a different partner.

A certificate PATCH takes either a base64 certificate in "data", which
replaces a partner certificate, or a base64 PKCS12 in "data" plus the password
that opens it in "password", which replaces a certificate and its private key
together so an identity of our own can be rotated.

Replacing a key pair did not work before. importPrivateKey staged the
certificate with setCertificateEntry, which a keystore refuses on an alias
that already holds a private key because it would orphan the key, so the
import failed for exactly the alias anyone would want to rotate. The key entry
is now written in a single operation instead, which replaces whatever the alias
held, so there is no window where the alias has a certificate but no key. The
previous key pair is put back if the write fails, leaving the running server
able to keep signing with the identity it already had. The entry is stored
under the keystore password rather than the password of the file the key came
from, because that is what getPrivateKey reads it back with. This fixes the
console "cert import <alias> <file.p12> <password>" path as well.

Neither a partner nor a partnership can be renamed by an update. Other records
reference them by name, so a rename would silently orphan them; the error says
to add the replacement and delete the old one instead.

An unknown partner or partnership is returned as a command error rather than
raised, so an API caller gets the reason back instead of a server error, which
matches how the delete command already behaves.

Also fixes the neighbouring verbs, which were unusable: PUT and DELETE dropped
the first character of the item name because they pass the ID to a method that
expects it to still carry the leading path separator, and PUT and HEAD bound a
path parameter named "param" that is not in their path template, so the
resource name always arrived null.
@JakeHuneau
JakeHuneau force-pushed the feature/patch-endpoints branch from 8296a61 to 2037dd7 Compare September 8, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant