cms icon indicating copy to clipboard operation
cms copied to clipboard

Merge users and admins

Open valiro21 opened this issue 8 years ago • 7 comments

This means:

  • add another column in the database for each user specifying the algorithm used to hashing (or empty for clear text). Example: MD5, SHA512, SHA256
  • based on the above field, the password field will be either the clear text password or the hashed password
  • in the CWS, at login, compare the passwords based on the algorithm specified in the database
  • don't show passwords for users that use a hash function in the AWS but display a message showing the algorithm used. the admin can still change the password
  • add option to specify hash in cmsAddUser

Pros:

  • different types of users (users for a local contest with clear text password and users for remote contests in the same database)
  • in the future, this will work well with an option for users to change their password only if allowed by the admins
  • better security overall
  • users can use password that are easier to remember for them and I can guarantee my users that I can't see their passwords

Cons:

  • additional column in the database - bigger database
  • with password hash, the admins can't recover a password, only change it

valiro21 avatar Feb 06 '17 09:02 valiro21

Hi Valentin, completely agree this is necessary for online contests or when registration is not done by admins. To add some "prior art", administrative accounts work similarly, but using only one field (the format is algo:password (or something equivalent), where the default algo is using bcrypt. I wanted to extend this schema to users too.

stefano-maggiolo avatar Feb 06 '17 09:02 stefano-maggiolo

Hi Stefano,

Thank you for considering this feature!

A couple of remarks that should be taken into consideration:

  • the additional column assures an easier upgrade of the database whereas a single field would require to modify the current password fields in the database to something like text:password (for plain text)
  • from the pycrypto documentation, I can see that it has the necessary SHA2 algorithms so there is no need to add more package requirements, but bcrypt is also good and it is already used for the administrative accounts

valiro21 avatar Feb 06 '17 12:02 valiro21

In my opinion, Admin accounts should be merged with User accounts. This is evident from the fact that, since an Admin cannot login in ContestWebServer, we implemented silly workarounds in order to perform admin operations (e.g. the boolean unrestricted field of users).

In practice, I find myself using two admin accounts everytime: one for AdminWebServer, and another (hidden + unrestricted) for ContestWebServer.

It would make much more sense to just have "unrestricted Users" be the same thing as "Admins".

wil93 avatar Feb 06 '17 15:02 wil93

I would prefer keeping administrator accounts separate from participants. In most cases except one you mentioned there is little overlap between administrator and user functionality. It would add unnecessary information about administrator settings to all users. Having a hidden test account doesn't seem like that much of problem, you can use a password manager and after logging in there is almost no visible difference.

karliss avatar Feb 06 '17 18:02 karliss

In most cases except one you mentioned there is little overlap between administrator and user functionality

Here's another case: with the multi contest functionality, it can happen (and it actually happened to me) that an Admin should only moderate some contests (not all of them).

For example, every year we hold an online contest before the national finals. This contest is usually prepared (and moderated) by some volunteers, often chosen among the best students from the previous year. Last year, I remember creating a special Admin user for them, and I manually tweaked AdminWebServer in order to avoid letting these students access information on other contests with the provided Admin account.

To solve this, these are the options:

  1. Adding a field to Admin table, with a whitelist (or blacklist) of contests that the admin user can moderate.
  2. Spending another year implementing an AdminParticipation concept (where an Admin user "participates" in a Contest) just like we did in the past for Users.
  3. Just use the existing Participation concept.

I would do option 3.

It would add unnecessary information about administrator settings to all users

There is already. The User.unrestricted field is all you need to distinguish normal users from admins.

Having a hidden test account doesn't seem like that much of problem, you can use a password manager and after logging in there is almost no visible difference

I fail to see how a password manager helps.

wil93 avatar Feb 06 '17 18:02 wil93

That makes sense, but requires much bigger changes. Lets do it in separately.

karliss avatar Feb 08 '17 06:02 karliss

The title issue has been solved: User and Participation passwords can now be stored hashed. I'm changing the title to reflect the second feature request that emerged from the discussion.

lw avatar Apr 24 '18 06:04 lw