From 4fd1b1001847eaef73b3da9a222c011068a3b2b7 Mon Sep 17 00:00:00 2001 From: Pascal de Bruijn Date: Tue, 6 Sep 2022 15:31:43 +0200 Subject: [PATCH 1/5] models/user.py: properly guard plain_text_password property Resolves the following issue, which occurs with force_otp enabled and OAuth authentication sources: File "/srv/powerdnsadmin/powerdnsadmin/models/user.py", line 481, in update_profile "utf-8") if self.plain_text_password else user.password AttributeError: 'User' object has no attribute 'plain_text_password' --- powerdnsadmin/models/user.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/powerdnsadmin/models/user.py b/powerdnsadmin/models/user.py index 2f8b87c..1ac7b60 100644 --- a/powerdnsadmin/models/user.py +++ b/powerdnsadmin/models/user.py @@ -107,7 +107,7 @@ class User(db.Model): def check_password(self, hashed_password): # Check hashed password. Using bcrypt, the salt is saved into the hash itself - if (self.plain_text_password): + if hasattr(self, "plain_text_password"): return bcrypt.checkpw(self.plain_text_password.encode('utf-8'), hashed_password.encode('utf-8')) return False @@ -423,7 +423,7 @@ class User(db.Model): name='Administrator').first().id self.password = self.get_hashed_password( - self.plain_text_password) if self.plain_text_password else '*' + self.plain_text_password) if hasattr(self, "plain_text_password") else '*' if self.password and self.password != '*': self.password = self.password.decode("utf-8") @@ -459,7 +459,7 @@ class User(db.Model): user.email = self.email # store new password hash (only if changed) - if self.plain_text_password: + if hasattr(self, "plain_text_password"): user.password = self.get_hashed_password( self.plain_text_password).decode("utf-8") @@ -478,7 +478,7 @@ class User(db.Model): user.lastname = self.lastname if self.lastname else user.lastname user.password = self.get_hashed_password( self.plain_text_password).decode( - "utf-8") if self.plain_text_password else user.password + "utf-8") if hasattr(self, "plain_text_password") else user.password if self.email: # Can not update to a new email that From 846c03f154ff9953641628e33632f4ba951b8bf9 Mon Sep 17 00:00:00 2001 From: Pascal de Bruijn Date: Wed, 7 Sep 2022 14:23:34 +0200 Subject: [PATCH 2/5] models/user.py: add non-zero valid_window to totp.verify PyOTP's totp.verify defaults to the valid_window of zero, which means it will reject valid codes, if submitted just past the 30 sec window. It also means, users will run into authentication issues very quickly if their phones time-sync isn't perfect. Therefore valid_window should at the very least be 1 or more, settting it higher trades security for robustness, especially with regard to time desync issues. --- powerdnsadmin/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/powerdnsadmin/models/user.py b/powerdnsadmin/models/user.py index 2f8b87c..228f8f3 100644 --- a/powerdnsadmin/models/user.py +++ b/powerdnsadmin/models/user.py @@ -94,7 +94,7 @@ class User(db.Model): def verify_totp(self, token): totp = pyotp.TOTP(self.otp_secret) - return totp.verify(token) + return totp.verify(token, valid_window = 5) def get_hashed_password(self, plain_text_password=None): # Hash a password for the first time From d25a22272eb1ee589ca9425b77ebc49f4b177478 Mon Sep 17 00:00:00 2001 From: WhatshallIbreaktoday <75358410+WhatshallIbreaktoday@users.noreply.github.com> Date: Wed, 12 Oct 2022 08:10:35 +0200 Subject: [PATCH 3/5] allow null/None JSON data This change permits to proxy pdns zone notify api requests (which are expected to be with empty body) --- powerdnsadmin/lib/helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/powerdnsadmin/lib/helper.py b/powerdnsadmin/lib/helper.py index a5925ef..1b5a082 100644 --- a/powerdnsadmin/lib/helper.py +++ b/powerdnsadmin/lib/helper.py @@ -14,9 +14,9 @@ def forward_request(): msg_str = "Sending request to powerdns API {0}" if request.method != 'GET' and request.method != 'DELETE': - msg = msg_str.format(request.get_json(force=True)) + msg = msg_str.format(request.get_json(force=True, silent=True)) current_app.logger.debug(msg) - data = request.get_json(force=True) + data = request.get_json(force=True, silent=True) verify = False From 25ebbf132c16f0068b7b44759ab8a83e0aa38635 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Fri, 4 Nov 2022 11:59:59 +1100 Subject: [PATCH 4/5] Fix handling of passwords with % in the SQLALCHEMY_DATABASE_URI Fix Flask-Migrate ValueError from occurring when a password has '%' characters in it when specified via SQLALCHEMY_DATABASE_URI. --- migrations/env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/env.py b/migrations/env.py index 6a10e6d..4742e14 100755 --- a/migrations/env.py +++ b/migrations/env.py @@ -19,7 +19,7 @@ logger = logging.getLogger('alembic.env') # target_metadata = mymodel.Base.metadata from flask import current_app config.set_main_option('sqlalchemy.url', - current_app.config.get('SQLALCHEMY_DATABASE_URI')) + current_app.config.get('SQLALCHEMY_DATABASE_URI').replace("%","%%")) target_metadata = current_app.extensions['migrate'].db.metadata # other values from the config, defined by the needs of env.py, From 2656242b45439ece9e50afd95667b83a24fb1439 Mon Sep 17 00:00:00 2001 From: Bernward Sanchez <109844019+pneb@users.noreply.github.com> Date: Fri, 9 Dec 2022 09:33:17 +0800 Subject: [PATCH 5/5] Update api_key.py I added the parentheses to the `db.session.rollback` line to call the method, which will now properly roll back any changes made to the database if an error occurs. --- powerdnsadmin/models/api_key.py | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/powerdnsadmin/models/api_key.py b/powerdnsadmin/models/api_key.py index 4c26cd2..7bd0fda 100644 --- a/powerdnsadmin/models/api_key.py +++ b/powerdnsadmin/models/api_key.py @@ -60,31 +60,31 @@ class ApiKey(db.Model): def update(self, role_name=None, description=None, domains=None, accounts=None): try: - if role_name: - role = Role.query.filter(Role.name == role_name).first() - self.role_id = role.id + if role_name: + role = Role.query.filter(Role.name == role_name).first() + self.role_id = role.id - if description: - self.description = description + if description: + self.description = description - if domains is not None: - domain_object_list = Domain.query \ - .filter(Domain.name.in_(domains)) \ - .all() - self.domains[:] = domain_object_list + if domains is not None: + domain_object_list = Domain.query \ + .filter(Domain.name.in_(domains)) \ + .all() + self.domains[:] = domain_object_list - if accounts is not None: - account_object_list = Account.query \ - .filter(Account.name.in_(accounts)) \ - .all() - self.accounts[:] = account_object_list + if accounts is not None: + account_object_list = Account.query \ + .filter(Account.name.in_(accounts)) \ + .all() + self.accounts[:] = account_object_list - db.session.commit() + db.session.commit() except Exception as e: - msg_str = 'Update of apikey failed. Error: {0}' - current_app.logger.error(msg_str.format(e)) - db.session.rollback - raise e + msg_str = 'Update of apikey failed. Error: {0}' + current_app.logger.error(msg_str.format(e)) + db.session.rollback() # fixed line + raise e def get_hashed_password(self, plain_text_password=None): # Hash a password for the first time