Skip to content

Commit

Permalink
disable UI non-editable users/groups + error API side (fixes #164)
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault committed Jul 29, 2020
1 parent 05fae0a commit e6f4dff
Show file tree
Hide file tree
Showing 14 changed files with 241 additions and 113 deletions.
6 changes: 5 additions & 1 deletion HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,21 @@ Features / Changes
* Add tag description into generated Swagger API documentation.
* Add more usage details to start `Magpie` web application in documentation.
* Add database migration for new ``discoverable`` column of groups.
* Allow logged user to update its own information.
* Allow logged user to update its own information
(relates to `#170 <https://github.com/Ouranosinc/Magpie/issues/170>`_).
* Allow logged user to join *discoverable* groups.
* Change some UI CSS for certain pages to improve table readability.
* Add UI page to render unauthorized and forbidden views due to insufficient permissions.
* Add more validation and inputs to update group information.
* Allow combined configuration files (providers, permissions, users, groups) with inter-references between them
specified with ``MAGPIE_CONFIG_PATH`` environment variable or ``magpie.config_path`` setting (example in ``configs``).
* Add disabled checkboxes for UI rendering of non-editable items
(relates to `#164 <https://github.com/Ouranosinc/Magpie/issues/164>`_).

Bug Fixes
~~~~~~~~~~~~~~~~~~~~~
* Fix invalid API documentation of request body for ``POST /users/{user_name}/groups``.
* Fix `#164 <https://github.com/Ouranosinc/Magpie/issues/164>`_ (forbid *special* users and groups update and delete).
* Fix minor HTML issues in mako templates.

`1.11.0 <https://github.com/Ouranosinc/Magpie/tree/1.11.0>`_ (2020-06-19)
Expand Down
3 changes: 0 additions & 3 deletions magpie/api/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ def unauthorized_or_forbidden(request):
path = request.route_path("error").replace("/magpie", "")
data = {"error_request": content, "error_code": http_err.code}
return request_api(request, path, "POST", data)
subreq = Request.blank(location, base_url=request.application_url, POST=data,
headers={"Content-Type": CONTENT_TYPE_JSON})
return request.invoke_subrequest(subreq, use_tweens=True)
return raise_http(nothrow=True, http_error=http_err, detail=content["detail"], content=content, content_type=accept)


Expand Down
2 changes: 1 addition & 1 deletion magpie/api/login/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from magpie.api import schemas as s
from magpie.api.management.user.user_formats import format_user
from magpie.api.management.user.user_utils import create_user
from magpie.api.requests import get_multiformat_post, get_value_multiformat_post_checked
from magpie.api.requests import get_multiformat_post, get_principals, get_value_multiformat_post_checked
from magpie.constants import get_constant
from magpie.security import authomatic_setup, get_provider_names
from magpie.utils import CONTENT_TYPE_JSON, convert_response, get_logger, get_magpie_url
Expand Down
2 changes: 1 addition & 1 deletion magpie/api/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from magpie import __meta__
from magpie.constants import get_constant
from magpie.permissions import Permission
from magpie.utils import CONTENT_TYPE_HTML, CONTENT_TYPE_JSON, get_magpie_url
from magpie.utils import CONTENT_TYPE_HTML, CONTENT_TYPE_JSON

if TYPE_CHECKING:
# pylint: disable=W0611,unused-import
Expand Down
2 changes: 1 addition & 1 deletion magpie/typedefs.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
AnyKey = Union[Str, int]
AnyValue = Union[Str, Number, bool, None]
BaseJSON = Union[AnyValue, List["BaseJSON"], Dict[AnyKey, "BaseJSON"]]
JSON = Dict[AnyKey, BaseJSON]
JSON = Union[Dict[AnyKey, Union[BaseJSON, "JSON"]], List[BaseJSON]]

UserServicesType = Union[Dict[Str, Dict[Str, Any]], List[Dict[Str, Any]]]
ServiceOrResourceType = Union[models.Service, models.Resource]
Expand Down
22 changes: 14 additions & 8 deletions magpie/ui/management/templates/edit_group.mako
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,21 @@
<table>
%for user in users:
<tr>
<td><label>
% if user in members:
<td>
<label>
<input type="checkbox" value="${user}" name="member"
onchange="document.getElementById('edit_members').submit()" checked>${user}
% else:
<input type="checkbox" value="${user}" name="member"
onchange="document.getElementById('edit_members').submit()">${user}
% endif
</label></td>
%if user in members:
checked
%endif
%if group_name in MAGPIE_FIXED_GROUP_MEMBERSHIPS:
disabled
%else:
onchange="document.getElementById('edit_members').submit()"
%endif
>
${user}
</label>
</td>
</tr>
%endfor
</table>
Expand Down
32 changes: 15 additions & 17 deletions magpie/ui/management/templates/edit_user.mako
Original file line number Diff line number Diff line change
Expand Up @@ -144,23 +144,21 @@
<table>
%for group in groups:
<tr>
% if group in own_groups:
<td>
<label>
<input type="checkbox" value="${group}" name="member" checked
onchange="document.getElementById('edit_membership').submit()">
${group}
</label>
</td>
% else:
<td>
<label>
<input type="checkbox" value="${group}" name="member"
onchange="document.getElementById('edit_membership').submit()">
${group}
</label>
</td>
% endif
<td>
<label>
<input type="checkbox" value="${group}" name="member"
%if group in own_groups:
checked
%endif
%if group in MAGPIE_FIXED_GROUP_MEMBERSHIPS:
disabled
%else:
onchange="document.getElementById('edit_membership').submit()"
%endif
>
${group}
</label>
</td>
</tr>
%endfor
</table>
Expand Down
32 changes: 15 additions & 17 deletions magpie/ui/user/templates/edit_current_user.mako
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,21 @@
<table>
%for group in groups:
<tr>
% if group in joined_groups:
<td>
<label>
<input type="checkbox" value="${group}" name="member" checked
onchange="document.getElementById('edit_membership').submit()">
${group}
</label>
</td>
% else:
<td>
<label>
<input type="checkbox" value="${group}" name="member"
onchange="document.getElementById('edit_membership').submit()">
${group}
</label>
</td>
% endif
<td>
<label>
<input type="checkbox" value="${group}" name="member"
% if group in joined_groups:
checked
%endif
%if group in MAGPIE_FIXED_GROUP_MEMBERSHIPS:
disabled
%else:
onchange="document.getElementById('edit_membership').submit()"
%endif
>
${group}
</label>
</td>
</tr>
%endfor
</table>
Expand Down
8 changes: 4 additions & 4 deletions magpie/ui/user/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ def leave_discoverable_group(self, group_name):

@view_config(route_name="edit_current_user", renderer="templates/edit_current_user.mako", permission=Authenticated)
def edit_current_user(self):
own_groups = self.get_current_user_groups()
joined_groups = self.get_current_user_groups()
public_groups = self.get_discoverable_groups()
user_info = self.get_current_user_info()
user_info[u"edit_mode"] = u"no_edit"
user_info[u"own_groups"] = own_groups
user_info[u"joined_groups"] = joined_groups
user_info[u"groups"] = public_groups
error_message = ""

Expand Down Expand Up @@ -101,8 +101,8 @@ def edit_current_user(self):
# edits to groups checkboxes
if is_edit_group_membership:
selected_groups = self.request.POST.getall("member")
removed_groups = list(set(own_groups) - set(selected_groups))
new_groups = list(set(selected_groups) - set(own_groups))
removed_groups = list(set(joined_groups) - set(selected_groups))
new_groups = list(set(selected_groups) - set(joined_groups))
for group in removed_groups:
self.leave_discoverable_group(group)
for group in new_groups:
Expand Down
11 changes: 10 additions & 1 deletion magpie/ui/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import json
from typing import TYPE_CHECKING

from pyramid.httpexceptions import HTTPBadRequest, HTTPUnauthorized, HTTPForbidden, exception_response
from pyramid.httpexceptions import HTTPBadRequest, exception_response
from pyramid.request import Request

from magpie.api.requests import get_logged_user
from magpie.constants import get_constant
from magpie.utils import CONTENT_TYPE_JSON, get_header, get_logger, get_magpie_url

if TYPE_CHECKING:
Expand Down Expand Up @@ -83,16 +84,24 @@ def wrap(*args, **kwargs):


class BaseViews(object):
"""Base methods for Magpie UI pages."""

def __init__(self, request):
self.request = request
self.magpie_url = get_magpie_url(request.registry)
self.logged_user = get_logged_user(request)

self.MAGPIE_FIXED_GROUP_MEMBERSHIPS = [
get_constant("MAGPIE_ANONYMOUS_GROUP", settings_container=request),
]
"""Special groups membership that cannot be edited."""

def add_template_data(self, data=None):
# type: (Optional[Dict[Str, Any]]) -> Dict[Str, Any]
"""Adds required template data for the 'heading' mako template applied to every UI page."""
all_data = data or {}
all_data.setdefault("MAGPIE_SUB_TITLE", "Administration")
all_data.setdefault("MAGPIE_FIXED_GROUP_MEMBERSHIPS", self.MAGPIE_FIXED_GROUP_MEMBERSHIPS)
magpie_logged_user = get_logged_user(self.request)
if magpie_logged_user:
all_data.update({"MAGPIE_LOGGED_USER": magpie_logged_user.user_name})
Expand Down
15 changes: 8 additions & 7 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ search = __version__ = "{current_version}"
replace = __version__ = "{new_version}"

[bumpversion:file:HISTORY.rst]
search =
search =
`Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
------------------------------------------------------------------------------------
replace =
replace =
`Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
------------------------------------------------------------------------------------

* Nothing yet.

`{new_version} <https://github.com/Ouranosinc/Magpie/tree/{new_version}>`_ ({now:%%Y-%%m-%%d})
------------------------------------------------------------------------------------

Expand All @@ -39,7 +39,7 @@ ignore-path = docs/_build
[flake8]
ignore = E501,W291,W503,W504
max-line-length = 120
exclude =
exclude =
.git,
__pycache__,
docs,
Expand Down Expand Up @@ -67,17 +67,18 @@ known_second_party = twitcher
skip_glob = *pyramid.httpexceptions*

[tool:pytest]
addopts =
addopts =
--strict
--tb=native
markers =
markers =
defaults: magpie default users, providers and views
register: magpie methods employed in 'register' module
login: magpie login operations
services: magpie services operations
resources: magpie resources operations
groups: magpie groups operations
users: magpie users operations
logged: magpie logged user operations (current)
status: magpie views validation found/displayed as per permissions
remote: magpie tests running on remote instance specified by url
local: magpie tests running on local instance created by test suite
Expand Down
Loading

0 comments on commit e6f4dff

Please sign in to comment.