UMBC-CMSC447-Spring2017-Team5 / college-JUMP

https://lassa.xen.prgmr.com/collegejump/
GNU General Public License v3.0
0 stars 0 forks source link

User can set themselves to be an administrator #82

Closed alexander-bauer closed 7 years ago

alexander-bauer commented 7 years ago

@ksilber1 @ichniow1 If you can, please write a test for this -- it is a security issue that I need to fix.

Mjacks3 commented 7 years ago

super classs admin form build

alexander-bauer commented 7 years ago

Malik Jackson (mjacks3@umbc.edu) So Alex, I don't see any documentation for hiding form fields on quick_form. Looks like quick_form is just for generating a form that you no modifications to make on. If you want, since this is just a single instance, I can use the flask-bootstrap wtform fields to a make a custom form that doesn't include the admin field (and disable admin field validation in views, of course.)

@Mjacks3 I'm putting my reply here, for the sake of documentation.

I think the approach we should take, and I don't know if this is a common pattern or not, is to write the minimally-allowed UserInfoForm as the collection of options a user can change for themselves, and then write a subclass UserInfoAdminForm which includes the additional options, including email change, semester change, and delete. Then, the pattern for serving these can be

@app.route(...)
def view_page(user_id):
    if current_user.admin:
        form = forms.UserInfoAdminForm()
    elif current_user.id == user.id:
        form = forms.UserInfoForm()
    else:
        return flask.abort(403)

    if form.validate_on_submit():
        # Same code, different forms. If UserInfoForm() is submitted with admin options,
        # they will fail to validate, or just not be picked up at all.
        # ...

    # ...
Mjacks3 commented 7 years ago

This should be fixed now, Reopen if there are any issues