views: access control #18
Conversation
nharraud
force-pushed
the
access_control
branch
from
463c9d1 to
71d9611
Compare
| self._update_permission_factory = None | ||
| self._delete_permission_factory = None | ||
|
|
||
| @property |
There was a problem hiding this comment.
IMHO it can be werkzeug.utils:cached_property
| $ curl -XPOST http://localhost:5000/login -d 'email=admin%40invenio-software.org&password=123456' -c mycookie | ||
| $ curl -XGET http://localhost:5000/records/1 -b mycookie | ||
|
|
||
| Login as [email protected] (password 123456), hwo has not |
|
@lnielsen @jirikuncar One thing you can see in the code but which I forgot to mention is that the tests and example use the UI login as we don't have oauth yet. As the search-ui will be also use cookie authentication to access this REST API this seemed a valid usage even if not the most common. |
|
@nharraud both OAuthClient and OAuth2Server should be ready on GitHub (not PyPI yet). |
nharraud
force-pushed
the
access_control
branch
from
71d9611 to
fadf8d7
Compare
|
@jirikuncar Sorry I meant Application Token. There is no need for an OAuth authentication here. Using an application token added directly to the database would remove the dependency on UI login. |
nharraud
force-pushed
the
access_control
branch
4 times, most recently
from
93d39c6 to
b60b025
Compare
nharraud
force-pushed
the
access_control
branch
from
b60b025 to
2a924a0
Compare
|
Coverage decreased because I added the delete permission factory methods even though there is no delete method implemented yet. |
|
@jirikuncar can we release a new version of invenio-rest? This depends on the last commits. |
nharraud
force-pushed
the
access_control
branch
2 times, most recently
from
b16c6d2 to
8c70ec8
Compare
|
@jirikuncar I removed the example with permissions, removed default permission factories and copied permissions.py from invenio_records for the tests. Ready for merge. |
nharraud
force-pushed
the
access_control
branch
from
8c70ec8 to
831b590
Compare
| try: | ||
| record.delete() | ||
| # mark all PIDs as DELETED | ||
| all_pids = PersistentIdentifier.query \ |
There was a problem hiding this comment.
IMHO you can use only one call of .filter() (or db.and_(cond1, cond2)).
all_pids = PersistentIdentifier.query.filter(
PersistentIdentifier.object_type == pid.object_type,
PersistentIdentifier.object_uuid == pid.object_uuid
).all()
nharraud
force-pushed
the
access_control
branch
2 times, most recently
from
1c38793 to
4bd5968
Compare
| """Load default read permission factory.""" | ||
| if self._read_permission_factory is None: | ||
| imp = self.app.config[ | ||
| "RECORDS_REST_DEFAULT_READ_PERMISSION_FACTORY" |
There was a problem hiding this comment.
It is not critical, but can you use single quotes (') for strings for consistency with rest of the code. They are randomly mixed (") through the code also in other files. Thanks
There was a problem hiding this comment.
@jirikuncar Yes you are right. Can I fix that in the next pull request (search integration) so that I don't spend too much time rebasing?
There was a problem hiding this comment.
IMHO it's not so much pain to run search&replace. You won't have many conflicts when you rebase other branches.
There was a problem hiding this comment.
Ok. Is there a guideline for quotes in Invenio? I think we should also fix the cookie cutter as it is putting " in some places.
There was a problem hiding this comment.
Just use your judgement and be consistent. When in doubt, be consistent with the surrounding code.
nharraud
force-pushed
the
access_control
branch
3 times, most recently
from
89666b1 to
5efe984
Compare
| @@ -0,0 +1,25 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
Please remove this file as its usage is discouraged with py.test.
avoid “init.py” files in your test directories. This way your tests can run easily against an installed version of mypkg, independently from the installed package if it contains the tests or not.
|
nharraud
force-pushed
the
access_control
branch
from
5efe984 to
0160301
Compare
* NEW Adds customizable access control to record views. Allow configuring different permissions per endpoint. (addresses inveniosoftware#14) Signed-off-by: Nicolas Harraudeau <[email protected]>
* NEW Adds DELETE support to the records REST API. Signed-off-by: Nicolas Harraudeau <[email protected]>
Signed-off-by: Nicolas Harraudeau <[email protected]>
|
@jirikuncar Done. Only the |
configuring different permissions per endpoint. (reference views: integrate API authentication/access control #14)
Signed-off-by: Nicolas Harraudeau [email protected]