ory / hydra

OpenID Certified™ OpenID Connect and OAuth Provider written in Go - cloud native, security-first, open source API security for your infrastructure. SDKs for any language. Works with Hardware Security Modules. Compatible with MITREid.
https://www.ory.sh/hydra/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.26k stars 1.47k forks source link

Deletes are not respecting the time boundaries with CockroachDB #3753

Open david7joy opened 3 months ago

david7joy commented 3 months ago

Preflight checklist

Ory Network Project

No response

Describe the bug

We are observing that a record is being attempted to be deleted which was created as recently as 5 mins ago. Hydra is pushing for a delete which may not be necessary or requested in cockroachDB.

Here is the delete query example :

DELETE FROM
  hydra_oauth2_access AS hydra_oauth2_access
WHERE
  nid = '39bba160-9c5d-4bd1-9f80-170c4f7c4a06':::UUID AND request_id = '591b4158-1431-47e7-b707-66b75b5edf75':::STRING

Here is the timestamp we observed in CockroachDB :

select crdb_internal.approximate_timestamp(crdb_internal_mvcc_timestamp) FROM hydra_oauth2_access AS hydra_oauth2_access WHERE nid = '39bba160-9c5d-4bd1-9f80-170c4f7c4a06' AND request_id ='591b4158-1431-47e7-b707-66b75b5edf75';

  crdb_internal.approximate_timestamp
---------------------------------------
  2024-03-28 18:46:16.506874

Observation: The transaction was only created 5 mins ago, but Hydra is attempting a delete.

It may be a good idea to use CRDB Built-in Row Level TTL capability over adding delete in Hydra.

Reproducing the bug

Relevant log output

No response

Relevant configuration

No response

Version

v2.2.0 and v2.2.0-rc.2

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes

Additional Context

@viragtripathi @nollenr

aeneasr commented 2 months ago

Under normal circumstances, rows in hydra_oauth2_access are not deleted within 5 minutes, as the default expiry for access tokens is at least an hour. Depending on the clean up strategy (e.g. using Ory Hydra Janitor), one can choose how much time should pass before these stale records are removed.

Cockroach TTL is in our view not the best solution here as our SQL migrations are immutable files. Operators however want to choose how long they want to keep these records on file as it is often used in forensic investigations around account takeover (Answering: "who issued which token at what time and used it for what?"). Since we don't know how long these recods should be kept, we can't set a fixed time for TTL, which would be the case with row level TTL. I'm sure there is a way to engineer around this, but we believe that the Janitor is good enough even for larger-scale environments.