User administration module

[dartar]

A module to manage Wikka users.

Status

beta (code cleanup needed)

Refs

•  http://wikkawiki.org/UserAdmin

Related tickets

• #26 Page administration module
• #442 Removing spam/test users
• #608 Enhanced User Administration module (single/mass user feedback)
• #647 Update codebase to reflect new user status fields

[DarTar]

Mahefa, I'm reassigning this to myself if you don't mind.

[DarTar]

(In [772]) First draft of adminusers action. The code generates a simple GUI that can be used later to plug in different sorts of handlers and user-related functionality. Icons and general CSS selectors for alternate coloring of tables are included, refs #25 and #562

[DarTar]

(In [810]) Minor stylistic change to adminusers, refs #25

[DarTar]

(In [811]) Prefixing constants in adminusers to avoid name collision, adding CSS class for sort order information (to be used by any module that generates sortable lists or tables), refs #25

[DarTar]

(In [813]) Admin modules:

• Adding todos in phpdoc header;
• Adding missing icons;
• Hiding features yet to be implemented;

refs #25 and #26

[DarTar]

(In [814]) Minor phpdoc change, refs #25

[DarTar]

(In [815]) Styling: line-height for filter fieldsets in admin modules, refs #25 and #26

[BrianKoontz]

(In [830]) Mass user deletions, including password lockout and session tracking to "kick" banned users who might still be accessing the site via an existing session. Currently, there is no functionality to keep session table pruned of sessions that aren't deleted through a user logout (refs #25).

[BrianKoontz]

Replying to BrianKoontz:

(In [830]) Mass user deletions, including password lockout and session tracking to "kick" banned users who might still be accessing the site via an existing session. Currently, there is no functionality to keep session table pruned of sessions that aren't deleted through a user logout (refs #25).

Forgot to mention: This change requires the following DB change:

create table wikka_sessions (sessionid char(16) NOT NULL, userid varchar(75) NOT NULL, PRIMARY KEY (sessionid, userid));

Also, note to self: Single-user delete might not be working correctly.

[BrianKoontz]

(In [832]) Implemented cleanup routine for session records in sessions table (keyed off the persistent cookie lifetime) to keep the table pruned of old sessions. Minor code cleanup. (refs #25)

[raffa]

Replying to BrianKoontz:

Forgot to mention: This change requires the following DB change: create table wikka_sessions (sessionid char(16) NOT NULL, userid varchar(75) NOT NULL, PRIMARY KEY (sessionid, userid));

This should be added to the install script then.

cheers,

raffa

[raffa]

btw, the create string is not correct, since:

Query failed: INSERT INTO wikka_sessions (sessionid, userid, session_start) VALUES('some_session...', 'admin', FROM_UNIXTIME(1196582038.32)) (Unknown column 'session_start' in 'field list')

[BrianKoontz]

Replying to raffa:

btw, the create string is not correct, since: Query failed: INSERT INTO wikka_sessions (sessionid, userid, session_start) VALUES('some_session...', 'admin', FROM_UNIXTIME(1196582038.32)) (Unknown column 'session_start' in 'field list')

You'll need to do an update, raffa...this has been fixed.

[BrianKoontz]

Replying to BrianKoontz:

Replying to raffa:

btw, the create string is not correct, since: Query failed: INSERT INTO wikka_sessions (sessionid, userid, session_start) VALUES('some_session...', 'admin', FROM_UNIXTIME(1196582038.32)) (Unknown column 'session_start' in 'field list')

You'll need to do an update, raffa...this has been fixed.

Spoke too quickly...you'll need to add the session_start field:

alter table wikka_sessions add session_start datetime NOT NULL;

Will add this to the install script...

[BrianKoontz]

(In [833]) Forgot to create sessions table for new installs, fixed. Also, if one is upgrading between 1.1.6.4 revisions, DB updates will need to be done manually (refs #25).

[raffa]

the javascript checks in the install file bug me:

• CamelCase in username *should* be used. JS forces me to use it
• Email addresses like name@… are not recognized to be valid, there could also be other combinations?

By the way: wikka is still giving away too much information, which makes attacking vulnerable installations easier. See ticket #560 & #597.

[DarTar]

(In [834]) Adding default pages (AdminUsers and AdminPages) and removing redundant update of FormattingRules, refs #25 and #26 and #562

[BrianKoontz]

Migrated over from dupe ticket #442:

(In [823]) User status is a feature introduced in 1.1.6.4 so the upgrader should add it for any version <1.1.6.4, refs #442

(In [837]) Delete links disabled for admins; error message displayed when attempting to mass-delete admin users; delete link fixed (refs #442).

[DarTar]

(In [846]) Stripping single/mass user feedback from the AdminUser module included in 1.1.6.4 since they depend on the enhanced feedback module that will be released with 1.1.7, refs #25, #486, #608

[DarTar]

(In [854]) Installation of admin modules in 1.1.6.4: - creating new CategoryAdmin for default pages for admin modules; - creating new DatabaseInfo page as a default page for {{dbinfo}}; - setting ACL for AdminUsers, AdminPages and DatabaseInfo to admin-only; refs #25, #26 and #27

[BrianKoontz]

Tests successful using revision 863.

[DarTar]

Brian, a few random question on adminusers (not sure if we need to address them all before the 1.1.6.4 release):

• could user deletion display a confirmation screen? (the same that appears when trying to mass-delete)
• could we add a filter option to display deleted users? (in which case I think an "undelete" option could be useful)
• do other actions that process users data take into account the new status field? If they don't have we considered the issues that could possibly arise from this?

* if we give admins the possibility to completely restrict access, it would be necessary to have a form to be able to add new users from the admin panel, otherwise this will only be possible by directly feeding the DB.

Let me know what you think of these issues so we can plan some further coding if needed before definitely closing the ticket for the release.

[DarTar]

Also, is there a reason why the pw value for deleted users is replaced with a "!"? Why not just leave it there (assuming that the status field is sufficient to determine whether a user can still try to login or not).

This makes me think, incidentally, of the consequences of the status field on functionality such as {{emailpassword}}.

[DarTar]

I just realised that I haven't committed yet the userpages and userchanges actions required by the user admin module (!). I'll take care of this.

[BrianKoontz]

Replying to DarTar:

Also, is there a reason why the pw value for deleted users is replaced with a "!"?

Yes, because there is no possible MD5 hash that will match that value. It's the same principle used in Unix to lock a user account.

Why not just leave it there (assuming that the status field is sufficient to determine whether a user can still try to login or not).

Consider it an additional safeguard. If we've gone so far as to delete a user, there's no need for a password. And if said user has access restored by mistake, they still can't access until positive measures are taken to restore the password.

This makes me think, incidentally, of the consequences of the status field on functionality such as {{emailpassword}}.

Yes, there are probably several places in the Wikka codebase that needs to check the status of a user when an action would not depend on the user actually being logged in.

[BrianKoontz]

Replying to DarTar:

Brian, a few random question on adminusers (not sure if we need to address them all before the 1.1.6.4 release)

I've moved your comments to #608, which I created to handle just these types of questions. I realize there are a lot of additional enhancements that can be made to the user/page admin code, but at some point you have to draw the line in the sand and say "this is it for 1.1.6.4". I think we're at that point with the user/page admin modules.

[DarTar]

Replying to BrianKoontz:

Yes, because there is no possible MD5 hash that will match that value. It's the same principle used in Unix to lock a user account. [...] Consider it an additional safeguard. If we've gone so far as to delete a user, there's no need for a password.

ok so this answers my question re: the possibility of reinstating a previously removed user: this won't be possible with deleted users. I assume disabled users will be treated otherwise.

Yes, there are probably several places in the Wikka codebase that needs to check the status of a user when an action would not depend on the user actually being logged in.

ok, I suggest we open a separate ticket for this.

[BrianKoontz]

Since tickets #608 and #647 address further upgrades and modifications to the User Administration module as requested here, there appear to be no more issues to address with respect to this ticket and 1.1.6.4, so I'm closing it (again).

[DarTar]

Brian,

as per my last email and comment above, there's an important issue still pending for this ticket:

I haven't committed yet the userpages and userchanges actions required by the user admin module (!). I'll take care of this.

I'm not sure yet if I can find a good internet connection here, if I do I can try to fix these.

[BrianKoontz]

Missed that, sorry.

[BrianKoontz]

(In [915]) These changes all stem from enabling the {{my*}} actions in AdminUsers. There is an important fix to the comment actions (hidden comments were being displayed) that needs to be ported to 1.1.7. New parameter ($user) added to Load...Comment() calls in main library.

Refs #25, #562, #575

[DarTar]

Excellent, thanks for taking care of this, Brian.

[DarTar]

Reviewed and tested - closing the ticket.

[BrianKoontz]

The problem with the module is that one form is trying to do the work of multiple forms, so all POST fields are being processed, which results in things getting out of sync (for instance, clearing the search and then resubmitting returns unexpected results if num of records displayed has been modified previously...likewise, modifying the num of records displayed after a search has been performed does not return the expected results).

The proper fix is to completely refactor this module, and perform only one action at a time (i.e., use the value of 'submit' to determine if the user has asked for a search or something else). The quick fix is to insert enough branching instructions so as to properly process all permutations of fields that are submitted en masse.

See below for the quick fix, which I believe captures all combinations of searches and record display settings. It's rather ugly, though, and I doubt I'll remember what I did next week...

[BrianKoontz]

(In [1007]) Addressed issues involving the simultaneous submission of multiple POST params in the same POST action. Refs #25

[DarTar]

works for me now

[DarTar]

Brian, I just realized that the upgrader needs a:

ALTER TABLE trunk_users ADD status enum('invited','signed-up','pending','active','suspended','banned','deleted')

My current AdminUser in trunk is broken and I had to fix the DB manually.

[BrianKoontz]

Replying to DarTar:

Brian, I just realized that the upgrader needs a: {{{ ALTER TABLE trunk_users ADD status enum('invited','signed-up','pending','active','suspended','banned','deleted') }}} My current AdminUser in trunk is broken and I had to fix the DB manually.

Looks like this was fixed in [1139].