user_auth_token_logs opschonen?

Fout
1

We hebben een forum waar user_auth_token_logs 61 miljoen rijen heeft (en groeiende).

Er zijn slechts 25k user_auth_tokens.

Van de 61 miljoen rijen verwijzen 54 miljoen rijen naar een user_auth_token die niet meer bestaat (d.w.z. een database-integriteitsprobleem). En van de 61 miljoen rijen zijn ongeveer 58 miljoen ouder dan 2 maanden (d.w.z. schijnbaar nutteloos?)

Vragen:

  • Kunnen we dit gewoon opschonen zonder verdere integriteitsproblemen te riskeren?
  • Zou het een idee zijn om hier automatisch een taak voor te laten lopen?
db=# select count(*) from user_auth_tokens;
 count
-------
 25648

db=# select count(*) from user_auth_token_logs;
  count
----------
 61415352

db=# select count(*) from user_auth_token_logs where user_auth_token_id not in (select id from user_auth_tokens);
  count
----------
 54558442

db=# select count(*) from user_auth_token_logs where created_at < '2024-07-13';
  count
----------
 58565943
4 likes
3

Yes, user_auth_token_logs are there only for debugging purposes. All rows can be emptied, the only consequence will be that you won’t have any logs to debug.

This should be covered by:

https://github.com/discourse/discourse/blob/main/app/jobs/scheduled/weekly.rb#L13

2 likes
5

Thank you for that, I did not realize that was there.

So
 the cleanup only runs when verbose_auth_token_logging is true (which is not the case on this instance)

but non-verbose logging always happens regardless of the setting :scream:

per 8fb823c

Moving this to bug :slight_smile:

4 likes
6

Ah yes, good catch. Looks like lines 214 to 217 need fixing as well.

I would be comfortable with a global cleanup after a certain timeframe. @osama (since you’re the author of the commit linked above), do you think we can clean up all of these logs after some time (and if so, after how long)? It sounds like we need to keep some of them for detecting suspicious logins.

4 likes
7

Why does it need fixing? :thinking: That piece of code is about cleaning up rotated UserAuthTokens, not about the log records?

Update: after enabling SiteSetting.verbose_auth_token_logging, triggering the weekly job and running VACUUM FULL user_auth_token_logs the table went from 16GB to 687MB :+1:

Saved some trees today :deciduous_tree:

3 likes
8

Yes, I think we can clean up most of the logs, but some of them have to stay. Specifically, I think any records that have suspicious, generate, or rotate for action will need to be kept around because they’re used for detecting and generating reports for suspicious logins.

3 likes
12

I’m seeing on my forum this bug was never fixed :eyes:

The suspicious logins report seems to only apply to staff members, is there a reason these logs need to be kept for non-admins?

For the report to work, does it need data from the beginning of the account’s history? Can the log be shortened to something like the past 6 months?

Right now there is no cleanup whatsoever, which is a privacy concern.

2 likes
13

I don’t understand the above discussion either.

The bug is very simple: if the mode is not verbose, then no cleanup of UserAuthTokenLog is performed at all, ever. The if must go.

image
image500×259 5.06 KB

The original implementation only logged when SiteSetting.verbose_auth_token_logging is true. Which still had the problem that after disabling it, the most recent remaining logs would stay, but that’s a small thing.

But this change made the logging unconditional (“The generate, rotate and suspicious auth token logs are now always logged regardless of the verbose_auth_token_logging setting”).

TLDR; That change forgot to make the removal unconditional as well.

3 likes
14

Zeker, we zullen het de komende weken oplossen. Als er haast bij is, stuur gerust een pull request (die getest is en bevestigt dat dit naar verwachting werkt).

1 like
17

I’ve made a PR Fix: cleanup UserAuthTokenLog unconditionally by communiteq · Pull Request #34288 · discourse/discourse · GitHub, would be cool if this made the cut for 3.5

19

And it seems I was beaten to it :slight_smile:

2 likes
20

Inderdaad, die PR is nu samengevoegd dankzij @Osama. Het pakt de meeste soorten user_auth_token_logs aan, maar niet allemaal. We zullen binnenkort een oplossing voor de generate-vermeldingen volgen. (Zie de discussie in de PR-link hierboven voor meer context).

Ik houd dit onderwerp open terwijl we de follow-up aanpakken.

4 likes