From 4ae1ea3bb07a62dd966a94dbb9b6a3370f0d2246 Mon Sep 17 00:00:00 2001 From: Bryan Date: Thu, 12 Mar 2015 02:31:15 -0400 Subject: [PATCH] major security overhaul for #420 and #423 cleaned a lot of code, centralized security definitions into models, moved security deployment from templates to views, enhanced template consistency for buttons, improved ajax handling for tags, removed redundant and divergent tag operations. --- karmaworld/apps/notes/models.py | 85 ++++++++++++++++++--- karmaworld/apps/notes/templatetags/notes.py | 14 ++++ karmaworld/apps/notes/views.py | 72 ++++++++++++----- karmaworld/apps/users/models.py | 10 +-- karmaworld/assets/js/note-detail.js | 5 ++ karmaworld/templates/notes/note_base.html | 55 +++++++------ 6 files changed, 181 insertions(+), 60 deletions(-) diff --git a/karmaworld/apps/notes/models.py b/karmaworld/apps/notes/models.py index 9bb0a1c..fe91c3b 100644 --- a/karmaworld/apps/notes/models.py +++ b/karmaworld/apps/notes/models.py @@ -6,10 +6,13 @@ Models for the notes django app. Contains only the minimum for handling files and their representation """ +import os +import time +import urllib +import logging import datetime import traceback -import logging -from allauth.account.signals import user_logged_in + from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.core.urlresolvers import reverse @@ -19,25 +22,28 @@ from django.core.files.storage import default_storage from django.db.models import SET_NULL from django.db.models.signals import post_save, post_delete, pre_save from django.dispatch import receiver -from karmaworld.apps.users.models import NoteKarmaEvent, GenericKarmaEvent -from karmaworld.utils.filepicker import encode_fp_policy, sign_fp_policy -import os -import time -import urllib - from django.conf import settings from django.core.files import File from django.core.files.storage import FileSystemStorage from django.db import models from django.utils.text import slugify + +import markdown import django_filepicker from taggit.managers import TaggableManager -import markdown +from allauth.account.signals import user_logged_in from karmaworld.apps.notes import sanitizer +from karmaworld.apps.users.models import UserProfile +from karmaworld.apps.users.models import NoteKarmaEvent +from karmaworld.apps.users.models import GenericKarmaEvent +from karmaworld.apps.notes.search import SearchIndex from karmaworld.apps.courses.models import Course from karmaworld.apps.licenses.models import License -from karmaworld.apps.notes.search import SearchIndex + +from karmaworld.utils.filepicker import sign_fp_policy +from karmaworld.utils.filepicker import encode_fp_policy + from karmaworld.settings.manual_unique_together import auto_add_check_unique_together FILEPICKER_API_KEY = os.environ['FILEPICKER_API_KEY'] @@ -366,6 +372,65 @@ class Note(Document): return self.category in Note.EDITABLE_CATEGORIES + def allows_edit_by(self, user, cache={}): + """ + Security policy for editing notes. Hand rolled for now. + cache is a dictionary to return some calculated info pass-by-ref. + (does nothing, maintained for consistency) + Returns True if the user can edit the note. + Checks if the user can tag and the note is editable. + """ + # Of course the note must be editable in the first place. + # If so, user must be able to tag note to edit it. + return self.is_editable() and self.allows_tags_by(user) + + def allows_tags_by(self, user, cache={}): + """ + Security policy for tagging notes. Hand rolled for now. + cache is a dictionary to return some calculated info pass-by-ref. + Returns True if the user can edit the note. + Checks if the user can delete and there are enough points to tag. + """ + # If the user cannot delete the note, the user cannot add tags. + if not self.allows_delete_by(user, cache): + return False + # Staff can edit, no worries about points. + if cache.has_key('staff') and cache['staff']: + return True + # Users must have 20 points to edit tags. + if cache.has_key('profile') and not cache.has_key('points'): + cache['points'] = cache['profile'].get_points() + return cache.has_key('points') and cache['points'] >= 20 + + def allows_delete_by(self, user, cache={}): + """ + Security policy for deleting notes. Hand rolled for now. + cache is a dictionary to return some calculated info pass-by-ref + Returns True if the user can delete the note. + Checks if user is admin or owns the note. + """ + # do not allow unauthenticated users access + if hasattr(user, 'is_authenticated') and not user.is_authenticated(): + return False + cache['user'] = user + # ensure user is a UserProfile by contract. + if not hasattr(user, 'has_staff_status'): + # assume it's a regular User and try to resolve it + user = UserProfile.objects.get(user_id=user.id) + cache['profile'] = user + # if the user is staff, the user can delete. + cache['staff'] = False + if user.has_staff_status(): + cache['staff'] = True + return True + # apparently some notes are un-owned. so, user is not the owner. + if self.user is None: + return False + # pull user id for this note + owner_id = self.user.id + # Note owner may edit, others may not. + return owner_id == user.get_id() + class NoteMarkdown(models.Model): note = models.OneToOneField(Note, primary_key=True) markdown = models.TextField(blank=True, null=True) diff --git a/karmaworld/apps/notes/templatetags/notes.py b/karmaworld/apps/notes/templatetags/notes.py index cba5016..6d376cd 100644 --- a/karmaworld/apps/notes/templatetags/notes.py +++ b/karmaworld/apps/notes/templatetags/notes.py @@ -22,3 +22,17 @@ def ordinal_letter(value): def keyvalue(dict, key): return dict[key] + +@register.filter() +def can_edit(user,note): + return note.allows_edit_by(user) + + +@register.filter() +def can_tag(user,note): + return note.allows_tags_by(user) + + +@register.filter() +def can_del(user,note): + return note.allows_delete_by(user) diff --git a/karmaworld/apps/notes/views.py b/karmaworld/apps/notes/views.py index 0e38024..4efd0de 100644 --- a/karmaworld/apps/notes/views.py +++ b/karmaworld/apps/notes/views.py @@ -2,31 +2,42 @@ # -*- coding:utf8 -*- # Copyright (C) 2012 FinalsClub Foundation -import traceback +import json import logging +import traceback + from django.contrib import messages from django.core import serializers from django.core.exceptions import ValidationError from django.forms.formsets import formset_factory -from karmaworld.apps.courses.forms import CourseForm -from karmaworld.apps.courses.models import Course -from karmaworld.apps.notes.search import SearchIndex -from karmaworld.apps.quizzes.create_quiz import quiz_from_keywords -from karmaworld.apps.quizzes.forms import KeywordForm -from karmaworld.apps.quizzes.models import Keyword -from karmaworld.apps.quizzes.tasks import submit_extract_keywords_hit, get_extract_keywords_results -from karmaworld.apps.users.models import NoteKarmaEvent -from karmaworld.utils.ajax_utils import * -from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden +from django.http import HttpResponse +from django.http import HttpResponseRedirect +from django.http import HttpResponseForbidden +from django.http import HttpResponseBadRequest from django.views.generic import DetailView, ListView, TemplateView from django.views.generic import UpdateView, FormView from django.views.generic import View from django.views.generic.detail import SingleObjectMixin -from karmaworld.apps.notes.models import Note, NoteMarkdown, KEYWORD_MTURK_THRESHOLD -from karmaworld.apps.notes.forms import NoteForm, NoteDeleteForm +from karmaworld.apps.notes.forms import NoteForm +from karmaworld.apps.notes.forms import NoteDeleteForm +from karmaworld.apps.notes.models import Note +from karmaworld.apps.notes.models import NoteMarkdown +from karmaworld.apps.notes.models import KEYWORD_MTURK_THRESHOLD +from karmaworld.apps.notes.search import SearchIndex +from karmaworld.apps.users.models import NoteKarmaEvent +from karmaworld.apps.courses.forms import CourseForm +from karmaworld.apps.quizzes.forms import KeywordForm +from karmaworld.apps.quizzes.tasks import submit_extract_keywords_hit +from karmaworld.apps.quizzes.tasks import get_extract_keywords_results +from karmaworld.apps.courses.models import Course +from karmaworld.apps.quizzes.models import Keyword +from karmaworld.apps.quizzes.create_quiz import quiz_from_keywords + +from karmaworld.utils.ajax_utils import ajax_pk_base +from karmaworld.utils.ajax_utils import ajax_increment logger = logging.getLogger(__name__) @@ -39,6 +50,10 @@ USER_PROFILE_FLAGS_FIELD = 'flagged_notes' def note_page_context_helper(note, request, context): if request.method == 'POST': + if not note.allows_edit_by(request.user): + # This user is Balrog. It. Shall. Not. Pass. + return HttpResponseForbidden() + # Only save tags if not forbidden above. context['note_edit_form'] = NoteForm(request.POST) else: tags_string = ','.join([str(tag) for tag in note.tags.all()]) @@ -76,6 +91,15 @@ class NoteView(UpdateView): def get_success_url(self): return self.object.get_absolute_url() + def form_valid(self, form): + self.note = self.object + # Ensure that the requesting user has permission to edit. + if self.note.allows_edit_by(self.request.user): + return super(NoteView, self).form_valid(form) + else: + messages.error(self.request, 'Permission denied.') + return HttpResponseRedirect(self.get_success_url()) + def get_context_data(self, **kwargs): context = super(NoteView, self).get_context_data(**kwargs) context['show_note_container'] = True @@ -108,9 +132,8 @@ class NoteDeleteView(FormView): def form_valid(self, form): self.note = Note.objects.get(id=form.cleaned_data['note']) - u = self.request.user # Ensure that the requesting user has permission to delete. - if (u.is_authenticated() and u.id == self.note.user_id) or u.is_staff: + if self.note.allows_delete_by(self.request.user): self.note.is_hidden = True self.note.save() messages.success(self.request, 'The note "{0}" was deleted successfully.'.format(self.note.name)) @@ -158,7 +181,10 @@ class NoteKeywordsView(FormView, SingleObjectMixin): kwargs['keywords'] = Keyword.objects.filter(note=self.get_object()) kwargs['show_keywords'] = True - note_page_context_helper(self.get_object(), self.request, kwargs) + ret = note_page_context_helper(self.get_object(), self.request, kwargs) + # check for errors returned by the helper. + if ret: + return ret return super(NoteKeywordsView, self).get_context_data(**kwargs) @@ -202,7 +228,10 @@ class NoteQuizView(TemplateView): def get_context_data(self, **kwargs): note = Note.objects.get(slug=self.kwargs['slug']) - note_page_context_helper(note, self.request, kwargs) + ret = note_page_context_helper(note, self.request, kwargs) + # check for errors returned by the helper. + if ret: + return ret kwargs['note'] = note kwargs['questions'] = quiz_from_keywords(note) @@ -328,8 +357,8 @@ def edit_note_tags(request, pk): """ Saves the posted string of tags """ - if request.method == "POST" and request.is_ajax() and request.user.is_authenticated() and request.user.get_profile().can_edit_items(): - note = Note.objects.get(pk=pk) + note = Note.objects.get(pk=pk) + if request.method == "POST" and request.is_ajax() and note.allows_tags_by(request.user): note.tags.set(request.body) note_json = serializers.serialize('json', [note,]) @@ -337,7 +366,10 @@ def edit_note_tags(request, pk): resp['fields']['tags'] = list(note.tags.names()) return HttpResponse(json.dumps(resp), mimetype="application/json") - else: + if request.method != "POST" or not request.is_ajax(): return HttpResponseBadRequest(json.dumps({'status': 'fail', 'message': 'Invalid request'}), mimetype="application/json") + else: + return HttpResponseForbidden(json.dumps({'status': 'fail', 'message': 'Not permitted'}), + mimetype="application/json") diff --git a/karmaworld/apps/users/models.py b/karmaworld/apps/users/models.py index 1caea61..c349175 100644 --- a/karmaworld/apps/users/models.py +++ b/karmaworld/apps/users/models.py @@ -40,11 +40,11 @@ class UserProfile(models.Model): return sum - def can_edit_items(self): - if self.user.is_staff: - return True - else: - return (self.get_points() >= 20) + def get_id(self): + return self.user.id + + def has_staff_status(self): + return self.user.is_staff NO_BADGE = 0 PROSPECT = 1 diff --git a/karmaworld/assets/js/note-detail.js b/karmaworld/assets/js/note-detail.js index 0ea09a2..5862621 100644 --- a/karmaworld/assets/js/note-detail.js +++ b/karmaworld/assets/js/note-detail.js @@ -123,6 +123,11 @@ function initNoteContentPage() { $('.tags').append($('', { class: 'tag-span', text: tag })); }); $('#note-tag-dialog').foundation('reveal', 'close'); + }, + error: function(data) { + $('#note_tags_form').slideUp(); + $('#note-tag-dialog').foundation('reveal', 'close'); + confirm('Could not add tags: ' + data.responseJSON.message); } }); }); diff --git a/karmaworld/templates/notes/note_base.html b/karmaworld/templates/notes/note_base.html index beab745..4622171 100644 --- a/karmaworld/templates/notes/note_base.html +++ b/karmaworld/templates/notes/note_base.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% load url from future %} {% load compress %} +{% load notes %} {% block title %} {{ note.name }} @@ -57,7 +58,7 @@ }); {% endif %} - {% if note.is_editable %} + {% if user|can_edit:note %}