Add error handling in login view (#55)

* Add 403 error handler * Raise 403 if user is not authenticated * Display helper text and highlight input fields on 403 in login form * Remove 403 related styling and text on login errors other than 403 * Fix failing test * Remove redundant user check and exception * Raise NotAuthenticated when Zino authentication fails --------- Co-authored-by: Hanne Moa <[email protected]>
Uninett · Jan 24, 2024 · cfbe2be · cfbe2be
1 parent 166458f
commit cfbe2be
Show file tree

Hide file tree

Showing 10 changed files with 188 additions and 111 deletions.
diff --git a/src/howitz/__init__.py b/src/howitz/__init__.py
@@ -7,12 +7,12 @@
 from flask.logging import default_handler
 from flask_assets import Bundle, Environment
 from flask_login import LoginManager, logout_user
-from werkzeug.exceptions import HTTPException, BadRequest, NotFound
+from werkzeug.exceptions import HTTPException, BadRequest, NotFound, Forbidden
 
 from howitz.config.utils import load_config
 from howitz.config.zino1 import make_zino1_config
 from howitz.config.howitz import make_howitz_config
-from howitz.error_handlers import handle_generic_exception, handle_generic_http_exception, handle_400, handle_404
+from howitz.error_handlers import handle_generic_exception, handle_generic_http_exception, handle_400, handle_404, handle_403
 from howitz.users.db import UserDB
 from howitz.users.commands import user_cli
 from zinolib.controllers.zino1 import Zino1EventManager
@@ -29,6 +29,7 @@ def create_app(test_config=None):
     app.register_error_handler(HTTPException, handle_generic_http_exception)
     app.register_error_handler(BadRequest, handle_400)
     app.register_error_handler(NotFound, handle_404)
+    app.register_error_handler(Forbidden, handle_403),
 
     # load config
     app = load_config(app, test_config)

diff --git a/src/howitz/endpoints.py b/src/howitz/endpoints.py
@@ -20,7 +20,7 @@
 from zinolib.controllers.zino1 import Zino1EventManager, RetryError, EventClosedError
 from zinolib.event_types import Event, AdmState, PortState, BFDState, ReachabilityState
 from zinolib.compat import StrEnum
-from zinolib.ritz import NotConnectedError
+from zinolib.ritz import NotConnectedError, AuthenticationError
 
 from howitz.users.utils import authenticate_user
 from .utils import login_check
@@ -41,30 +41,30 @@ def auth_handler(username, password):
     # check user credentials in database
     with current_app.app_context():
         user = authenticate_user(current_app.database, username, password)
-        if user:  # is registered in database
-            current_app.logger.debug('User %s', user)
-
-            if not current_app.event_manager.is_connected:
-                current_app.event_manager = Zino1EventManager.configure(current_app.zino_config)
-                current_app.event_manager.connect()
-                current_app.logger.info('Connected to Zino %s', current_app.event_manager.is_connected)
-
-            if not current_app.event_manager.is_authenticated:
-                current_app.event_manager.authenticate(username=user.username, password=user.token)
-                current_app.logger.info('Authenticated in Zino %s', current_app.event_manager.is_authenticated)
-
-            if current_app.event_manager.is_authenticated:  # is zino authenticated
-                current_app.logger.debug('User is Zino authenticated %s', current_app.event_manager.is_authenticated)
-                current_app.logger.debug('HOWITZ CONFIG %s', current_app.howitz_config)
-                login_user(user, remember=True)
-                flash('Logged in successfully.')
-                session["selected_events"] = []
-                session["expanded_events"] = {}
-                session["errors"] = {}
-                session["not_connected_counter"] = 0
-                return user
-    return None
-
+        current_app.logger.debug('User %s', user)
+
+        if not current_app.event_manager.is_connected:
+            current_app.event_manager = Zino1EventManager.configure(current_app.zino_config)
+            current_app.event_manager.connect()
+            current_app.logger.info('Connected to Zino %s', current_app.event_manager.is_connected)
+
+        if not current_app.event_manager.is_authenticated:
+            current_app.event_manager.authenticate(username=user.username, password=user.token)
+            current_app.logger.info('Authenticated in Zino %s', current_app.event_manager.is_authenticated)
+
+        if current_app.event_manager.is_authenticated:  # is zino authenticated
+            current_app.logger.debug('User is Zino authenticated %s', current_app.event_manager.is_authenticated)
+            current_app.logger.debug('HOWITZ CONFIG %s', current_app.howitz_config)
+            login_user(user, remember=True)
+            flash('Logged in successfully.')
+            session["selected_events"] = []
+            session["expanded_events"] = {}
+            session["errors"] = {}
+            session["not_connected_counter"] = 0
+            return user
+
+        raise AuthenticationError('Unexpected error on Zino authentication')
+    
 
 def logout_handler():
     with current_app.app_context():

diff --git a/src/howitz/error_handlers.py b/src/howitz/error_handlers.py
@@ -64,3 +64,14 @@ def handle_404(e):
         current_app.logger.warn('Path not found: %s', request.path)
     return render_template('/responses/404-not-found.html',
                            err_msg=e.description), 404
+
+
+def handle_403(e):
+    current_app.logger.exception("403 Forbidden has occurred %s", e)
+
+    response = make_response(render_template('/responses/403.html',
+                                             err_msg=e.description))
+
+    response.headers['HX-Trigger'] = 'htmx:responseError'
+
+    return response, 403
diff --git a/src/howitz/static/main.css b/src/howitz/static/main.css
@@ -1,102 +1,123 @@
+/***** BULK UPDATE REQUEST INDICATOR ****/
+.bulk-update-htmx-indicator {
+    display: none;
+}
 
-    .bulk-update-htmx-indicator {
-        display: none;
-    }
-
-    .htmx-request .bulk-update-htmx-indicator {
-        display: inline;
-    }
+.htmx-request .bulk-update-htmx-indicator {
+    display: inline;
+}
 
-    .htmx-request.bulk-update-htmx-indicator {
-        display: inline;
-    }
+.htmx-request.bulk-update-htmx-indicator {
+    display: inline;
+}
 
-    /***** MODAL DIALOG ****/
+/***** MODAL DIALOG ****/
 #modal {
-	/* Underlay covers entire screen. */
-	position: fixed;
-	top:0px;
-	bottom: 0px;
-	left:0px;
-	right:0px;
-	background-color:rgba(0,0,0,0.5);
-	z-index:1000;
-
-	/* Flexbox centers the .modal-content vertically and horizontally */
-	display:flex;
-	flex-direction:column;
-	align-items:center;
-
-	/* Animate when opening */
-	animation-name: fadeIn;
-	animation-duration:150ms;
-	animation-timing-function: ease;
+    /* Underlay covers entire screen. */
+    position: fixed;
+    top: 0px;
+    bottom: 0px;
+    left: 0px;
+    right: 0px;
+    background-color: rgba(0, 0, 0, 0.5);
+    z-index: 1000;
+
+    /* Flexbox centers the .modal-content vertically and horizontally */
+    display: flex;
+    flex-direction: column;
+    align-items: center;
+
+    /* Animate when opening */
+    animation-name: fadeIn;
+    animation-duration: 150ms;
+    animation-timing-function: ease;
 }
 
 /*#modal > .modal-underlay*/
 /*#event-details-modal > .modal-underlay*/
 #update-event-status-modal > .modal-underlay, .modal-underlay {
-	/* underlay takes up the entire viewport. This is only
-	required if you want to click to dismiss the popup */
-	position: absolute;
-	z-index: -1;
-	top:0px;
-	bottom:0px;
-	left: 0px;
-	right: 0px;
+    /* underlay takes up the entire viewport. This is only
+    required if you want to click to dismiss the popup */
+    position: absolute;
+    z-index: -1;
+    top: 0px;
+    bottom: 0px;
+    left: 0px;
+    right: 0px;
 }
 
 #update-event-status-modal > .modal-content {
-	/* Position visible dialog near the top of the window */
-	margin-top:10vh;
-
-	/* Sizing for visible dialog */
-	width:80%;
-	max-width:600px;
-
-	/* Display properties for visible dialog*/
-	border:solid 1px #999;
-	border-radius:8px;
-	box-shadow: 0px 0px 20px 0px rgba(0,0,0,0.3);
-	background-color:white;
-	padding:20px;
-
-	/* Animate when opening */
-	animation-name:zoomIn;
-	animation-duration:150ms;
-	animation-timing-function: ease;
+    /* Position visible dialog near the top of the window */
+    margin-top: 10vh;
+
+    /* Sizing for visible dialog */
+    width: 80%;
+    max-width: 600px;
+
+    /* Display properties for visible dialog*/
+    border: solid 1px #999;
+    border-radius: 8px;
+    box-shadow: 0px 0px 20px 0px rgba(0, 0, 0, 0.3);
+    background-color: white;
+    padding: 20px;
+
+    /* Animate when opening */
+    animation-name: zoomIn;
+    animation-duration: 150ms;
+    animation-timing-function: ease;
 }
 
 #modal.closing {
-	/* Animate when closing */
-	animation-name: fadeOut;
-	animation-duration:150ms;
-	animation-timing-function: ease;
+    /* Animate when closing */
+    animation-name: fadeOut;
+    animation-duration: 150ms;
+    animation-timing-function: ease;
 }
 
 #modal.closing > .modal-content {
-	/* Animate when closing */
-	animation-name: zoomOut;
-	animation-duration:150ms;
-	animation-timing-function: ease;
+    /* Animate when closing */
+    animation-name: zoomOut;
+    animation-duration: 150ms;
+    animation-timing-function: ease;
 }
 
 @keyframes fadeIn {
-	0% {opacity: 0;}
-	100% {opacity: 1;}
+    0% {
+        opacity: 0;
+    }
+    100% {
+        opacity: 1;
+    }
 }
 
 @keyframes fadeOut {
-	0% {opacity: 1;}
-	100% {opacity: 0;}
+    0% {
+        opacity: 1;
+    }
+    100% {
+        opacity: 0;
+    }
 }
 
 @keyframes zoomIn {
-	0% {transform: scale(0.9);}
-	100% {transform: scale(1);}
+    0% {
+        transform: scale(0.9);
+    }
+    100% {
+        transform: scale(1);
+    }
 }
 
 @keyframes zoomOut {
-	0% {transform: scale(1);}
-	100% {transform: scale(0.9);}
-}
+    0% {
+        transform: scale(1);
+    }
+    100% {
+        transform: scale(0.9);
+    }
+}
+
+/***** INPUT FILED ERROR ****/
+.error-input input {
+    box-shadow: var(--tw-ring-inset) 0 0 0 calc(2px + var(--tw-ring-offset-width)) #f87171;
+}
diff --git a/src/howitz/templates/components/login/sign-in-form.html b/src/howitz/templates/components/login/sign-in-form.html
@@ -4,21 +4,30 @@
         method="post"
         hx-post="/auth"
         hx-indicator="#spinner-indicator"
+        hx-validate="true"
+        hx-on:htmx:response-error="htmx.addClass(htmx.find('form'), 'error-input')"
+        hx-target-403="form p"
 >
     <div>
         <label for="username" class="block text-sm font-medium leading-6 text-sky-100">Username</label>
-        <div class="mt-2">
-            <input id="username" name="username" type="text" autocomplete="current-username" required
-                   class="block w-full rounded-md border-0 py-1.5 bg-slate-900 text-sky-100 shadow-sm ring-1 ring-inset ring-slate-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-sky-600 sm:text-sm sm:leading-6">
-        </div>
+        <input id="username" name="username" type="text" autocomplete="current-username" required
+               class="mt-2 block w-full rounded-md border-0 py-1.5 bg-slate-900 text-sky-100 shadow-sm ring-1 ring-inset ring-slate-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-sky-600 sm:text-sm sm:leading-6">
+        <p
+                id="username-helper-text"
+                hidden
+        >
+        </p>
     </div>
 
     <div>
         <label for="password" class="block text-sm font-medium leading-6 text-sky-100">Password</label>
-        <div class="mt-2">
-            <input id="password" name="password" type="password" autocomplete="current-password" required
-                   class="block w-full rounded-md border-0 py-1.5 bg-slate-900 text-sky-100 shadow-sm ring-1 ring-inset ring-slate-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-sky-600 sm:text-sm sm:leading-6">
-        </div>
+        <input id="password" name="password" type="password" autocomplete="current-password" required
+               class="mt-2 block w-full rounded-md border-0 py-1.5 bg-slate-900 text-sky-100 shadow-sm ring-1 ring-inset ring-slate-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-sky-600 sm:text-sm sm:leading-6">
+        <p
+                id="password-helper-text"
+                hidden
+        >
+        </p>
     </div>
 
     <div>

diff --git a/src/howitz/templates/components/popups/alerts/error/error-alert.html b/src/howitz/templates/components/popups/alerts/error/error-alert.html
@@ -38,3 +38,5 @@
     </div>
 
 </div>
+
+{% include "/responses/remove-403-login.html" %}
diff --git a/src/howitz/templates/responses/403.html b/src/howitz/templates/responses/403.html
@@ -0,0 +1,15 @@
+<p
+        id="username-helper-text"
+        hx-swap-oob="outerHTML"
+        class="flex-inline text-sm text-red-400"
+>
+    {{ err_msg }}
+</p>
+
+<p
+        id="password-helper-text"
+        hx-swap-oob="outerHTML"
+        class="flex-inline text-sm text-red-400"
+>
+    {{ err_msg }}
+</p>
diff --git a/src/howitz/templates/responses/remove-403-login.html b/src/howitz/templates/responses/remove-403-login.html
@@ -0,0 +1,14 @@
+<p
+        id="username-helper-text"
+        hx-swap-oob="outerHTML"
+        hx-on:htmx:load="htmx.removeClass(htmx.find('form'), 'error-input')"
+        hidden
+>
+</p>
+
+<p
+        id="password-helper-text"
+        hx-swap-oob="outerHTML"
+        hidden
+>
+</p>
diff --git a/src/howitz/users/utils.py b/src/howitz/users/utils.py
@@ -1,5 +1,6 @@
 import hashlib
 
+from werkzeug.exceptions import Forbidden
 from werkzeug.security import generate_password_hash, check_password_hash
 
 
@@ -14,7 +15,8 @@ def authenticate_user(database, username: str, password: str):
     user = database.get(username)
     if user and user.authenticate(password):
         return user
-    return None
+    else:
+        raise Forbidden('Wrong username or password')
 
 
 def encode_password(password: str):
Original file line number	Diff line number	Diff line change
Expand Up		@@ -38,3 +38,5 @@
		</div>

		</div>

		{% include "/responses/remove-403-login.html" %}