inveniosoftware / invenio-access

Invenio module for common role based access control.

Invenio-Access

Role-based access control (RBAC) for Invenio.

Invenio-Access works together with Invenio-Accounts to provide a full-fledge authentication and authorization system for Flask and Invenio based on a suite of existing Flask extensions such as:

• Flask-Security
• Flask-Login
• Flask-Principal
• passlib

Features:

• Role-based access control with object level permissions.
• CLI and administration interface for allowing/denying actions to users, roles or system roles.
• Support for superuser privileges.

Further documentation is available on https://invenio-access.readthedocs.io/

invenio-access's Issues

admin: add warning message when cascade delete

When a Role with some ActionRoles is deleted both the Role and the group of ActionRoles are silently deleted. We should warn the user about it.

Invenio-Access: Review administration interface

docs: usage documentation

• Defining actions and permissions
• Requiring permissions
• Read The Docs not rendering properly

installation: investigate correct dependency order (Accounts)

• investigate what is the correct dependency order with Invenio-Accounts
• add missing dependencies

cc @hachreak @lnielsen

tests: simplify doctest execution

The following cookiecutter change:

inveniosoftware/cookiecutter-invenio-module#98

should be propagated to this Invenio module.

Namely, in run-tests.sh, the sphinx for doctests is invoked after pytest run:

$ tail -3 ./\{\{\ cookiecutter.project_shortname\ \}\}/run-tests.sh
sphinx-build -qnNW docs docs/_build/html && python setup.py test && sphinx-build -qnNW -b doctest docs docs/_build/doctest

This sometimes led to problems on Travis CI with the second sphinx-build run due
to "disappearing" dependencies after the example application was tested.

A solution that worked for invenio-marc21 (see
inveniosoftware/invenio-marc21#49 (comment))
and that was integrated in cookiecutter (see
inveniosoftware/cookiecutter-invenio-module#98) was to
run doctest execution in pytest, removing the second sphinx-build invocation.

This both solved Travis CI build failures and simplified test suite execution.

Note that this change may necessitate to amend the code tests etc so that things
would be executed with the Flask application context (see
inveniosoftware/invenio-marc21@09e98fc).

travis: fix failing build

Looks to be related to bug in SQLAlchemy-Continuum:

inveniosoftware/invenio-db#113

model: be resilient to mysql bug #77591 (NULL change on foreign key)

Problem:
Travis currently uses mysql version 5.6.33 which has bug #77591.
This bug prevents this alembic recipe from running correctly.

Solution:
One solution would be to install mysql-5.7 on travis but we would have to use it in every module which is not acceptable given how travis is slow. We will instead change the alembic recipe.

cli: add `access add role` command

addrole would also be good. We need to add roles to users, and there's nothing I see on the front end side to do that yet. I may make a start on this soon, since we really need it.

by @eamonnmag in inveniosoftware/invenio-accounts#2 (comment)

setup: define entry point groups for extension.

cli: deprecation warning for callback

Triggered by process_action and process_email:

Warning: Invoked legacy parameter callback "<function process_email at 0x7effcaebb230>".  The new signature for such callbacks starting with click 2.0 is (ctx, param, value).

global: fix import error

Same as:

• invenio-pidstore #130
• invenio-access #170

https://travis-ci.org/inveniosoftware/invenio-access/jobs/656676593

Warning, treated as error:
autodoc: failed to import module u'ext' from module u'invenio_access'; the following exception was raised:
cannot import name config
The command "./run-tests.sh" exited with 2.

Fix also deprecation warning in Travis config:
https://travis-ci.org/inveniosoftware/invenio-access/jobs/656676593/config

cli: command to remove authorisation + model fixes

• Primary key for model actionusers/roles
• CLI to list user/role authorisations
• CLI to remove an authorisation
• Problems with listing partial action needs in access list

cli: allow/deny action for all users

addresses #71

global: make new release v1.2.0

tests: simplify doctest execution

The following cookiecutter change:

inveniosoftware/cookiecutter-invenio-module#98

should be propagated to this Invenio module.

Namely, in run-tests.sh, the sphinx for doctests is invoked after pytest run:

$ tail -3 ./\{\{\ cookiecutter.project_shortname\ \}\}/run-tests.sh
sphinx-build -qnNW docs docs/_build/html && python setup.py test && sphinx-build -qnNW -b doctest docs docs/_build/doctest

This sometimes led to problems on Travis CI with the second sphinx-build run due
to "disappearing" dependencies after the example application was tested.

A solution that worked for invenio-marc21 (see
inveniosoftware/invenio-marc21#49 (comment))
and that was integrated in cookiecutter (see
inveniosoftware/cookiecutter-invenio-module#98) was to
run doctest execution in pytest, removing the second sphinx-build invocation.

This both solved Travis CI build failures and simplified test suite execution.

Note that this change may necessitate to amend the code tests etc so that things
would be executed with the Flask application context (see
inveniosoftware/invenio-marc21@09e98fc).

RFC Complex access rights for records

Current (legacy) situation

On Invenio 1.X records are restricted based on collection rights (if the record is not in any collection it is restricted to the owner and the admins).
Also there is the possibility to grant access to a specific record inside a restricted collection by using certain MARC tags, i.e 506

Migration problem

Sometimes two collections were created artificially to restrict a set of records, i.e. "Videos" and "Restricted Videos".
Moreover in the situation where one gives access to a record inside the restricted collection to a given person, this person can't search for the record she/he is allow to see inside the restricted collection. This kind os technic allows the users to see the record only if they know the URL.
This artificial separation between public and restricted records can now be achieve using facets.

Proposal

Store the access rights inside the metadata of the record, i.e.

{
  "_access": {
    "read": {
      "system": ["group_a", "group_b"], 
      "user": ["[email protected]"]
    }
  }
}

The so called system field stores the access rights derived by the system from, for example, the collections/communities that the record belongs to. This field should not be modified by anyone except by the system.
On the other hand the user field is meant to be modify by the users, for example upon submission, so the user submitting something can decide to share her/his work with some else which is not part of the community.

If both fields are empty, the record is public.

Possible problems with this solution

• Potentially the records will get modify just to change the access rights and this will generate unnecessary versions.
• If we combine this solution with collections (or any other thing defining access rights) we need to be sure the access rights are correctly propagated to the record.

Possible benefits of this solution

• Potentially the records will get modify just to change the access rights and this will generate a useful trace of the evolution of the access rights (maybe easier to combine with memento API if we want to expose all the versions).
• Makes migration much easier eliminating the headache of cleaning the collection tree and the access right definitions for viewrestrcoll action.
• Access rights are defined in one place, so answering questions like "why don't I have access to this record?" are much easier to answer.

We have made a small proof of concept on the CDS overlay

Counterproposal
Migrate the viewrestrcoll action as it is, creating a parametrised action, and when migrating the records also migrate the collections they belong to so they can be used to decide whether a user has access to a given record or not.

Possible problems with this solution

• The "user" access rights definitions have to be migrated and treated in the same way as in the first proposal.
• All the access rights definitions are going to be migrated, meaning a cleanup in the current system needs to be done (always talking from the point of view of v1.2 -> v3.0 migration)

Possible benefits of this solution

• Access rights don't need to be propagated to the record.

Note for the reader
The RFC might seems "migration driven" but, regardless if one starts clean or from an existing system, the way access rights are defined IMHO should be the same for the sake of everyone 😉

CC @inveniosoftware/triagers @omelkonian

permissions: remove DynamicPermission

fix issue with superuser and parameters

Here is the deal: in Invenio, if there is no explicit restrictions, it means that the action should be available for everybody. Thus, when we have the following table:

user	action	parameter
1	viewrecord	5

it means that all the record are freely accessible, except the record 5 that can only be seen by the user 1.

And it works!!!

The problem

The problem comes when we have a superuser. consider the following table:

user	action	parameter
1	viewrecord	5
0	superuser

We now have a superuser, which is able to access everything. What should it change from before? now, all the record should be freely viewable, except the record 5 accessible only by the user 1 (explicit action) and the user 0 (superuser).

BUT this is not true. Now ALL the records are restricted! and only the superuser can see them (except for the record 5 that can also be seen by the user 1).

The source of the problem

As you can see here: https://github.com/mattupstate/flask-principal/blob/master/flask_principal.py#L328-L339, Flask-Principal allows a user to access a resource if there is a common value between what the user provides and what the action needs.

As long as there is no superuser, both sets will be empty for the records (except for the record 5).

But as soon as we add a superuser, all the actions will have a set with the superuser inside! See https://github.com/inveniosoftware/invenio-access/blob/master/invenio_access/permissions.py#L85

Tests for it

tests already exists and are currently commented because they obviously don't pass (see #56): https://github.com/remileduc/invenio-access/blob/c677ee4d5592829c67835ca8864122dc349cc6ea/tests/test_invenio_access_permissions.py#L181-L190

Note that these tests work if you remove the line https://github.com/remileduc/invenio-access/blob/c677ee4d5592829c67835ca8864122dc349cc6ea/tests/test_invenio_access_permissions.py#L127
(and obviously the corresponding tests https://github.com/remileduc/invenio-access/blob/c677ee4d5592829c67835ca8864122dc349cc6ea/tests/test_invenio_access_permissions.py#L159-L165)

tests for MySQL not passing

The tests for MySQL are not passing.

The first issue is the use of a wrong user: see https://travis-ci.org/inveniosoftware/invenio-access/builds/259488732

This is fixed in #128 with this commit: ef685ba
However there is still an error related to alembic: https://travis-ci.org/inveniosoftware/invenio-access/builds/260114107

This error happens here: https://github.com/inveniosoftware/invenio-access/blob/master/tests/test_invenio_access.py#L130-L131
a test table is apparently created... But this is not bugged in PSQL \o/

good luck with that ^^

release v1.0.0a2

• Depends on #32

admin: Flask-Admin view

docs: update structure

• introduce new documentation structure inveniosoftware/cookiecutter-invenio-module#56
• document user perspective
• document signals (if not done)
• document example app (if not done)

acl: define actions in separate packages

Split default actions from https://github.com/inveniosoftware/invenio-access/blob/master/invenio_access/local_config.py#L143-L188 to separate modules (acl.py).

invenio_documents module required on access testsuite

Inside access module we have tests that require invenio_documents module.

tests: fix import sorting (build failure)

#160

test fail with release and devel requirements

https://travis-ci.org/inveniosoftware/invenio-access/jobs/106216062

views: add a REST API

Note: This discussion might be a little premature as @egabancho is working on an alternative access control, but it is still interesting to have it just as a reminder.

This module needs a REST API which would enable to assign and unassign permissions to users and roles.

Relates to #72

IP based access control

In order to provide access control based on IP ranges system roles will be used. Tasks:

• Write an RFC:
  • Gather use cases from the partners
  • Define how IP ranges are represented
• Implement it:
  • Define how the configuration should look like
  • Implement dynamic registration of system roles based on the configuration
  • Implement identity loading based on the user / configuration

Add a way to allow any users or deny all users

See #56.
With this PR we prevented the creation of ActionUsers without any users, because the behavior was undeterminated.
We need to find a way for specific case to allow or deny all users from an action.

Adding a user to the superuser-access role locks out from viewing all unregistered users

Minimal reproduction: https://github.com/jacquerie/invenio-records-ui/blob/40ecddfe45c1a5fc17b87fdb73be8c060912580d/examples/permsapp.py
When visiting /records/1 as an unregistered user one gets a login page, rather than the record.

I understand that this is expected behaviour given the code, but I want to argue that is not reasonable behaviour given the objective of the library.

Locking out from viewing all unregistered users when a superadmin exists is reasonable behaviour when we are talking about files on a machine: if a sudo user exists, by default all files should LOCK access by guests.

This is not reasonable when we are talking about a web server: the mere fact that an admin exists should NOT LOCK access by guests.

Applications built using Invenio are more like web servers, so by default should have the latter behaviour.

CC: @jmartinm @david-caro @kaplun

cache: improve performance of DynamicPermissions

• caching of identities
• caching of DynamicPermissions (or Action and ParametrizedAction)
• cache invalidation (database table is changed)
• performance test - generate users, roles, actions
• configurable instance of Flask-Cache passed to InvenioAccess extension

travis: master build failure

https://travis-ci.org/inveniosoftware/invenio-access/builds/491238373

permissions: DynamicPermissions default allow or deny?

In https://github.com/inveniosoftware/invenio-access/blob/master/examples/app.py#L70-L72 it says

If you are using an action in your app which does not have any role or user
assigned, the action will be allowed to perform by everyone (Anonymous users
included).

Is there a why a permission is allowed by default with DynamicPermission?

It seems that Flask-Principal by default denies https://github.com/mattupstate/flask-principal/blob/master/flask_principal.py#L328-L339

E.g., take the following code using Permission and DynamicPermission.

from flask_principal import ActionNeed, Permission
from invenio_access.permissions import DynamicPermission

open_action = ActionNeed('open')
perm = Permission(open_action)
dynamic_perm = DynamicPermission(open_action)

In above example is a user does not have the ActionNeed('open') then perm.can() will deny access, while dynamic_perm.can() will allow access (if no users/roles where assigned the permission).

ext: cache cannot be set when using invenio-base to load invenio-access

Currently the cache has to be injected to the extension when being initialized, e.g.:

cache = RedisCache()
InvenioAccess(app, cache=cache)

This works if you manually initialise the extension, however if you use Invenio-Base there's no way to set the cache.

models: rename exclude to deny

https://github.com/inveniosoftware/invenio-access/blob/master/invenio_access/models.py#L48

global: release checklist

readthedocs: example app fix

add ``explicit_excludes`` to permissions

Invenio-Access Permission class adds the field excplicit_needs to the object. This is expanded and assuming that an action type need has excludes they will be added. See here.

However, if I have excludes that are mere Needs they will:

• Not be added (We should not have to access _permissions.excludes directly from a children class)
• Be added to the needs if passed through explicit_needs. See here

permissions: assign any_user/authenticated_user to identities

We have created new system roles such as any_user/authenticated_user. These needs however still need to be provided to the identities via the identity_loaded/changed signals

Breaking installation due to missing dependency

Traceback (most recent call last):
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/sphinx/ext/autodoc/importer.py", line 32, in import_module
    return importlib.import_module(modname)
  File "/Users/ppanero/.pyenv/versions/3.6.9/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/ppanero/Workspace/inveniosw/invenio-records-permissions/invenio_records_permissions/__init__.py", line 15, in <module>
    from .factories import record_create_permission_factory, \
  File "/Users/ppanero/Workspace/inveniosw/invenio-records-permissions/invenio_records_permissions/factories/__init__.py", line 12, in <module>
    from .records import record_create_permission_factory, \
  File "/Users/ppanero/Workspace/inveniosw/invenio-records-permissions/invenio_records_permissions/factories/records.py", line 12, in <module>
    from ..policies import get_record_permission_policy
  File "/Users/ppanero/Workspace/inveniosw/invenio-records-permissions/invenio_records_permissions/policies/__init__.py", line 12, in <module>
    from .base import BasePermissionPolicy
  File "/Users/ppanero/Workspace/inveniosw/invenio-records-permissions/invenio_records_permissions/policies/base.py", line 15, in <module>
    from invenio_access import Permission
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/invenio_access/__init__.py", line 408, in <module>
    from .ext import InvenioAccess
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/invenio_access/ext.py", line 19, in <module>
    from .loaders import load_permissions_on_identity_loaded
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/invenio_access/loaders.py", line 10, in <module>
    from flask_security import current_user
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/flask_security/__init__.py", line 13, in <module>
    from .core import Security, RoleMixin, UserMixin, AnonymousUser, current_user
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/flask_security/core.py", line 28, in <module>
    from .forms import ChangePasswordForm, ConfirmRegisterForm, \
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/flask_security/forms.py", line 69, in <module>
    email_validator = Email(message='INVALID_EMAIL_ADDRESS')
  File "/Users/ppanero/.virtualenvs/invenio-records-permissions/lib/python3.6/site-packages/wtforms/validators.py", line 332, in __init__
    raise Exception("Install 'email_validator' for email validation support.")
Exception: Install 'email_validator' for email validation support.

caching: arguments are not taken into account leading to wrong permissions

The caching layer does not take the action argument into account.

E..g if the action is edit with argument 1 , and the caching layer will just get any previous cached result for the edit action, which could be e.g. edit with argument 2. This is essentially leads to wrong permissions being applied when caching is used.

global: no application context when discovering admin.py

I just tried invenio-access master and I get the following error when I try to call inveniomanage:

Traceback (most recent call last):
  File "/Users/nharraud/.virtualenvs/b2share-evolution/bin/inveniomanage", line 7, in <module>
    from invenio_base.manage import main
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/invenio_base/manage.py", line 28, in <module>
    manager = Manager(create_app(), with_default_commands=False)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/invenio_base/factory.py", line 279, in create_app
    extensions=ExtensionRegistry(app),
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask_registry/registries/appdiscovery.py", line 153, in __init__
    self.register(app, ext_name)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask_registry/registries/appdiscovery.py", line 166, in register
    ext(app)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/invenio_ext/admin/__init__.py", line 120, in setup_app
    'admin', app=app, with_setup=True, admin=admin
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/invenio_ext/admin/__init__.py", line 89, in __init__
    super(AdminDiscoveryRegistry, self).__init__(*args, **kwargs)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask_registry/registries/modulediscovery.py", line 254, in __init__
    self.discover(app=app)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask_registry/registries/modulediscovery.py", line 175, in discover
    self._discover_module(pkg)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask_registry/registries/modulediscovery.py", line 184, in _discover_module
    module = import_string(import_str, silent=self.silent)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/werkzeug/utils.py", line 412, in import_string
    __import__(import_name)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/src/invenio-access/invenio_access/admin.py", line 32, in <module>
    from .models import AccAuthorization, AccROLE, UserAccROLE
  File "/Users/nharraud/.virtualenvs/b2share-evolution/src/invenio-access/invenio_access/models.py", line 39, in <module>
    from .local_config import CFG_ACC_ACTIVITIES_URLS, \
  File "/Users/nharraud/.virtualenvs/b2share-evolution/src/invenio-access/invenio_access/local_config.py", line 222, in <module>
    'runbibedit' : (_("Run Record Editor"), "%s/%s/edit/?ln=%%s" % (cfg['CFG_SITE_URL'], cfg['CFG_SITE_RECORD'])),
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/werkzeug/local.py", line 368, in <lambda>
    __getitem__ = lambda x, i: x._get_current_object()[i]
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/werkzeug/local.py", line 297, in _get_current_object
    return self.__local()
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/invenio_base/globals.py", line 36, in <lambda>
    cfg = LocalProxy(lambda: current_app.config)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/werkzeug/local.py", line 338, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/werkzeug/local.py", line 297, in _get_current_object
    return self.__local()
  File "/Users/nharraud/.virtualenvs/b2share-evolution/lib/python2.7/site-packages/flask/globals.py", line 34, in _find_app
    raise RuntimeError('working outside of application context')
RuntimeError: working outside of application context

This error does not appear with 0.2.0 version. I suspect that the bug existed before and just got more visible when commit d434c35 modified invenio_access/local_config.py.

api: stabilise and document

• check existing API functionality
• add missing important API functionality
• check API function signatures and parameters
• enhance API docstrings (param, returns, raises, versionadded)
• plug API functions to existing docs

release: make beta release

permissions: allow/deny action with argument

• add column for denying (excluding) action
• add column for defining argument(s) for action
  • Q: Do we need to support multiple arguments per action? (cc @tiborsimko @egabancho @lnielsen )

tests: addition of test_example_app.py

The package contains examples/app.py example application, but there is no test
for it in tests/test_example_app.py. It should be added. See existing examples:

• https://github.com/inveniosoftware/invenio-records/blob/master/tests/test_example_app.py
• https://github.com/inveniosoftware/invenio-marc21/blob/master/tests/test_example_app.py

example: `tear_down.sh` is malfunctioning on BSD

When running ./app-teardown.sh the following error is retrieved when running on BSD:

rm: ./instance: is a directory
rm: -Rf: No such file or directory

It seems that the order in which the parameters are provided to rm in app_teardown.sh is making it fail. Since GNU doesn't care about argument order I think we should always put options first.

api: ActionUsers and ActionRoles argument value suggest

It seems to me that there is currently no way to know what argument values are valid or not for a given action.

What about enabling the registration of "suggests" and "validators" in setup.py under invenio_access.actions alongside their permission?

This could be used in the CLI and the REST API.

@inveniosoftware/maintainers

Relates to #72

models: constraint naming conventions fix

global: review CLI for invenio-access

1. Do we need an invenio access init command?
2. Do we need an invenio access permissions <user> command to list which actions are allowed/denied for a given user.