Merge pull request #371 from Ouranosinc/fix-ui-edit-pwd

Fix UI user edit fields error message

CHANGES.rst

@@ -7,7 +7,20 @@ Changes
 `Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
 ------------------------------------------------------------------------------------
 
-* Nothing yet.
+Features / Changes
+~~~~~~~~~~~~~~~~~~~~~
+* Add better details of HTTP error cause in returned UI page
+  (resolves `#369 <https://github.com/Ouranosinc/Magpie/issues/369>`_).
+* Ensure that general programming internal errors are not bubbled up in UI error page.
+* Add function to parse output body and redact potential leaks of flagged fields.
+* Align HTML format and structure of all edit forms portions of `Users`, `Groups` and `Services` UI pages to simplify
+  and unify their rendering.
+* Add inline UI error messages to `User` edition fields.
+
+Bug Fixes
+~~~~~~~~~~~~~~~~~~~~~
+* Fix validation of edited user fields to handle and adequately indicate returned error on UI
+  (resolves `#370 <https://github.com/Ouranosinc/Magpie/issues/370>`_).
 
 `3.2.1 <https://github.com/Ouranosinc/Magpie/tree/3.2.1>`_ (2020-11-17)
 ------------------------------------------------------------------------------------

Makefile

@@ -519,6 +519,12 @@ test-remote: install-dev install	## run only remote tests with the environment P
 	@echo "Running remote tests..."
 	@bash -c '$(CONDA_CMD) pytest tests -vv -m "remote" --junitxml "$(APP_ROOT)/tests/results.xml"'
 
+.PHONY: test-custom
+test-custom: install-dev install	## run custom marker tests using SPEC="<marker-specification>"
+	@echo "Running custom tests..."
+	@[ "${SPEC}" ] || ( echo ">> 'TESTS' is not set"; exit 1 )
+	@bash -c '$(CONDA_CMD) pytest tests -vv -m "${SPEC}" --junitxml "$(APP_ROOT)/tests/results.xml"'
+
 .PHONY: test-docker
 test-docker: docker-test			## alias for 'docker-test' target - WARNING: could build image if missing
 

docs/permissions.rst

@@ -444,7 +444,7 @@ priority over any :term:`Group` :term:`Permission`. Also, ``deny`` access is pri
 the default interpretation of protected access control defined by `Magpie`. When ``match`` and ``recursive`` scopes
 cause ambiguous resolution, the ``match`` :term:`Permission` is prioritized over inherited access via parent ``scope``.
 
-As a general of thumb, all :term:`Permission` are resolved such that more restrictive access applied *closer* to
+As a general rule of thumb, all :term:`Permission` are resolved such that more restrictive access applied *closer* to
 the actual :term:`Resource` for the targeted :term:`User` will have priority, both in terms of inheritance by tree
 hierarchy and by :term:`Group` memberships.
 

docs/services.rst

@@ -169,7 +169,7 @@ its children :term:`Resource` are :attr:`Permission.BROWSE`, :attr:`Permission.R
     provides *metadata* access to that file.
 
 Permission :attr:`Permission.READ` can be applied to all of the resources, but will only effectively make sense when
-attempting access of a specific :term:`Resource of type :class:`magpie.models.File`.
+attempting access of a specific :term:`Resource` of type :class:`magpie.models.File`.
 
 .. versionchanged:: 3.1
     Permission :attr:`Permission.READ` does not offer *metadata* content listing of :class:`magpie.models.Directory`

magpie/security.py

@@ -16,12 +16,52 @@
 
 if TYPE_CHECKING:
     # pylint: disable=W0611,unused-import
-    from magpie.typedefs import JSON, AnySettingsContainer
+    from typing import List, Optional
+
+    from magpie.typedefs import AnySettingsContainer, JSON, Str
 
 AUTHOMATIC_LOGGER = get_logger("magpie.authomatic", level=logging.DEBUG)
 LOGGER = get_logger(__name__)
 
 
+def mask_credentials(container, redact="[REDACTED]", flags=None, parent=None):
+    # type: (JSON, Str, Optional[List[Str]], Optional[Str]) -> JSON
+    """
+    Masks away any credential matched against :paramref:`flags` recursively from JSON :paramref:`container`.
+
+    Matched credential entries are replaced by :paramref:`redact`. List items are all replaced by the same
+    :paramref:`redact` when their :paramref:`parent` field name is matched.
+
+    :param container: JSON container to mask.
+        If starting with a list on top-level, first level children will not be masked unless parent is provided.
+    :param redact: string by which to replace flagged fields.
+    :param flags: field names (partial matches) to flag for masking.
+    :param parent: reference to contained elements if in a listing format rather than mapping.
+    :return: masked credentials JSON container.
+    """
+    flags = flags or ["password", "pwd"]
+
+    def flagged(_compare):
+        if isinstance(_compare, (dict, tuple, list, set, type(None))):
+            return False
+        return any(_flag in _compare for _flag in flags)
+
+    if isinstance(container, (list, tuple, set)):
+        for i, item in enumerate(container):
+            container[i] = mask_credentials(item, redact=redact, flags=flags, parent=parent)
+        return container
+
+    if isinstance(container, dict):
+        for key in list(container):
+            container[key] = mask_credentials(container[key], redact=redact, flags=flags, parent=key)
+        return container
+
+    if flagged(parent):
+        return redact
+
+    return container
+
+
 def get_auth_config(container):
     # type: (AnySettingsContainer) -> Configurator
     """

magpie/ui/home/static/style.css

@@ -246,26 +246,71 @@ table thead {
 }
 
 .panel-line {
-    margin: 0.5em;
+    margin: 0.25em 0.5em;
+    font-family: arial, sans-serif;  /* same as table to ensure they match when table is not actually used */
 }
 
 table.panel-line {
-    margin: 0.25em;
+    margin: 0.25em 0.5em;
 }
 
 table.panel-line th,
 table.panel-line td {
     border: 0;
+    margin: 0.25em 0.25em;
     white-space: nowrap;
     padding: 0;  /* override global th/td */
 }
 
-.panel-error {
-    margin-left: 0.5em;
+.panel-line-entry {
+    margin: 0.25em 0;
+    min-height: 1em;
+}
+
+.panel-form-error>img {
+    width: 1em;
+    height: 1em;
+    margin: 0.1em 0.25em 0.15em 0;
+}
+
+.panel-form-error {
+    width: auto;
+    display: inline-flex;
+    float: left;
+    text-align: center;
+    margin-left: 0.25em;
+    margin-top: 0;
 }
 
-.panel-line input[type="submit"] {
+.panel-line input[type="submit"],
+.panel-line-entry input[type="submit"] {
     height: auto;
+    float: right;
+    margin-left: 0.5em;
+    min-width: 4em;  /* for save/edit buttons alignment when editing one field and others are still in non-edit mode */
+}
+
+/* textual field entry when in edit mode */
+.panel-line-entry input[type="password"],
+.panel-line-entry input[type="text"],
+.panel-line-entry input[type="url"] {
+    -webkit-appearance: none;
+    appearance: none;
+    height: 1.125em;
+    border: solid #666 1px;
+    font-size: 14px;
+    margin-bottom: 0.1em;
+}
+
+/* textual field entry when in non-edit mode */
+.panel-line-entry span {
+/*
+    background-color: lightgray;
+    border-color: black;
+    border-width: 0.15em;
+    border-style: inset;
+    padding: 0 0.25em;
+*/
 }
 
 .panel-line-checkbox {
@@ -279,6 +324,7 @@ table.panel-line td {
 
 .panel-entry {
     font-weight: bold;
+    margin-right: 0.5em;
 }
 
 .panel-value {
@@ -671,7 +717,7 @@ table.simple-list input[type="submit"] {
 }
 
 .tree-button.goto-service {
-    margin: 0.1em 0 0 -4em;
+    margin: 0.25em 0 0 -4em;
 }
 
 .tree-item-message {
@@ -692,7 +738,7 @@ div.tree-button {
     width: 5em;
     float: right;
     text-align: center;
-    margin: 0.1em 0 0 0;
+    margin-bottom: 0.15em;
 }
 
 .clear {
@@ -941,6 +987,10 @@ table.request-details thead tbody td th {
     padding: 1px;
 }
 
+.tab-panel-selector {
+    margin: 0.1em;
+}
+
 .current-tab-panel {
     background: white;
     padding: 1em;
@@ -961,6 +1011,7 @@ a.current-tab:visited {
 
 .tab {
     min-width: 6em;  /* make tabs same width regardless of text length, but must be adjusted for longest one */
+    margin-bottom: 0.1em;   /* in case page is not wide enough, avoid wrapped tabs to be sticking on top of another */
     border-color: black;
 }