diff --git a/tests/test_25_environments.py b/tests/test_25_environments.py index f9622b3d6..957d0611e 100644 --- a/tests/test_25_environments.py +++ b/tests/test_25_environments.py @@ -1,6 +1,8 @@ import pytest from fixtures.utils import retry_fast from selenium.webdriver.common.by import By +from selenium.webdriver.support import expected_conditions as EC +from selenium.webdriver.support.ui import WebDriverWait def test_create(prod, browser): @@ -44,7 +46,11 @@ def test_add_member(alice_member, browse_prod_members): def test_remove_member(alice_member, browse_prod_members): browser = browse_prod_members browser.select("tbody tr:nth-child(1) button").click() - username = browser.select(".modal-body strong").text + wait = WebDriverWait(browser, 10) + element = wait.until( + EC.visibility_of_element_located((By.CSS_SELECTOR, ".modal-body strong")) + ) + username = element.text assert username.startswith("a") # admin or alice browser.select("#buttonDelete").click() browser.absent("tbody tr:nth-child(2)") diff --git a/ui/.husky/pre-commit b/ui/.husky/pre-commit index f86f55459..c5b67da06 100755 --- a/ui/.husky/pre-commit +++ b/ui/.husky/pre-commit @@ -3,5 +3,3 @@ cd ui npx lint-staged -ruff check -ruff format --check diff --git a/ui/package.json b/ui/package.json index fed713d81..e568f631b 100644 --- a/ui/package.json +++ b/ui/package.json @@ -44,7 +44,11 @@ "vite": "^8.1.5" }, "lint-staged": { - "*.{js,css,vue}": "prettier --write" + "*.{js,css,vue}": "prettier --write", + "*.py": [ + "ruff check", + "ruff format --check" + ] }, "allowScripts": { "vue-demi@0.14.8": true, diff --git a/ui/share/sql/dev-fixture.sql b/ui/share/sql/dev-fixture.sql index 19195c71a..671ff4feb 100644 --- a/ui/share/sql/dev-fixture.sql +++ b/ui/share/sql/dev-fixture.sql @@ -33,6 +33,11 @@ VALUES ('admin', (SELECT id FROM application.groups WHERE name = 'stable/dba')), ('admin', (SELECT id FROM application.groups WHERE name = 'mass/dba')); +INSERT INTO application.acl (role, action, resource) +VALUES +('trn:temboard:core:group:mass/dba', '*', 'trn:temboard:core:instance:mass'), +('trn:temboard:core:group:stable/dba', '*', 'trn:temboard:core:instance:stable'); + -- Pre-register agents INSERT INTO application.instances diff --git a/ui/temboardui/acl.py b/ui/temboardui/acl.py new file mode 100644 index 000000000..6c55432b4 --- /dev/null +++ b/ui/temboardui/acl.py @@ -0,0 +1,91 @@ +import logging + +from flask import abort + +logger = logging.getLogger(__name__) + + +class TRN: + def __init__(self, scope, type, name): + self.scope = scope + self.type = type + self.name = name + + @staticmethod + def parse(trn): + elems = str.split(trn, ":") + if len(elems) < 5: + raise Exception("Malformed TRN") + return TRN(elems[2], elems[3], elems[4]) + + def __str__(self): + return f"trn:temboard:{self.scope}:{self.type}:{self.name}" + + def parent(self): + parent = TRN(self.scope, self.type, self.name) + + if self.name != "*": + parent.name = "*" + if "/" in self.name: + names = str.split(self.name, "/") + parent.name = "/".join(names[:-1]) + return parent + if self.type != "*": + parent.type = "*" + return parent + parent.scope = "*" + return parent + + def expand(self): + trns = ["*"] + trn = TRN(self.scope, self.type, self.name) + while str(trn) != "trn:temboard:*:*:*": + if str(trn) not in trns: + trns.append(str(trn)) + trn = trn.parent() + trns.append(str(trn)) + return trns + + +class ACLResult: + def __init__(self, role, action, resource, decision="allowed", statements=None): + self.role = role + self.action = action + self.resource = resource + self.decision = decision + self.statements = statements or [] + + def raise_for_decision(self): + log_prefix = "Access <%s %s on %s> " + log_args = (self.role, self.action, self.resource or "*") + + if self.decision == "allowed": + logger.debug( + log_prefix + "allowed by %s", + *log_args, + ", ".join(repr(s) for s in self.statements), + ) + return True + else: + if self.decision == "implicitDeny": + logger.debug(log_prefix + "implicitly denied.", *log_args) + else: + logger.debug( + log_prefix + "denied by %s", + *log_args, + ", ".join(repr(s) for s in self.statements if s.deny), + ) + raise abort(403) + + +def expand_actions(action): + """Returns the list of pattern relevant for this action.""" + actions = ["*"] + if action != "*": + method, _, endpoint = action.partition(":") + if method != "*": + actions.append("*:" + endpoint) + elif endpoint != "*": + actions.append(method + ":*") + actions.append(action) + return actions diff --git a/ui/temboardui/application.py b/ui/temboardui/application.py index c46c8167c..d6ae98b36 100644 --- a/ui/temboardui/application.py +++ b/ui/temboardui/application.py @@ -12,6 +12,7 @@ from flask import current_app as app from itsdangerous import URLSafeTimedSerializer +from sqlalchemy.orm import selectinload from sqlalchemy.orm.exc import NoResultFound from temboardui.errors import TemboardUIError @@ -96,6 +97,7 @@ def get_role_by_cookie(session, content): try: role = ( session.query(Role) + .options(selectinload(Role.groups)) .filter(Role.role_name == str(c_role_name), Role.is_active.is_(True)) .one() ) diff --git a/ui/temboardui/handlers/settings/metadata.py b/ui/temboardui/handlers/settings/metadata.py index 0c24b6f28..e3b2790ba 100644 --- a/ui/temboardui/handlers/settings/metadata.py +++ b/ui/temboardui/handlers/settings/metadata.py @@ -1,6 +1,6 @@ import logging -from temboardui.web.tornado import admin_required, app, render_template +from temboardui.web.tornado import app, render_template from ...version import inspect_versions @@ -8,7 +8,6 @@ @app.route(r"/settings/metadata") -@admin_required def metadata(request): versions_info = inspect_versions() infos = { diff --git a/ui/temboardui/model/orm.py b/ui/temboardui/model/orm.py index ab2f8ee4d..3619f9cb9 100644 --- a/ui/temboardui/model/orm.py +++ b/ui/temboardui/model/orm.py @@ -8,6 +8,7 @@ from sqlalchemy.orm import Query, relationship from temboardtoolkit.utils import utcnow +from ..acl import TRN from . import QUERIES Model = declarative_base() @@ -62,6 +63,15 @@ def select_secret(cls, secret): def expired(self): return self.edate < utcnow() + def trn(self): + return TRN("core", "apikey", str(self.id)) + + def role_trns(self): + return self.trn().expand() + + def resource_trns(self): + return self.trn().expand() + class Plugin(Model): __tablename__ = "plugins" @@ -181,8 +191,8 @@ def asdict(self): phone=self.role_phone, active=self.is_active, admin=self.is_admin, - groups=[g.name for g in self.groups], - environments=[g.environment.name for g in self.groups], + groups=[g.name for g in self.groups if g.environment], + environments=[g.environment.name for g in self.groups if g.environment], ) def select_environments(self): @@ -199,6 +209,19 @@ def select_instances(self): .columns(Instance.__mapper__.c.values()) ) + def trn(self): + return TRN("core", "user", self.role_name) + + def role_trns(self): + trns = set() + trns.update(self.trn().expand()) + for g in self.groups: + trns.update(g.trn().expand()) + return trns + + def resource_trns(self): + return self.trn().expand() + class StubRole: # Fake object for roles not in database. @@ -256,6 +279,12 @@ def delete_member(self, name, username): group=name, role=username ) + def trn(self): + return TRN("core", "group", self.name) + + def resource_trns(self): + return self.trn().expand() + class Environment(Model): __tablename__ = "environments" @@ -327,6 +356,12 @@ def asdict(self): dba_group=self.dba_group.name, ) + def trn(self): + return TRN("core", "environment", self.name) + + def resource_trn(self): + return self.trn().expand() + class Instance(Model): __tablename__ = "instances" @@ -522,3 +557,63 @@ def enable_plugin(self, plugin): def disable_plugin(self, plugin): return Plugin.delete(self, plugin) + + def trn(self): + return TRN( + "core", + "instance", + f"{self.environment.name}/{self.agent_address}:{self.agent_port}", + ) + + def resource_trns(self): + return self.trn().expand() + + +class ACLRule(Model): + __tablename__ = "acl" + __table_args__ = {"schema": "application"} + + id = Column(types.BigInteger, primary_key=True) + role = Column(types.UnicodeText) + action = Column(types.UnicodeText) + resource = Column(types.UnicodeText) + deny = Column(types.Boolean) + cdate = Column(types.TIMESTAMP(timezone=True)) + origin = Column(types.UnicodeText) + + @classmethod + def insert(cls, role, action, resource, deny=False): + return Query(cls).from_statement( + text(QUERIES["acl-insert"]).bindparams( + role=role, action=action, resource=resource, deny=deny + ) + ) + + @classmethod + def delete(cls, role, action, resource): + return Query(cls).from_statement( + text(QUERIES["acl-delete"]).bindparams( + role=role, action=action, resource=resource + ) + ) + + @classmethod + def match(cls, roles, actions, resources): + return Query(cls).from_statement( + text(QUERIES["acl-get"]).bindparams( + roles=roles, actions=actions, resources=resources + ) + ) + + def __repr__(self): + return f"" + + +class Anonymous: + @staticmethod + def trn(): + return TRN("*", "*", "*") + + @staticmethod + def role_trns(): + return [Anonymous.trn()] diff --git a/ui/temboardui/model/queries/acl-delete.sql b/ui/temboardui/model/queries/acl-delete.sql new file mode 100644 index 000000000..80da09873 --- /dev/null +++ b/ui/temboardui/model/queries/acl-delete.sql @@ -0,0 +1,6 @@ +DELETE FROM + application.acl +WHERE + role = :role + AND action = :action + AND resource = :resource RETURNING *; diff --git a/ui/temboardui/model/queries/acl-get.sql b/ui/temboardui/model/queries/acl-get.sql new file mode 100644 index 000000000..0297d7fbd --- /dev/null +++ b/ui/temboardui/model/queries/acl-get.sql @@ -0,0 +1,8 @@ +SELECT + * +FROM + application.acl +WHERE + role = ANY(:roles) + AND ACTION = ANY(:actions) + AND resource = ANY(:resources); \ No newline at end of file diff --git a/ui/temboardui/model/queries/acl-insert.sql b/ui/temboardui/model/queries/acl-insert.sql new file mode 100644 index 000000000..afcc3b17e --- /dev/null +++ b/ui/temboardui/model/queries/acl-insert.sql @@ -0,0 +1,5 @@ +INSERT INTO + application.acl(role, action, resource, deny) +VALUES + (:role, :action, :resource, :deny) +RETURNING *; \ No newline at end of file diff --git a/ui/temboardui/model/versions/014_acl.sql b/ui/temboardui/model/versions/014_acl.sql new file mode 100644 index 000000000..b5fe445e9 --- /dev/null +++ b/ui/temboardui/model/versions/014_acl.sql @@ -0,0 +1,119 @@ +------------------------------------------------ +-- ACL MANAGEMENT -- +------------------------------------------------ +CREATE TABLE application.acl ( + "id" BIGSERIAL PRIMARY KEY, + "role" TEXT NOT NULL, + "action" TEXT NOT NULL, + "resource" TEXT NOT NULL, + "deny" BOOLEAN DEFAULT FALSE, + "cdate" TIMESTAMP WITH TIME ZONE DEFAULT NOW(), + "origin" TEXT, + UNIQUE("role", "action", "resource") +); + +INSERT INTO + application.acl (role, action, resource) +VALUES + -- All users can access root + ( + 'trn:temboard:*:*:*', + 'GET:/', + '*' + ), + -- All users can access login page + ( + 'trn:temboard:*:*:*', + '*:/login', + '*' + ), + -- All users can submit login form + ( + 'trn:temboard:*:*:*', + '*:/json/login', + '*' + ), + -- All users can access reset password page + ( + 'trn:temboard:*:*:*', + '*:/reset-password', + '*' + ), + -- All users can submit reset-password form + ( + 'trn:temboard:*:*:*', + 'POST:/json/reset-password', + '*' + ), + -- All identified users can access logout + ( + 'trn:temboard:core:user:*', + '*:/logout', + '*' + ), + -- All identified users can access home + ( + 'trn:temboard:core:user:*', + '*:/home', + '*' + ), + -- All identified users can retrieve instances list + ( + 'trn:temboard:core:user:*', + 'GET:/json/instances/home', + '*' + ), + -- All identified user can access about page + ( + 'trn:temboard:core:user:*', + '*:/about', + '*' + ), + -- All users from admins group have access to ALL requests + ( + 'trn:temboard:core:group:admins', + '*', + '*' + ), + -- ApiKey have access to open metrics + ( + 'trn:temboard:core:apikey:*', + 'GET:/proxy/
//monitoring/metrics', + '*' + ); + +-- Insert ACL for all existing dba groups +-- e.g. : mass/dba => "trn:temboard:core:group:mass/dba" "*" "trn:temboard:core:instance:mass" +INSERT INTO + application.acl (role, action, resource) +SELECT + 'trn:temboard:core:group:' || g.name AS group, + '*' AS action, + 'trn:temboard:core:instance:' || e.name AS instance +FROM + application.groups g + JOIN application.environments e ON g.id = e.dba_group_id; + +-- Create group admins +INSERT INTO + application.groups (name, description) +VALUES + ('admins', 'Admin'); + +--Add every user having is_admin to true in admins group +INSERT INTO + application.memberships (role_name, group_id) +SELECT + r.role_name, + ( + SELECT + id + FROM + application.groups + WHERE + name = 'admins' + ) +FROM + application.roles r +WHERE + r.is_admin; \ No newline at end of file diff --git a/ui/temboardui/plugins/monitoring/routes.py b/ui/temboardui/plugins/monitoring/routes.py index 7956d1d2a..d764ed847 100644 --- a/ui/temboardui/plugins/monitoring/routes.py +++ b/ui/temboardui/plugins/monitoring/routes.py @@ -1,10 +1,9 @@ # Flask routes from flask import current_app -from ...web.flask import apikey_allowed, instance_proxy +from ...web.flask import instance_proxy @instance_proxy.route("/monitoring/metrics") -@apikey_allowed def get_metrics(): return current_app.instance.proxy() diff --git a/ui/temboardui/web/flask.py b/ui/temboardui/web/flask.py index 1a652473f..ef5feb47e 100644 --- a/ui/temboardui/web/flask.py +++ b/ui/temboardui/web/flask.py @@ -26,10 +26,11 @@ from tornado.web import decode_signed_value from werkzeug.exceptions import HTTPException +from ..acl import TRN, ACLResult, expand_actions from ..agentclient import TemboardAgentClient from ..application import get_instance, get_role_by_cookie from ..model import Session -from ..model.orm import ApiKey, StubRole +from ..model.orm import ACLRule, Anonymous, ApiKey, StubRole from .tornado import serialize_querystring from .vitejs import ViteJSExtension @@ -84,7 +85,8 @@ def create_app(temboard_app): SQLAlchemy(app) APIKeyMiddleware(app) UserMiddleware(app) - AuthMiddleware(app) + AuthenticationMiddleware(app) + InstanceMiddleware(app) app.register_error_handler(Exception, error_handler) # unsafe-eval is for jquery. unsafe-inline because we have @@ -126,11 +128,11 @@ def finalize_app(): app = current_app app.secret_key = app.temboard.config.temboard.cookie_secret - # This middleware registers instance_proxy blueprint, loads g.instance - # object and provides helpers in app.instance. instance is loaded only for - # instance_proxy blueprint. instance_proxy must be registered after plugins - # loading. - InstanceMiddleware(app) + # instance_routes and instance_proxy must be registered after plugins loading. + app.register_blueprint(instance_proxy) + app.register_blueprint(instance_routes) + + AuthorizationsMiddleware(app) return app @@ -246,7 +248,7 @@ def before(self): g.apikey = key -class AuthMiddleware: +class AuthenticationMiddleware: # Flask extension enforcing authentication def __init__(self, app=None): @@ -271,21 +273,6 @@ def before(self): if not func: abort(404) - apikey_allowed = getattr(func, "__apikey_allowed", False) - if apikey_allowed and g.apikey: - logger.debug("Endpoint authorized by API key.") - return - - anonymous_allowed = getattr(func, "__anonymous_allowed", False) - if not anonymous_allowed and g.current_user is None: - logger.debug("Refusing anonymous access.") - abort(401) - - admin_required = getattr(func, "__admin_required", False) - if admin_required and not g.current_user.is_admin: - logger.debug("Refusing access to non-admin user.") - abort(403) - class UserMiddleware: # Flask extension to load current user. @@ -332,11 +319,8 @@ def __init__(self, app=None): def init_app(self, app): app.instance = self - app.register_blueprint(instance_proxy) - app.register_blueprint(instance_routes) - brf = app.before_request_funcs - brf[instance_proxy.name] = [self.load_instance_before_request] - brf[instance_routes.name] = [self.load_instance_before_request] + + app.before_request(self.load_instance_before_request) @app.teardown_request def teardown_instance(*_): @@ -350,6 +334,15 @@ def context_processor(): return dict(instance=g.instance) def load_instance_before_request(self): + # instance is loaded only for + # instance_routes and instance_proxy blueprint + current_blueprint = "" + allowed_blueprints = ["instance_proxy", "instance_routes"] + if request.endpoint: + current_blueprint = request.endpoint.split(".")[0] + if current_blueprint not in allowed_blueprints: + return + address = request.view_args.pop("address") port = request.view_args.pop("port") @@ -357,16 +350,6 @@ def load_instance_before_request(self): if not g.instance: abort(404) - func = self.app.view_functions.get(request.endpoint) - apikey_allowed = g.apikey and getattr(func, "__apikey_allowed", False) - user_allowed = ( - g.current_user - and g.db_session.execute( - g.instance.has_dba(g.current_user.role_name) - ).scalar() - ) - if not apikey_allowed and not user_allowed: - abort(403) g.instance.status = None prefix = current_app.blueprints[request.blueprint].url_prefix request.instance_path = request.url_rule.rule.replace(prefix, "") @@ -429,22 +412,77 @@ def check_active_plugin(self, name): raise abort(408, "Plugin %s not activated." % name) -def anonymous_allowed(func): - # Decorator marking a route as public. - func.__anonymous_allowed = True - return func +class AuthorizationsMiddleware: + def __init__(self, app=None): + self.app = app + if app: + self.init_app(app) + + def init_app(self, app): + app.before_request(self.check) + + def check(self, role=None, action=None, resource=None): + func = self.app.view_functions.get(request.endpoint) + if getattr(func, "__nocheck", None): + return + + role = role or getattr(func, "__authorization_role", None) + action = action or getattr(func, "__authorization_action", None) + resource = resource or getattr(func, "__authorization_resource", None) + + if role: + roles = TRN.parse(role).expand() + elif g.apikey: + role = g.apikey.trn() + roles = [str(trn) for trn in g.apikey.role_trns()] + elif g.current_user: + role = g.current_user.role_name + roles = [str(trn) for trn in g.current_user.role_trns()] + else: + role = Anonymous.trn() + roles = [str(trn) for trn in Anonymous.role_trns()] + + action = action or f"{request.method}:{request.url_rule.rule}" + actions = expand_actions(action) + + if resource: + resources = TRN.parse(resource).expand() + elif hasattr(g, "instance") and g.instance: + resource = str(g.instance.trn()) + resources = [str(trn) for trn in g.instance.resource_trns()] + else: + resource = "*" + resources = ["*"] + + if request.url_rule and request.url_rule.rule.startswith("/static"): + return + + statements = ( + ACLRule.match(roles, actions, resources).with_session(g.db_session).all() + ) + denies = [s for s in statements if s.deny] + if not statements: + result = ACLResult(role, action, resource, "implicitDeny") + elif denies: + result = ACLResult(role, action, resource, "denied", statements) + else: + result = ACLResult(role, action, resource, statements=statements) + + result.raise_for_decision() + +def configure_authorization(role=None, action=None, resource=None): + def decorator(func): + func.__authorization_role = role + func.__authorization_action = action + func.__authorization_resource = resource + return func -def apikey_allowed(func): - # Decorator allowing a route by apikey auth - func.__apikey_allowed = True - return func + return decorator -def admin_required(func): - # Similar to flask_security.roles_required, but limited to admin role. - func.__admin_required = True - return func +def nocheck(func): + func.__nocheck = True def transaction(func): diff --git a/ui/temboardui/web/routes/auth.py b/ui/temboardui/web/routes/auth.py index add8d0dde..c68cbd788 100644 --- a/ui/temboardui/web/routes/auth.py +++ b/ui/temboardui/web/routes/auth.py @@ -15,6 +15,8 @@ ) from itsdangerous import SignatureExpired from temboardtoolkit import validators +from tornado.web import create_signed_value + from temboardui.application import ( gen_cookie, get_reset_token_serializer, @@ -23,10 +25,9 @@ send_mail, ) from temboardui.errors import TemboardUIError -from tornado.web import create_signed_value from ...model import orm -from ..flask import admin_required, anonymous_allowed, transaction, validating +from ..flask import transaction, validating logger = logging.getLogger(__name__) @@ -39,7 +40,6 @@ def logout(): @app.route("/login") -@anonymous_allowed def login(): if g.current_user: return redirect("/home") @@ -47,7 +47,6 @@ def login(): @app.route(r"/json/login", methods=["POST"]) -@anonymous_allowed def json_login(): username = request.json["username"] password = request.json["password"] @@ -81,13 +80,11 @@ def json_login(): @app.route("/reset-password", methods=["GET"]) -@anonymous_allowed def reset_password(): return render_template("reset-password.html", headerbar=False) @app.route("/json/reset-password", methods=["POST"]) -@anonymous_allowed def json_reset_pwd(): if request.method == "POST": data = request.json @@ -155,7 +152,6 @@ def get_role_name_for_token(token): @app.route("/reset-password/", methods=["GET"]) -@anonymous_allowed def reset_token(token): try: get_role_name_for_token(token) @@ -166,7 +162,6 @@ def reset_token(token): @app.route("/json/reset-password/", methods=["POST"]) -@anonymous_allowed def json_reset_password(token): try: role_name = get_role_name_for_token(token) @@ -199,13 +194,11 @@ def json_reset_password(token): @app.route("/json/users") -@admin_required def get_users(): return jsonify([u.asdict() for u in orm.Role.all().with_session(g.db_session)]) @app.route("/json/users", methods=["POST"]) -@admin_required @transaction def post_user(): if "password" not in request.json: @@ -216,7 +209,6 @@ def post_user(): @app.route("/json/users/") -@admin_required def get_user(name): user = orm.Role.get(name).with_session(g.db_session).one_or_none() if user is None: @@ -225,7 +217,6 @@ def get_user(name): @app.route("/json/users/", methods=["PUT"]) -@admin_required @transaction def put_user(name=None, user=None): if user is None: @@ -260,7 +251,6 @@ def put_user(name=None, user=None): @app.route("/json/users/", methods=["DELETE"]) -@admin_required @transaction def delete_user(name): result = g.db_session.execute(orm.Role.delete(name)) @@ -270,7 +260,6 @@ def delete_user(name): @app.route("/json/groups//members/") -@admin_required def get_group_membership(groupname, username): row = g.db_session.execute(orm.Group.select_membership(groupname, username)).first() if not row: @@ -280,7 +269,6 @@ def get_group_membership(groupname, username): @app.route("/json/groups//members", methods=["POST"]) -@admin_required @transaction def post_group_membership(groupname): gr = orm.Group.get(groupname).with_session(g.db_session).one_or_none() @@ -294,7 +282,6 @@ def post_group_membership(groupname): @app.route("/json/groups//members/", methods=["DELETE"]) -@admin_required @transaction def delete_group_membership(groupname, username): """Remove a user from a group.""" diff --git a/ui/temboardui/web/routes/core.py b/ui/temboardui/web/routes/core.py index 7206803ce..ec8b2f4aa 100644 --- a/ui/temboardui/web/routes/core.py +++ b/ui/temboardui/web/routes/core.py @@ -2,11 +2,9 @@ from flask import g, jsonify, redirect from ...model.orm import Instance -from ..flask import admin_required, anonymous_allowed @app.route("/") -@anonymous_allowed def index(): if g.current_user: return redirect("/home") @@ -27,7 +25,6 @@ def get_instance_home(): @app.route("/json/plugins") -@admin_required def get_plugins(): """List plugins.""" return jsonify(sorted(app.temboard.config.temboard.plugins)) diff --git a/ui/temboardui/web/routes/inventory.py b/ui/temboardui/web/routes/inventory.py index bf8a8e00c..bf5af2e6c 100644 --- a/ui/temboardui/web/routes/inventory.py +++ b/ui/temboardui/web/routes/inventory.py @@ -9,14 +9,14 @@ from temboardtoolkit.utils import utcnow from ... import agentclient +from ...acl import TRN from ...model import QUERIES, orm -from ..flask import admin_required, transaction, validating +from ..flask import transaction, validating logger = logging.getLogger(__name__) @current_app.route("/json/environments") -@admin_required def get_environments(): return flask.jsonify( [e.asdict() for e in orm.Environment.all().with_session(g.db_session)] @@ -24,14 +24,12 @@ def get_environments(): @current_app.route("/json/environments", methods=["POST"]) -@admin_required @transaction def post_environments(): return put_environment(environment=orm.Environment(dba_group=orm.Group())) @current_app.route("/json/environments/") -@admin_required def get_environment(name): environment = orm.Environment.get(name).with_session(g.db_session).first() if environment is None: @@ -40,7 +38,6 @@ def get_environment(name): @current_app.route("/json/environments//members") -@admin_required def get_environment_members(name): return flask.jsonify( [ @@ -51,11 +48,16 @@ def get_environment_members(name): @current_app.route("/json/environments/", methods=["PUT"]) -@admin_required @transaction def put_environment(name=None, environment=None): - if environment is None: - environment = orm.Environment.get(name).with_session(g.db_session).first() + if environment is None and ( + environment := orm.Environment.get(name).with_session(g.db_session).first() + ): + orm.ACLRule.delete( + str(TRN("core", "group", f"{environment.name}/dba")), + "*", + str(TRN("core", "instance", environment.name)), + ).with_session(g.db_session).first() if environment is None: flask.abort(404, "No such environment.") @@ -63,6 +65,11 @@ def put_environment(name=None, environment=None): environment.name = validators.slug(flask.request.json["name"]) environment.description = flask.request.json["description"] environment.dba_group.name = f"{environment.name}/dba" + orm.ACLRule.insert( + str(TRN("core", "group", environment.dba_group.name)), + "*", + str(TRN("core", "instance", environment.name)), + ).with_session(g.db_session).first() # Used as profile name in /settings/environment/<>/members environment.dba_group.description = "DBA" g.db_session.add(environment) # When called from post_environment @@ -72,13 +79,17 @@ def put_environment(name=None, environment=None): @current_app.route("/json/environments/", methods=["DELETE"]) -@admin_required @transaction def delete_environment(name): # Delete DBA group, cascding to environment. result = g.db_session.execute(orm.Group.delete(f"{name}/dba")) if result.rowcount == 0: flask.abort(404, "No such environment.") + orm.ACLRule.delete( + str(TRN("core", "group", f"{name}/dba")), + "*", + str(TRN("core", "instance", name)), + ).with_session(g.db_session).first() return flask.jsonify() @@ -116,7 +127,6 @@ def post_instance(): @current_app.route("/json/instances/
/") -@admin_required def get_instance(address, port): try: instance = orm.Instance.get(address, port).with_session(g.db_session).one() @@ -126,7 +136,6 @@ def get_instance(address, port): @current_app.route("/json/instances/
/", methods=["PUT"]) -@admin_required @transaction def put_instance(address=None, port=None, instance=None): if not instance: @@ -171,7 +180,6 @@ def put_instance(address=None, port=None, instance=None): @current_app.route("/json/instances/
/", methods=["DELETE"]) -@admin_required @transaction def delete_instance(address, port): out = g.db_session.execute(orm.Instance.delete(address, port)) @@ -182,7 +190,6 @@ def delete_instance(address, port): # Special proxy for unregistered instance. @current_app.route("/json/instances/
//discover") -@admin_required def discover(address, port): client = agentclient.TemboardAgentClient.factory( current_app.temboard.config, address, port, username=g.current_user.role_name @@ -201,7 +208,6 @@ def discover(address, port): @current_app.route("/instances.csv") -@admin_required def get_instances_csv(): search = flask.request.args.get("filter") pattern = "%%%s%%" % search if search else "%" diff --git a/ui/temboardui/web/routes/settings.py b/ui/temboardui/web/routes/settings.py index ca6990ac2..f3fd7a6d6 100644 --- a/ui/temboardui/web/routes/settings.py +++ b/ui/temboardui/web/routes/settings.py @@ -3,11 +3,9 @@ from ...application import send_mail, send_sms from ...model import orm -from ..flask import admin_required @app.route("/settings/instances") -@admin_required def settings_instances(): return render_template( "settings/instances.html", @@ -17,7 +15,6 @@ def settings_instances(): @app.route("/settings/environments") -@admin_required def settings_environments(): return render_template( "settings/environments.html", @@ -27,13 +24,11 @@ def settings_environments(): @app.route("/settings/environments//members") -@admin_required def settings_environment_members(name): return render_template("settings/members.html", sidebar=True, environment=name) @app.route("/settings/users") -@admin_required def settings_users(): return render_template( "settings/users.html", @@ -43,7 +38,6 @@ def settings_users(): @app.route("/settings/notifications") -@admin_required def settings_notifications(): return render_template( "settings/notifications.html", @@ -54,7 +48,6 @@ def settings_notifications(): @app.route("/json/test_email", methods=["POST"]) -@admin_required def post_test_email(): email = request.json.get("email") if not email: @@ -80,7 +73,6 @@ def post_test_email(): @app.route("/json/test_sms", methods=["POST"]) -@admin_required def post_test_sms(): phone = request.json.get("phone") if not phone: diff --git a/ui/temboardui/web/tornado.py b/ui/temboardui/web/tornado.py index 189713838..4968215c1 100644 --- a/ui/temboardui/web/tornado.py +++ b/ui/temboardui/web/tornado.py @@ -13,30 +13,16 @@ from tornado.web import Application as TornadoApplication from tornado.web import HTTPError, RequestHandler +from ..acl import ACLResult, expand_actions from ..agentclient import TemboardAgentClient from ..application import get_instance, get_role_by_cookie from ..errors import TemboardUIError from ..model import Session as DBSession +from ..model.orm import ACLRule, Anonymous logger = logging.getLogger(__name__) -def admin_required(func): - # Similar to flask_security.roles_required, but limited to admin role. - func.__admin_required = True - return func - - -def anonymous_allowed(func): - # Reverse of flask_security.login_required. - # - # In temboard, very few pages are anonymous. Thus we have implicit - # login_required. This behaviour can be disabled by using - # @anonymous_allowed. - func.__anonymous_allowed = True - return func - - def serialize_querystring(query): return "&".join( [ @@ -364,51 +350,55 @@ def json_middleware(request, *args): return json_middleware -def add_user_instance_middleware(func): - # Ensures user is allowed to access to the instance - @functools.wraps(func) - def user_instance_middleware(request, *args): - user = request.current_user - - if user is None: - # Not logged in - raise HTTPError(401, "Restricted area.") - - user_allowed = request.db_session.execute( - request.instance.has_dba(user.role_name) - ).scalar() - if not user_allowed: - raise HTTPError(403, "Restricted area.") - - return func(request, *args) - - return user_instance_middleware - - class UserHelper: @classmethod def add_middleware(cls, func): @functools.wraps(func) def user_middleware(request, *args): - role = request.current_user = request.handler.current_user + request.current_user = request.handler.current_user + return func(request, *args) - anonymous_allowed = getattr(func, "__anonymous_allowed", False) - if not anonymous_allowed and role is None: - logger.debug("Redirecting anonymous to /login.") - raise Redirect("/login") + return user_middleware - admin_required = getattr(func, "__admin_required", False) - if admin_required and not role.is_admin: - logger.debug("Refusing access to non-admin user.") - raise HTTPError(403) - return func(request, *args) +class AuthorizationsHelper: + @classmethod + def add_middleware(cls, func): + @functools.wraps(func) + def authorizations_middleware(request, *args): + if request.current_user: + role = request.current_user.role_name + roles = [str(trn) for trn in request.current_user.role_trns()] + else: + role = Anonymous.trn() + roles = [str(trn) for trn in Anonymous.role_trns()] + + action = f"{request.method}:{request.uri}" + actions = expand_actions(action) + if hasattr(request, "instance") and request.instance: + resource = str(request.instance.trn()) + resources = [str(trn) for trn in request.instance.resource_trns()] + else: + resource = "*" + resources = ["*"] + statements = ( + ACLRule.match(roles, actions, resources) + .with_session(request.db_session) + .all() + ) + denies = [s for s in statements if s.deny] + if not statements: + result = ACLResult(role, action, resource, "implicitDeny") + elif denies: + result = ACLResult(role, action, resource, "denied", statements) + else: + result = ACLResult(role, action, resource, statements=statements) - return user_middleware + result.raise_for_decision() + return func(request, *args) -# Ensure @functools.wraps preserves User middleware attributes. -functools.WRAPPER_UPDATES += ("__admin_required", "__anonymous_allowed") + return authorizations_middleware class Blueprint: @@ -462,13 +452,8 @@ def route(self, url, methods=None, with_instance=False, json=None): def decorator(func): logger_name = func.__module__ + "." + func.__name__ + func = AuthorizationsHelper.add_middleware(func) if with_instance: - if url.startswith("/server/") or url.startswith("/proxy/"): - # Limit user/instance access control to /server/ and - # /proxy/. - # Admin area access control has already been performed - func = add_user_instance_middleware(func) - func = InstanceHelper.add_middleware(func) func = UserHelper.add_middleware(func) diff --git a/ui/tests/unit/test_acl.py b/ui/tests/unit/test_acl.py new file mode 100644 index 000000000..8b47189d3 --- /dev/null +++ b/ui/tests/unit/test_acl.py @@ -0,0 +1,56 @@ +import pytest +from temboardui.acl import TRN, expand_actions + + +def test_trn_parse(): + with pytest.raises(Exception) as e: + TRN.parse("malformed:trn") + assert str(e.value) == "Malformed TRN" + + trn = TRN.parse("trn:temboard:core:user:alice") + + assert trn.scope == "core" + assert trn.type == "user" + assert trn.name == "alice" + + assert str(trn) == "trn:temboard:core:user:alice" + + trn = TRN.parse("trn:temboard:core:instance:prod/pg001.bridoulou.fr") + + assert trn.scope == "core" + assert trn.type == "instance" + assert trn.name == "prod/pg001.bridoulou.fr" + + +def test_trn_parent(): + trn = TRN.parse("trn:temboard:core:user:alice") + assert str(trn.parent()) == "trn:temboard:core:user:*" + + trn = TRN.parse("trn:temboard:core:group:prod/dba") + assert str(trn.parent()) == "trn:temboard:core:group:prod" + + trn = TRN.parse("trn:temboard:core:group:prod/dba/indus") + assert str(trn.parent()) == "trn:temboard:core:group:prod/dba" + + trn = TRN.parse("trn:temboard:*:*:*") + assert str(trn.parent()) == "trn:temboard:*:*:*" + + +def test_trn_expand(): + trn = TRN.parse("trn:temboard:core:user:alice") + parents = TRN.expand(trn) + assert len(parents) == 5 + assert str(parents[0]) == "*" + assert str(parents[1]) == "trn:temboard:core:user:alice" + assert str(parents[2]) == "trn:temboard:core:user:*" + assert str(parents[3]) == "trn:temboard:core:*:*" + assert str(parents[4]) == "trn:temboard:*:*:*" + + +def test_expand_action(): + action = "POST:/login" + actions = expand_actions(action) + assert len(actions) == 3 + assert actions[0] == "*" + assert actions[1] == "*:/login" + assert actions[2] == "POST:/login"