From 3e462dab1749887a24bfdc39fabdecea5bf7d9cb Mon Sep 17 00:00:00 2001 From: corubba Date: Fri, 27 May 2022 12:53:19 +0200 Subject: [PATCH] Fix csrf configuration CSRF has been initialized *before* the app config was fully read. That made it impossible to configure CSRF properly. Moved the CSRF init into the routes module, and switched from programmatic to decorated exemptions. GET routes don't need to be exempted because they are by default. --- powerdnsadmin/__init__.py | 28 +--------------------------- powerdnsadmin/routes/__init__.py | 3 ++- powerdnsadmin/routes/api.py | 20 ++++++++++++++++++++ powerdnsadmin/routes/base.py | 4 ++++ powerdnsadmin/routes/index.py | 5 ++++- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/powerdnsadmin/__init__.py b/powerdnsadmin/__init__.py index c70b273..02479a5 100755 --- a/powerdnsadmin/__init__.py +++ b/powerdnsadmin/__init__.py @@ -1,7 +1,6 @@ import os import logging from flask import Flask -from flask_seasurf import SeaSurf from flask_mail import Mail from werkzeug.middleware.proxy_fix import ProxyFix from flask_session import Session @@ -33,31 +32,6 @@ def create_app(config=None): # Proxy app.wsgi_app = ProxyFix(app.wsgi_app) - # CSRF protection - csrf = SeaSurf(app) - csrf.exempt(routes.index.dyndns_checkip) - csrf.exempt(routes.index.dyndns_update) - csrf.exempt(routes.index.saml_authorized) - csrf.exempt(routes.api.api_login_create_zone) - csrf.exempt(routes.api.api_login_delete_zone) - csrf.exempt(routes.api.api_generate_apikey) - csrf.exempt(routes.api.api_delete_apikey) - csrf.exempt(routes.api.api_update_apikey) - csrf.exempt(routes.api.api_zone_subpath_forward) - csrf.exempt(routes.api.api_zone_forward) - csrf.exempt(routes.api.api_create_zone) - csrf.exempt(routes.api.api_create_account) - csrf.exempt(routes.api.api_delete_account) - csrf.exempt(routes.api.api_update_account) - csrf.exempt(routes.api.api_create_user) - csrf.exempt(routes.api.api_delete_user) - csrf.exempt(routes.api.api_update_user) - csrf.exempt(routes.api.api_list_account_users) - csrf.exempt(routes.api.api_add_account_user) - csrf.exempt(routes.api.api_remove_account_user) - csrf.exempt(routes.api.api_zone_cryptokeys) - csrf.exempt(routes.api.api_zone_cryptokey) - # Load config from env variables if using docker if os.path.exists(os.path.join(app.root_path, 'docker_config.py')): app.config.from_object('powerdnsadmin.docker_config') @@ -69,7 +43,7 @@ def create_app(config=None): if 'FLASK_CONF' in os.environ: app.config.from_envvar('FLASK_CONF') - # Load app sepecified configuration + # Load app specified configuration if config is not None: if isinstance(config, dict): app.config.update(config) diff --git a/powerdnsadmin/routes/__init__.py b/powerdnsadmin/routes/__init__.py index b22324b..7d8aa9a 100644 --- a/powerdnsadmin/routes/__init__.py +++ b/powerdnsadmin/routes/__init__.py @@ -1,5 +1,5 @@ from .base import ( - login_manager, handle_bad_request, handle_unauthorized_access, + csrf, login_manager, handle_bad_request, handle_unauthorized_access, handle_access_forbidden, handle_page_not_found, handle_internal_server_error ) @@ -13,6 +13,7 @@ from .api import api_bp, apilist_bp def init_app(app): login_manager.init_app(app) + csrf.init_app(app) app.register_blueprint(index_bp) app.register_blueprint(user_bp) diff --git a/powerdnsadmin/routes/api.py b/powerdnsadmin/routes/api.py index 672e84a..3df31e1 100644 --- a/powerdnsadmin/routes/api.py +++ b/powerdnsadmin/routes/api.py @@ -6,6 +6,7 @@ from flask import ( ) from flask_login import current_user +from .base import csrf from ..models.base import db from ..models import ( User, Domain, DomainUser, Account, AccountUser, History, Setting, ApiKey, @@ -187,6 +188,7 @@ def index(): @api_bp.route('/pdnsadmin/zones', methods=['POST']) @api_basic_auth @api_can_create_domain +@csrf.exempt def api_login_create_zone(): pdns_api_url = Setting().get('pdns_api_url') pdns_api_key = Setting().get('pdns_api_key') @@ -255,6 +257,7 @@ def api_login_list_zones(): @api_bp.route('/pdnsadmin/zones/', methods=['DELETE']) @api_basic_auth @api_can_create_domain +@csrf.exempt def api_login_delete_zone(domain_name): pdns_api_url = Setting().get('pdns_api_url') pdns_api_key = Setting().get('pdns_api_key') @@ -310,6 +313,7 @@ def api_login_delete_zone(domain_name): @api_bp.route('/pdnsadmin/apikeys', methods=['POST']) @api_basic_auth +@csrf.exempt def api_generate_apikey(): data = request.get_json() description = None @@ -466,6 +470,7 @@ def api_get_apikey(apikey_id): @api_bp.route('/pdnsadmin/apikeys/', methods=['DELETE']) @api_basic_auth +@csrf.exempt def api_delete_apikey(apikey_id): apikey = ApiKey.query.get(apikey_id) @@ -503,6 +508,7 @@ def api_delete_apikey(apikey_id): @api_bp.route('/pdnsadmin/apikeys/', methods=['PUT']) @api_basic_auth +@csrf.exempt def api_update_apikey(apikey_id): # if role different and user is allowed to change it, update # if apikey domains are different and user is allowed to handle @@ -664,6 +670,7 @@ def api_list_users(username=None): @api_bp.route('/pdnsadmin/users', methods=['POST']) @api_basic_auth @api_role_can('create users', allow_self=True) +@csrf.exempt def api_create_user(): """ Create new user @@ -737,6 +744,7 @@ def api_create_user(): @api_bp.route('/pdnsadmin/users/', methods=['PUT']) @api_basic_auth @api_role_can('update users', allow_self=True) +@csrf.exempt def api_update_user(user_id): """ Update existing user @@ -809,6 +817,7 @@ def api_update_user(user_id): @api_bp.route('/pdnsadmin/users/', methods=['DELETE']) @api_basic_auth @api_role_can('delete users') +@csrf.exempt def api_delete_user(user_id): user = User.query.get(user_id) if not user: @@ -860,6 +869,7 @@ def api_list_accounts(account_name): @api_bp.route('/pdnsadmin/accounts', methods=['POST']) @api_basic_auth +@csrf.exempt def api_create_account(): if current_user.role.name not in ['Administrator', 'Operator']: msg = "{} role cannot create accounts".format(current_user.role.name) @@ -904,6 +914,7 @@ def api_create_account(): @api_bp.route('/pdnsadmin/accounts/', methods=['PUT']) @api_basic_auth @api_role_can('update accounts') +@csrf.exempt def api_update_account(account_id): data = request.get_json() name = data['name'] if 'name' in data else None @@ -945,6 +956,7 @@ def api_update_account(account_id): @api_bp.route('/pdnsadmin/accounts/', methods=['DELETE']) @api_basic_auth @api_role_can('delete accounts') +@csrf.exempt def api_delete_account(account_id): account_list = [] or Account.query.filter(Account.id == account_id).all() if len(account_list) == 1: @@ -996,6 +1008,7 @@ def api_list_account_users(account_id): methods=['PUT']) @api_basic_auth @api_role_can('add user to account') +@csrf.exempt def api_add_account_user(account_id, user_id): account = Account.query.get(account_id) if not account: @@ -1023,6 +1036,7 @@ def api_add_account_user(account_id, user_id): methods=['DELETE']) @api_basic_auth @api_role_can('remove user from account') +@csrf.exempt def api_remove_account_user(account_id, user_id): account = Account.query.get(account_id) if not account: @@ -1054,6 +1068,7 @@ def api_remove_account_user(account_id, user_id): @apikey_auth @apikey_can_access_domain @apikey_can_configure_dnssec(http_methods=['POST']) +@csrf.exempt def api_zone_cryptokeys(server_id, zone_id): resp = helper.forward_request() return resp.content, resp.status_code, resp.headers.items() @@ -1065,6 +1080,7 @@ def api_zone_cryptokeys(server_id, zone_id): @apikey_auth @apikey_can_access_domain @apikey_can_configure_dnssec() +@csrf.exempt def api_zone_cryptokey(server_id, zone_id, cryptokey_id): resp = helper.forward_request() return resp.content, resp.status_code, resp.headers.items() @@ -1075,6 +1091,7 @@ def api_zone_cryptokey(server_id, zone_id, cryptokey_id): methods=['GET', 'POST', 'PUT', 'PATCH', 'DELETE']) @apikey_auth @apikey_can_access_domain +@csrf.exempt def api_zone_subpath_forward(server_id, zone_id, subpath): resp = helper.forward_request() return resp.content, resp.status_code, resp.headers.items() @@ -1090,6 +1107,7 @@ def api_zone_subpath_forward(server_id, zone_id, subpath): @callback_if_request_body_contains_key(apikey_can_configure_dnssec()(), http_methods=['PUT'], keys=['dnssec', 'nsec3param']) +@csrf.exempt def api_zone_forward(server_id, zone_id): resp = helper.forward_request() if not Setting().get('bg_domain_updates'): @@ -1127,6 +1145,7 @@ def api_zone_forward(server_id, zone_id): @api_bp.route('/servers/', methods=['GET', 'PUT']) @apikey_auth @apikey_is_admin +@csrf.exempt def api_server_sub_forward(subpath): resp = helper.forward_request() return resp.content, resp.status_code, resp.headers.items() @@ -1135,6 +1154,7 @@ def api_server_sub_forward(subpath): @api_bp.route('/servers//zones', methods=['POST']) @apikey_auth @apikey_can_create_domain +@csrf.exempt def api_create_zone(server_id): resp = helper.forward_request() diff --git a/powerdnsadmin/routes/base.py b/powerdnsadmin/routes/base.py index 48ef1c0..16ed00a 100644 --- a/powerdnsadmin/routes/base.py +++ b/powerdnsadmin/routes/base.py @@ -1,9 +1,13 @@ import base64 + from flask import render_template, url_for, redirect, session, request, current_app from flask_login import LoginManager +from flask_seasurf import SeaSurf from ..models.user import User + +csrf = SeaSurf() login_manager = LoginManager() diff --git a/powerdnsadmin/routes/index.py b/powerdnsadmin/routes/index.py index 93823ce..3a6f55c 100644 --- a/powerdnsadmin/routes/index.py +++ b/powerdnsadmin/routes/index.py @@ -10,7 +10,7 @@ from yaml import Loader, load from flask import Blueprint, render_template, make_response, url_for, current_app, g, session, request, redirect, abort from flask_login import login_user, logout_user, login_required, current_user -from .base import login_manager +from .base import csrf, login_manager from ..lib import utils from ..decorators import dyndns_login_required from ..models.base import db @@ -763,6 +763,7 @@ def resend_confirmation_email(): @index_bp.route('/nic/checkip.html', methods=['GET', 'POST']) +@csrf.exempt def dyndns_checkip(): # This route covers the default ddclient 'web' setting for the checkip service return render_template('dyndns.html', @@ -771,6 +772,7 @@ def dyndns_checkip(): @index_bp.route('/nic/update', methods=['GET', 'POST']) +@csrf.exempt @dyndns_login_required def dyndns_update(): # dyndns protocol response codes in use are: @@ -961,6 +963,7 @@ def saml_metadata(): @index_bp.route('/saml/authorized', methods=['GET', 'POST']) +@csrf.exempt def saml_authorized(): errors = [] if not current_app.config.get('SAML_ENABLED'):