נקה את יומני ה-auth_token של המשתמש?

Bug
1

יש לנו פורום שבו לטבלה user_auth_token_logs יש 61 מיליון רשומות (והיא גדלה).

יש רק 25 אלף user_auth_tokens.

מתוך 61 מיליון הרשומות, 54 מיליון רשומות מתייחסות ל-user_auth_token שכבר לא קיים (כלומר, בעיית שלמות נתונים). ומתוך 61 מיליון הרשומות, כ-58 מיליון הן בנות יותר מחודשיים (כלומר, חסרות תועלת לכאורה?)

שאלות:

  • האם נוכל פשוט לנקות את זה מבלי לסכן בעיות שלמות נוספות?
  • האם תהיה רעיון שיהיה תהליך שינקה את זה אוטומטית?
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 לייקים
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 לייקים
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 לייקים
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 לייקים
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 לייקים
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 לייקים
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 לייקים
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 לייקים
14

בטח, נסדר את זה במהלך השבועות הקרובים, אם יש לחץ אל תהססו לשלוח בקשת משיכה (PR) (שתהיה בבדיקה ותאשר שזה עובד כמצופה)

לייק 1
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 לייקים
20

אכן, אותו PR אושר כעת בזכות @Osama. הוא מטפל ברוב סוגי user_auth_token_logs, אך לא בכולם, אנו נמשיך עם תיקון עבור ערכי generate בקרוב. (ראה את הדיון בקישור ה-PR למעלה להקשר נוסף).

אני אשאיר נושא זה פתוח בזמן שאנו מטפלים בהמשך.

4 לייקים