From c39c3a0ab6c5c1e961868b7acb73d3059a79a844 Mon Sep 17 00:00:00 2001 From: Andrea Mistrali Date: Mon, 29 Jul 2024 13:39:25 +0200 Subject: [PATCH] Fixed permissions and referrers --- app/__init__.py | 10 ++--- app/lib.py | 82 +++++++++++++++++++++++--------------- app/templates/index.html | 16 +++++++- app/templates/node.html | 15 ++++++- app/templates/nodes.html | 2 +- app/templates/user.html | 4 +- app/views/main.py | 26 ++++++------ app/views/rest.py | 86 +++++++++++----------------------------- config.py | 2 +- 9 files changed, 124 insertions(+), 119 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index dc00fe3..dc420de 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -30,7 +30,7 @@ def create_app(environment='development'): config[env].configure(app) app.config['APP_TZ'] = os.environ.get('TZ', 'UTC') app.config['ADMIN_GROUPS'] = list( - map(str.strip, app.config['ADMIN_GROUPS'].split(','))) + map(str.strip, app.config.get('ADMIN_GROUPS', "").split(','))) app.logger.debug(f"admin groups: {app.config['ADMIN_GROUPS']}") app.logger.info("middleware init: mobility") @@ -41,12 +41,12 @@ def create_app(environment='development'): # Register blueprints. from .views import main_blueprint, rest_blueprint - app.logger.info(f"registering main blueprint with prefix '{ - main_blueprint.url_prefix}'") + app.logger.info(f"register blueprint: 'main' [prefix '{ + main_blueprint.url_prefix}']") app.register_blueprint(main_blueprint) - app.logger.info(f"registering rest blueprint with prefix '{ - rest_blueprint.url_prefix}'") + app.logger.info(f"register blueprint: 'rest' [prefix '{ + rest_blueprint.url_prefix}']") app.register_blueprint(rest_blueprint) app.logger.info("jinja2 custom filters loaded") diff --git a/app/lib.py b/app/lib.py index 84364f2..303650e 100644 --- a/app/lib.py +++ b/app/lib.py @@ -2,7 +2,7 @@ import os import functools from flask import request, abort, current_app -from flask import session as flask_session +from flask import session as flask_session, jsonify from flask_pyoidc import OIDCAuthentication as _OIDCAuth from flask_pyoidc.user_session import UserSession from flask_pyoidc.provider_configuration import ProviderConfiguration, ClientMetadata @@ -21,19 +21,6 @@ def remote_ip() -> str: return str(request.environ.get('REMOTE_ADDR')) -def username() -> str: - userinfo = flask_session['userinfo'] - return userinfo['email'].split('@')[0] - - -def login_name() -> str: - userinfo = flask_session['userinfo'] - if 'preferred_username' in userinfo: - return userinfo['preferred_username'] - else: - return username() - - def webMode() -> bool: is_gunicorn = "gunicorn" in os.environ.get('SERVER_SOFTWARE', '') is_werkzeug = os.environ.get('WERKZEUG_RUN_MAIN', False) == "true" @@ -64,6 +51,53 @@ class OIDCAuthentication(_OIDCAuth): super().init_app(app) app.auth = self + @property + def username(self) -> str: + userinfo = flask_session['userinfo'] + return userinfo['email'].split('@')[0] + + @property + def login_name(self) -> str: + userinfo = flask_session['userinfo'] + return userinfo.get('preferred_username', self.username) + + @property + def isAdmin(self) -> bool: + userinfo = flask_session['userinfo'] + user_groups = userinfo.get('groups', []) + with current_app.app_context(): + admin_groups = current_app.config.get('ADMIN_GROUPS', []) + admin_users = current_app.config.get('ADMIN_USERS', []) + + authorized_groups = set(admin_groups).intersection(user_groups) + + if len(authorized_groups): + log.debug(f"'{self.username}' is a member of { + authorized_groups}") + return True + + if self.username in admin_users: + log.debug(f"'{self.username}' is an admin user") + return True + return False + + @property + def unathorized(self): + response = jsonify( + {'message': f"not authorized", + 'comment': 'nice try, info logged', + 'logged': f"'{self.username}@{remote_ip()}", + 'result': 'GO AWAY!'}) + log.warning( + f"user '{self.username}' attempted denied operation from {remote_ip()}") + return response, 403 + + def userOrAdmin(self, username: str): + """ + Check is the current user is an admin OR the username passed as argument + """ + return self.isAdmin or self.username == username + def authorize(self, provider_name: str, authz_fn: Callable, **kwargs): if provider_name not in self._provider_configurations: raise ValueError( @@ -76,7 +110,7 @@ class OIDCAuthentication(_OIDCAuth): # Decorator def oidc_decorator(view_func): - @ functools.wraps(view_func) + @functools.wraps(view_func) def wrapper(*args, **kwargs): # Retrieve session and client session = UserSession(flask_session, provider_name) @@ -165,23 +199,7 @@ class OIDCAuthentication(_OIDCAuth): """ def _authz_fn(session) -> bool: - user_groups = session.userinfo.get('groups', []) - username = session.userinfo.get('preferred_username', "") - with current_app.app_context(): - admin_groups = current_app.config.get('ADMIN_GROUPS', []) - admin_users = current_app.config.get('ADMIN_USERS', []) - - authorized_groups = set(admin_groups).intersection(user_groups) - - if len(authorized_groups): - log.debug(f"'{username}' is a member of { - authorized_groups}") - return True - - if username in admin_users: - log.debug(f"'{username}' is an admin user") - return True - return False + return self.isAdmin return self.authorize(provider_name, authz_fn=_authz_fn) diff --git a/app/templates/index.html b/app/templates/index.html index 231994b..982ec7a 100644 --- a/app/templates/index.html +++ b/app/templates/index.html @@ -55,6 +55,18 @@ {% endfor %} +
+
+ access level +
+
+ {% if auth.isAdmin %} + ADMIN + {% else %} + USER + {% endif %} +
+

your devices

@@ -67,7 +79,7 @@ {% for node in userNodeList %}
- {{ node.givenName}} + {{ node.givenName}}
@@ -84,7 +96,7 @@
- + diff --git a/app/templates/node.html b/app/templates/node.html index c3a1e94..9f3f232 100644 --- a/app/templates/node.html +++ b/app/templates/node.html @@ -74,7 +74,10 @@ {% for ip in node.ipAddresses %}
- {{ ip }} + + {{ ip }} +
{% endfor %} @@ -210,3 +213,13 @@
{% endblock %} + +{% block scripts %} + +{% endblock %} diff --git a/app/templates/nodes.html b/app/templates/nodes.html index 37c35c6..3dce487 100644 --- a/app/templates/nodes.html +++ b/app/templates/nodes.html @@ -52,7 +52,7 @@ {% if node.expireDate and not node.expired %} - + diff --git a/app/templates/user.html b/app/templates/user.html index 41e6a90..27aecb2 100644 --- a/app/templates/user.html +++ b/app/templates/user.html @@ -49,7 +49,7 @@ {% if node.expireDate and not node.expired %} - + @@ -57,7 +57,7 @@ {% endif %} - + diff --git a/app/views/main.py b/app/views/main.py index 840a217..4b05f30 100644 --- a/app/views/main.py +++ b/app/views/main.py @@ -26,6 +26,15 @@ def health(): return jsonify(dict(status="OK", version=current_app.config['APP_VERSION'])) +@main_blueprint.route('/token', methods=['GET', 'POST']) +@auth.authorize_admins('default') +def token(): + user_session = UserSession(session) + return jsonify(access_token=user_session.access_token, + id_token=user_session.id_token, + userinfo=user_session.userinfo) + + @main_blueprint.route('/', methods=['GET', 'POST']) @auth.access_control('default') def index(): @@ -34,17 +43,8 @@ def index(): userNodeList = [n for n in Node().list().nodes if n.user.name == hs_user] return render_template('index.html', userNodeList=userNodeList, - session=user_session) - - -@main_blueprint.route('/token', methods=['GET', 'POST']) -@auth.authorize_admins('default') -def token(): - user_session = UserSession(session) - # return jsonify(user_session.userinfo) - return jsonify(access_token=user_session.access_token, - id_token=user_session.id_token, - userinfo=user_session.userinfo) + session=user_session, + auth=auth) @main_blueprint.route('/logout') @@ -62,12 +62,14 @@ def nodes(): @main_blueprint.route('/node/', methods=['GET']) -@auth.authorize_admins('default') +@auth.access_control('default') def node(nodeId): # There is a bug in HS api with retrieving a single node # and we added a workaround to hsapi, so node.get() returns a # v1Node object instead of v1NodeResponse, so we access directly # `node`, instead of `node.node` + if not auth.userOrAdmin(auth.username): + return auth.unathorized node = Node().get(nodeId) routes = Node().routes(nodeId) isExitNode = any( diff --git a/app/views/rest.py b/app/views/rest.py index 7b59624..3bfb381 100644 --- a/app/views/rest.py +++ b/app/views/rest.py @@ -4,7 +4,7 @@ from flask import Blueprint, request from flask import redirect, url_for from app import auth -from ..lib import login_name, username +# from ..lib import login_name, username from flask import jsonify @@ -35,86 +35,46 @@ def routeToggle(routeId: int): action = 'enabled' log.info( f"route '{route.prefix}' via '{route.node.givenName}'" - f"{action} by '{username()}'") - return redirect(url_for("main.routes")) + f"{action} by '{auth.username}'") + return redirect(request.referrer) @rest_blueprint.route('/node//expire', methods=['GET']) -@auth.authorize_admins('default') +@auth.access_control('default') def expireNode(nodeId: int): - """ - This expires a node from the node page. - The difference from above is that it returns to the /node/nodeId page - """ - Node().expire(nodeId) - log.info(f"node '{nodeId}' expired by '{username()}'") - return redirect(url_for("main.node", nodeId=nodeId)) - - -@rest_blueprint.route('/node//user-expire', methods=['GET']) -@auth.authorize_admins('default') -def expireNodeUser(nodeId: int): """ This expires a node from the node page. The difference from above is that it returns to the /node/nodeId page """ node = Node().get(nodeId) - userName = node.user.name + if not auth.userOrAdmin(node.user.name): + return auth.unathorized Node().expire(nodeId) - log.info(f"node '{nodeId}' expired by '{username()}'") - return redirect(url_for("main.user", userName=userName)) + log.info(f"node '{nodeId}' expired by '{auth.username}'") + return redirect(request.referrer) -@rest_blueprint.route('/node//list-expire', methods=['GET']) -@auth.authorize_admins('default') -def expireNodeList(nodeId: int): - """ - This expires a node from the node list. - The difference from above is that it returns to the /nodes page - """ - Node().expire(nodeId) - log.info(f"node '{nodeId}' expired by '{username()}'") - return redirect(url_for("main.nodes")) - - -@ rest_blueprint.route('/node//delete', methods=['GET']) -@ auth.authorize_admins('default') +@rest_blueprint.route('/node//delete', methods=['GET']) +@auth.access_control('default') def deleteNode(nodeId: int): - Node().delete(nodeId) - log.info(f"node '{nodeId}' deleted by '{username()}'") - return redirect(url_for("main.nodes")) - - -@rest_blueprint.route('/node//delete-own', methods=['GET']) -@auth.access_control('default') -def deleteOwnNode(nodeId: int): node = Node().get(nodeId) - if node.user.name != username(): - response = jsonify({'message': 'not authorized'}) - return response, 401 + if not auth.userOrAdmin(node.user.name): + return auth.unathorized + Node().expire(nodeId) Node().delete(nodeId) - log.info(f"'{username()}' delete their own node '{nodeId}'") - return redirect(url_for("main.index")) + log.info(f"node '{nodeId}' deleted by '{auth.username}'") + return redirect(request.referrer) -@rest_blueprint.route('/node//delete-user', methods=['GET']) -@auth.access_control('default') -def deleteNodeUser(nodeId: int): - node = Node().get(nodeId) - Node().delete(nodeId) - log.info(f"'{username()}' delete their own node '{nodeId}'") - return redirect(url_for("main.user", userName=node.user.name)) - - -@ rest_blueprint.route('/node//rename/', methods=['GET']) -@ auth.authorize_admins('default') +@rest_blueprint.route('/node//rename/', methods=['GET']) +@auth.authorize_admins('default') def renameNode(nodeId: int, newName: str): Node().rename(nodeId, newName) return jsonify(dict(newName=newName)) -@ rest_blueprint.route('/user//delete', methods=['GET']) -@ auth.authorize_admins('default') +@rest_blueprint.route('/user//delete', methods=['GET']) +@auth.authorize_admins('default') def deleteUser(userName: str): nodes = Node().byUser(userName) for node in nodes.nodes: @@ -124,8 +84,8 @@ def deleteUser(userName: str): return redirect(url_for("main.users")) -@ rest_blueprint.route('/user//pakcreate', methods=['POST']) -@ auth.authorize_admins('default') +@rest_blueprint.route('/user//pakcreate', methods=['POST']) +@auth.authorize_admins('default') def createPKA(userName: str): data = request.json log.debug(data) @@ -138,8 +98,8 @@ def createPKA(userName: str): return jsonify(dict(key=pak.preAuthKey.key)) -@ rest_blueprint.route('/user//expire/', methods=['GET']) -@ auth.authorize_admins('default') +@rest_blueprint.route('/user//expire/', methods=['GET']) +@auth.authorize_admins('default') def expirePKA(userName: str, key: str): log.debug(key) req = v1ExpirePreAuthKeyRequest(user=userName, key=key) diff --git a/config.py b/config.py index 45ae942..ab2f506 100644 --- a/config.py +++ b/config.py @@ -17,7 +17,7 @@ class BaseConfig(object): # All the followinf vars can be overriden # in the environment, using `HSMAN_` prefix SECRET_KEY = "secreto" - ADMIN_GROUPS = ["adminGroup"] + ADMIN_GROUPS = "adminGroup" OIDC_CLIENT_ID = 'client-id' OIDC_CLIENT_SECRET = 'client-secreto' OIDC_URL = "https://myidp.example.com/auth"