From 61d092922cc2b8da4ede8c98f35b9b0b7bde98d3 Mon Sep 17 00:00:00 2001 From: Dimitri Huisman Date: Fri, 25 Nov 2022 11:21:33 +0000 Subject: [PATCH] Process review comments (PR2464) --- core/admin/mailu/__init__.py | 2 +- core/admin/mailu/api/__init__.py | 20 ++++++++----------- core/admin/mailu/api/common.py | 26 ++++++++++++------------- core/admin/mailu/api/v1/__init__.py | 9 ++++----- core/admin/mailu/api/v1/alias.py | 12 ++++++------ core/admin/mailu/api/v1/domains.py | 30 ++++++++++++++--------------- core/admin/mailu/api/v1/relay.py | 10 +++++----- core/admin/mailu/api/v1/user.py | 10 +++++----- core/admin/mailu/configuration.py | 2 +- docs/configuration.rst | 5 ++--- 10 files changed, 60 insertions(+), 66 deletions(-) diff --git a/core/admin/mailu/__init__.py b/core/admin/mailu/__init__.py index 89e06c9b..de3503d0 100644 --- a/core/admin/mailu/__init__.py +++ b/core/admin/mailu/__init__.py @@ -75,7 +75,7 @@ def create_app_from_config(config): app.register_blueprint(internal.internal, url_prefix='/internal') app.register_blueprint(sso.sso, url_prefix='/sso') if app.config.get('API_TOKEN'): - api.register(app, web_api=app.config.get('WEB_API')) + api.register(app, web_api_root=app.config.get('WEB_API')) return app diff --git a/core/admin/mailu/api/__init__.py b/core/admin/mailu/api/__init__.py index 44cda962..3fd9d495 100644 --- a/core/admin/mailu/api/__init__.py +++ b/core/admin/mailu/api/__init__.py @@ -1,24 +1,20 @@ from flask import redirect, url_for from flask_restx import apidoc -from . import v1 +from . import v1 as APIv1 -def register(app, web_api): +def register(app, web_api_root): - ACTIVE=v1 - ROOT=web_api - v1.app = app + APIv1.app = app # register api bluprint(s) - apidoc.apidoc.url_prefix = f'{ROOT}/v{int(v1.VERSION)}' - v1.api_token = app.config['API_TOKEN'] - app.register_blueprint(v1.blueprint, url_prefix=f'{ROOT}/v{int(v1.VERSION)}') - - + apidoc.apidoc.url_prefix = f'{web_api_root}/v{int(APIv1.VERSION)}' + APIv1.api_token = app.config['API_TOKEN'] + app.register_blueprint(APIv1.blueprint, url_prefix=f'{web_api_root}/v{int(APIv1.VERSION)}') # add redirect to current api version - @app.route(f'{ROOT}/') + @app.route(f'{web_api_root}/') def redir(): - return redirect(url_for(f'{ACTIVE.blueprint.name}.root')) + return redirect(url_for(f'{APIv1.blueprint.name}.root')) # swagger ui config app.config.SWAGGER_UI_DOC_EXPANSION = 'list' diff --git a/core/admin/mailu/api/common.py b/core/admin/mailu/api/common.py index 26372bd9..1b69f4d7 100644 --- a/core/admin/mailu/api/common.py +++ b/core/admin/mailu/api/common.py @@ -5,13 +5,15 @@ import flask import hmac from functools import wraps from flask_restx import abort +from sqlalchemy.sql.expression import label -def fqdn_in_use(*names): - for name in names: - for model in models.Domain, models.Alternative, models.Relay: - if model.query.get(name): - return model - return None +def fqdn_in_use(name): + d = models.db.session.query(label('name', models.Domain.name)) + a = models.db.session.query(label('name', models.Alternative.name)) + r = models.db.session.query(label('name', models.Relay.name)) + if d.union_all(a).union_all(r).filter_by(name=name).count() > 0: + return True + return False """ Decorator for validating api token for authentication """ def api_token_authorization(func): @@ -20,14 +22,12 @@ def api_token_authorization(func): client_ip = flask.request.headers.get('X-Real-IP', flask.request.remote_addr) if utils.limiter.should_rate_limit_ip(client_ip): abort(429, 'Too many attempts from your IP (rate-limit)' ) - if (request.args.get('api_token') == '' or - request.args.get('api_token') == None): - abort(401, 'A valid API token is expected as query string parameter') - if not hmac.compare_digest(request.args.get('api_token'), v1.api_token): + if not request.headers.get('Authorization'): + abort(401, 'A valid API token is expected which is provided as request header') + if not hmac.compare_digest(request.headers.get('Authorization'), v1.api_token): utils.limiter.rate_limit_ip(client_ip) flask.current_app.logger.warn(f'Invalid API token provided by {client_ip}.') - abort(403, 'A valid API token is expected as query string parameter') - else: - flask.current_app.logger.info(f'Valid API token provided by {client_ip}.') + abort(403, 'A valid API token is expected which is provided as request header') + flask.current_app.logger.info(f'Valid API token provided by {client_ip}.') return func(*args, **kwds) return decorated_function diff --git a/core/admin/mailu/api/v1/__init__.py b/core/admin/mailu/api/v1/__init__.py index 5cc878c9..5cb1fc82 100644 --- a/core/admin/mailu/api/v1/__init__.py +++ b/core/admin/mailu/api/v1/__init__.py @@ -8,20 +8,19 @@ api_token = None blueprint = Blueprint(f'api_v{int(VERSION)}', __name__) authorization = { - 'apikey': { + 'Bearer': { 'type': 'apiKey', - 'in': 'query', - 'name': 'api_token' + 'in': 'header', + 'name': 'Authorization' } } - api = Api( blueprint, version=f'{VERSION:.1f}', title='Mailu API', default_label='Mailu', validate=True, authorizations=authorization, - security='apikey', + security='Bearer', doc='/swaggerui/' ) diff --git a/core/admin/mailu/api/v1/alias.py b/core/admin/mailu/api/v1/alias.py index 29c03195..7635aa19 100644 --- a/core/admin/mailu/api/v1/alias.py +++ b/core/admin/mailu/api/v1/alias.py @@ -24,7 +24,7 @@ alias_fields = alias.inherit('Alias',alias_fields_update, { class Aliases(Resource): @alias.doc('list_alias') @alias.marshal_with(alias_fields, as_list=True, skip_none=True, mask=None) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def get(self): """ List aliases """ @@ -35,7 +35,7 @@ class Aliases(Resource): @alias.response(200, 'Success', response_fields) @alias.response(400, 'Input validation exception', response_fields) @alias.response(409, 'Duplicate alias', response_fields) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def post(self): """ Create a new alias """ @@ -60,7 +60,7 @@ class Alias(Resource): @alias.doc('find_alias') @alias.response(200, 'Success', alias_fields) @alias.response(404, 'Alias not found', response_fields) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def get(self, alias): """ Find alias """ @@ -75,7 +75,7 @@ class Alias(Resource): @alias.response(200, 'Success', response_fields) @alias.response(404, 'Alias not found', response_fields) @alias.response(400, 'Input validation exception', response_fields) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def put(self, alias): """ Update alias """ @@ -97,7 +97,7 @@ class Alias(Resource): @alias.doc('delete_alias') @alias.response(200, 'Success', response_fields) @alias.response(404, 'Alias not found', response_fields) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def delete(self, alias): """ Delete alias """ @@ -113,7 +113,7 @@ class AliasWithDest(Resource): @alias.doc('find_alias_filter_domain') @alias.response(200, 'Success', alias_fields) @alias.response(404, 'Alias or domain not found', response_fields) - @alias.doc(security='apikey') + @alias.doc(security='Bearer') @common.api_token_authorization def get(self, domain): """ Find aliases of domain """ diff --git a/core/admin/mailu/api/v1/domains.py b/core/admin/mailu/api/v1/domains.py index 35393b81..4eabe22e 100644 --- a/core/admin/mailu/api/v1/domains.py +++ b/core/admin/mailu/api/v1/domains.py @@ -78,7 +78,7 @@ alternative_fields = api.model('AlternativeDomain', { class Domains(Resource): @dom.doc('list_domain') @dom.marshal_with(domain_fields_get, as_list=True, skip_none=True, mask=None) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def get(self): """ List domains """ @@ -89,7 +89,7 @@ class Domains(Resource): @dom.response(200, 'Success', response_fields) @dom.response(400, 'Input validation exception', response_fields) @dom.response(409, 'Duplicate domain/alternative name', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def post(self): """ Create a new domain """ @@ -133,7 +133,7 @@ class Domain(Resource): @dom.doc('find_domain') @dom.response(200, 'Success', domain_fields) @dom.response(404, 'Domain not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def get(self, domain): """ Find domain by name """ @@ -150,7 +150,7 @@ class Domain(Resource): @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'Domain not found', response_fields) @dom.response(409, 'Duplicate domain/alternative name', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def put(self, domain): """ Update an existing domain """ @@ -194,7 +194,7 @@ class Domain(Resource): @dom.response(200, 'Success', response_fields) @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'Domain not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def delete(self, domain): """ Delete domain """ @@ -213,7 +213,7 @@ class Domain(Resource): @dom.response(200, 'Success', response_fields) @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'Domain not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def post(self, domain): """ Generate new DKIM/DMARC keys for domain """ @@ -232,7 +232,7 @@ class Manager(Resource): @dom.marshal_with(manager_fields, as_list=True, skip_none=True, mask=None) @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'domain not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def get(self, domain): """ List managers of domain """ @@ -249,7 +249,7 @@ class Manager(Resource): @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'User or domain not found', response_fields) @dom.response(409, 'Duplicate domain manager', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def post(self, domain): """ Create a new domain manager """ @@ -275,7 +275,7 @@ class Domain(Resource): @dom.doc('find_manager') @dom.response(200, 'Success', manager_fields) @dom.response(404, 'Manager not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def get(self, domain, email): """ Find manager by email address """ @@ -301,7 +301,7 @@ class Domain(Resource): @dom.response(200, 'Success', response_fields) @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'Manager not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def delete(self, domain, email): if not validators.email(email): @@ -327,7 +327,7 @@ class User(Resource): @dom.marshal_with(user.user_fields_get, as_list=True, skip_none=True, mask=None) @dom.response(400, 'Input validation exception', response_fields) @dom.response(404, 'Domain not found', response_fields) - @dom.doc(security='apikey') + @dom.doc(security='Bearer') @common.api_token_authorization def get(self, domain): """ List users from domain """ @@ -343,7 +343,7 @@ class Alternatives(Resource): @alt.doc('list_alternative') @alt.marshal_with(alternative_fields, as_list=True, skip_none=True, mask=None) - @alt.doc(security='apikey') + @alt.doc(security='Bearer') @common.api_token_authorization def get(self): """ List alternatives """ @@ -356,7 +356,7 @@ class Alternatives(Resource): @alt.response(400, 'Input validation exception', response_fields) @alt.response(404, 'Domain not found or missing', response_fields) @alt.response(409, 'Duplicate alternative domain name', response_fields) - @alt.doc(security='apikey') + @alt.doc(security='Bearer') @common.api_token_authorization def post(self): """ Create new alternative (for domain) """ @@ -379,7 +379,7 @@ class Alternatives(Resource): @alt.route('/') class Alternative(Resource): @alt.doc('find_alternative') - @alt.doc(security='apikey') + @alt.doc(security='Bearer') @common.api_token_authorization def get(self, alt): """ Find alternative (of domain) """ @@ -395,7 +395,7 @@ class Alternative(Resource): @alt.response(400, 'Input validation exception', response_fields) @alt.response(404, 'Alternative/Domain not found or missing', response_fields) @alt.response(409, 'Duplicate domain name', response_fields) - @alt.doc(security='apikey') + @alt.doc(security='Bearer') @common.api_token_authorization def delete(self, alt): """ Delete alternative (for domain) """ diff --git a/core/admin/mailu/api/v1/relay.py b/core/admin/mailu/api/v1/relay.py index 67089d9a..6b52c7d5 100644 --- a/core/admin/mailu/api/v1/relay.py +++ b/core/admin/mailu/api/v1/relay.py @@ -24,7 +24,7 @@ relay_fields_update = api.model('RelayUpdate', { class Relays(Resource): @relay.doc('list_relays') @relay.marshal_with(relay_fields, as_list=True, skip_none=True, mask=None) - @relay.doc(security='apikey') + @relay.doc(security='Bearer') @common.api_token_authorization def get(self): "List relays" @@ -35,7 +35,7 @@ class Relays(Resource): @relay.response(200, 'Success', response_fields) @relay.response(400, 'Input validation exception') @relay.response(409, 'Duplicate relay', response_fields) - @relay.doc(security='apikey') + @relay.doc(security='Bearer') @common.api_token_authorization def post(self): """ Create relay """ @@ -61,7 +61,7 @@ class Relay(Resource): @relay.doc('find_relay') @relay.response(400, 'Input validation exception', response_fields) @relay.response(404, 'Relay not found', response_fields) - @relay.doc(security='apikey') + @relay.doc(security='Bearer') @common.api_token_authorization def get(self, name): """ Find relay """ @@ -79,7 +79,7 @@ class Relay(Resource): @relay.response(400, 'Input validation exception', response_fields) @relay.response(404, 'Relay not found', response_fields) @relay.response(409, 'Duplicate relay', response_fields) - @relay.doc(security='apikey') + @relay.doc(security='Bearer') @common.api_token_authorization def put(self, name): """ Update relay """ @@ -105,7 +105,7 @@ class Relay(Resource): @relay.response(200, 'Success', response_fields) @relay.response(400, 'Input validation exception', response_fields) @relay.response(404, 'Relay not found', response_fields) - @relay.doc(security='apikey') + @relay.doc(security='Bearer') @common.api_token_authorization def delete(self, name): """ Delete relay """ diff --git a/core/admin/mailu/api/v1/user.py b/core/admin/mailu/api/v1/user.py index 01dac786..cbe1fe6d 100644 --- a/core/admin/mailu/api/v1/user.py +++ b/core/admin/mailu/api/v1/user.py @@ -64,7 +64,7 @@ user_fields_put = api.model('UserUpdate', { class Users(Resource): @user.doc('list_users') @user.marshal_with(user_fields_get, as_list=True, skip_none=True, mask=None) - @user.doc(security='apikey') + @user.doc(security='Bearer') @common.api_token_authorization def get(self): "List users" @@ -75,7 +75,7 @@ class Users(Resource): @user.response(200, 'Success', response_fields) @user.response(400, 'Input validation exception') @user.response(409, 'Duplicate user', response_fields) - @user.doc(security='apikey') + @user.doc(security='Bearer') @common.api_token_authorization def post(self): """ Create user """ @@ -141,7 +141,7 @@ class User(Resource): @user.doc('find_user') @user.response(400, 'Input validation exception', response_fields) @user.response(404, 'User not found', response_fields) - @user.doc(security='apikey') + @user.doc(security='Bearer') @common.api_token_authorization def get(self, email): """ Find user """ @@ -159,7 +159,7 @@ class User(Resource): @user.response(400, 'Input validation exception', response_fields) @user.response(404, 'User not found', response_fields) @user.response(409, 'Duplicate user', response_fields) - @user.doc(security='apikey') + @user.doc(security='Bearer') @common.api_token_authorization def put(self, email): """ Update user """ @@ -222,7 +222,7 @@ class User(Resource): @user.response(200, 'Success', response_fields) @user.response(400, 'Input validation exception', response_fields) @user.response(404, 'User not found', response_fields) - @user.doc(security='apikey') + @user.doc(security='Bearer') @common.api_token_authorization def delete(self, email): """ Delete user """ diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 612bcb18..b77e2cf1 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -71,7 +71,7 @@ DEFAULT_CONFIG = { 'LOGO_BACKGROUND': None, 'API': False, # Advanced settings - 'API' : 'false', + 'API' : False, 'WEB_API' : '/api', 'API_TOKEN': None, 'LOG_LEVEL': 'WARNING', diff --git a/docs/configuration.rst b/docs/configuration.rst index 3a7d8c66..b37bec08 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -203,14 +203,13 @@ Depending on your particular deployment you most probably will want to change th Advanced settings ----------------- -The ``API`` (default: False) setting controls if the API endpoint is publicly -reachable. +The ``API`` (default: False) setting controls if the API endpoint is reachable. The ``WEB_API`` (default: /api) setting configures the endpoint that the API listens on publicly&interally. The path must always start with a leading slash. The ``API_TOKEN`` (default: None) enables the API endpoint. This token must be -passed as query parameter with requests to the API as authentication token. +passed as request header to the API as authentication token. The ``CREDENTIAL_ROUNDS`` (default: 12) setting is the number of rounds used by the password hashing scheme. The number of rounds can be reduced in case faster