Skip to content
Permalink
Browse files

use alternate token for login

  • Loading branch information...
vansika committed Jan 4, 2019
1 parent c436e6f commit 98f3a15523dc99897ae5b706d38fca70cfc18ca1
@@ -12,6 +12,7 @@ CREATE TABLE "user" (
);
ALTER TABLE "user" ADD CONSTRAINT user_musicbrainz_id_key UNIQUE (musicbrainz_id);
ALTER TABLE "user" ADD CONSTRAINT user_musicbrainz_row_id_key UNIQUE (musicbrainz_row_id);
ALTER TABLE "user" ADD COLUMN user_login_id VARCHAR;

CREATE TABLE spotify_auth (
user_id INTEGER NOT NULL, -- PK and FK to user.id
@@ -0,0 +1,6 @@
BEGIN;

-- Add column for alternative user id for login purpose to user table
ALTER TABLE "user" ADD COLUMN user_login_id VARCHAR;

COMMIT;
@@ -26,13 +26,14 @@ def create(musicbrainz_row_id, musicbrainz_id):
"""
with db.engine.connect() as connection:
result = connection.execute(sqlalchemy.text("""
INSERT INTO "user" (musicbrainz_id, musicbrainz_row_id, auth_token)
VALUES (:mb_id, :mb_row_id, :token)
INSERT INTO "user" (musicbrainz_id, musicbrainz_row_id, auth_token, user_login_id)
VALUES (:mb_id, :mb_row_id, :token, :login_id)
RETURNING id
"""), {
"mb_id": musicbrainz_id,
"token": str(uuid.uuid4()),
"mb_row_id": musicbrainz_row_id,
"login_id" : str(uuid.uuid4()),
})
return result.fetchone()["id"]

@@ -58,7 +59,7 @@ def update_token(id):
raise


USER_GET_COLUMNS = ['id', 'created', 'musicbrainz_id', 'auth_token', 'last_login', 'latest_import', 'gdpr_agreed', 'musicbrainz_row_id']
USER_GET_COLUMNS = ['id', 'created', 'musicbrainz_id', 'auth_token', 'last_login', 'latest_import', 'gdpr_agreed', 'musicbrainz_row_id', 'user_login_id']


def get(id):
@@ -85,6 +86,30 @@ def get(id):
row = result.fetchone()
return dict(row) if row else None

def get_by_user_login_id(login_id):
"""Get user with a specified login ID.
Args:
id (int): login ID of a user.
Returns:
Dictionary with the following structure:
{
"id": <user id>,
"created": <account creation time>,
"musicbrainz_id": <MusicBrainz username>,
"auth_token": <authentication token>,
}
"""
with db.engine.connect() as connection:
result = connection.execute(sqlalchemy.text("""
SELECT {columns}
FROM "user"
WHERE user_login_id = :user_login_id
""".format(columns=','.join(USER_GET_COLUMNS))), {"user_login_id": login_id})
row = result.fetchone()
return dict(row) if row else None


def get_by_mb_id(musicbrainz_id):
"""Get user with a specified MusicBrainz ID.
@@ -14,6 +14,7 @@ class User(db.Model):
latest_import = db.Column(db.DateTime(timezone=True), default=lambda: datetime.fromutctimestamp(0))
gdpr_agreed = db.Column(db.DateTime(timezone=True))
musicbrainz_row_id = db.Column(db.Integer, nullable=False)
user_login_id = db.Column(db.String)

class UserAdminView(AdminModelView):
form_columns = [
@@ -29,6 +30,7 @@ class UserAdminView(AdminModelView):
'gdpr_agreed',
'last_login',
'latest_import',
'user_login_id',
]
column_searchable_list = [
'id',
@@ -28,7 +28,7 @@ def test_export(self):
Test for the user export of ListenBrainz data.
"""
# test get requests to export view first
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
resp = self.client.get(url_for('profile.export_data'))
self.assert200(resp)

@@ -21,7 +21,7 @@ def test_admin_views_when_not_logged_in(self):

def test_admin_views_when_authorized_logged_in(self):
self.app.config['ADMINS'] = [self.authorized_user['musicbrainz_id']]
self.temporary_login(self.authorized_user['id'])
self.temporary_login(self.authorized_user['user_login_id'])
# flask-admin seems to do a few redirects before going to the actual
# final web page, so we have to follow redirects
r = self.client.get('/admin', follow_redirects=True)
@@ -30,7 +30,7 @@ def test_admin_views_when_authorized_logged_in(self):

def test_admin_views_when_unauthorized_logged_in(self):
self.app.config['ADMINS'] = [self.authorized_user['musicbrainz_id']]
self.temporary_login(self.unauthorized_user['id'])
self.temporary_login(self.unauthorized_user['user_login_id'])
r = self.client.get('/admin', follow_redirects=True)
self.assert200(r)
self.assertNotIn('BDFL Zone', r.data.decode('utf-8'))
@@ -8,12 +8,16 @@


class User(UserMixin):
def __init__(self, id, created, musicbrainz_id, auth_token, gdpr_agreed):
def __init__(self, id, created, musicbrainz_id, auth_token, gdpr_agreed, user_login_id):
self.id = id
self.created = created
self.musicbrainz_id = musicbrainz_id
self.auth_token = auth_token
self.gdpr_agreed = gdpr_agreed
self.user_login_id = user_login_id

def get_id(self):
return self.user_login_id

@classmethod
def from_dbrow(cls, user):
@@ -23,11 +27,12 @@ def from_dbrow(cls, user):
musicbrainz_id=user['musicbrainz_id'],
auth_token=user['auth_token'],
gdpr_agreed=user['gdpr_agreed'],
user_login_id=user['user_login_id'],
)

@login_manager.user_loader
def load_user(user_id):
user = db_user.get(user_id)
def load_user(user_login_id):
user = db_user.get_by_user_login_id(user_login_id)
if user:
return User.from_dbrow(user)
else:
@@ -14,9 +14,9 @@ def create_app(self):
app.config['TESTING'] = True
return app

def temporary_login(self, user_id):
def temporary_login(self, user_login_id):
with self.client.session_transaction() as session:
session['user_id'] = user_id
session['user_id'] = user_login_id
session['_fresh'] = True


@@ -1,10 +1,11 @@

from flask import Blueprint, request, redirect, render_template, url_for, session
from flask import Blueprint, request, redirect, render_template, url_for, session, current_app
from flask_login import login_user, logout_user, login_required
from listenbrainz.webserver.login import login_forbidden, provider
from listenbrainz.webserver import flash
import listenbrainz.db.user as db_user
import time
import datetime

login_bp = Blueprint('login', __name__)

@@ -29,7 +30,7 @@ def musicbrainz_post():
if provider.validate_post_login():
user = provider.get_user()
db_user.update_last_login(user.musicbrainz_id)
login_user(user)
login_user(user, remember=True, duration=datetime.timedelta(current_app.config['SESSION_REMEMBER_ME_DURATION']))
next = session.get('next')
if next:
return redirect(next)
@@ -102,26 +102,27 @@ def test_menu_not_logged_in(self, mock_user_get):
self.assertNotIn('My Listens', data)
mock_user_get.assert_not_called()

@mock.patch('listenbrainz.db.user.get')
@mock.patch('listenbrainz.db.user.get_by_user_login_id')
def test_menu_logged_in(self, mock_user_get):
""" If the user is logged in, check that we perform a database query to get user data """
user = db_user.get_or_create(1, 'iliekcomputers')
db_user.agree_to_gdpr(user['musicbrainz_id'])
user = db_user.get_or_create(1, 'iliekcomputers')

mock_user_get.return_value = user
self.temporary_login(user['id'])
self.temporary_login(user['user_login_id'])
resp = self.client.get(url_for('index.index'))
data = resp.data.decode('utf-8')

# username (menu header)
self.assertIn('iliekcomputers', data)
self.assertIn('Import', data)
# item in user menu

self.assertIn('My Listens', data)
mock_user_get.assert_called_with(user['id'])

@mock.patch('listenbrainz.db.user.get')
@mock.patch('listenbrainz.db.user.get_by_user_login_id')
def test_menu_logged_in_error_show(self, mock_user_get):
""" If the user is logged in, if we show a 400 or 404 error, show the user menu"""
@self.app.route('/page_that_returns_400')
@@ -136,7 +137,7 @@ def view404():
db_user.agree_to_gdpr(user['musicbrainz_id'])
user = db_user.get_or_create(1, 'iliekcomputers')
mock_user_get.return_value = user
self.temporary_login(user['id'])
self.temporary_login(user['user_login_id'])
resp = self.client.get('/page_that_returns_400')
data = resp.data.decode('utf-8')
self.assert400(resp)
@@ -145,6 +146,7 @@ def view404():
self.assertIn('iliekcomputers', data)
self.assertIn('Import', data)
# item in user menu

self.assertIn('My Listens', data)
mock_user_get.assert_called_with(user['id'])

@@ -155,6 +157,7 @@ def view404():
self.assertIn('iliekcomputers', data)
self.assertIn('Import', data)
# item in user menu

self.assertIn('My Listens', data)
mock_user_get.assert_called_with(user['id'])

@@ -170,15 +173,15 @@ def view500():
db_user.agree_to_gdpr(user['musicbrainz_id'])
user = db_user.get_or_create(1, 'iliekcomputers')
mock_user_get.return_value = user
self.temporary_login(user['id'])
self.temporary_login(user['user_login_id'])
resp = self.client.get('/page_that_returns_500')
data = resp.data.decode('utf-8')
# item not in user menu
self.assertNotIn('My Listens', data)
self.assertNotIn('Sign in', data)
self.assertIn('Import', data)

@mock.patch('listenbrainz.db.user.get')
@mock.patch('listenbrainz.db.user.get_by_user_login_id')
def test_menu_logged_in_error_dont_show_user_loaded(self, mock_user_get):
""" If the user is logged in, if we show a 500 error, do not show the user menu
If the user has previously been loaded in the view, check that it's not
@@ -194,10 +197,10 @@ def test_menu_logged_in_error_dont_show_user_loaded(self, mock_user_get):
@login_required
def view500():
# flask-login user is loaded during @login_required, so check that the db has been queried
mock_user_get.assert_called_with(user['id'])
mock_user_get.assert_called_with(user['user_login_id'])
raise InternalServerError('error')

self.temporary_login(user['id'])
self.temporary_login(user['user_login_id'])
resp = self.client.get('/page_that_returns_500')
data = resp.data.decode('utf-8')
self.assertIn('Import', data)
@@ -22,21 +22,21 @@ def tearDown(self):
DatabaseTestCase.tearDown(self)

def test_reset_import_timestamp_get(self):
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
response = self.client.get(url_for('profile.reset_latest_import_timestamp'))
self.assertTemplateUsed('profile/resetlatestimportts.html')
self.assert200(response)

def test_profile_view(self):
"""Tests the user info view and makes sure auth token is present there"""
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
response = self.client.get(url_for('profile.info', user_name=self.user['musicbrainz_id']))
self.assertTemplateUsed('profile/info.html')
self.assert200(response)
self.assertIn(self.user['auth_token'], response.data.decode('utf-8'))

def test_reset_import_timestamp(self):
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
val = int(time.time())
db_user.update_latest_import(self.user['musicbrainz_id'], val)

@@ -53,7 +53,7 @@ def test_reset_import_timestamp(self):
self.assertEqual(int(ts), 0)

def test_reset_import_timestamp_post(self):
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
val = int(time.time())
db_user.update_latest_import(self.user['musicbrainz_id'], val)

@@ -78,7 +78,7 @@ def test_user_info_not_logged_in(self):

@patch('listenbrainz.webserver.views.user.publish_data_to_queue')
def test_delete(self, mock_publish_data_to_queue):
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
r = self.client.get(url_for('profile.delete'))
self.assert200(r)

@@ -93,7 +93,7 @@ def test_delete(self, mock_publish_data_to_queue):
@patch('listenbrainz.webserver.views.profile.spotify.get_spotify_oauth')
def test_connect_spotify(self, mock_get_spotify_oauth, mock_remove_user):
mock_get_spotify_oauth.return_value.get_authorize_url.return_value = 'someurl'
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
r = self.client.get(url_for('profile.connect_spotify'))
self.assert200(r)

@@ -110,7 +110,7 @@ def test_spotify_callback(self, mock_add_new_user, mock_get_access_token):
'refresh_token': 'refresh',
'expires_in': 3600,
}
self.temporary_login(self.user['id'])
self.temporary_login(self.user['user_login_id'])
r = self.client.get(url_for('profile.connect_spotify_callback', code='code'))
self.assertStatus(r, 302)
mock_get_access_token.assert_called_once_with('code')

0 comments on commit 98f3a15

Please sign in to comment.
You can’t perform that action at this time.