From eb7f65275a07a350f94c5db2c8965c7d8c3cbf99 Mon Sep 17 00:00:00 2001 From: keanemind Date: Sun, 29 Jul 2018 14:47:19 -0500 Subject: [PATCH] =?UTF-8?q?Add=20unique=20URLs=20for=20registration=20?= =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BB=E2=80=8D=F0=9F=92=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #15 --- lostnphoned/schema.sql | 9 ++++- lostnphoned/sms.py | 81 ++++++++++++++++++++++++------------------ lostnphoned/sql.py | 51 ++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 35 deletions(-) diff --git a/lostnphoned/schema.sql b/lostnphoned/schema.sql index 996d2d4..005eea9 100644 --- a/lostnphoned/schema.sql +++ b/lostnphoned/schema.sql @@ -1,6 +1,7 @@ DROP TABLE IF EXISTS users; DROP TABLE IF EXISTS passwords; DROP TABLE IF EXISTS bannable_clients; +DROP TABLE IF EXISTS register_ids; CREATE TABLE users ( phone_number TEXT PRIMARY KEY, @@ -22,4 +23,10 @@ CREATE TABLE bannable_clients ( phone_number TEXT PRIMARY KEY, last_attempt TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, attempts INTEGER NOT NULL DEFAULT 1 -); \ No newline at end of file +); + +CREATE TABLE register_ids ( + uuid TEXT PRIMARY KEY, + phone_number TEXT UNIQUE NOT NULL, + created TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +) diff --git a/lostnphoned/sms.py b/lostnphoned/sms.py index 64a5467..98e2676 100644 --- a/lostnphoned/sms.py +++ b/lostnphoned/sms.py @@ -2,6 +2,7 @@ import os import difflib +import uuid import flask import phonenumbers import schedule @@ -58,6 +59,15 @@ def message_received(): if sql.existing_user(phone_number_e164, connection): message = "This phone number has already been registered." else: + # Remove old ID from database if it's there (user wants a new URL) + sql.remove_register_id(phone_number_e164, connection) + + # Generate unique ID + clientid = generate_clientid() + + # Add ID to database + sql.add_register_id(clientid, phone_number_e164, connection) + message = ( "Welcome to Lost-n-Phoned! " "To create your account, please click the link and " @@ -65,7 +75,7 @@ def message_received(): "After authorizing with Google, you will receive " "additional instructions on how to add a password. {}" .format( - flask.url_for('authorize', phone=phone_number_e164, _external=True) + flask.url_for('authorize', clientid=clientid, _external=True) ) ) else: @@ -117,10 +127,39 @@ def message_received(): return str(resp) +def generate_clientid() -> str: + """Create a unique ID for the client using UUID.""" + base16 = uuid.uuid4() + return int_to_base58(base16.int) # pylint: disable=E1101 + + +def int_to_base58(num: int) -> str: + """Convert an int to a base58 string.""" + alphabet = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" + converted = "" + while num: + num, digit = divmod(num, 58) + converted += alphabet[digit] + return converted[::-1] + + @app.route('/authorize') def authorize(): """Authorization link.""" + connection = sql.connect() + clientid = flask.request.args.get('clientid', type=str) + if not clientid or not sql.get_register_number(clientid, connection): + return "Error: Invalid link or link expired." + + # Get phone number corresponding to the clientid from the database + phone_number_e164 = sql.get_register_number(clientid, connection) + + # Remove the link immediately to prevent the user from clicking the + # link multiple times and potentially getting through OAuth multiple + # times. + sql.remove_register_id(phone_number_e164, connection) + # Create flow instance to manage the OAuth 2.0 Authorization Grant Flow steps. flow = google_auth_oauthlib.flow.Flow.from_client_secrets_file( CLIENT_SECRETS_FILE, scopes=SCOPES) @@ -145,30 +184,7 @@ def authorize(): flask.session['state'] = state # Store the user's phone number (from the request parameter) for the callback to use. - - phone_number = flask.request.args.get('phone', type=str) - try: - phone_number_obj = phonenumbers.parse(phone_number) - # Region is None because the number should already be in E164 - except phonenumbers.NumberParseException: - return "Error: Invalid phone number." - - if not phonenumbers.is_valid_number(phone_number_obj): - return "Error: Invalid phone number." - - phone_number_e164 = phonenumbers.format_number( - phone_number_obj, - phonenumbers.PhoneNumberFormat.E164 - ) - - # Check if phone number is already registered, to prevent - # SQLite UNIQUE constraint error. This could happen if the user - # clicks the link multiple times. - connection = sql.connect() - if sql.existing_user(phone_number_e164, connection): - return "Error: You have already registered this phone number." - - flask.session['phone_number'] = flask.request.args.get('phone', type=str) + flask.session['phone_number'] = phone_number_e164 return flask.redirect(authorization_url) @@ -179,7 +195,7 @@ def oauth2callback(): # Specify the state when creating the flow in the callback so that it can # verified in the authorization server response. state = flask.session['state'] - phone_number = flask.session['phone_number'] # Guaranteed to be in E164 + phone_number_e164 = flask.session['phone_number'] # Guaranteed to be in E164 flow = google_auth_oauthlib.flow.Flow.from_client_secrets_file( CLIENT_SECRETS_FILE, scopes=SCOPES, state=state) @@ -190,21 +206,18 @@ def oauth2callback(): flow.fetch_token(authorization_response=authorization_response) credentials = flow.credentials - # In case the phone number is already registered, clear the existing - # database entry to avoid SQLite3 UNIQUE constraint error. - # If we chose to not store anything, then the user would soon have - # to reregister from the start because the refresh token in - # the database has been invalidated, and the entry would be deleted. + # I don't think it's possible for the user to already be registered, + # but just in case... connection = sql.connect() - sql.remove_user(phone_number, connection) # No error even if number - # isn't in database + sql.remove_user(phone_number_e164, connection) # No error if user doesn't exist # Store credentials in the database. - sql.add_user(phone_number, credentials, connection) + sql.add_user(phone_number_e164, credentials, connection) connection.close() return flask.redirect('/authorize-success') + def get_phone_number_obj(phone_number): """Returns phonenumbers.PhoneNumber object if possible, or None if the input could not possibly be a phone number.""" diff --git a/lostnphoned/sql.py b/lostnphoned/sql.py index eecc97e..4c9d2b8 100644 --- a/lostnphoned/sql.py +++ b/lostnphoned/sql.py @@ -15,6 +15,7 @@ def init_app(app): # Set up periodic SQL operation schedule.every().day.do(remove_clients) + schedule.every().hour.do(remove_register_ids) @click.command('init-db') @@ -244,3 +245,53 @@ def remove_clients(): "WHERE last_attempt < DATETIME('now', '-7 days')") cursor.execute(command) connection.commit() + + +def add_register_id(uuid: str, number: str, connection): + """Add a database entry for a uuid and phone number.""" + + cursor = connection.cursor() + command = ("INSERT INTO register_ids (uuid, phone_number) " + "VALUES (?, ?)") + cursor.execute(command, (uuid, number)) + connection.commit() + + +def remove_register_id(number: str, connection): + """Remove the uuid corresponding to the phone number + if it is in the database.""" + + cursor = connection.cursor() + command = ("DELETE FROM register_ids " + "WHERE phone_number = ?") + cursor.execute(command, (number,)) + connection.commit() + + +def get_register_number(uuid: str, connection): + """Get a phone number associated with a uuid. + Returns None if the uuid is not in the database + or is expired.""" + + cursor = connection.cursor() + command = ("SELECT phone_number FROM register_ids " + "WHERE uuid = ? AND created >= DATETIME('now', '-5 minutes')") + cursor.execute(command, (uuid,)) + + data = cursor.fetchone() + + if data: + return data[0] + + return None + + +def remove_register_ids(): + """Remove all expired uuids.""" + + connection = connect() + cursor = connection.cursor() + command = ("DELETE FROM register_ids " + "WHERE created < DATETIME('now', '-5 minutes')") + cursor.execute(command) + connection.commit()