From 8b320ff978265fa05093a8b4ff473ae06466c219 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Thu, 1 Mar 2018 07:21:54 -0600 Subject: [PATCH] Address pull request comments * Reformatted javascript * Appeased pycodestyle * TODO Add Expiration and Note fields to tokens? --- .gitignore | 5 +--- client/html/user_tokens.tpl | 2 +- client/js/api.js | 12 ++++----- client/js/controllers/user_controller.js | 8 +++--- client/js/models/user_token.js | 26 +++++++++---------- client/js/views/user_tokens_view.js | 13 +++++----- server/szurubooru/api/user_token_api.py | 19 +++++++++----- server/szurubooru/func/user_tokens.py | 20 +++++++++----- server/szurubooru/func/users.py | 9 +++---- server/szurubooru/middleware/authenticator.py | 2 +- .../a39c7f98a7fa_add_user_token_table.py | 23 ++++++++-------- server/szurubooru/model/__init__.py | 4 +-- .../tests/api/test_user_token_creating.py | 9 +++---- .../tests/api/test_user_token_deleting.py | 15 ++++++----- .../tests/api/test_user_token_retrieving.py | 14 +++++----- .../tests/api/test_user_token_updating.py | 19 +++++++------- .../szurubooru/tests/func/test_user_tokens.py | 5 ++-- .../tests/middleware/test_authenticator.py | 4 +-- 18 files changed, 108 insertions(+), 101 deletions(-) diff --git a/.gitignore b/.gitignore index 30cd78cf..8862e290 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,4 @@ config.yaml */*_modules/ .coverage -.cache -__pycache__ -.idea/ -*.iml \ No newline at end of file +.cache \ No newline at end of file diff --git a/client/html/user_tokens.tpl b/client/html/user_tokens.tpl index 7c3306da..03b3a163 100644 --- a/client/html/user_tokens.tpl +++ b/client/html/user_tokens.tpl @@ -10,7 +10,7 @@
<%= token.token %>
-
+
diff --git a/client/js/api.js b/client/js/api.js index d8cde471..637acdf9 100644 --- a/client/js/api.js +++ b/client/js/api.js @@ -89,11 +89,11 @@ class Api extends events.EventTarget { loginFromCookies() { const auth = cookies.getJSON('auth'); return auth && auth.user && auth.token ? - this.login_with_token(auth.user, auth.token, true) : + this.loginWithToken(auth.user, auth.token, true) : Promise.resolve(); } - login_with_token(userName, token, doRemember) { + loginWithToken(userName, token, doRemember) { this.cache = {}; return new Promise((resolve, reject) => { this.userName = userName; @@ -118,7 +118,7 @@ class Api extends events.EventTarget { }); } - create_token(userName, options) { + createToken(userName, options) { return new Promise((resolve, reject) => { this.post('/user-token/' + userName, {}) .then(response => { @@ -135,7 +135,7 @@ class Api extends events.EventTarget { }); } - delete_token(userName, userToken) { + deleteToken(userName, userToken) { return new Promise((resolve, reject) => { this.delete('/user-token/' + userName + '/' + userToken, {}) .then(response => { @@ -162,7 +162,7 @@ class Api extends events.EventTarget { if (doRemember) { options.expires = 365; } - this.create_token(this.userName, options); + this.createToken(this.userName, options); this.user = response; resolve(); this.dispatchEvent(new CustomEvent('login')); @@ -175,7 +175,7 @@ class Api extends events.EventTarget { logout() { let self = this; - this.delete_token(this.userName, this.userToken) + this.deleteToken(this.userName, this.userToken) .then(response => { self._logout(); }, error => { diff --git a/client/js/controllers/user_controller.js b/client/js/controllers/user_controller.js index 4b02657f..4a7c0221 100644 --- a/client/js/controllers/user_controller.js +++ b/client/js/controllers/user_controller.js @@ -87,12 +87,12 @@ class UserController { this._view.addEventListener('create-token', e => this._evtCreateToken(e)); this._view.addEventListener('delete-token', e => this._evtDeleteToken(e)); - for (let i = 0; i < this._successMessages.length; i++) { - this.showSuccess(this._successMessages[i]); + for (let message of this._successMessages) { + this.showSuccess(message); } - for (let i = 0; i < this._errorMessages.length; i++) { - this.showError(this._errorMessages[i]); + for (let message of this._errorMessages) { + this.showError(message); } }, error => { diff --git a/client/js/models/user_token.js b/client/js/models/user_token.js index 24e0ccdb..0ca1b9b9 100644 --- a/client/js/models/user_token.js +++ b/client/js/models/user_token.js @@ -11,20 +11,20 @@ class UserToken extends events.EventTarget { this._updateFromResponse({}); } - get token() { return this._token; } - get enabled() { return this._enabled; } - get version() { return this._version; } - get creation_time() { return this._creation_time; } + get token() { return this._token; } + get enabled() { return this._enabled; } + get version() { return this._version; } + get creationTime() { return this._creationTime; } static fromResponse(response) { if (typeof response.results !== 'undefined') { - let token_list = []; - for (let i = 0; i < response.results.length; i++) { + let tokenList = []; + for (let responseToken of response.results) { const token = new UserToken(); - token._updateFromResponse(response.results[i]); - token_list.push(token) + token._updateFromResponse(responseToken); + tokenList.push(token) } - return token_list; + return tokenList; } else { const ret = new UserToken(); ret._updateFromResponse(response); @@ -62,10 +62,10 @@ class UserToken extends events.EventTarget { _updateFromResponse(response) { const map = { - _token: response.token, - _enabled: response.enabled, - _version: response.version, - _creation_time: response.creationTime, + _token: response.token, + _enabled: response.enabled, + _version: response.version, + _creationTime: response.creationTime, }; Object.assign(this, map); diff --git a/client/js/views/user_tokens_view.js b/client/js/views/user_tokens_view.js index 0a5218b1..f98f8d8c 100644 --- a/client/js/views/user_tokens_view.js +++ b/client/js/views/user_tokens_view.js @@ -22,11 +22,12 @@ class UserTokenView extends events.EventTarget { } _decorateTokenForms() { + this._tokenFormNodes = []; for (let i = 0; i < this._tokens.length; i++) { - let formNode = this._hostNode.querySelector('#token' + i); + let formNode = this._hostNode.querySelector('.token[data-token-id=\"' + i + '\"]'); views.decorateValidator(formNode); formNode.addEventListener('submit', e => this._evtDelete(e)); - this._tokenFormNodes.push(formNode) + this._tokenFormNodes.push(formNode); } } @@ -44,23 +45,21 @@ class UserTokenView extends events.EventTarget { enableForm() { views.enableForm(this._formNode); - for (let i = 0; i < this._tokenFormNodes.length; i++) { - let formNode = this._tokenFormNodes[i]; + for (let formNode of this._tokenFormNodes) { views.enableForm(formNode); } } disableForm() { views.disableForm(this._formNode); - for (let i = 0; i < this._tokenFormNodes.length; i++) { - let formNode = this._tokenFormNodes[i]; + for (let formNode of this._tokenFormNodes) { views.disableForm(formNode); } } _evtDelete(e) { e.preventDefault(); - const userToken = this._tokens[parseInt(e.target.id.replace('token', ''))]; + const userToken = this._tokens[parseInt(e.target.getAttribute('data-token-id'))]; this.dispatchEvent(new CustomEvent('delete', { detail: { user: this._user, diff --git a/server/szurubooru/api/user_token_api.py b/server/szurubooru/api/user_token_api.py index f28d4e58..2323a97e 100644 --- a/server/szurubooru/api/user_token_api.py +++ b/server/szurubooru/api/user_token_api.py @@ -13,7 +13,8 @@ def _serialize( @rest.routes.get('/user-tokens/(?P[^/]+)/?') -def get_user_tokens(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: +def get_user_tokens( + ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: user = users.get_user_by_name(params['user_name']) infix = 'self' if ctx.user.user_id == user.user_id else 'any' auth.verify_privilege(ctx.user, 'user_tokens:list:%s' % infix) @@ -24,7 +25,8 @@ def get_user_tokens(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Resp @rest.routes.post('/user-token/(?P[^/]+)/?') -def create_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: +def create_user_token( + ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: user = users.get_user_by_name(params['user_name']) infix = 'self' if ctx.user.user_id == user.user_id else 'any' auth.verify_privilege(ctx.user, 'user_tokens:create:%s' % infix) @@ -33,27 +35,30 @@ def create_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Re @rest.routes.put('/user-token/(?P[^/]+)/(?P[^/]+)/?') -def update_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: +def update_user_token( + ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response: user = users.get_user_by_name(params['user_name']) infix = 'self' if ctx.user.user_id == user.user_id else 'any' auth.verify_privilege(ctx.user, 'user_tokens:edit:%s' % infix) - user_token = user_tokens.get_user_token_by_user_and_token(user, params['user_token']) + user_token = user_tokens.get_by_user_and_token(user, params['user_token']) versions.verify_version(user_token, ctx) versions.bump_version(user_token) if ctx.has_param('enabled'): auth.verify_privilege(ctx.user, 'user_tokens:edit:%s' % infix) - user_tokens.update_user_token_enabled(user_token, ctx.get_param_as_bool('enabled')) + user_tokens.update_user_token_enabled(user_token, + ctx.get_param_as_bool('enabled')) user_tokens.update_user_token_edit_time(user_token) ctx.session.commit() return _serialize(ctx, user_token) @rest.routes.delete('/user-token/(?P[^/]+)/(?P[^/]+)/?') -def delete_user_token(ctx: rest.Context, params: Dict[str, str]) -> rest.Response: +def delete_user_token( + ctx: rest.Context, params: Dict[str, str]) -> rest.Response: user = users.get_user_by_name(params['user_name']) infix = 'self' if ctx.user.user_id == user.user_id else 'any' auth.verify_privilege(ctx.user, 'user_tokens:delete:%s' % infix) - user_token = user_tokens.get_user_token_by_user_and_token(user, params['user_token']) + user_token = user_tokens.get_by_user_and_token(user, params['user_token']) if user_token is not None: ctx.session.delete(user_token) ctx.session.commit() diff --git a/server/szurubooru/func/user_tokens.py b/server/szurubooru/func/user_tokens.py index bad0819c..398c54a2 100644 --- a/server/szurubooru/func/user_tokens.py +++ b/server/szurubooru/func/user_tokens.py @@ -1,10 +1,13 @@ from datetime import datetime from typing import Any, Optional, List, Dict, Callable - -from szurubooru import db, model, rest +from szurubooru import db, model, rest, errors from szurubooru.func import auth, serialization, users +class InvalidEnabledFieldError(errors.ValidationError): + pass + + class UserTokenSerializer(serialization.BaseSerializer): def __init__( self, @@ -51,9 +54,11 @@ def serialize_user_token( return UserTokenSerializer(user_token, auth_user).serialize(options) -def get_user_token_by_user_and_token(user: model.User, token: str) -> model.UserToken: +def get_by_user_and_token( + user: model.User, token: str) -> model.UserToken: return (db.session.query(model.UserToken) - .filter(model.UserToken.user_id == user.user_id, model.UserToken.token == token) + .filter(model.UserToken.user_id == user.user_id, + model.UserToken.token == token) .one_or_none()) @@ -76,9 +81,12 @@ def create_user_token(user: model.User) -> model.UserToken: return user_token -def update_user_token_enabled(user_token: model.UserToken, enabled: bool) -> None: +def update_user_token_enabled( + user_token: model.UserToken, enabled: bool) -> None: assert user_token - user_token.enabled = enabled if enabled is not None else True + if enabled is None: + raise InvalidEnabledFieldError('Enabled cannot be empty.') + user_token.enabled = enabled def update_user_token_edit_time(user_token: model.UserToken) -> None: diff --git a/server/szurubooru/func/users.py b/server/szurubooru/func/users.py index da43647a..c6b12d99 100644 --- a/server/szurubooru/func/users.py +++ b/server/szurubooru/func/users.py @@ -172,8 +172,7 @@ def get_user_count() -> int: def try_get_user_by_name(name: str) -> Optional[model.User]: - return ( - db.session + return (db.session .query(model.User) .filter(sa.func.lower(model.User.name) == sa.func.lower(name)) .one_or_none()) @@ -189,11 +188,11 @@ def get_user_by_name(name: str) -> model.User: def try_get_user_by_name_or_email(name_or_email: str) -> Optional[model.User]: return ( db.session - .query(model.User) - .filter( + .query(model.User) + .filter( (sa.func.lower(model.User.name) == sa.func.lower(name_or_email)) | (sa.func.lower(model.User.email) == sa.func.lower(name_or_email))) - .one_or_none()) + .one_or_none()) def get_user_by_name_or_email(name_or_email: str) -> model.User: diff --git a/server/szurubooru/middleware/authenticator.py b/server/szurubooru/middleware/authenticator.py index 285b8ef6..581c190a 100644 --- a/server/szurubooru/middleware/authenticator.py +++ b/server/szurubooru/middleware/authenticator.py @@ -16,7 +16,7 @@ def _authenticate(username: str, password: str) -> model.User: def _authenticate_token(username: str, token: str) -> model.User: """Try to authenticate user. Throw AuthError for invalid users.""" user = users.get_user_by_name(username) - user_token = user_tokens.get_user_token_by_user_and_token(user, token) + user_token = user_tokens.get_by_user_and_token(user, token) if not auth.is_valid_token(user_token): raise errors.AuthError('Invalid token.') return user diff --git a/server/szurubooru/migrations/versions/a39c7f98a7fa_add_user_token_table.py b/server/szurubooru/migrations/versions/a39c7f98a7fa_add_user_token_table.py index c61ee3ea..95286bbf 100644 --- a/server/szurubooru/migrations/versions/a39c7f98a7fa_add_user_token_table.py +++ b/server/szurubooru/migrations/versions/a39c7f98a7fa_add_user_token_table.py @@ -17,17 +17,18 @@ depends_on = None def upgrade(): op.create_table('user_token', - sa.Column('id', sa.Integer(), nullable=False), - sa.Column('user_id', sa.Integer(), nullable=False), - sa.Column('token', sa.Unicode(length=36), nullable=False), - sa.Column('enabled', sa.Boolean(), nullable=False), - sa.Column('creation_time', sa.DateTime(), nullable=False), - sa.Column('last_edit_time', sa.DateTime(), nullable=True), - sa.Column('version', sa.Integer(), nullable=False), - sa.ForeignKeyConstraint(['user_id'], ['user.id'], ondelete='CASCADE'), - sa.PrimaryKeyConstraint('id') - ) - op.create_index(op.f('ix_user_token_user_id'), 'user_token', ['user_id'], unique=False) + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('token', sa.Unicode(length=36), nullable=False), + sa.Column('enabled', sa.Boolean(), nullable=False), + sa.Column('creation_time', sa.DateTime(), nullable=False), + sa.Column('last_edit_time', sa.DateTime(), nullable=True), + sa.Column('version', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], + ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id')) + op.create_index(op.f('ix_user_token_user_id'), 'user_token', + ['user_id'], unique=False) def downgrade(): diff --git a/server/szurubooru/model/__init__.py b/server/szurubooru/model/__init__.py index 9f87b489..7f5009c7 100644 --- a/server/szurubooru/model/__init__.py +++ b/server/szurubooru/model/__init__.py @@ -1,7 +1,5 @@ from szurubooru.model.base import Base -from szurubooru.model.user import ( - User, - UserToken) +from szurubooru.model.user import (User, UserToken) from szurubooru.model.tag_category import TagCategory from szurubooru.model.tag import Tag, TagName, TagSuggestion, TagImplication from szurubooru.model.post import ( diff --git a/server/szurubooru/tests/api/test_user_token_creating.py b/server/szurubooru/tests/api/test_user_token_creating.py index 004aa974..11b2f95b 100644 --- a/server/szurubooru/tests/api/test_user_token_creating.py +++ b/server/szurubooru/tests/api/test_user_token_creating.py @@ -11,7 +11,8 @@ def inject_config(config_injector): config_injector({'privileges': {'user_tokens:create:self': 'regular'}}) -def test_creating_user_token(user_token_factory, context_factory, fake_datetime): +def test_creating_user_token( + user_token_factory, context_factory, fake_datetime): user_token = user_token_factory() with patch('szurubooru.func.user_tokens.create_user_token'), \ patch('szurubooru.func.user_tokens.serialize_user_token'), \ @@ -21,9 +22,7 @@ def test_creating_user_token(user_token_factory, context_factory, fake_datetime) user_tokens.serialize_user_token.return_value = 'serialized user token' user_tokens.create_user_token.return_value = user_token result = api.user_token_api.create_user_token( - context_factory( - user=user_token.user), + context_factory(user=user_token.user), {'user_name': user_token.user.name}) assert result == 'serialized user token' - user_tokens.create_user_token.assert_called_once_with( - user_token.user) + user_tokens.create_user_token.assert_called_once_with(user_token.user) diff --git a/server/szurubooru/tests/api/test_user_token_deleting.py b/server/szurubooru/tests/api/test_user_token_deleting.py index cddbfec8..16bc1f23 100644 --- a/server/szurubooru/tests/api/test_user_token_deleting.py +++ b/server/szurubooru/tests/api/test_user_token_deleting.py @@ -11,19 +11,20 @@ def inject_config(config_injector): config_injector({'privileges': {'user_tokens:delete:self': 'regular'}}) -def test_deleting_user_token(user_token_factory, context_factory, fake_datetime): +def test_deleting_user_token( + user_token_factory, context_factory, fake_datetime): user_token = user_token_factory() db.session.add(user_token) db.session.commit() - with patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'), \ + with patch('szurubooru.func.user_tokens.get_by_user_and_token'), \ patch('szurubooru.func.users.get_user_by_name'), \ fake_datetime('1969-02-12'): users.get_user_by_name.return_value = user_token.user - user_tokens.get_user_token_by_user_and_token.return_value = user_token + user_tokens.get_by_user_and_token.return_value = user_token result = api.user_token_api.delete_user_token( - context_factory( - user=user_token.user), - {'user_name': user_token.user.name, 'user_token': user_token.token}) + context_factory(user=user_token.user), + {'user_name': user_token.user.name, + 'user_token': user_token.token}) assert result == {} - user_tokens.get_user_token_by_user_and_token.assert_called_once_with( + user_tokens.get_by_user_and_token.assert_called_once_with( user_token.user, user_token.token) diff --git a/server/szurubooru/tests/api/test_user_token_retrieving.py b/server/szurubooru/tests/api/test_user_token_retrieving.py index 27a3833f..8b2fc7ea 100644 --- a/server/szurubooru/tests/api/test_user_token_retrieving.py +++ b/server/szurubooru/tests/api/test_user_token_retrieving.py @@ -11,7 +11,8 @@ def inject_config(config_injector): config_injector({'privileges': {'user_tokens:list:self': 'regular'}}) -def test_retrieving_user_tokens(user_token_factory, context_factory, fake_datetime): +def test_retrieving_user_tokens( + user_token_factory, context_factory, fake_datetime): user_token1 = user_token_factory() user_token2 = user_token_factory(user=user_token1.user) user_token3 = user_token_factory(user=user_token1.user) @@ -21,11 +22,10 @@ def test_retrieving_user_tokens(user_token_factory, context_factory, fake_dateti fake_datetime('1969-02-12'): users.get_user_by_name.return_value = user_token1.user user_tokens.serialize_user_token.return_value = 'serialized user token' - user_tokens.get_user_tokens.return_value = [user_token1, user_token2, user_token3] + user_tokens.get_user_tokens.return_value = [user_token1, user_token2, + user_token3] result = api.user_token_api.get_user_tokens( - context_factory( - user=user_token1.user), + context_factory(user=user_token1.user), {'user_name': user_token1.user.name}) - assert result == {'results': ['serialized user token', 'serialized user token', 'serialized user token']} - user_tokens.get_user_tokens.assert_called_once_with( - user_token1.user) + assert result == {'results': ['serialized user token'] * 3} + user_tokens.get_user_tokens.assert_called_once_with(user_token1.user) diff --git a/server/szurubooru/tests/api/test_user_token_updating.py b/server/szurubooru/tests/api/test_user_token_updating.py index 97b7acb3..0c8e3d3e 100644 --- a/server/szurubooru/tests/api/test_user_token_updating.py +++ b/server/szurubooru/tests/api/test_user_token_updating.py @@ -15,7 +15,7 @@ def test_edit_user_token(user_token_factory, context_factory, fake_datetime): user_token = user_token_factory() db.session.add(user_token) db.session.commit() - with patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'), \ + with patch('szurubooru.func.user_tokens.get_by_user_and_token'), \ patch('szurubooru.func.user_tokens.update_user_token_enabled'), \ patch('szurubooru.func.user_tokens.update_user_token_edit_time'), \ patch('szurubooru.func.user_tokens.serialize_user_token'), \ @@ -23,17 +23,16 @@ def test_edit_user_token(user_token_factory, context_factory, fake_datetime): fake_datetime('1969-02-12'): users.get_user_by_name.return_value = user_token.user user_tokens.serialize_user_token.return_value = 'serialized user token' - user_tokens.get_user_token_by_user_and_token.return_value = user_token + user_tokens.get_by_user_and_token.return_value = user_token result = api.user_token_api.update_user_token( - context_factory( - params={ - 'version': user_token.version, - 'enabled': False, - }, - user=user_token.user), - {'user_name': user_token.user.name, 'user_token': user_token.token}) + context_factory(params={'version': user_token.version, + 'enabled': False, + }, + user=user_token.user), + {'user_name': user_token.user.name, + 'user_token': user_token.token}) assert result == 'serialized user token' - user_tokens.get_user_token_by_user_and_token.assert_called_once_with( + user_tokens.get_by_user_and_token.assert_called_once_with( user_token.user, user_token.token) user_tokens.update_user_token_enabled.assert_called_once_with( user_token, False) diff --git a/server/szurubooru/tests/func/test_user_tokens.py b/server/szurubooru/tests/func/test_user_tokens.py index d3c8dc9b..bb71862a 100644 --- a/server/szurubooru/tests/func/test_user_tokens.py +++ b/server/szurubooru/tests/func/test_user_tokens.py @@ -27,12 +27,13 @@ def test_serialize_user_token_none(): assert result is None -def test_get_user_token_by_user_and_token(user_token_factory): +def test_get_by_user_and_token(user_token_factory): user_token = user_token_factory() db.session.add(user_token) db.session.flush() db.session.commit() - result = user_tokens.get_user_token_by_user_and_token(user_token.user, user_token.token) + result = user_tokens.get_by_user_and_token(user_token.user, + user_token.token) assert result == user_token diff --git a/server/szurubooru/tests/middleware/test_authenticator.py b/server/szurubooru/tests/middleware/test_authenticator.py index 6c02fffc..0d9120b7 100644 --- a/server/szurubooru/tests/middleware/test_authenticator.py +++ b/server/szurubooru/tests/middleware/test_authenticator.py @@ -32,9 +32,9 @@ def test_process_request_token_auth_valid(context_factory, user_token_factory): }) with patch('szurubooru.func.auth.is_valid_token'), \ patch('szurubooru.func.users.get_user_by_name'), \ - patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'): + patch('szurubooru.func.user_tokens.get_by_user_and_token'): users.get_user_by_name.return_value = user_token.user - user_tokens.get_user_token_by_user_and_token.return_value = user_token + user_tokens.get_by_user_and_token.return_value = user_token auth.is_valid_password.return_value = True authenticator.process_request(ctx) assert ctx.user == user_token.user