From bb3d9e62e665fe6b5e88affbdc555478d35dcf5d Mon Sep 17 00:00:00 2001 From: s3lph Date: Mon, 16 Jul 2018 19:47:54 +0200 Subject: [PATCH 1/5] Ported code to Python 3.7. TODO: Port CI pipeline and deployment images. --- README.md | 2 +- matemat/db/facade.py | 15 +++----- matemat/db/test/test_facade.py | 14 +++---- matemat/db/wrapper.py | 3 +- matemat/primitives/Product.py | 55 ++++----------------------- matemat/primitives/User.py | 65 +++++--------------------------- matemat/webserver/requestargs.py | 7 ++-- 7 files changed, 36 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index 95aa201..e5995c0 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ This project intends to provide a well-tested and maintainable alternative to ## Dependencies -- Python 3 (>=3.6) +- Python 3 (>=3.7) - Python dependencies: - jinja2 diff --git a/matemat/db/facade.py b/matemat/db/facade.py index 38f324c..de6aa1e 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -1,4 +1,5 @@ +from __future__ import annotations from typing import List, Optional, Any, Type import crypt @@ -9,12 +10,6 @@ from matemat.exceptions import AuthenticationError, DatabaseConsistencyError from matemat.db import DatabaseWrapper from matemat.db.wrapper import Transaction -# TODO: Change to METHOD_BLOWFISH when adopting Python 3.7 -""" -The method to use for password hashing. -""" -_CRYPT_METHOD = crypt.METHOD_SHA512 - class MatematDatabase(object): """ @@ -42,7 +37,7 @@ class MatematDatabase(object): """ self.db: DatabaseWrapper = DatabaseWrapper(filename) - def __enter__(self) -> 'MatematDatabase': + def __enter__(self) -> MatematDatabase: # Pass context manager stuff through to the database wrapper self.db.__enter__() return self @@ -100,7 +95,7 @@ class MatematDatabase(object): :raises ValueError: If a user with the same name already exists. """ # Hash the password. - pwhash: str = crypt.crypt(password, crypt.mksalt(_CRYPT_METHOD)) + pwhash: str = crypt.crypt(password, crypt.mksalt()) user_id: int = -1 with self.db.transaction() as c: # Look up whether a user with the same name already exists. @@ -178,7 +173,7 @@ class MatematDatabase(object): if verify_password and not compare_digest(crypt.crypt(oldpass, row[0]), row[0]): raise AuthenticationError('Old password does not match.') # Hash the new password and write it to the database. - pwhash: str = crypt.crypt(newpass, crypt.mksalt(_CRYPT_METHOD)) + pwhash: str = crypt.crypt(newpass, crypt.mksalt()) c.execute(''' UPDATE users SET password = :pwhash, lastchange = STRFTIME('%s', 'now') WHERE user_id = :user_id ''', { @@ -208,7 +203,7 @@ class MatematDatabase(object): if verify_password and not compare_digest(crypt.crypt(password, row[0]), row[0]): raise AuthenticationError('Password does not match.') # Hash the new touchkey and write it to the database. - tkhash: Optional[str] = crypt.crypt(touchkey, crypt.mksalt(_CRYPT_METHOD)) if touchkey is not None else None + tkhash: Optional[str] = crypt.crypt(touchkey, crypt.mksalt()) if touchkey is not None else None c.execute(''' UPDATE users SET touchkey = :tkhash, lastchange = STRFTIME('%s', 'now') WHERE user_id = :user_id ''', { diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index b17262f..1d63e19 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -58,7 +58,7 @@ class DatabaseTest(unittest.TestCase): u = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com') # Add a touchkey without using the provided function c.execute('''UPDATE users SET touchkey = :tkhash WHERE user_id = :user_id''', { - 'tkhash': crypt.crypt('0123', crypt.mksalt(crypt.METHOD_SHA512)), + 'tkhash': crypt.crypt('0123', crypt.mksalt()), 'user_id': u.id }) user = db.login('testuser', 'supersecurepassword') @@ -99,7 +99,7 @@ class DatabaseTest(unittest.TestCase): with self.assertRaises(AuthenticationError): db.login('testuser', 'mynewpassword') db.login('testuser', 'adminpasswordreset') - user._user_id = -1 + user.id = -1 with self.assertRaises(AuthenticationError): # Password change for an inexistent user should fail db.change_password(user, 'adminpasswordreset', 'passwordwithoutuser') @@ -123,7 +123,7 @@ class DatabaseTest(unittest.TestCase): with self.assertRaises(AuthenticationError): db.login('testuser', touchkey='4567') db.login('testuser', touchkey='89ab') - user._user_id = -1 + user.id = -1 with self.assertRaises(AuthenticationError): # Touchkey change for an inexistent user should fail db.change_touchkey(user, '89ab', '048c') @@ -139,7 +139,7 @@ class DatabaseTest(unittest.TestCase): self.assertEqual('newaddress@example.com', checkuser.email) self.assertFalse(checkuser.is_admin) self.assertFalse(checkuser.is_member) - user._user_id = -1 + user.id = -1 with self.assertRaises(DatabaseConsistencyError): db.change_user(user) @@ -205,7 +205,7 @@ class DatabaseTest(unittest.TestCase): self.assertEqual('Flora Power Mate', checkproduct.name) self.assertEqual(150, checkproduct.price_member) self.assertEqual(250, checkproduct.price_non_member) - product._product_id = -1 + product.id = -1 with self.assertRaises(DatabaseConsistencyError): db.change_product(product) product2 = db.create_product('Club Mate', 200, 200) @@ -248,7 +248,7 @@ class DatabaseTest(unittest.TestCase): with self.assertRaises(ValueError): # Should fail, negative amount db.deposit(user, -42) - user._user_id = -1 + user.id = -1 with self.assertRaises(DatabaseConsistencyError): # Should fail, user id -1 does not exist db.deposit(user, 42) @@ -267,7 +267,7 @@ class DatabaseTest(unittest.TestCase): self.assertEqual(42, c.fetchone()[0]) c.execute('''SELECT stock FROM products WHERE product_id = ?''', [product2.id]) self.assertEqual(0, c.fetchone()[0]) - product._product_id = -1 + product.id = -1 with self.assertRaises(DatabaseConsistencyError): # Should fail, product id -1 does not exist db.restock(product, 42) diff --git a/matemat/db/wrapper.py b/matemat/db/wrapper.py index 34af430..f57c95f 100644 --- a/matemat/db/wrapper.py +++ b/matemat/db/wrapper.py @@ -1,4 +1,5 @@ +from __future__ import annotations from typing import Any, Optional import sqlite3 @@ -76,7 +77,7 @@ class DatabaseWrapper(object): self._filename: str = filename self._sqlite_db: Optional[sqlite3.Connection] = None - def __enter__(self) -> 'DatabaseWrapper': + def __enter__(self) -> DatabaseWrapper: self.connect() return self diff --git a/matemat/primitives/Product.py b/matemat/primitives/Product.py index d2cb043..071b1c4 100644 --- a/matemat/primitives/Product.py +++ b/matemat/primitives/Product.py @@ -1,51 +1,10 @@ -from typing import Any +from dataclasses import dataclass -class Product(object): - - def __init__(self, - product_id: int, - name: str, - price_member: int, - price_non_member: int) -> None: - self._product_id: int = product_id - self._name: str = name - self._price_member: int = price_member - self._price_non_member: int = price_non_member - - def __eq__(self, other: Any) -> bool: - if other is None or not isinstance(other, Product): - return False - return self._product_id == other._product_id \ - and self._name == other._name \ - and self._price_member == other._price_member \ - and self._price_non_member == other._price_non_member - - @property - def id(self) -> int: - return self._product_id - - @property - def name(self) -> str: - return self._name - - @name.setter - def name(self, name: str) -> None: - self._name = name - - @property - def price_member(self) -> int: - return self._price_member - - @price_member.setter - def price_member(self, price: int) -> None: - self._price_member = price - - @property - def price_non_member(self) -> int: - return self._price_non_member - - @price_non_member.setter - def price_non_member(self, price: int) -> None: - self._price_non_member = price +@dataclass +class Product: + id: int + name: str + price_member: int + price_non_member: int diff --git a/matemat/primitives/User.py b/matemat/primitives/User.py index e49e52b..ba001c3 100644 --- a/matemat/primitives/User.py +++ b/matemat/primitives/User.py @@ -1,58 +1,13 @@ -from typing import Optional, Any +from typing import Optional + +from dataclasses import dataclass -class User(object): - - def __init__(self, - user_id: int, - username: str, - email: Optional[str] = None, - admin: bool = False, - member: bool = True) -> None: - self._user_id: int = user_id - self._username: str = username - self._email: Optional[str] = email - self._admin: bool = admin - self._member: bool = member - - def __eq__(self, other: Any) -> bool: - if other is None or not isinstance(other, User): - return False - return self._user_id == other._user_id \ - and self._username == other._username \ - and self._email == other._email \ - and self._admin == other._admin \ - and self._member == other._member - - @property - def id(self) -> int: - return self._user_id - - @property - def name(self) -> str: - return self._username - - @property - def email(self) -> Optional[str]: - return self._email - - @email.setter - def email(self, email: str) -> None: - self._email = email - - @property - def is_admin(self) -> bool: - return self._admin - - @is_admin.setter - def is_admin(self, admin: bool) -> None: - self._admin = admin - - @property - def is_member(self) -> bool: - return self._member - - @is_member.setter - def is_member(self, member: bool) -> None: - self._member = member +@dataclass +class User: + id: int + name: str + email: Optional[str] = None + is_admin: bool = False + is_member: bool = False diff --git a/matemat/webserver/requestargs.py b/matemat/webserver/requestargs.py index 373db90..6ce11ef 100644 --- a/matemat/webserver/requestargs.py +++ b/matemat/webserver/requestargs.py @@ -1,4 +1,5 @@ +from __future__ import annotations from typing import Dict, Iterator, List, Tuple, Union @@ -24,7 +25,7 @@ class RequestArguments(object): """ self.__container: Dict[str, RequestArgument] = dict() - def __getitem__(self, key: str) -> 'RequestArgument': + def __getitem__(self, key: str) -> RequestArgument: """ Retrieve the argument for the given name, creating it on the fly, if it doesn't exist. @@ -40,7 +41,7 @@ class RequestArguments(object): # Return the argument for the name return self.__container[key] - def __getattr__(self, key: str) -> 'RequestArgument': + def __getattr__(self, key: str) -> RequestArgument: """ Syntactic sugar for accessing values with a name that can be used in Python attributes. The value will be returned as an immutable view. @@ -278,7 +279,7 @@ class RequestArgument(object): # Yield an immutable scalar view for each (ctype, value) element in the array yield _View(self.__name, v) - def __getitem__(self, index: Union[int, slice]) -> 'RequestArgument': + def __getitem__(self, index: Union[int, slice]) -> RequestArgument: """ Index the argument with either an int or a slice. The returned values are represented as immutable RequestArgument views. From 97d175d62a2b90a37f1030adf1f5b4acd7d9b3ac Mon Sep 17 00:00:00 2001 From: s3lph Date: Thu, 2 Aug 2018 20:37:54 +0200 Subject: [PATCH 2/5] Upgraded Dockerfiles to work with Python 3.7. With some weird stuff in the unittest dockerfile. --- Dockerfile | 2 +- testing/Dockerfile | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index c427a2e..553cb64 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ -FROM python:3.6-alpine +FROM python:3.7-alpine RUN mkdir -p /var/matemat/db /var/matemat/upload RUN apk --update add libmagic diff --git a/testing/Dockerfile b/testing/Dockerfile index 94644e6..1c19268 100644 --- a/testing/Dockerfile +++ b/testing/Dockerfile @@ -1,11 +1,15 @@ -FROM debian:buster +# There is no buster image yet and stretch doesn't have a docker package. So let's just "upgrade" the image to buster. +FROM python:3.7-stretch -RUN useradd -d /home/matemat -m matemat -RUN mkdir -p /var/matemat/db && chown matemat:matemat -R /var/matemat/db -RUN mkdir -p /var/matemat/upload && chown matemat:matemat -R /var/matemat/upload -RUN apt-get update -qy -RUN apt-get install -y --no-install-recommends file sudo openssh-client git docker.io python3-dev python3-pip python3-coverage python3-setuptools build-essential -RUN pip3 install wheel pycodestyle mypy +RUN sed -re 's/stretch/buster/g' -i /etc/apt/sources.list \ + && useradd -d /home/matemat -m matemat \ + && mkdir -p /var/matemat/db /var/matemat/upload \ + && chown matemat:matemat -R /var/matemat/db \ + && chown matemat:matemat -R /var/matemat/upload \ + && apt-get update -qy \ + && apt-get install -y --no-install-recommends file sudo openssh-client git docker.io build-essential \ + && python3.7 -m pip install wheel pycodestyle mypy \ + && rm -rf /var/lib/apt/lists/* WORKDIR /home/matemat From 9f5c556417027664795a9c6e0648341d247a3331 Mon Sep 17 00:00:00 2001 From: s3lph Date: Thu, 2 Aug 2018 20:44:17 +0200 Subject: [PATCH 3/5] Updated GitLab CI to use the latest docker image. --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9ac4c38..cf02ea3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,5 @@ --- -image: s3lph/matemat-ci:20180720-01 +image: s3lph/matemat-ci:20180802-01 stages: - test From 51c804abf3f662aa6658b7cfdeaf1daec23d281a Mon Sep 17 00:00:00 2001 From: s3lph Date: Thu, 2 Aug 2018 20:47:24 +0200 Subject: [PATCH 4/5] Added missing python-coverage to testing docker image. --- .gitlab-ci.yml | 2 +- testing/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cf02ea3..84bda47 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,5 @@ --- -image: s3lph/matemat-ci:20180802-01 +image: s3lph/matemat-ci:20180802-02 stages: - test diff --git a/testing/Dockerfile b/testing/Dockerfile index 1c19268..a67d602 100644 --- a/testing/Dockerfile +++ b/testing/Dockerfile @@ -9,7 +9,7 @@ RUN sed -re 's/stretch/buster/g' -i /etc/apt/sources.list \ && chown matemat:matemat -R /var/matemat/upload \ && apt-get update -qy \ && apt-get install -y --no-install-recommends file sudo openssh-client git docker.io build-essential \ - && python3.7 -m pip install wheel pycodestyle mypy \ + && python3.7 -m pip install coverage wheel pycodestyle mypy \ && rm -rf /var/lib/apt/lists/* WORKDIR /home/matemat From 5537f2e1f37e0fbc583340662d6055c76cbd22a2 Mon Sep 17 00:00:00 2001 From: s3lph Date: Thu, 2 Aug 2018 21:39:05 +0200 Subject: [PATCH 5/5] GitLab CI: Fixed coverage invocation. --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 84bda47..155ee2d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -10,8 +10,8 @@ test: stage: test script: - pip3 install -r requirements.txt - - sudo -u matemat python3-coverage run --branch -m unittest discover matemat - - sudo -u matemat python3-coverage report -m --include 'matemat/*' --omit '*/test_*.py' + - sudo -u matemat python3 -m coverage run --branch -m unittest discover matemat + - sudo -u matemat python3 -m coverage report -m --include 'matemat/*' --omit '*/test_*.py' codestyle: stage: test