From c6b193b8e83d61922a86b69076204ed8a6fc85f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sat, 21 Jul 2018 10:16:24 +0200 Subject: [PATCH 1/9] Init privileges CRUD and docs --- docs/usage/index.rst | 1 + docs/usage/privileges.rst | 3 +++ repocribro/controllers/admin.py | 6 +++++- repocribro/models.py | 5 ++++- repocribro/templates/admin/role.html | 8 ++++++-- repocribro/templates/admin/tabs/roles.html | 10 ++++++++-- 6 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 docs/usage/privileges.rst diff --git a/docs/usage/index.rst b/docs/usage/index.rst index 16b719a..f7a8ccc 100644 --- a/docs/usage/index.rst +++ b/docs/usage/index.rst @@ -7,3 +7,4 @@ Usage start commands web + privileges diff --git a/docs/usage/privileges.rst b/docs/usage/privileges.rst new file mode 100644 index 0000000..3adcc37 --- /dev/null +++ b/docs/usage/privileges.rst @@ -0,0 +1,3 @@ +Privileges and Roles +==================== + diff --git a/repocribro/controllers/admin.py b/repocribro/controllers/admin.py index c069b41..7911a8b 100644 --- a/repocribro/controllers/admin.py +++ b/repocribro/controllers/admin.py @@ -173,12 +173,15 @@ def role_edit(name): if role is None: flask.abort(404) name = flask.request.form.get('name', '') + priv = flask.request.form.get('privileges', '') desc = flask.request.form.get('description', None) if name == '': flask.flash('Couldn\'t make that role...', 'warning') return flask.redirect(flask.url_for('admin.index', tab='roles')) + # TODO: validate priv (chars, separators, etc.) try: role.name = name + role.privileges = priv role.description = desc db.session.commit() except sqlalchemy.exc.IntegrityError as e: @@ -213,12 +216,13 @@ def role_create(): db = flask.current_app.container.get('db') name = flask.request.form.get('name', '') + priv = flask.request.form.get('privileges', '') desc = flask.request.form.get('description', None) if name == '': flask.flash('Couldn\'t make that role...', 'warning') return flask.redirect(flask.url_for('admin.index', tab='roles')) try: - role = Role(name, desc) + role = Role(name, priv, desc) db.session.add(role) db.session.commit() except sqlalchemy.exc.IntegrityError as e: diff --git a/repocribro/models.py b/repocribro/models.py index 3d89f7f..689df8d 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -257,13 +257,16 @@ class Role(db.Model, RoleMixin): name = sqlalchemy.Column(sqlalchemy.String(80), unique=True) #: Description (purpose, notes, ...) of the role description = sqlalchemy.Column(sqlalchemy.UnicodeText) + #: Serialized list of privileges + privileges = sqlalchemy.Column(sqlalchemy.Text) #: User accounts assigned to the role user_accounts = sqlalchemy.orm.relationship( 'UserAccount', back_populates='roles', secondary=roles_users ) - def __init__(self, name, description): + def __init__(self, name, privileges, description): self.name = name + self.privileges = privileges self.description = description def __repr__(self): diff --git a/repocribro/templates/admin/role.html b/repocribro/templates/admin/role.html index 7b655c6..7c635f0 100644 --- a/repocribro/templates/admin/role.html +++ b/repocribro/templates/admin/role.html @@ -21,8 +21,12 @@

{{ role.name }}
- - + + +
+
+ +
diff --git a/repocribro/templates/admin/tabs/roles.html b/repocribro/templates/admin/tabs/roles.html index 1705c30..329bb97 100644 --- a/repocribro/templates/admin/tabs/roles.html +++ b/repocribro/templates/admin/tabs/roles.html @@ -15,8 +15,12 @@

Roles

- - + + +
+
+ +
@@ -25,6 +29,7 @@

Roles

Name Description + Privileges Users @@ -34,6 +39,7 @@

Roles

{{ role.name }} {{ role.description }} + {{ role.privileges }} {{ role.accounts|length }} From af5983e4cf864116e59766dc3617a591d4198115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sat, 21 Jul 2018 10:29:45 +0200 Subject: [PATCH 2/9] Fixing tests due to model changes --- repocribro/commands/assign_role.py | 3 ++- tests/conftest.py | 2 +- tests/test_models.py | 8 ++++---- tests/test_security.py | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/repocribro/commands/assign_role.py b/repocribro/commands/assign_role.py index b9da273..b01f02a 100644 --- a/repocribro/commands/assign_role.py +++ b/repocribro/commands/assign_role.py @@ -17,7 +17,8 @@ def _assign_role(login, role_name): role = db.session.query(Role).filter_by(name=role_name).first() if role is None: print('Role {} not in DB... adding'.format(role_name)) - role = Role(role_name, '') + print('WARNING - created role has all privileges by default!') + role = Role(role_name, '*', '') db.session.add(role) user.user_account.roles.append(role) db.session.commit() diff --git a/tests/conftest.py b/tests/conftest.py index 87ff6d8..56b9e2b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -164,7 +164,7 @@ def filled_db_session(empty_db_session): Organization, Repository, Commit, Release, Push # Setup admin role - admin_role = Role('admin', 'Administrators') + admin_role = Role('admin', '*', 'Administrators') session.add(admin_role) account_banned = UserAccount() diff --git a/tests/test_models.py b/tests/test_models.py index f7ffb8b..df06455 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -150,9 +150,9 @@ def test_repo_push(session, github_data_loader): def test_role_mixin(): - roleA = Role('admin', 'Admin of this great app') - roleB = Role('admin', 'Administrator of the app') - roleC = Role('admininistrator', 'Admin of this great app') + roleA = Role('admin', '*', 'Admin of this great app') + roleB = Role('admin', '*', 'Administrator of the app') + roleC = Role('admininistrator', '*', 'Admin of this great app') assert roleA == roleB assert roleB == roleA assert hash(roleA) == hash(roleB) @@ -205,7 +205,7 @@ def test_user_mixin(): repo.visibility_type = Repository.VISIBILITY_PUBLIC assert account.sees_repo(repo) - role = Role('admin', '') + role = Role('admin', '*', '') account.roles.append(role) assert account.has_role('admin') assert account.rolenames == ['admin'] diff --git a/tests/test_security.py b/tests/test_security.py index 36d7d6b..e79eda0 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -28,7 +28,7 @@ def test(): with pytest.raises(Forbidden): assert test() == 200 - role_admin = Role('admin', '') + role_admin = Role('admin', '*', '') account = UserAccount() account.id = 666 account.roles.append(role_admin) From fe8a0039dc176fcbf815f6cd3b127dbf0d106443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sun, 22 Jul 2018 08:17:08 +0200 Subject: [PATCH 3/9] Extensible security with actions and roles --- .../13a18cc60ee5_privileges_for_role.py | 28 +++++++++++ repocribro/controllers/admin.py | 26 +++++----- repocribro/controllers/core.py | 2 + repocribro/ext_core.py | 12 ++++- repocribro/extending/extension.py | 26 ++++++++++ repocribro/models.py | 17 +++++++ repocribro/repocribro.py | 4 ++ repocribro/security.py | 50 +++++++++++++++---- tests/test_security.py | 2 +- 9 files changed, 141 insertions(+), 26 deletions(-) create mode 100644 migrations/versions/13a18cc60ee5_privileges_for_role.py diff --git a/migrations/versions/13a18cc60ee5_privileges_for_role.py b/migrations/versions/13a18cc60ee5_privileges_for_role.py new file mode 100644 index 0000000..e8966c2 --- /dev/null +++ b/migrations/versions/13a18cc60ee5_privileges_for_role.py @@ -0,0 +1,28 @@ +"""Privileges for role + +Revision ID: 13a18cc60ee5 +Revises: 891cf9712a20 +Create Date: 2018-07-21 09:54:25.502179 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '13a18cc60ee5' +down_revision = '891cf9712a20' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('Role', sa.Column('privileges', sa.Text(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('Role', 'privileges') + # ### end Alembic commands ### diff --git a/repocribro/controllers/admin.py b/repocribro/controllers/admin.py index 7911a8b..c6f0f37 100644 --- a/repocribro/controllers/admin.py +++ b/repocribro/controllers/admin.py @@ -9,7 +9,7 @@ @admin.route('') -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def index(): """Administration zone dashboard (GET handler)""" ext_master = flask.current_app.container.get('ext_master') @@ -26,7 +26,7 @@ def index(): @admin.route('/account/') -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def account_detail(login): """Account administration (GET handler)""" db = flask.current_app.container.get('db') @@ -38,7 +38,7 @@ def account_detail(login): @admin.route('/account//ban', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def account_ban(login): """Ban (make inactive) account (POST handler)""" db = flask.current_app.container.get('db') @@ -66,7 +66,7 @@ def account_ban(login): @admin.route('/account//delete', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def account_delete(login): """Delete account (POST handler)""" db = flask.current_app.container.get('db') @@ -84,7 +84,7 @@ def account_delete(login): @admin.route('/repository//') -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def repo_detail(login, reponame): """Repository administration (GET handler)""" db = flask.current_app.container.get('db') @@ -101,7 +101,7 @@ def repo_detail(login, reponame): @admin.route('/repository///visibility', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def repo_visibility(login, reponame): """Change repository visibility (POST handler)""" db = flask.current_app.container.get('db') @@ -134,7 +134,7 @@ def repo_visibility(login, reponame): @admin.route('/repository///delete', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def repo_delete(login, reponame): """Delete repository (POST handler)""" db = flask.current_app.container.get('db') @@ -152,7 +152,7 @@ def repo_delete(login, reponame): @admin.route('/role/') -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_detail(name): """Role administration (GET handler)""" db = flask.current_app.container.get('db') @@ -164,7 +164,7 @@ def role_detail(name): @admin.route('/role//edit', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_edit(name): """Edit role (POST handler)""" db = flask.current_app.container.get('db') @@ -194,7 +194,7 @@ def role_edit(name): @admin.route('/role//delete', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_delete(name): """Delete role (POST handler)""" db = flask.current_app.container.get('db') @@ -210,7 +210,7 @@ def role_delete(name): @admin.route('/roles/create', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_create(): """Create new role (POST handler)""" db = flask.current_app.container.get('db') @@ -234,7 +234,7 @@ def role_create(): @admin.route('/role//add', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_assignment_add(name): """Assign role to user (POST handler)""" db = flask.current_app.container.get('db') @@ -257,7 +257,7 @@ def role_assignment_add(name): @admin.route('/role//remove', methods=['POST']) -@permissions.admin_role.require(404) +@permissions.roles.admin.require(404) def role_assignment_remove(name): """Remove assignment of role to user (POST handler)""" db = flask.current_app.container.get('db') diff --git a/repocribro/controllers/core.py b/repocribro/controllers/core.py index 4e8cd0d..6779023 100644 --- a/repocribro/controllers/core.py +++ b/repocribro/controllers/core.py @@ -2,6 +2,7 @@ import flask_login from ..models import User, Organization, Repository +from ..security import permissions #: Core controller blueprint core = flask.Blueprint('core', __name__, url_prefix='') @@ -21,6 +22,7 @@ def index(): @core.route('/search/') @core.route('/search') @core.route('/search/') +# @permissions.actions.search.require(403) def search(query=''): """Search page (GET handler) diff --git a/repocribro/ext_core.py b/repocribro/ext_core.py index f535e43..694ddda 100644 --- a/repocribro/ext_core.py +++ b/repocribro/ext_core.py @@ -4,7 +4,7 @@ from .extending import Extension from .extending.helpers import ViewTab, Badge -from .models import Push, Release, Repository +from .models import Push, Release, Repository, Role from .github import GitHubAPI @@ -164,6 +164,16 @@ def provide_filters(): from .filters import all_filters return all_filters + @staticmethod + def provide_roles(): + return { + 'admin': Role('admin', '*', 'Service administrators'), + } + + @staticmethod + def provide_actions(): + return ['login', 'search'] + @staticmethod def get_gh_webhook_processors(): """Get all GitHub webhooks processory""" diff --git a/repocribro/extending/extension.py b/repocribro/extending/extension.py index 233f63b..416b555 100644 --- a/repocribro/extending/extension.py +++ b/repocribro/extending/extension.py @@ -108,6 +108,24 @@ def provide_filters(): """ return {} + @staticmethod + def provide_roles(): + """Extension can define roles for user accounts + + :return: Dictionary with name + Role entity + :rtype: dict of str: ``repocribro.models.Role`` + """ + return {} + + @staticmethod + def provide_actions(): + """Extension can define actions for privileges + + :return: List of action names + :rtype: list of str + """ + return [] + def init_models(self): """Hook operation for initiating the models and registering them within db @@ -126,6 +144,14 @@ def init_filters(self): """ self.register_filters_from_dict(self.provide_filters()) + def init_security(self): + """Hook operation to setup privileges (roles and actions)""" + permissions = config = self.app.container.get('permissions') + for role_name in self.provide_roles().keys(): + permissions.register_role(role_name) + for action_name in self.provide_actions(): + permissions.register_action(action_name) + def introduce(self): """Hook operation for getting short introduction of extension (mostly for debug/log purpose) diff --git a/repocribro/models.py b/repocribro/models.py index 689df8d..29e987b 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -79,6 +79,14 @@ def __hash__(self): """ return hash(self.name) + def permits(self, privilege): + privileges = self.privileges.split(':') + if privilege in privileges: + return True + if '*' in privileges: # TODO: better wildcards + return True + return False + class Anonymous(flask_login.AnonymousUserMixin): """Anonymous (not logged) user representation""" @@ -246,6 +254,15 @@ def __repr__(self): """ return ''.format(self.id) + def privileges(self, all_privileges=frozenset()): + privileges = set() + for priv in all_privileges: + for role in self.roles: + if role.permits(priv): + privileges.add(priv) + break + return privileges + class Role(db.Model, RoleMixin): """User account role in the application""" diff --git a/repocribro/repocribro.py b/repocribro/repocribro.py index e65dc82..87824bd 100644 --- a/repocribro/repocribro.py +++ b/repocribro/repocribro.py @@ -112,9 +112,13 @@ def create_app(cfg_files=['DEFAULT']): ext_names = ext_master.call('introduce', 'unknown') print('Loaded extensions: {}'.format(', '.join(ext_names))) + from .security import permissions + app.container.set_singleton('permissions', permissions) + ext_master.call('init_first') ext_master.call('init_models') ext_master.call('init_business') + ext_master.call('init_security') ext_master.call('init_filters') ext_master.call('init_blueprints') ext_master.call('init_container') diff --git a/repocribro/security.py b/repocribro/security.py index 972f036..9c59bae 100644 --- a/repocribro/security.py +++ b/repocribro/security.py @@ -28,16 +28,39 @@ def load_user(user_id): return login_manager, principals +class PermissionsContainer: + + def __init__(self, name): + self.x_name = name + self.x_dict = dict() + + def __getattr__(self, key): + return flask_principal.Permission(*self.x_dict[key]) + + class Permissions: - """ Class for prividing various permissions + """ Class for prividing various permissions""" - .. todo:: allow extensions provide permissions to others - """ + def __init__(self): + self.roles = PermissionsContainer('roles') + self.actions = PermissionsContainer('actions') + + def register_role(self, role_name): + self.roles.x_dict[role_name] = \ + (flask_principal.RoleNeed(role_name),) + + def register_action(self, priv_name): + self.actions.x_dict[priv_name] = \ + (flask_principal.ActionNeed(priv_name),) + + @property + def all_roles(self): + return set(self.roles.x_dict.keys()) + + @property + def all_actions(self): + return set(self.actions.x_dict.keys()) - #: Administrator role permission - admin_role = flask_principal.Permission( - flask_principal.RoleNeed('admin') - ) #: All permissions in the app permissions = Permissions() @@ -83,15 +106,20 @@ def on_identity_loaded(sender, identity): :param identity: Identity container :type identity: ``flask_principal.Identity`` """ - identity.user = flask_login.current_user + user = flask_login.current_user + identity.user = user - if hasattr(flask_login.current_user, 'id'): + if hasattr(user, 'id'): identity.provides.add( flask_principal.UserNeed(flask_login.current_user.id) ) - if hasattr(flask_login.current_user, 'roles'): - for role in flask_login.current_user.roles: + if hasattr(user, 'roles'): + for role in user.roles: identity.provides.add( flask_principal.RoleNeed(role.name) ) + for priviledge in user.privileges(permissions.all_actions): + identity.provides.add( + flask_principal.ActionNeed(priviledge) + ) diff --git a/tests/test_security.py b/tests/test_security.py index e79eda0..3f3e4f5 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -21,7 +21,7 @@ def test_login_logout(app, empty_db_session): def test_permission_admin(app, empty_db_session): with app.test_request_context('/'): - @permissions.admin_role.require(403) + @permissions.roles.admin.require(403) def test(): return 200 From c27fcbebe4a8d43e272480dbdfd6c4ff704ded6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sun, 22 Jul 2018 17:07:37 +0200 Subject: [PATCH 4/9] Privileges with basic wildcards --- repocribro/models.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/repocribro/models.py b/repocribro/models.py index 29e987b..525a5f4 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -2,6 +2,7 @@ import sqlalchemy import flask_login import datetime +import fnmatch from .database import db @@ -83,8 +84,9 @@ def permits(self, privilege): privileges = self.privileges.split(':') if privilege in privileges: return True - if '*' in privileges: # TODO: better wildcards - return True + for p in privileges: + if fnmatch.fnmatch(privilege, p): + return True return False From 743038a376dc51b181bf5d540f4759e00f67bf31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sun, 22 Jul 2018 18:12:24 +0200 Subject: [PATCH 5/9] Default anonymous role --- repocribro/controllers/admin.py | 8 +++- repocribro/controllers/core.py | 2 +- repocribro/ext_core.py | 7 +++- repocribro/models.py | 73 +++++++++++++++++---------------- repocribro/repocribro.py | 2 +- repocribro/security.py | 11 ++++- 6 files changed, 61 insertions(+), 42 deletions(-) diff --git a/repocribro/controllers/admin.py b/repocribro/controllers/admin.py index c6f0f37..6e8c8eb 100644 --- a/repocribro/controllers/admin.py +++ b/repocribro/controllers/admin.py @@ -1,8 +1,8 @@ import flask import sqlalchemy -from ..models import User, Role, Repository -from ..security import permissions +from ..models import User, Role, Repository, Anonymous +from ..security import permissions, reload_anonymous_role #: Admin controller blueprint admin = flask.Blueprint('admin', __name__, url_prefix='/admin') @@ -184,6 +184,8 @@ def role_edit(name): role.privileges = priv role.description = desc db.session.commit() + if name == Anonymous.rolename: + reload_anonymous_role(flask.current_app, db) except sqlalchemy.exc.IntegrityError as e: flask.flash('Couldn\'t make that role... {}'.format(str(e)), 'warning') @@ -225,6 +227,8 @@ def role_create(): role = Role(name, priv, desc) db.session.add(role) db.session.commit() + if name == Anonymous.rolename: + reload_anonymous_role(flask.current_app, db) except sqlalchemy.exc.IntegrityError as e: flask.flash('Couldn\'t make that role... {}'.format(str(e)), 'warning') diff --git a/repocribro/controllers/core.py b/repocribro/controllers/core.py index 6779023..a8c8699 100644 --- a/repocribro/controllers/core.py +++ b/repocribro/controllers/core.py @@ -22,7 +22,7 @@ def index(): @core.route('/search/') @core.route('/search') @core.route('/search/') -# @permissions.actions.search.require(403) +@permissions.actions.search.require(403) def search(query=''): """Search page (GET handler) diff --git a/repocribro/ext_core.py b/repocribro/ext_core.py index 694ddda..0fb78f1 100644 --- a/repocribro/ext_core.py +++ b/repocribro/ext_core.py @@ -4,7 +4,7 @@ from .extending import Extension from .extending.helpers import ViewTab, Badge -from .models import Push, Release, Repository, Role +from .models import Push, Release, Repository, Role, Anonymous from .github import GitHubAPI @@ -168,6 +168,7 @@ def provide_filters(): def provide_roles(): return { 'admin': Role('admin', '*', 'Service administrators'), + 'anonymous': Role(Anonymous.rolename, 'search*:login', 'Not-logged user') } @staticmethod @@ -201,7 +202,9 @@ def setup_config(self): def init_business(self): """Init business layer (other extensions, what is needed)""" - from .security import init_login_manager + from .security import init_login_manager, reload_anonymous_role + + reload_anonymous_role(self.app, self.db) login_manager, principals = init_login_manager(self.db) login_manager.init_app(self.app) principals.init_app(self.app) diff --git a/repocribro/models.py b/repocribro/models.py index 525a5f4..2c2d037 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -90,51 +90,49 @@ def permits(self, privilege): return False -class Anonymous(flask_login.AnonymousUserMixin): - """Anonymous (not logged) user representation""" - +class UserMixin(flask_login.UserMixin): @property def is_active(self): """Check whether is user active - :return: False, anonymous is not active + :return: If user is active (can login) :rtype: bool """ - return False + return self.active def has_role(self, role): """Check whether has the role :param role: Role to be checked - :type role: ``repocribro.models.RoleMixin`` - :return: False, anonymous has no roles + :type role: str + :return: If user has a role :rtype: bool """ - return False + return role in (role.name for role in self.roles) @property def rolenames(self): """Get names of all roles of that user - :return: Empty list, anonymous has no roles + :return: List of names of roles of user :rtype: list of str """ - return [] + return [role.name for role in self.roles] def owns_repo(self, repo): """Check if user owns the repository :param repo: Repository which shoudl be tested :type repo: ``repocribro.models.Repository`` - :return: False, anonymous can not own repository + :return: If user owns repo :rtype: bool """ - return False + return repo.owner.github_id == self.github_user.github_id def sees_repo(self, repo, has_secret=False): """Check if user is allowed to see the repo - Anonymous can see only public repos + Must be admin or owner to see not public repo :param repo: Repository which user want to see :type repo: ``repocribro.models.Repository`` @@ -144,52 +142,66 @@ def sees_repo(self, repo, has_secret=False): :rtype: bool """ visible = repo.is_public or (has_secret and repo.is_hidden) - return visible + return visible or self.has_role('admin') or self.owns_repo(repo) + def privileges(self, all_privileges=frozenset()): + privileges = set() + for priv in all_privileges: + for role in self.roles: + if role.permits(priv): + privileges.add(priv) + break + return privileges + + +class Anonymous(flask_login.AnonymousUserMixin, UserMixin): + """Anonymous (not logged) user representation""" + + rolename = 'anonymous' + roles = [] -class UserMixin(flask_login.UserMixin): @property def is_active(self): """Check whether is user active - :return: If user is active (can login) + :return: False, anonymous is not active :rtype: bool """ - return self.active + return False def has_role(self, role): """Check whether has the role :param role: Role to be checked - :type role: str - :return: If user has a role + :type role: ``repocribro.models.RoleMixin`` + :return: False, anonymous has no roles :rtype: bool """ - return role in (role.name for role in self.roles) + return role == self.rolename @property def rolenames(self): """Get names of all roles of that user - :return: List of names of roles of user + :return: Empty list, anonymous has no roles :rtype: list of str """ - return [role.name for role in self.roles] + return [self.rolename] def owns_repo(self, repo): """Check if user owns the repository :param repo: Repository which shoudl be tested :type repo: ``repocribro.models.Repository`` - :return: If user owns repo + :return: False, anonymous can not own repository :rtype: bool """ - return repo.owner.github_id == self.github_user.github_id + return False def sees_repo(self, repo, has_secret=False): """Check if user is allowed to see the repo - Must be admin or owner to see not public repo + Anonymous can see only public repos :param repo: Repository which user want to see :type repo: ``repocribro.models.Repository`` @@ -199,7 +211,7 @@ def sees_repo(self, repo, has_secret=False): :rtype: bool """ visible = repo.is_public or (has_secret and repo.is_hidden) - return visible or self.has_role('admin') or self.owns_repo(repo) + return visible #: Many-to-many relationship between user accounts and roles @@ -256,15 +268,6 @@ def __repr__(self): """ return ''.format(self.id) - def privileges(self, all_privileges=frozenset()): - privileges = set() - for priv in all_privileges: - for role in self.roles: - if role.permits(priv): - privileges.add(priv) - break - return privileges - class Role(db.Model, RoleMixin): """User account role in the application""" diff --git a/repocribro/repocribro.py b/repocribro/repocribro.py index 87824bd..b4cc6c8 100644 --- a/repocribro/repocribro.py +++ b/repocribro/repocribro.py @@ -117,8 +117,8 @@ def create_app(cfg_files=['DEFAULT']): ext_master.call('init_first') ext_master.call('init_models') - ext_master.call('init_business') ext_master.call('init_security') + ext_master.call('init_business') ext_master.call('init_filters') ext_master.call('init_blueprints') ext_master.call('init_container') diff --git a/repocribro/security.py b/repocribro/security.py index 9c59bae..7d2f91a 100644 --- a/repocribro/security.py +++ b/repocribro/security.py @@ -2,7 +2,7 @@ import flask_login import flask_principal -from .models import UserAccount, Anonymous +from .models import UserAccount, Anonymous, Role def init_login_manager(db): @@ -98,6 +98,15 @@ def clear_session(*args): flask.session.pop(key, None) +def reload_anonymous_role(app, db): + with app.app_context(): + anonymous_role = db.session.query(Role).filter_by( + name=Anonymous.rolename + ).first() + if anonymous_role is not None: + Anonymous.roles.append(anonymous_role) + + @flask_principal.identity_loaded.connect def on_identity_loaded(sender, identity): """Principal helper for loading the identity of logged user From 7aa2f6282ee270ae474dc4e32de1e045a59b5e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sun, 22 Jul 2018 19:00:44 +0200 Subject: [PATCH 6/9] Default user role --- repocribro/controllers/auth.py | 7 ++++++- repocribro/ext_core.py | 5 +++-- repocribro/models.py | 2 ++ repocribro/security.py | 8 ++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/repocribro/controllers/auth.py b/repocribro/controllers/auth.py index 5983f92..edaaf51 100644 --- a/repocribro/controllers/auth.py +++ b/repocribro/controllers/auth.py @@ -2,7 +2,9 @@ import flask_sqlalchemy from ..models import User, UserAccount -from ..security import login as security_login, logout as security_logout +from ..security import login as security_login, \ + logout as security_logout, \ + get_default_user_role #: Auth controller blueprint auth = flask.Blueprint('auth', __name__, url_prefix='/auth') @@ -33,6 +35,9 @@ def github_callback_get_account(db, gh_api): is_new = False if gh_user is None: user_account = UserAccount() + default_role = get_default_user_role(flask.current_app, db) + if default_role is not None: + user_account.roles.append(default_role) db.session.add(user_account) gh_user = User.create_from_dict(user_data, user_account) db.session.add(gh_user) diff --git a/repocribro/ext_core.py b/repocribro/ext_core.py index 0fb78f1..3bb2df7 100644 --- a/repocribro/ext_core.py +++ b/repocribro/ext_core.py @@ -4,7 +4,7 @@ from .extending import Extension from .extending.helpers import ViewTab, Badge -from .models import Push, Release, Repository, Role, Anonymous +from .models import Push, Release, Repository, Role, Anonymous, UserAccount from .github import GitHubAPI @@ -168,7 +168,8 @@ def provide_filters(): def provide_roles(): return { 'admin': Role('admin', '*', 'Service administrators'), - 'anonymous': Role(Anonymous.rolename, 'search*:login', 'Not-logged user') + 'user': Role(UserAccount.default_rolename, 'search*', 'Regular users'), + 'anonymous': Role(Anonymous.rolename, 'search*:login', 'Not-logged users') } @staticmethod diff --git a/repocribro/models.py b/repocribro/models.py index 2c2d037..8117da4 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -230,6 +230,8 @@ class UserAccount(db.Model, UserMixin, SearchableMixin): """UserAccount in the repocribro app""" __tablename__ = 'UserAccount' + default_rolename = 'user' + #: Unique identifier of the user account id = sqlalchemy.Column(sqlalchemy.Integer, primary_key=True) #: Timestamp where account was created diff --git a/repocribro/security.py b/repocribro/security.py index 7d2f91a..4da0298 100644 --- a/repocribro/security.py +++ b/repocribro/security.py @@ -107,6 +107,14 @@ def reload_anonymous_role(app, db): Anonymous.roles.append(anonymous_role) +def get_default_user_role(app, db): + with app.app_context(): + user_role = db.session.query(Role).filter_by( + name=UserAccount.default_rolename + ).first() + return user_role + + @flask_principal.identity_loaded.connect def on_identity_loaded(sender, identity): """Principal helper for loading the identity of logged user From 6a92ce94101c7341f41cf2893589897e98c1525c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Sun, 22 Jul 2018 19:07:36 +0200 Subject: [PATCH 7/9] Creation of default roles if not present --- repocribro/extending/extension.py | 6 ++++-- repocribro/security.py | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/repocribro/extending/extension.py b/repocribro/extending/extension.py index 416b555..f4fc22f 100644 --- a/repocribro/extending/extension.py +++ b/repocribro/extending/extension.py @@ -146,9 +146,11 @@ def init_filters(self): def init_security(self): """Hook operation to setup privileges (roles and actions)""" - permissions = config = self.app.container.get('permissions') - for role_name in self.provide_roles().keys(): + from ..security import create_default_role + permissions = self.app.container.get('permissions') + for role_name, role in self.provide_roles().items(): permissions.register_role(role_name) + create_default_role(self.app, self.db, role) for action_name in self.provide_actions(): permissions.register_action(action_name) diff --git a/repocribro/security.py b/repocribro/security.py index 4da0298..e8b51e2 100644 --- a/repocribro/security.py +++ b/repocribro/security.py @@ -115,6 +115,16 @@ def get_default_user_role(app, db): return user_role +def create_default_role(app, db, role): + with app.app_context(): + existing_role = db.session.query(Role).filter_by( + name=role.name + ).first() + if existing_role is None: + db.session.add(role) + db.session.commit() + + @flask_principal.identity_loaded.connect def on_identity_loaded(sender, identity): """Principal helper for loading the identity of logged user From c89c064f9e68710922f01bd7edea39b0c54a2d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Mon, 23 Jul 2018 12:01:49 +0200 Subject: [PATCH 8/9] Validation of serialized privileges string --- repocribro/controllers/admin.py | 18 +++++++++++++----- repocribro/models.py | 14 ++++++++++++-- repocribro/templates/admin/role.html | 1 + repocribro/templates/admin/tabs/roles.html | 1 + 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/repocribro/controllers/admin.py b/repocribro/controllers/admin.py index 6e8c8eb..fa8cf60 100644 --- a/repocribro/controllers/admin.py +++ b/repocribro/controllers/admin.py @@ -178,11 +178,15 @@ def role_edit(name): if name == '': flask.flash('Couldn\'t make that role...', 'warning') return flask.redirect(flask.url_for('admin.index', tab='roles')) - # TODO: validate priv (chars, separators, etc.) + + role.name = name + role.privileges = priv.lower() + role.description = desc + if not role.valid_privileges(): + flask.flash('Unsaved - incorrect characters in privileges ' + 'for role {}'.format(name), 'warning') + return flask.redirect(flask.url_for('admin.role_detail', name=role.name)) try: - role.name = name - role.privileges = priv - role.description = desc db.session.commit() if name == Anonymous.rolename: reload_anonymous_role(flask.current_app, db) @@ -223,8 +227,12 @@ def role_create(): if name == '': flask.flash('Couldn\'t make that role...', 'warning') return flask.redirect(flask.url_for('admin.index', tab='roles')) + role = Role(name, priv, desc) + if not role.valid_privileges(): + flask.flash('Unsaved - incorrect characters in privileges ' + 'for role {}'.format(name), 'warning') + return flask.redirect(flask.url_for('admin.role_detail', name=role.name)) try: - role = Role(name, priv, desc) db.session.add(role) db.session.commit() if name == Anonymous.rolename: diff --git a/repocribro/models.py b/repocribro/models.py index 8117da4..d822931 100644 --- a/repocribro/models.py +++ b/repocribro/models.py @@ -3,6 +3,7 @@ import flask_login import datetime import fnmatch +import re from .database import db @@ -51,6 +52,8 @@ def fulltext_query(cls, query_str, db_query): class RoleMixin: """Mixin for models representing roles""" + priv_regex = re.compile('[a-z_\?\*]+') + def __eq__(self, other): """Equality of roles is based on names @@ -84,11 +87,18 @@ def permits(self, privilege): privileges = self.privileges.split(':') if privilege in privileges: return True - for p in privileges: - if fnmatch.fnmatch(privilege, p): + for priv in privileges: + if fnmatch.fnmatch(privilege, priv): return True return False + def valid_privileges(self): + privileges = self.privileges.split(':') + for priv in privileges: + if not self.priv_regex.search(priv): + return False + return True + class UserMixin(flask_login.UserMixin): @property diff --git a/repocribro/templates/admin/role.html b/repocribro/templates/admin/role.html index 7c635f0..f35f595 100644 --- a/repocribro/templates/admin/role.html +++ b/repocribro/templates/admin/role.html @@ -23,6 +23,7 @@

{{ role.name }}
+ Privileges separated by semicolon : (see documentation)
diff --git a/repocribro/templates/admin/tabs/roles.html b/repocribro/templates/admin/tabs/roles.html index 329bb97..bb92eee 100644 --- a/repocribro/templates/admin/tabs/roles.html +++ b/repocribro/templates/admin/tabs/roles.html @@ -17,6 +17,7 @@

Roles

+ Privileges separated by semicolon : (see documentation)
From 4eaf62a17a9cf878cb9af1f92080b3cbdcfb9572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Such=C3=A1nek?= Date: Mon, 23 Jul 2018 14:22:07 +0200 Subject: [PATCH 9/9] Adjusting tests and fixing --- repocribro/controllers/admin.py | 9 ++++++--- repocribro/controllers/core.py | 1 - repocribro/ext_core.py | 6 ++++-- repocribro/repocribro.py | 4 ++++ repocribro/security.py | 35 ++++++++++++++++++++++----------- tests/conftest.py | 5 ++++- tests/test_controller_admin.py | 9 +++++---- tests/test_controller_core.py | 2 +- tests/test_models.py | 2 +- 9 files changed, 48 insertions(+), 25 deletions(-) diff --git a/repocribro/controllers/admin.py b/repocribro/controllers/admin.py index fa8cf60..7ba9849 100644 --- a/repocribro/controllers/admin.py +++ b/repocribro/controllers/admin.py @@ -185,7 +185,8 @@ def role_edit(name): if not role.valid_privileges(): flask.flash('Unsaved - incorrect characters in privileges ' 'for role {}'.format(name), 'warning') - return flask.redirect(flask.url_for('admin.role_detail', name=role.name)) + return flask.redirect(flask.url_for('admin.role_detail', + name=role.name)) try: db.session.commit() if name == Anonymous.rolename: @@ -231,7 +232,8 @@ def role_create(): if not role.valid_privileges(): flask.flash('Unsaved - incorrect characters in privileges ' 'for role {}'.format(name), 'warning') - return flask.redirect(flask.url_for('admin.role_detail', name=role.name)) + return flask.redirect(flask.url_for('admin.role_detail', + name=role.name)) try: db.session.add(role) db.session.commit() @@ -242,7 +244,8 @@ def role_create(): 'warning') db.session.rollback() return flask.redirect(flask.url_for('admin.index', tab='roles')) - return flask.redirect(flask.url_for('admin.role_detail', name=role.name)) + return flask.redirect(flask.url_for('admin.role_detail', + name=role.name)) @admin.route('/role//add', methods=['POST']) diff --git a/repocribro/controllers/core.py b/repocribro/controllers/core.py index a8c8699..1d046a3 100644 --- a/repocribro/controllers/core.py +++ b/repocribro/controllers/core.py @@ -22,7 +22,6 @@ def index(): @core.route('/search/') @core.route('/search') @core.route('/search/') -@permissions.actions.search.require(403) def search(query=''): """Search page (GET handler) diff --git a/repocribro/ext_core.py b/repocribro/ext_core.py index 3bb2df7..011d133 100644 --- a/repocribro/ext_core.py +++ b/repocribro/ext_core.py @@ -168,8 +168,10 @@ def provide_filters(): def provide_roles(): return { 'admin': Role('admin', '*', 'Service administrators'), - 'user': Role(UserAccount.default_rolename, 'search*', 'Regular users'), - 'anonymous': Role(Anonymous.rolename, 'search*:login', 'Not-logged users') + 'user': Role(UserAccount.default_rolename, 'search*', + 'Regular users'), + 'anonymous': Role(Anonymous.rolename, 'search*:login', + 'Not-logged users') } @staticmethod diff --git a/repocribro/repocribro.py b/repocribro/repocribro.py index b4cc6c8..66b31a5 100644 --- a/repocribro/repocribro.py +++ b/repocribro/repocribro.py @@ -80,6 +80,10 @@ def __init__(self): super().__init__(PROG_NAME) self.container = DI_Container() + def ext_call(self, what_to_call): + ext_master = self.container.get('ext_master') + ext_master.call(what_to_call) + def create_app(cfg_files=['DEFAULT']): """Factory for making the web Flask application diff --git a/repocribro/security.py b/repocribro/security.py index e8b51e2..d670c04 100644 --- a/repocribro/security.py +++ b/repocribro/security.py @@ -100,29 +100,40 @@ def clear_session(*args): def reload_anonymous_role(app, db): with app.app_context(): - anonymous_role = db.session.query(Role).filter_by( - name=Anonymous.rolename - ).first() + anonymous_role = None + try: + anonymous_role = db.session.query(Role).filter_by( + name=Anonymous.rolename + ).first() + except: + pass if anonymous_role is not None: Anonymous.roles.append(anonymous_role) def get_default_user_role(app, db): + user_role = None with app.app_context(): - user_role = db.session.query(Role).filter_by( - name=UserAccount.default_rolename - ).first() + try: + user_role = db.session.query(Role).filter_by( + name=UserAccount.default_rolename + ).first() + except: + pass return user_role def create_default_role(app, db, role): with app.app_context(): - existing_role = db.session.query(Role).filter_by( - name=role.name - ).first() - if existing_role is None: - db.session.add(role) - db.session.commit() + try: + existing_role = db.session.query(Role).filter_by( + name=role.name + ).first() + if existing_role is None: + db.session.add(role) + db.session.commit() + except: + pass @flask_principal.identity_loaded.connect diff --git a/tests/conftest.py b/tests/conftest.py index 56b9e2b..e634caf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -114,6 +114,8 @@ def teardown(): _db.app = app _db.create_all() + app.ext_call('init_security') # create default roles + request.addfinalizer(teardown) return _db @@ -161,11 +163,12 @@ def filled_db_session(empty_db_session): session = empty_db_session import datetime from repocribro.models import Role, UserAccount, User, \ - Organization, Repository, Commit, Release, Push + Organization, Repository, Commit, Release, Push, Anonymous # Setup admin role admin_role = Role('admin', '*', 'Administrators') session.add(admin_role) + Anonymous.permits = lambda x: True account_banned = UserAccount() account_banned.active = False diff --git a/tests/test_controller_admin.py b/tests/test_controller_admin.py index 62e4965..7641db5 100644 --- a/tests/test_controller_admin.py +++ b/tests/test_controller_admin.py @@ -118,17 +118,18 @@ def test_account_delete(filled_db_session, app_client): def test_role_create_edit_delete(filled_db_session, app_client): from repocribro.models import Role app_client.get('/test/login/admin') - existing_role = {'name': 'admin', 'description': ''} + existing_role = {'name': 'admin', 'privileges': '*', 'description': ''} ret = app_client.post('/admin/roles/create', data=existing_role) assert ret.status == '302 FOUND' - bad_role = {'name': '', 'description': ''} + bad_role = {'name': '', 'privileges': '', 'description': ''} ret = app_client.post('/admin/roles/create', data=bad_role) assert ret.status == '302 FOUND' role = filled_db_session.query(Role).filter_by(name='testadmin').first() assert role is None - new_role = {'name': 'testadmin', 'description': ''} + new_role = {'name': 'testadmin', 'privileges': 'login:s*', + 'description': ''} ret = app_client.post('/admin/roles/create', data=new_role) assert ret.status == '302 FOUND' role = filled_db_session.query(Role).filter_by(name='testadmin').first() @@ -147,7 +148,7 @@ def test_role_create_edit_delete(filled_db_session, app_client): role = filled_db_session.query(Role).filter_by(name='test_admin').first() assert role is None - edit_role = {'name': 'test_admin', 'description': ''} + edit_role = {'name': 'test_admin', 'privileges': '*', 'description': ''} ret = app_client.post('/admin/role/testadmin/edit', data=edit_role) assert ret.status == '302 FOUND' role = filled_db_session.query(Role).filter_by(name='test_admin').first() diff --git a/tests/test_controller_core.py b/tests/test_controller_core.py index d1e880b..2b3926f 100644 --- a/tests/test_controller_core.py +++ b/tests/test_controller_core.py @@ -6,7 +6,7 @@ def test_landing(app_client): app_client.get('/').data.decode('utf-8') -def test_search(app_client): +def test_search(filled_db_session, app_client): res = app_client.get('/search') assert res.status == '200 OK' assert '

Search

' in res.data.decode('utf-8') diff --git a/tests/test_models.py b/tests/test_models.py index df06455..999b4da 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -171,7 +171,7 @@ def test_anonymous(): assert not anonym.has_role('user') assert not anonym.is_active assert not anonym.is_authenticated - assert anonym.rolenames == [] + assert anonym.rolenames == ['anonymous'] assert anonym.is_anonymous repo = Repository(777, None, 'some/repo', 'repo', 'C++', '', '', '', False, None, None, Repository.VISIBILITY_PRIVATE)