Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB\CORE: IP HISTORY LOGGING #29926

Closed
wants to merge 9 commits into from

Conversation

acidmanifesto
Copy link
Contributor

@acidmanifesto acidmanifesto commented Apr 17, 2024

Changes proposed:

  • new Auth Table account_ip_history
  • Enables the account ip history logging to account_ip_history of the auth table.
  • Primary Keys are Account and IP.
  • Assist in logging account ip history of the user every time they login.
  • Loggs Account, RealmID,IP, date_recent, date_first.
  • comment collumn added for any manual review notes
  • conf set to disable for whatever nsa\fbi\dickpic\gdprroflmaocopter requirement.
  • Added a del prep statement to clear account specific ip history log when account is deleted.

Issues addressed:

Closes # (insert issue tracker number)

Tests performed:

Builds and Tested in game
image
Observe the screen shot.
Account 1 logged in at for the first time since this PR at 2024-04-17 17:54:03 in date_first
Account 1 then logged off and relogged in with the same IP at 2024-04-17 17:55:01 Date Recemt
Account 3 is logging in the same ip as Account 1. So since it is its first login, both date_recent and date_first are the same.
If Account 1 logged in from a different ip then a new row is created.

Known issues and TODO list: (add/remove lines as needed)

  • [ ]
  • [ ]

Enables the account ip history logging to account_ip_history of the auth table.
Primary Keys are Account and IP.
Assist in logging account ip history of the user every time they login.
@CraftedRO
Copy link
Contributor

what about some "auto-clean" old records function ?

@acidmanifesto
Copy link
Contributor Author

what about some "auto-clean" old records function ?

This is a history log for ip history of a player account. I am not entirely certain what you mean by a auto-clean?

Only new entries are created if the account and ip changes.

It will only update timestamps of exsisting accounts/ip address entries.

It isnt like say a server.log or anticheat.log where it just continuously fills.

@Jinnaix
Copy link
Contributor

Jinnaix commented Apr 18, 2024

Looks good to me

@Aokromes
Copy link
Member

i requested such feature long time ago :)

@Faq
Copy link
Contributor

Faq commented Apr 19, 2024

But what is real use case for such info?

@Jinnaix
Copy link
Contributor

Jinnaix commented Apr 19, 2024

Use cases are clear for anyone who ever had TC in a productive environment

@acidmanifesto
Copy link
Contributor Author

acidmanifesto commented Apr 19, 2024

But what is real use case for such info?

Identifying a user(s) that may be multiboxing, vpn ips, various ips for either tracking of potentional malicious behavior or ban bypassing just to name a few.
Pretty much any manual audit concerns which you need to see if ip history will make a determining decision.

@Faq
Copy link
Contributor

Faq commented Apr 20, 2024

I guess then some helper would be view to show accounts with same IPs? As now you will check this only if reported, but could be done proactive? You know best where you will use this info, just saying that it can be improved, as now, this looks like basic implementation. Just thinking next steps from use case perspective, nothing against such implementations.

@acidmanifesto
Copy link
Contributor Author

I guess then some helper would be view to show accounts with same IPs? As now you will check this only if reported, but could be done proactive? You know best where you will use this info, just saying that it can be improved, as now, this looks like basic implementation. Just thinking next steps from use case perspective, nothing against such implementations.

I am up for any suggestions.

@Aokromes
Copy link
Member

I guess then some helper would be view to show accounts with same IPs? As now you will check this only if reported, but could be done proactive? You know best where you will use this info, just saying that it can be improved, as now, this looks like basic implementation. Just thinking next steps from use case perspective, nothing against such implementations.

imho all this must be handled by minimanager taking data from this table, ie searching accounts who used certain ips etc.

@Jinnaix
Copy link
Contributor

Jinnaix commented May 14, 2024

@acidmanifesto

Just for logical reasons - can you please bind this to the following feature being enabled?

image

Would be stupid to suppress ip logging in database but still create logging history.

It would also make more sense to place your config option right below that setting here

AllowLoggingIPAddressesInDatabase = 1
instead of the file log area.

@acidmanifesto
Copy link
Contributor Author

@acidmanifesto

Just for logical reasons - can you please bind this to the following feature being enabled?

image

Would be stupid to suppress ip logging in database but still create logging history.

It would also make more sense to place your config option right below that setting here

AllowLoggingIPAddressesInDatabase = 1

instead of the file log area.

done ca06252

@acidmanifesto
Copy link
Contributor Author

@acidmanifesto

Just for logical reasons - can you please bind this to the following feature being enabled?

image

Would be stupid to suppress ip logging in database but still create logging history.

It would also make more sense to place your config option right below that setting here

AllowLoggingIPAddressesInDatabase = 1

instead of the file log area.

ab09dbf

@Shauren
Copy link
Member

Shauren commented May 31, 2024

I'm going to close this - it duplicates what Allow.IP.Based.Action.Logging (combined with AllowLoggingIPAddressesInDatabase) already does

Query to extract the same data as what this PR proposes

SELECT
    lia.account_id, 
    lia.ip, 
    MIN(lia.time) AS date_first, 
    MAX(lia.time) AS date_recent
FROM
    logs_ip_actions AS lia
WHERE
    lia.type = 0 -- successful logins
GROUP BY
    lia.account_id, 
    lia.ip

@Shauren Shauren closed this May 31, 2024
@acidmanifesto acidmanifesto deleted the iphistory branch May 31, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants