From 1c2cdfe06a0d8b506c00742d754297368976f601 Mon Sep 17 00:00:00 2001
From: Jens Langhammer <jens.langhammer@beryju.org>
Date: Thu, 24 Nov 2022 13:42:13 +0100
Subject: [PATCH] web/flows: improve error messages for failed duo push

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
---
 .../authenticator_validate/challenge.py       |  5 +-
 .../authenticator_validate/tests/test_duo.py  | 12 ++++-
 .../stages/access_denied/AccessDeniedStage.ts | 53 ++++++++++++-------
 .../AuthenticatorValidateStageDuo.ts          | 20 +++----
 4 files changed, 58 insertions(+), 32 deletions(-)

diff --git a/authentik/stages/authenticator_validate/challenge.py b/authentik/stages/authenticator_validate/challenge.py
index a8cf9b2b3..fabbddcd4 100644
--- a/authentik/stages/authenticator_validate/challenge.py
+++ b/authentik/stages/authenticator_validate/challenge.py
@@ -208,8 +208,7 @@ def validate_challenge_duo(device_pk: int, stage_view: StageView, user: User) ->
                 stage=stage_view.executor.current_stage,
                 device_class=DeviceClasses.DUO.value,
             )
-            raise ValidationError("Duo denied access")
-        device.save()
+            raise ValidationError("Duo denied access", code="denied")
         return device
     except RuntimeError as exc:
         Event.new(
@@ -217,4 +216,4 @@ def validate_challenge_duo(device_pk: int, stage_view: StageView, user: User) ->
             message=f"Failed to DUO authenticate user: {str(exc)}",
             user=user,
         ).from_http(stage_view.request, user)
-        raise ValidationError("Duo denied access")
+        raise ValidationError("Duo denied access", code="denied")
diff --git a/authentik/stages/authenticator_validate/tests/test_duo.py b/authentik/stages/authenticator_validate/tests/test_duo.py
index 4dfddaa52..6b779d6ce 100644
--- a/authentik/stages/authenticator_validate/tests/test_duo.py
+++ b/authentik/stages/authenticator_validate/tests/test_duo.py
@@ -73,7 +73,17 @@ class AuthenticatorValidateStageDuoTests(FlowTestCase):
             )
         with patch(
             "authentik.stages.authenticator_duo.models.AuthenticatorDuoStage.auth_client",
-            MagicMock(return_value=MagicMock(auth=MagicMock(return_value={"result": "deny"}))),
+            MagicMock(
+                return_value=MagicMock(
+                    auth=MagicMock(
+                        return_value={
+                            "result": "deny",
+                            "status": "deny",
+                            "status_msg": "foo",
+                        }
+                    )
+                )
+            ),
         ):
             with self.assertRaises(ValidationError):
                 validate_challenge_duo(
diff --git a/web/src/flow/stages/access_denied/AccessDeniedStage.ts b/web/src/flow/stages/access_denied/AccessDeniedStage.ts
index cb74205f8..c873a3a73 100644
--- a/web/src/flow/stages/access_denied/AccessDeniedStage.ts
+++ b/web/src/flow/stages/access_denied/AccessDeniedStage.ts
@@ -1,3 +1,4 @@
+import { AKElement } from "@goauthentik/elements/Base";
 import "@goauthentik/elements/EmptyState";
 import "@goauthentik/flow/FormStatic";
 import { BaseStage } from "@goauthentik/flow/stages/base";
@@ -5,7 +6,7 @@ import { BaseStage } from "@goauthentik/flow/stages/base";
 import { t } from "@lingui/macro";
 
 import { CSSResult, TemplateResult, css, html } from "lit";
-import { customElement } from "lit/decorators.js";
+import { customElement, property } from "lit/decorators.js";
 import { ifDefined } from "lit/directives/if-defined.js";
 
 import AKGlobal from "@goauthentik/common/styles/authentik.css";
@@ -18,18 +19,14 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css";
 
 import { AccessDeniedChallenge, FlowChallengeResponseRequest } from "@goauthentik/api";
 
-@customElement("ak-stage-access-denied")
-export class AccessDeniedStage extends BaseStage<
-    AccessDeniedChallenge,
-    FlowChallengeResponseRequest
-> {
+@customElement("ak-stage-access-denied-icon")
+export class AccessDeniedIcon extends AKElement {
+    @property()
+    errorMessage?: string;
+
     static get styles(): CSSResult[] {
         return [
             PFBase,
-            PFLogin,
-            PFForm,
-            PFList,
-            PFFormControl,
             PFTitle,
             AKGlobal,
             css`
@@ -50,6 +47,29 @@ export class AccessDeniedStage extends BaseStage<
         ];
     }
 
+    render(): TemplateResult {
+        return html` <div class="pf-c-form__group">
+            <p class="big-icon">
+                <i class="pf-icon pf-icon-error-circle-o"></i>
+            </p>
+            <h3 class="pf-c-title pf-m-3xl reason">${t`Request has been denied.`}</h3>
+            ${this.errorMessage
+                ? html`<hr />
+                      <p>${this.errorMessage}</p>`
+                : html``}
+        </div>`;
+    }
+}
+
+@customElement("ak-stage-access-denied")
+export class AccessDeniedStage extends BaseStage<
+    AccessDeniedChallenge,
+    FlowChallengeResponseRequest
+> {
+    static get styles(): CSSResult[] {
+        return [PFBase, PFLogin, PFForm, PFList, PFFormControl, PFTitle, AKGlobal];
+    }
+
     render(): TemplateResult {
         if (!this.challenge) {
             return html`<ak-empty-state ?loading="${true}" header=${t`Loading`}> </ak-empty-state>`;
@@ -70,15 +90,10 @@ export class AccessDeniedStage extends BaseStage<
                             >
                         </div>
                     </ak-form-static>
-                    <div class="pf-c-form__group">
-                        <p class="big-icon">
-                            <i class="pf-icon pf-icon-error-circle-o"></i>
-                        </p>
-                        <h3 class="pf-c-title pf-m-3xl reason">${t`Request has been denied.`}</h3>
-                        ${this.challenge?.errorMessage &&
-                        html`<hr />
-                            <p>${this.challenge.errorMessage}</p>`}
-                    </div>
+                    <ak-stage-access-denied-icon
+                        errorMessage=${ifDefined(this.challenge.errorMessage)}
+                    >
+                    </ak-stage-access-denied-icon>
                 </form>
             </div>
             <footer class="pf-c-login__main-footer">
diff --git a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageDuo.ts b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageDuo.ts
index 845a15e75..4ee7e3b32 100644
--- a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageDuo.ts
+++ b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageDuo.ts
@@ -69,15 +69,17 @@ export class AuthenticatorValidateStageWebDuo extends BaseStage<
                         </div>
                     </ak-form-static>
 
-                    ${errors.map((err) => {
-                        return html`<p>${err.string}</p>`;
-                    })}
-
-                    <div class="pf-c-form__group pf-m-action">
-                        <button type="submit" class="pf-c-button pf-m-primary pf-m-block">
-                            ${t`Continue`}
-                        </button>
-                    </div>
+                    ${errors.length > 0
+                        ? errors.map((err) => {
+                              if (err.code === "denied") {
+                                  return html` <ak-stage-access-denied-icon
+                                      errorMessage=${err.string}
+                                  >
+                                  </ak-stage-access-denied-icon>`;
+                              }
+                              return html`<p>${err.string}</p>`;
+                          })
+                        : html`${t`Sending Duo push notification`}`}
                 </form>
             </div>
             <footer class="pf-c-login__main-footer">