search

robogals / myrobogals

myRobogals is the global intranet and record-keeping tool for Robogals. It has been built to simplify many of our day-to-day tasks including organising school visits, maintaining a member database, communicating with members, storing records reliably for future generations and easily collecting statistics on a global scale.
https://my.robogals.org
10 stars 21 forks source link

Exec-only fields editable by all #68

Closed manhinli closed 11 years ago

manhinli commented 11 years ago

If the appropriately named fields from the "Exec-only fields" set in the user profile edit page are injected or somehow make their way into the POST request, then the information is edited regardless of the submitting user's status.

"Exec-only" fields are always overridden every time someone updates their user profile via the frontend, irregardless of their status.

This results in wiping of previously stored data in those fields, or potentially insertion of data which they should not have access to if the POST request is modified in a specific way.

More checks need to be made to disregard such information.

yfcheung commented 11 years ago

rgprofile/views.py:

                    if 'internal_notes' in data:
                        u.internal_notes = data['internal_notes']
                    if 'trained' in data:
                        u.trained = data['trained']
'internal_notes' in data # is always true, therefore original data would be nullified
'trained' in data # is always true, therefore original data would be nullified
manhinli commented 11 years ago

Wow. This bug is even larger than I first thought!

Yes, I can confirm that every time a user updates their profile via. the frontend, all the exec-only data is wiped (as the received data is NULL)

yfcheung commented 11 years ago

Done

yfcheung commented 11 years ago

current approach: if the request is improper, discard the whole request.

Alternative approach: if the request is improper, still sign up the new user and only stores legitimate fields, or still modify legitimate fields of existing user.

Which one do you prefer?

yfcheung commented 11 years ago

original feature: staff can add users to other chapter and initialise new user's exec fields.

after the adjustment made: staff can still add users to other chapter but can not initialise exec fields. (This may be an inconvenience)