Skip to content
Draft

Acl #1628

Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ui/.husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,3 @@

cd ui
npx lint-staged
ruff check
ruff format --check
6 changes: 5 additions & 1 deletion ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
40 changes: 40 additions & 0 deletions ui/temboardui/acl.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
class TRN:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a short docstring explain what TRN is, what it looks like.

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])
Comment on lines +7 to +12

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use a class method here:

    @classmethod
    def parse(cls, trn):
        elems = str.split(trn, ":")
        if len(elems) < 5:
            raise Exception("Malformed TRN")
        return cls(elems[2], elems[3], elems[4])

You avoid repeating the name of the class. It prevents errors if the class is renamed. And it works better with inheritance.


def __str__(self):
return f"trn:temboard:{self.scope}:{self.type}:{self.name}"

def parent(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would IMO be a good candidate for @property.

Then it would be called like this:
parent = TRN.parse("trn:temboard:core:user:alice").parent

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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about having a property called parents instead?

trns = ["*"]
trn = TRN(self.scope, self.type, self.name)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self is already a TRN. You shouldn't have to recreate one.

while str(trn) != "trn:temboard:*:*:*":
if str(trn) not in trns:
trns.append(str(trn))
trn = trn.parent()
trns.append(str(trn))
return trns

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning a list of strings, why not returning a list of TRN instances?

2 changes: 2 additions & 0 deletions ui/temboardui/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
)
Expand Down
89 changes: 87 additions & 2 deletions ui/temboardui/model/orm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Comment on lines +66 to +73

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could possibly use @property.



class Plugin(Model):
__tablename__ = "plugins"
Expand Down Expand Up @@ -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],
Comment on lines +194 to +195

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it related to current commit?

)

def select_environments(self):
Expand All @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -522,3 +557,53 @@ 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"<ACL stmt deny={self.deny} {self.role} for {self.action} on {self.resource}>"
6 changes: 6 additions & 0 deletions ui/temboardui/model/queries/acl-delete.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
DELETE FROM
application.acl
WHERE
role = :role
AND action = :action
AND resource = :resource RETURNING *;
8 changes: 8 additions & 0 deletions ui/temboardui/model/queries/acl-get.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
SELECT
*
FROM
application.acl
WHERE
role = ANY(:roles)
AND ACTION = ANY(:actions)
Comment thread
pirlgon marked this conversation as resolved.
AND resource = ANY(:resources);
5 changes: 5 additions & 0 deletions ui/temboardui/model/queries/acl-insert.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
INSERT INTO
application.acl(role, action, resource, deny)
VALUES
(:role, :action, :resource, :deny)
RETURNING *;
119 changes: 119 additions & 0 deletions ui/temboardui/model/versions/014_acl.sql
Original file line number Diff line number Diff line change
@@ -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
(
Comment thread
pirlgon marked this conversation as resolved.
'trn:temboard:*:*:*',
'GET:/',
'*'
Comment thread
pgiraud marked this conversation as resolved.
),
-- 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/<address>/<port>/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;
4 changes: 2 additions & 2 deletions ui/temboardui/web/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def create_app(temboard_app):
SQLAlchemy(app)
APIKeyMiddleware(app)
UserMiddleware(app)
AuthMiddleware(app)
AuthenticationMiddleware(app)
app.register_error_handler(Exception, error_handler)

# unsafe-eval is for jquery. unsafe-inline because we have
Expand Down Expand Up @@ -246,7 +246,7 @@ def before(self):
g.apikey = key


class AuthMiddleware:
class AuthenticationMiddleware:
# Flask extension enforcing authentication

def __init__(self, app=None):
Expand Down
Loading