From a951467b5d16cbc81ef90b954651b95c91c94c3d Mon Sep 17 00:00:00 2001 From: Jeremy Archer Date: Fri, 22 Jan 2016 19:27:15 -0600 Subject: [PATCH] Allow edit and mark as sold for unauthenticated users. --- caravel/controllers/custom_fields.py | 24 ++--- caravel/controllers/listings.py | 26 ++++-- caravel/storage/config.py | 12 ++- caravel/templates/listings/fullpage.html | 41 ++++++++- caravel/tests/helper.py | 7 +- caravel/tests/test_listings.py | 49 +++++++++- .../test_listings_new_listing_expect.txt | 3 + .../test_listings_post_inquiry_expect.txt | 3 + ...stings_post_inquiry_with_cnetid_expect.txt | 4 +- .../test_listings_publish_listing_expect.txt | 92 ++++++++++--------- caravel/utils/principals.py | 18 ++++ 11 files changed, 204 insertions(+), 75 deletions(-) diff --git a/caravel/controllers/custom_fields.py b/caravel/controllers/custom_fields.py index 6c75436..3770787 100644 --- a/caravel/controllers/custom_fields.py +++ b/caravel/controllers/custom_fields.py @@ -12,7 +12,9 @@ from caravel import model, utils + class OnlyValid(object): + """Validates that the credential is correct.""" def __call__(self, form, field): @@ -21,7 +23,9 @@ def __call__(self, form, field): if not field.data or not field.data.valid: raise ValidationError("Invalid credential.") + class MatchesPrincipal(threading.local): + """Ensures that only the existing Principal is used.""" def __init__(self): @@ -41,7 +45,9 @@ def __call__(self, form, field): else: raise ValidationError("Please use the same account for edits.") + class PrincipalField(wtforms.StringField): + """A PrincipalField is one that represents an email address.""" def __init__(self, label, **kwargs): @@ -58,15 +64,10 @@ def __init__(self, label, **kwargs): def process_formdata(self, values): """Overrides the value sent in the form if asked.""" - device = utils.Device.from_request(request) - user = users.get_current_user() - if user: - self.data = utils.Principal(user.email(), device, "GOOGLE_APPS") - elif ((not self.requires_validation) and values and - re.match(r'^[^@]+@[^@]+$', values[0])): - self.data = utils.Principal(values[0], device, "EMAIL") - else: - self.data = None + email = None + if not self.requires_validation and values: + email = values[0] + self.data = utils.Principal.from_request(request, email) def _value(self): """Returns the user for this user.""" @@ -110,6 +111,7 @@ def __call__(self, **kwargs): class PhotoField(wtforms.StringField): + """A PhotoField represents a photo object stored in Cloud Storage.""" def process_formdata(self, values): @@ -141,7 +143,7 @@ def widget(self, field, **kwargs): return Markup("""
{} -
+
Remove
@@ -152,4 +154,4 @@ def widget(self, field, **kwargs): ) else: kwargs["type"] = "file" - return super(PhotoField, self).widget(field, **kwargs) \ No newline at end of file + return super(PhotoField, self).widget(field, **kwargs) diff --git a/caravel/controllers/listings.py b/caravel/controllers/listings.py index 78ac992..d45052f 100644 --- a/caravel/controllers/listings.py +++ b/caravel/controllers/listings.py @@ -17,6 +17,7 @@ import datetime import math import itertools +from google.appengine.ext import ndb TOR_DETECTOR = utils.TorDetector() @@ -182,17 +183,30 @@ def publish_listing(listing): Edits a listing. """ - if is_from_tor() or not users.get_current_user(): + if is_from_tor(): abort(403) - if listing.principal.email != users.get_current_user().email(): + requester = utils.Principal.from_request(request, + email=listing.principal.email) + + if not requester.can_act_as(listing.principal): abort(403) - listing = model.UnapprovedListing(id=listing.key.id(), **listing.__dict__) - listing.sold = (request.form.get("sold") == "true") - listing.put() + fields = listing.to_dict() + + new_listing = model.UnapprovedListing(id=listing.key.id()) + for key, value in listing.to_dict().items(): + try: + setattr(new_listing, key, value) + except ndb.ComputedPropertyError: + pass + + new_listing.sold = (request.form.get("sold") == "true") + new_listing.principal = requester + + new_listing.put() - if not isinstance(listing, model.Listing): + if not isinstance(new_listing, model.Listing): flash("Your edit is awaiting moderation. " "We'll email you when it is approved.") return redirect(url_for("show_listing", listing=listing)) diff --git a/caravel/storage/config.py b/caravel/storage/config.py index 5e684de..ac628d3 100644 --- a/caravel/storage/config.py +++ b/caravel/storage/config.py @@ -21,15 +21,21 @@ def lookup(key, default): """Look up the given key, or return the default.""" return Parameter.get_or_insert(key_name=key, value=default).value + def get_tor_addresses(): return [] send_grid_client = sendgrid.SendGridClient(lookup("sendgrid_client", "")) -app.secret_key = lookup("session_secret", base64.b64encode(os.urandom(32))) +app.secret_key = lookup( + "session_secret", + base64.b64encode( + os.urandom(32))) or os.urandom(32) slack_url = lookup("slack_url", "") -app.config["RECAPTCHA_PUBLIC_KEY"] = lookup("recaptcha_public_key", +app.config["RECAPTCHA_PUBLIC_KEY"] = lookup( + "recaptcha_public_key", "6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI") -app.config["RECAPTCHA_PRIVATE_KEY"] = lookup("recaptcha_private_key", +app.config["RECAPTCHA_PRIVATE_KEY"] = lookup( + "recaptcha_private_key", "6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe") app.config["RECAPTCHA_DATA_ATTRS"] = {"size": "compact"} diff --git a/caravel/templates/listings/fullpage.html b/caravel/templates/listings/fullpage.html index 1c0dc12..5610ebb 100644 --- a/caravel/templates/listings/fullpage.html +++ b/caravel/templates/listings/fullpage.html @@ -50,11 +50,11 @@

{{ listing.title }}

{% if not is_from_tor() %}
- {% if current_user and listing.principal.email == current_user.email() or is_admin %} + + {% if current_user and listing.principal.email == current_user.email() %}
-

- Manage Listing

+

Manage Listing

@@ -71,7 +71,40 @@

{% endif %}

- + {% elif listing.principal.auth_method == "GOOGLE_APPS" and not current_user %} +
+
+

+ Manage Listing

+
+
+ + Sign in with CNetID + to edit. +
+
+ {% elif listing.principal.auth_method != "GOOGLE_APPS" %} +
+
+

+ Suggest Changes

+
+
+ + + Edit + {% if listing.sold %} + + {% else %} + + {% endif %} +
+
+ {% endif %} {% if not listing.sold %}
diff --git a/caravel/tests/helper.py b/caravel/tests/helper.py index 78bce7f..88ffb09 100644 --- a/caravel/tests/helper.py +++ b/caravel/tests/helper.py @@ -125,12 +125,15 @@ def setUp(self): id="listing_a") self.listing_a.put() + seller_b = utils.Principal("seller-b@uchicago.edu", device, + utils.Principal.EMAIL) + seller_b.validated_by = "fixture for unit test" + self.listing_b = model.Listing( title=u"Listing \u2606B", body=u"Body of \u2606B", posted_at=datetime.datetime.now() - datetime.timedelta(hours=24), - principal=utils.Principal("seller-b@uchicago.edu", device, - utils.Principal.GOOGLE_APPS), + principal=seller_b, price=71.10, run_trigger=True, version=11, diff --git a/caravel/tests/test_listings.py b/caravel/tests/test_listings.py index 9b57354..30aabff 100644 --- a/caravel/tests/test_listings.py +++ b/caravel/tests/test_listings.py @@ -62,7 +62,7 @@ def test_post_inquiry(self): self.assertLongString(self.emails[0].text) self.emails.pop(0) - # Approve the first listing. + # Approve the first inquiry. with self.google_apps_user("admin@uchicago.edu"): print self.post("/moderation", data={ "csrf_token": self.ajax_csrf_token("/moderation"), @@ -238,7 +238,52 @@ def test_edit_listing(self): self.assertLongString(self.get("/listing_a").data) def test_publish_listing(self): - with self.google_apps_user("seller-a@uchicago.edu"): + self.post("/listing_b/publish", data=dict( + csrf_token=self.csrf_token("/listing_b"), + sold="true" + )) + + # Listing is not yet sold. + self.assertLongString(self.get("/listing_b").data) + + # Approve the edit. + with self.google_apps_user("admin@uchicago.edu"): + self.post("/moderation", data={ + "csrf_token": self.ajax_csrf_token("/moderation"), + "approve": model.UnapprovedListing.query().get().key.urlsafe(), + }).data + + # The listing is sold now! + self.assertLongString(self.get("/listing_b").data) + + # Listing is no longer visible in searches. + self.assertLongString(self.get("/").data) + self.assertLongString(self.get("/?q=listing").data) + + # Try unmarking a listing as sold. + self.post("/listing_b/publish", data=dict( + csrf_token=self.csrf_token("/listing_b/edit"), + sold="false" + )) + + # Listing is still sold. + self.assertLongString(self.get("/listing_b").data) + + # Approve the edit. + with self.google_apps_user("admin@uchicago.edu"): + self.post("/moderation", data={ + "csrf_token": self.ajax_csrf_token("/moderation"), + "approve": model.UnapprovedListing.query().get().key.urlsafe(), + }).data + + # Listing is no longer sold. + self.assertLongString(self.get("/listing_b").data) + + # Listing is visible again in searches. + self.assertLongString(self.get("/").data) + + def test_publish_listing_with_cnetid(self): + with self.google_apps_user("seller-b@uchicago.edu"): # Try marking a listing as sold. self.post("/listing_a/publish", data=dict( csrf_token=self.csrf_token("/listing_a"), diff --git a/caravel/tests/test_listings_new_listing_expect.txt b/caravel/tests/test_listings_new_listing_expect.txt index f014d8a..4e58723 100644 --- a/caravel/tests/test_listings_new_listing_expect.txt +++ b/caravel/tests/test_listings_new_listing_expect.txt @@ -19,6 +19,9 @@ apartments Posted now . Price: $3.44 Body of ☆D +Suggest Changes +Edit +Mark as Sold Contact Seller From Sign in with CNetID or diff --git a/caravel/tests/test_listings_post_inquiry_expect.txt b/caravel/tests/test_listings_post_inquiry_expect.txt index 7e56850..8cf1ca5 100644 --- a/caravel/tests/test_listings_post_inquiry_expect.txt +++ b/caravel/tests/test_listings_post_inquiry_expect.txt @@ -5,6 +5,9 @@ apartments Posted 2d ago . Price: $71.10 Body of ☆B +Suggest Changes +Edit +Mark as Sold Contact Seller From Sign in with CNetID or diff --git a/caravel/tests/test_listings_post_inquiry_with_cnetid_expect.txt b/caravel/tests/test_listings_post_inquiry_with_cnetid_expect.txt index 044fde3..dbc2faa 100644 --- a/caravel/tests/test_listings_post_inquiry_with_cnetid_expect.txt +++ b/caravel/tests/test_listings_post_inquiry_with_cnetid_expect.txt @@ -8,11 +8,11 @@ Listing ☆B apartments Posted 2d ago . Price: $71.10 -Validated by GOOGLE_APPS. Originally posted by +fixture for unit test. Originally posted by 1.2.3.4 with mozilla. Body of ☆B -Manage Listing +Suggest Changes Edit Mark as Sold Contact Seller diff --git a/caravel/tests/test_listings_publish_listing_expect.txt b/caravel/tests/test_listings_publish_listing_expect.txt index 712a73d..9a33462 100644 --- a/caravel/tests/test_listings_publish_listing_expect.txt +++ b/caravel/tests/test_listings_publish_listing_expect.txt @@ -1,68 +1,70 @@ New Listing -Logged in as -seller-a@uchicago.edu -My Listings -Logout -Listing ☆A -cars -Posted 5h ago . -Price: $3.10 +Your edit is awaiting moderation. We'll email you when it is approved. +Listing ☆B +apartments +Posted 2d ago . +Price: $71.10 +Body of ☆B +Suggest Changes +Edit +Mark as Sold +Contact Seller +From +Sign in with CNetID or +Message +--- +New Listing +Listing ☆B +apartments +Posted 2d ago . +Price: $71.10 This listing was sold, preventing it from showing up in searches and blocking future inquiries. -Validated by GOOGLE_APPS. Originally posted by -1.2.3.4 with -mozilla. -Body of ☆A -Manage Listing +Body of ☆B +Suggest Changes Edit Mark as Unsold --- New Listing -Logged in as -seller-a@uchicago.edu -My Listings -Logout -Listing ☆B -$71.10 -apartments -2d ago +Listing ☆A +$3.10 +cars +5h ago --- New Listing -Logged in as -seller-a@uchicago.edu -My Listings -Logout +Listing ☆A +$3.10 +cars +5h ago +--- +New Listing +Your edit is awaiting moderation. We'll email you when it is approved. Listing ☆B -$71.10 apartments -2d ago +Posted 2d ago . +Price: $71.10 +This listing was sold, preventing it from showing up in searches +and blocking future inquiries. +Body of ☆B +Suggest Changes +Edit +Mark as Unsold --- New Listing -Logged in as -seller-a@uchicago.edu -My Listings -Logout -Listing ☆A -cars -Posted 5h ago . -Price: $3.10 -Validated by GOOGLE_APPS. Originally posted by -1.2.3.4 with -mozilla. -Body of ☆A -Manage Listing +Listing ☆B +apartments +Posted 2d ago . +Price: $71.10 +Body of ☆B +Suggest Changes Edit Mark as Sold Contact Seller From -seller-a@uchicago.edu ( Logout ) +Sign in with CNetID or Message --- New Listing -Logged in as -seller-a@uchicago.edu -My Listings -Logout Listing ☆A $3.10 cars diff --git a/caravel/utils/principals.py b/caravel/utils/principals.py index b2faab9..78a85ad 100644 --- a/caravel/utils/principals.py +++ b/caravel/utils/principals.py @@ -1,5 +1,6 @@ import uuid import user_agents +import re class Device(object): @@ -28,6 +29,23 @@ class Principal(object): GOOGLE_APPS_DOMAIN = "uchicago.edu" + @classmethod + def from_request(klass, request, email=None): + from google.appengine.api import users + + if users.get_current_user(): + return Principal( + users.get_current_user().email(), + Device.from_request(request), + klass.GOOGLE_APPS + ) + elif email and re.match(r'^[^@]+@[^@]+$', email): + return Principal( + email, + Device.from_request(request), + klass.EMAIL + ) + def __init__(self, email, device, auth_method): if auth_method not in (self.GOOGLE_APPS, self.EMAIL, self.LEGACY): raise ValueError