Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Privacy features && social handles #242

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions app/api.py
Expand Up @@ -3,7 +3,7 @@
import time
from app import db, cas
from app.util import to_json, fail, succ, requires_login
from app.models import User, Person, Group
from app.models import User, Person, Group, PersonPersistent


api_bp = Blueprint('api', __name__)
Expand Down Expand Up @@ -50,7 +50,8 @@ def api_students():
if not criteria['filters'].get('school_code'):
criteria['filters']['school_code'] = []
criteria['filters']['school_code'].append('YC')
students = Person.search(criteria)
students = Person.search_respect_privacy_include_persistent(criteria)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function name is way too long


return to_json(students)


Expand All @@ -61,7 +62,8 @@ def api_people():
criteria = request.get_json(force=True) or {}
except:
criteria = {}
people = Person.search(criteria)
people = Person.search_respect_privacy_include_persistent(criteria)

return to_json(people)


Expand Down
46 changes: 45 additions & 1 deletion app/models.py
@@ -1,9 +1,11 @@
from app import app, db
from app.search import SearchableMixin
from app.util import get_now
from app.util import get_now, PERSISTENT_FIELDS, scrub_hidden_data
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I don't know how I feel about putting PERSISTENT_FIELDS under util... seems pretty specific to one file. Maybe make this a class variable under PersonPersistent?

import jwt
import datetime
from copy import copy
from sqlalchemy.sql import collate
from sqlalchemy.orm.session import make_transient


leaderships = db.Table(
Expand Down Expand Up @@ -199,7 +201,49 @@ def search(criteria):
people = person_query.all()
return people

def get_persistent_data(self):
return PersonPersistent.query.filter_by(person_id=self.id).first()

@staticmethod
def search_respect_privacy_include_persistent(criteria):
'''
Performs a search, including persistent data.
- Persistent data, including privacy and social information, is returned.
- If a person's privacy settings hide a certain field, it will be omitted from the results.
- The returned objects will be expunged, and changes to them will not be committed.
'''
people = Person.search(criteria)
people_copy = []
for person in people:
person_copy = copy(person)
make_transient(person_copy)

persistent_data = person.get_persistent_data()
if persistent_data is not None:
for field in PERSISTENT_FIELDS:
setattr(person_copy, field, getattr(persistent_data, field))

scrub_hidden_data(person_copy)
people_copy.append(person_copy)
return people_copy

class PersonPersistent(db.Model):
__tablename__ = 'person_persistent'
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
person_id = db.Column(db.Integer, db.ForeignKey('person.id'))

# socials
socials_instagram = db.Column(db.String)
socials_snapchat = db.Column(db.String)

# privacy
privacy_hide_image = db.Column(db.Boolean)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fundamentally disagree that we should keep track of what things people have marked as removed. It adds a ton of logic to our operations and risks giving people a totally false sense of security, since many people will stop with Yalies and not actually prevent their data from being accessed in any meaningful way. Instead, I think we should enable users to do a one-time removal of the field in question from our site when they do a removal (i.e. update the person table and remove that field), and aggressively warn that this serves only to accelerate the process of removal after removing one's data from the Face Book/directory. We really, really should not be putting ourselves in a position of needing to remove fields at request-time. I doubt there is any way to make that efficient.

privacy_hide_email = db.Column(db.Boolean)
privacy_hide_room = db.Column(db.Boolean)
privacy_hide_phone = db.Column(db.Boolean)
privacy_hide_address = db.Column(db.Boolean)
privacy_hide_major = db.Column(db.Boolean)
privacy_hide_birthday = db.Column(db.Boolean)
class Group(db.Model):
__tablename__ = 'group'
__searchable__ = (
Expand Down
71 changes: 56 additions & 15 deletions app/routes.py
@@ -1,7 +1,7 @@
from flask import render_template, make_response, request, redirect, url_for, jsonify, abort, g, session
from app import app, db, scraper, cas
from app.models import User, Person, Key
from app.util import requires_login, to_json, get_now, succ, fail
from app.models import User, Person, Key, PersonPersistent
from app.util import requires_login, forbidden_via_api, to_json, get_now, succ, fail, PERSISTENT_FIELDS, is_valid_instagram_username, is_valid_snapchat_username
from .cas_validate import validate
from sqlalchemy import distinct

Expand All @@ -25,12 +25,12 @@ def store_user():
token_cookie = request.cookies.get('token')
if token_cookie:
g.user = User.from_token(token_cookie)
method_used = 'cookie'
g.method_used = 'cookie'
authorization = request.headers.get('Authorization')
if g.user is None and authorization:
token = authorization.split(' ')[-1]
g.user = User.from_token(token)
method_used = 'header'
g.method_used = 'header'
if g.user is None and cas.username:
g.user = User.query.get(cas.username)
if not g.user:
Expand All @@ -41,14 +41,14 @@ def store_user():
registered_on=timestamp,
admin=is_first_user)
db.session.add(g.user)
method_used = 'CAS'
g.method_used = 'CAS'
if g.user:
g.person = Person.query.filter_by(netid=g.user.id, school_code='YC').first()
if g.user.banned or (not g.person and not g.user.admin):
# TODO: give a more graceful error than just a 403
abort(403)
try:
print(f'Authorized request by {g.person.first_name} {g.person.last_name} with {method_used} authentication.')
print(f'Authorized request by {g.person.first_name} {g.person.last_name} with {g.method_used} authentication.')
except AttributeError:
print('Could not render name.')
g.user.last_seen = timestamp
Expand Down Expand Up @@ -237,6 +237,7 @@ def hide_me():

@app.route('/keys', methods=['GET'])
@requires_login
@forbidden_via_api
def get_keys():
keys = Key.query.filter_by(user_id=g.user.id,
deleted=False).all()
Expand All @@ -245,24 +246,17 @@ def get_keys():

@app.route('/keys', methods=['POST'])
@requires_login
@forbidden_via_api
def create_key():
payload = request.get_json()
key = g.user.create_key(payload['description'])
db.session.add(key)
db.session.commit()
return to_json(key)


"""
@app.route('/keys/<key_id>', methods=['POST'])
@requires_login
def update_key(key_id):
pass
"""


@app.route('/keys/<key_id>', methods=['DELETE'])
@requires_login
@forbidden_via_api
def delete_key(key_id):
key = Key.query.get(key_id)
if key.user_id != g.user.id:
Expand All @@ -271,6 +265,53 @@ def delete_key(key_id):
db.session.commit()
return succ('Key deleted.')

@app.route('/edit', methods=['GET'])
@requires_login
def edit():
if g.person is None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we check this already

return fail('Could not find person in the database.', 403)
person_persistent = g.person.get_persistent_data()

if person_persistent is None:
return render_template('edit.html')
return render_template(
'edit.html',
socials_instagram=person_persistent.socials_instagram,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you can do **person_persistent.__dict__ instead of enumerating all these

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it won't really matter if you remove the privacy_hide_* fields, which I strongly believe you should do

socials_snapchat=person_persistent.socials_snapchat,
privacy_hide_image=person_persistent.privacy_hide_image,
privacy_hide_email=person_persistent.privacy_hide_email,
privacy_hide_room=person_persistent.privacy_hide_room,
privacy_hide_phone=person_persistent.privacy_hide_phone,
privacy_hide_address=person_persistent.privacy_hide_address,
privacy_hide_major=person_persistent.privacy_hide_major,
privacy_hide_birthday=person_persistent.privacy_hide_birthday
)

@app.route('/edit', methods=['POST'])
@requires_login
@forbidden_via_api
def edit_post():
payload = request.get_json()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already stored at g.json I think

socials_instagram = payload["socials_instagram"] if hasattr(payload, "socials_instagram") else None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: use single quotes in this function for consistency

socials_snapchat = payload["socials_snapchat"] if hasattr(payload, "socials_snapchat") else None
if (socials_instagram is not None) and (len(socials_instagram) != 0) and (not is_valid_instagram_username(payload["socials_instagram"])):
return fail('Invalid Instagram username.', 400)
if (socials_snapchat is not None) and (len(socials_snapchat) != 0) and (not is_valid_snapchat_username(payload["socials_snapchat"])):
return fail('Invalid Snapchat username.', 400)

if g.person is None:
return fail('Could not find person in the database.', 403)
person_persistent = g.person.get_persistent_data()
if person_persistent is None:
person_persistent = PersonPersistent(person_id=g.person.id)
db.session.add(person_persistent)

for key in PERSISTENT_FIELDS:
if key in payload:
setattr(person_persistent, key, payload[key])

db.session.commit()
return succ('Person edited.')

@app.route('/authorize', methods=['POST'])
def api_authorize_cas():
Expand Down
9 changes: 6 additions & 3 deletions app/scraper/__init__.py
@@ -1,5 +1,5 @@
from app import app, db, celery, elasticsearch
from app.models import leaderships, Group, Person
from app.models import leaderships, Group, Person, PersonPersistent
from app.mail import send_scraper_report

from app.scraper import sources
Expand Down Expand Up @@ -106,8 +106,11 @@ def scrape(caches_active, face_book_cookie, people_search_session_cookie, csrf_t
Group.query.delete()

Person.query.delete()
elasticsearch.indices.delete(index=Person.__tablename__)
elasticsearch.indices.create(index=Person.__tablename__)

if elasticsearch is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff thanks for fixing this

elasticsearch.indices.delete(index=Person.__tablename__)
elasticsearch.indices.create(index=Person.__tablename__)

num_inserted = 0
for person_dict in people:
if not person_dict.get('netid'):
Expand Down
21 changes: 21 additions & 0 deletions app/static/css/edit.css
@@ -0,0 +1,21 @@
.form-group {
display: flex;
flex-direction: row;
justify-content: flex-start;
align-items: center;
gap: 20px;
margin-bottom: 10px;
}

h3 {
margin-bottom: 20px;
font-size: 20px;
}

#error {
color: #FF0000;
}

#success {
color: #00FF00;
}
47 changes: 47 additions & 0 deletions app/static/js/edit.js
@@ -0,0 +1,47 @@
const FIELDS = [
'socials_instagram',
// 'socials_snapchat',
'privacy_hide_image',
'privacy_hide_email',
'privacy_hide_room',
'privacy_hide_phone',
'privacy_hide_address',
'privacy_hide_major',
'privacy_hide_birthday'
];

const submit = document.getElementById('submit');
const error = document.getElementById('error');
const success = document.getElementById('success');

submit.onclick = async (e) => {
error.innerText = '';
success.innerText = '';

const data = {};
for(const field of FIELDS) {
const elem = document.getElementById(field);
if(elem.type === 'checkbox') data[field] = elem.checked;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: put space in between if, for, etc. and the paren to match rest of the code

else if(elem.type === 'text') data[field] = elem.value;
}
let response;
try {
response = await fetch('/edit', {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(data)
})
} catch(e) {
console.error(e);
error.innerText = 'An unexpected error has occurred.';
}
if(response.status === 200) {
success.innerText = 'Your profile has been updated';
} else {
console.error(await response.text());
const json = await response.json();
error.innerText = json.message;
}
}
2 changes: 2 additions & 0 deletions app/static/js/index.js
Expand Up @@ -505,6 +505,8 @@ function loadNextPage() {
addPill(pills, "major", "Major", "book", person);
addPill(pills, "birthday", "Birthday", "birthday-cake", person);
addPill(pills, "address", "Address", "home", person);
addPill(pills, "socials_instagram", "Instagram", "instagram", person);
addPill(pills, "socials_snapchat", "Snapchat", "snapchat", person);

// Append the pills container to the person container
personContainer.appendChild(pills);
Expand Down
2 changes: 2 additions & 0 deletions app/templates/base.html
Expand Up @@ -21,6 +21,7 @@
<link rel="stylesheet" href="/static/css/controls.css">
<link rel="stylesheet" href="/static/css/list.css">
<link rel="stylesheet" href="/static/css/api.css">
<link rel="stylesheet" href="/static/css/edit.css">
<link rel="icon" href="/static/images/logo/favicon.png" type="image/png">
<link rel="icon" href="/static/images/logo/favicon.ico" type="image/x-icon">
<link rel="apple-touch-icon" href="/static/images/logo/apple-touch-icon-iphone-60x60.png">
Expand All @@ -39,6 +40,7 @@
<nav>
<a class="button outline" href="/">Home</a>
<a class="button outline" href="/about">About</a>
<a class="button outline" href="/edit">Edit Profile</a>
<a class="button outline" href="/faq">FAQ</a>
<a class="button outline" href="/apidocs">API</a>
{% if not unauthenticated %}
Expand Down