Ticket #543 (accepted enhancement)

Opened 7 years ago

Last modified 4 years ago

Enhanced support for handling IP addresses

Reported by: JavaWoman Owned by: JavaWoman
Priority: normal Milestone: blue-sky
Component: core Version: trunk
Severity: normal Keywords: username, ACL


There is a number of use cases for using an IP address for an anonymous user, but currently Wikka doesn't support this very well. What I envisage is the following:

Getting username

Currently GetUserName retrieves username for a logged-in user or IP address or hostname for an anonymous user - no choice to not retrieve anything in case user is not logged in, and only a global setting whether or not to allow reverse DNS lookup (which can be really slow).

  • with a slightly different interface, GetUserName() could be told explicitly whether to retrieve anything for an anonymous user, and if so, what (IP address only, or reverse lookup if allowed); the default (no parameter passed) would be the most usual case: name of logged in user only
  • for logging and reporting spam usually only an IP address is needed (and preferable), while for storing (and later displaying) 'username' on anonymous edits and comments generally the host name would be preferable

Access control

When an admin now enables reverse DNS lookup (for nicer display of anonymous "user names", the result is a lot of slower page displays because GetUserName() is called for every page name to be displayed from inside the HasAccess() method.

  • by specifying no parameter in the new situation, such lookups would never occur here (even if they are allowed at system level), speeding up access control considerably
  • by specifying only IP address in this context, this slowdown would not occur either (the IP address is already available in the $_SERVER super global) - but it would allow us to work with IP addresses in ACLs as well (to deny or grant access by IP address)
  • admin could still allow reverse DNS lookup for anonymous comment posts or page edits (which is a once-off lookup instead of a whole series for any page that shows some list of pages

These changes would be trivial to implement (I have the changed GetUserName() already implemented on my local development machine), and not only speed up access for anonymous users but make the system more flexible as well, allowing working with both IP addresses and host names.

Change History

Changed 7 years ago by JavaWoman

  • type changed from defect to enhancement

Changed 7 years ago by JavaWoman

(In [616]) - cleanup - preparation for retrieval of logged-in username only (refs #543)

Changed 7 years ago by JavaWoman

  • owner changed from unassigned to JavaWoman
  • status changed from new to assigned

Changed 7 years ago by JavaWoman

(In [699]) Improved and more structural user handling

New user handling

  • new object variables (mainly to support new user handling)
  • new methods for more structured user handling:
    • authenticateUserFromCookies() - replaces usage of LoadUser() in Run(); logs in user automatically if authenticated
    • loadUserData() - replaces usage of LoadUser() with only username given, where that was to actually retrieve the data
    • existsUser() - replaces usage of GetUser() when used ony to check if current user is logged in, or LoadUser() with only username given when used to check whethre the given username is a registered user
    • getAnonUserName() - returns IP address or hostname depending on configuration setting - but see #543
    • loginUser() - logs in user; returns result code; if database update failed, user is not logged in (replaces SetUser() but with modified interface
  • obsoleted methods (will be removed!):
    • LoadUser() - replaced by different methods depending on usage; left in place for now but returns FALSE only.
  • replaced methods (will be removed!):
    • SetUser() - functions as wrapper for loginUser() for now, but with old interface: no result code returned
  • methods with modified interface:
    • LoadPage() - $cache parameter is now TRUE: pass FALSE to not use cache; returns only FALSE instead of sometimes NULL if page was not retrieved
    • LoadPageById() - returns only FALSE instead of sometimes NULL if page was not retrieved
    • GetUser() - returns FALSE if user not logged in
    • LogoutUser() - like loginUser, now returns result code; if database update failed, user is not logged out
    • FormatUser() - $link parameter is now boolean: pass FALSE to prevent linking
  • renamed methods - (*) indicates change wrt
    • setWikkaCookie() instead of SetACookie()
    • deleteWikkaCookie() instead of DeleteCookie() (*)
    • deleteOldWikkaCookies() instead of DeleteOldCookies()
    • getWikkaCookie() instead of GetCookie() (*)
  • methods modified internally to leverage new user handling:
    • Run() - this is what makes it all more efficient: now uses authenticateUserFromCookies(), which logs in user and stores username + registered state if successful, or getAnonUserName(), which stores anonymous user name, if not: this means that username (anonymous or not) is derived just once per page load, slashing page load times for anonymous users when reverse lookup is happening, and generally speeding things up for all users.
    • SavePage() - replaced usage of GetUser()
    • GetUserName() - replaced usage of GetUser(); uses getAnonUserName() if user not logged in
    • LogoutUser() - updates object variable that tracks whether user is registered
    • FormatUser() - replaced usage of LoadUser() by existsUser()
    • UserIsOwner() - replaced usage of GetUser() by existsUser(); replaced usage of GetUserName() by direct retrieval of stored registered user name
    • IsAdmin() - replaced GetUsername() by existsUser() and direct retrieval of stored registered user name; also restructured to avoid looping over array
    • HasAccess() - replaced usage of GetUser() and GetUserName() with existsUser() and direct retrieval of (registered or anonymous) user name

In general:

  • object variables rearranged and (mostly) grouped within docblock templates
  • many docblocks added/updated
  • cleanup (coding guidelines)
  • minor tweaks

Still lots of debug code and commented-out code; this will be cleaned up in a later stage!

Fixes #542, fixes #368 (but this time by addressing cause of the problem instead of symptoms); refs #496

Changed 5 years ago by BrianKoontz

  • milestone changed from 1.2.1 to 1.3

Changed 4 years ago by BrianKoontz

  • milestone changed from 1.3 to blue-sky
Note: See TracTickets for help on using tickets.