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

Display IP address and port of user on Login and Quit #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

slook
Copy link
Contributor

@slook slook commented Feb 2, 2025

This PR is in preparation for writing failed logins to a log file, or taking some other DDOS mitigation actions. For now, simply write these details to the console...

image

  • Added: Display IP address on login "... denied" message
  • Added: Display IP address:port on successful login (the client's port is established during SetWaitPort)
  • Added: Display IP address:port of both involved peers during indirect ConnectToPeer
  • Added: Display IP address:port on "... quit" message
  • Changed: bool login_denied -> string login_error to keep track of the specific denial reason
  • Changed: "logging in" -> "logged in" and combined the "... is an admin" string into the main login string.

The new placement of the login success message in SetWaitPort (code 2) isn't entirely satisfactory, but the client's listening port isn't known until that point...

... However, this discrepancy makes me wonder if any SRoomList, SWishlistInterval, watch, set_status and get_pms_for calls are supposed to occur after the client wait port has been set (i.e. whence the login is considered to be fully established), as it seems like the program might be jumping the gun a bit here, so I'd like to revisit this to make sure the correct order of events does take place compared to the official server, and whether or not proc_message() should block other cases until the client has set the wait port, or perhaps even disconnect a non-compliant peer whose requests are out of order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant