feat: flush inactive/expired login and consent requests

[Benehiko]

Related issue

This is related to the hydra_oauth2_authentication_request table growth #1574 caused by expired/inactive login and consent flows never being purged from the database.

Proposed changes

Delete records from hydra_oauth2_authentication_request and hydra_oauth2_consent_request table which are of no use anymore causing the database growth to occur. This would then still follow the notAfter safe-guard approach to ensure we don't delete records the user does not want deleted.

Add this feature to the oauth2/flush endpoint https://github.com/Benehiko/hydra/blob/7f64e1a503646a3947af5aef9428dbc1b2792a3a/oauth2/handler.go#L520

This could also possibly be added to its own endpoint under the login/consent flows oauth2/auth/flush since this has to do specifically with login and consent flows rather than oauth2 tokens.
https://github.com/Benehiko/hydra/blob/7f64e1a503646a3947af5aef9428dbc1b2792a3a/consent/handler.go#L797

Add this to a janitor command in the CLI which will be a replacement for the current oauth2/flush endpoint.

Checklist

• I have read the contributing guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security. vulnerability, I
  confirm that I got green light (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further comments

Related test run to verify the growth of tables on Hydra:

Before change

This graph shows the growth over time after 200 000 clients have been created and each have tried the login / consent flows.
The failure-rate (number of clients that fail to log in) were set to be 97% which did not make a significant change in the growth over time against a test of successful client logins. The growth in clients created can be ignored as the tests were creating a singular client per login (hydra_client table - yellow). The hydra_oauth2_authentication_request table (purple) has been identified as being a problem table that would grow over time as clients perform the login and consent flows.

After change

This graph shows the growth over time after 20 000 clients have been created and each have tried the login / consent flows.
The failure-rate (number of clients that fail to log in) were set to be 97% which did not make a significant change in the growth over time against a test of successful client logins. The growth in clients created can be ignored as the tests were creating a singular client per login (hydra_client table - red). The hydra_oauth2_authentication_request table (light-blue) has been identified as being a problem table that would grow over time as clients perform the login and consent flows.

[zepatrik]

Can you add the charts you had? Just the discussion here, not in git
Do you have some with the flushing applied as in this PR?

[Benehiko]

Can you add the charts you had? Just the discussion here, not in git
Do you have some with the flushing applied as in this PR?

Can do, I do have one

[aeneasr]

Thank you, I think this is going in the right direction! I actually have another proposal - we currently have the oauth2 flush endpoint, but I think that's actually a bad pattern. We should probably have a subcommand hydra janitor (or hydra clean) which does what the flush endpoint does today. It would be similar to hydra migrate sql and connect directly to the database. We could then have flags (e.g. --keep-if-younger 30d) which would configure this stuff.

Its a bit of refactoring but since we tackle this anyways in this PR I think it makes sense. If you need some guidance I am happy to help with pointers!

[aeneasr]

@Benehiko good for another 👀 ?

[Benehiko]

@Benehiko good for another ?

@aeneasr
I still need to write tests for the persister:
#2381 (comment)

And when handler removes login/consent requests
#2381 (comment)

Other than that most of the small fixes have been done and 2/3 dockertests have been implemented alongside the 3/3 sqlite tests for the cli.

[Benehiko]

@aeneasr I have refactored the janitor cli.
I had to move how the cobra command was being created from the cmd package to the cli package. So all the flags and their descriptions are created in the cli package now. This is because I would've gotten a cyclic dependency error by importing the cmd constants inside the cli package as well as using the janitorCmd inside handler_janitor_test.go. This removes the duplication code for setting up the cli tests.

[aeneasr]

I've got some comments, you can address them beforehand or we do it together in the call!