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

First working API implementation #51

Closed
wants to merge 27 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
524feac
First working API implementation
jameswthorne Jan 12, 2017
bf93da4
Applied suggested fixes from jparise
jameswthorne Jan 13, 2017
7fc18ec
Deprecate support for py26 and py34
May 6, 2018
b8ce29e
Merge pull request #78 from pinterest/deprectate-py26-py34
nichochar May 6, 2018
331d421
Mock redis in tests using mockredis
May 6, 2018
2e0a296
Remove -s flag in tests
May 6, 2018
f59fd33
Remove 2.6 and 3.3 from .travis.yml
May 6, 2018
5a0beb1
Quote url to fix equal sign breaking outlook clients
May 6, 2018
9e7ca47
py2 and py3 support
May 6, 2018
13f294c
Use werkzeug quote/unquote functions instead of urllib
May 7, 2018
75b6a69
Make mock a requirement, not dev-requirement
May 7, 2018
68b4cec
Merge pull request #80 from pinterest/quoteurls
nichochar May 7, 2018
173f33f
Merge pull request #79 from pinterest/mock-redis
nichochar May 7, 2018
e45feb1
Bump version to 1.3.0
May 7, 2018
6fe4733
Merge pull request #81 from pinterest/bumpversion-1.3.0
nichochar May 7, 2018
699293b
Remove support for py26 and py33 from readme
May 7, 2018
548c998
Merge pull request #82 from pinterest/readme-update
nichochar May 7, 2018
e6eca0d
Use assertion methods introduced in Python 2.7
samueldg May 8, 2018
80f77a6
Fix assertEqual parameter order (expected, actual)
samueldg May 8, 2018
d407c26
Drop the dot in py.test (as recommended by pytest)
samueldg May 8, 2018
5ddecd4
Merge pull request #83 from samueldg/enhancement/modernize-tests
nichochar May 8, 2018
a2d4245
Add hiring plug in readme
May 12, 2018
a42815d
Merge pull request #84 from pinterest/shameless-hiring-plug
nichochar Jun 16, 2018
339dc0c
First working API implementation
jameswthorne Jan 12, 2017
23c3c81
Applied suggested fixes from jparise
jameswthorne Jan 13, 2017
7be6307
Rebased from master and incorporated suggested fixes
jameswthorne Jul 1, 2018
380af3c
Merge branch 'api' of https://github.com/jameswthorne/snappass into api
jameswthorne Jul 1, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 109 additions & 1 deletion snappass/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import redis
from redis.exceptions import ConnectionError

from flask import abort, Flask, render_template, request
from flask import abort, Flask, render_template, request, jsonify
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we retain the alphabetized order?



SNEAKY_USER_AGENTS = ('Slackbot', 'facebookexternalhit', 'Twitterbot', 'Facebot', 'WhatsApp')
Expand Down Expand Up @@ -90,12 +90,53 @@ def request_is_valid(request):
"""
return not SNEAKY_USER_AGENTS_RE.search(request.headers.get('User-Agent', ''))

def not_found_api(error=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The error parameter doesn't appear to be passed or used in any code paths.

message = {
'status': 404,
'message': 'Not Found: ' + request.url,
}
resp = jsonify(message)
resp.status_code = 404

return resp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Flask has a nice shortcut for this pattern:

return make_response(jsonify(message), 404)


def unsupported_media_type_api(error=None):
message = {
'status': 415,
'message': 'Unsupported Media Type',
}
resp = jsonify(message)
resp.status_code = 415

return resp

def bad_request_api(error=None):
message = {
'status': 400,
'message': 'Bad Request',
}
resp = jsonify(message)
resp.status_code = 400

return resp


@app.route('/', methods=['GET'])
def index():
return render_template('set_password.html')


@app.route('/api', methods=['GET'])
def index_api():
if NO_SSL:
base_url = request.url_root
else:
base_url = request.url_root.replace("http://", "https://")
Copy link
Collaborator

Choose a reason for hiding this comment

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

These four lines are repeated in multiple endpoint functions. Can we factor them into a common make_base_url() function?

Alternatively, we could just require flask-sslify, which would move this concern out of snappass.


return "Generate a password share link with the following command: \n\n" \
"curl -X POST -d \'{\"password\":\"password-here\",\"ttl\":\"week | day | hour\"}\' -H \"Content-Type:application/json\" " + base_url + "api\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: using single quotes to quote the string would allow to put unescaped double quotes in it. Makes it easier to read. 'curl -X POST -d \'{"password":"password-here","ttl":"week | day | hour"}\' -H ...'



@app.route('/', methods=['POST'])
def handle_password():
ttl, password = clean_input()
Expand All @@ -109,6 +150,52 @@ def handle_password():
return render_template('confirm.html', password_link=link)


@app.route('/api', methods=['POST'])
def handle_password_api():
if not request.headers['Content-Type'] == 'application/json':
return unsupported_media_type_api()

payload = request.get_json()

if 'password' in payload:
password = payload['password']

if not len(password) > 0:
return bad_request_api()
else:
return bad_request_api()
Copy link
Contributor

@coyotwill coyotwill Feb 9, 2018

Choose a reason for hiding this comment

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

Pretty complex logic here. How about something like :

password = payload.get('password', None)
if not password: #edit: not need to check len since empty strings are falsy   
    return bad_request_api()


if 'ttl' in payload:
time_period = payload['ttl'].lower()
if not time_period in time_conversion:
return bad_request_api()

ttl = time_conversion[time_period]
else:
# Set ttl to one week if not specified in the JSON
ttl = 604800
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, could do time_period = payload.get('ttl', 'week').lower()


key = set_password(password, ttl)

if NO_SSL:
base_url = request.url_root
else:
base_url = request.url_root.replace("http://", "https://")

link_web = base_url + key
link_api = base_url + "api/" + key

data = {
'web' : link_web,
'api' : link_api,
}

resp = jsonify(data)
resp.status_code = 200

return resp


@app.route('/<password_key>', methods=['GET'])
def show_password(password_key):
if not request_is_valid(request):
Expand All @@ -120,6 +207,27 @@ def show_password(password_key):
return render_template('password.html', password=password)


@app.route('/api/<password_key>', methods=['GET'])
def get_password_api(password_key):
password = get_password(password_key)
if not password:
return not_found_api()

if NO_SSL:
base_url = request.url_root
else:
base_url = request.url_root.replace("http://", "https://")

data = {
'password' : password
}

resp = jsonify(data)
resp.status_code = 200

return resp
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can just do:

return jsonify(data)

... because the response defaults to 200.



@check_redis_alive
def main():
app.run(host='0.0.0.0')
Expand Down