Skip to content

web/flows: resize captcha iframes (cherry-pick #12260)#12304

Merged
BeryJu merged 1 commit intoversion-2024.10from
cherry-pick-6426de-version-2024.10
Dec 9, 2024
Merged

web/flows: resize captcha iframes (cherry-pick #12260)#12304
BeryJu merged 1 commit intoversion-2024.10from
cherry-pick-6426de-version-2024.10

Conversation

@gcp-cherry-pick-bot
Copy link
Contributor

Cherry-picked web/flows: resize captcha iframes (#12260)

  • web: Add InvalidationFlow to Radius Provider dialogues

What

  • Bugfix: adds the InvalidationFlow to the Radius Provider dialogues
    • Repairs: {"invalidation_flow":["This field is required."]} message, which was not propagated
      to the Notification.
  • Nitpick: Pretties ?foo=${true} expressions: s/\?([^=]+)=\$\{true\}/\1/

Note

Yes, I know I'm going to have to do more magic when we harmonize the forms, and no, I didn't add the
Property Mappings to the wizard, and yes, I know I'm going to have pain with the new version of
the wizard. But this is a serious bug; you can't make Radius servers with either of the current
dialogues at the moment.

  • web: streamline CaptchaStage

What

This commit:

  1. Replaces the mass of if () { if() { if() } } with two state tables:
  • One for render()
  • One for renderBody()
  1. Breaks each Captcha out into "interactive" and "executive" versions
  2. Creates a handler table for each Captcha type
  3. Replaces the .forEach expression with a for loop.
  4. Move updated to the end of the class.
  5. Make captchDocument and captchaFrame constructed-on-demand with a cache.
  6. Remove a lot of @state handlers
  7. Give IframeMessageEvent its own type.
  8. Removed this.scriptElement
  9. Replaced window.removeEventListener with an AbortController()

Why

  1. Replacing if trees with a state table. The logic of the original was really hard to follow.
    With the state table, we can clearly see now that for the render() function, we care about the
    Boolean flags [embedded, challenged, interactive] and have appropriate effects for each. With
    renderBody(), we can see that we care about the Boolean flags [hasError, challenged], and can
    see the appropriate effects for each one.

  2. (and 3.) Breaking each Captcha clause into separate handlers. Again, the logic was convoluted,
    when what we really care about is "Does this captcha have a corresponding handler attached to
    window, and, if so, should we run the interactive or executive version of it?" By putting all
    of that into a table of [name, challenge, execute], we can clearly see what's being handled
    when.

  3. Replacing foreach() with for(): You cannot use a forEach() with async
    expressions
    .
    If you need asynchronous behavior in an ordered loop, for() is the safest way to handle it; if
    you need asynchronous behavior from multiple promises, Promise.allSettled(handlers.map()) is
    the way to go.

    I tried to tell if this function meant to run every handler it found simultaneously, or if it
    meant to test them in order; I went with the second option, breaking and exiting the loop once a
    handler had run successfully.

  4. Reordered the code a bit. We're trying to evolve a pattern in our source code: styles,
    properties, states, internal variables, constructor, getters & setters that are not @property()
    or @state(), DOM-related lifecycle handlers, event handlers, pre-render lifecycle handlers,
    renderers, and post-render lifecycle handlers. Helper methods (including subrenderers) go above
    the method(s) they help.

  5. Constructing Elements on demand with a cache. It is not guaranteed that we will actually need
    either of those. Constructing them on demand with a cache is both performant and cleaner.
    Likewise, I removed these from the Lit object's state() table, since they're constructed once
    and never change over the lifetime of an instance of ak-stage-captcha.

  6. Remove this.scriptElement: It was never referenced outside the one function where it was used.

  7. Remove removeEventListener(): AbortController() is a bit more verbose for small event
    handler collections, but it's considered much more reliable and much cleaner.

  • Didn't save the extracted ListenerController.

* web: Add InvalidationFlow to Radius Provider dialogues

## What

- Bugfix: adds the InvalidationFlow to the Radius Provider dialogues
  - Repairs: `{"invalidation_flow":["This field is required."]}` message, which was *not* propagated
    to the Notification.
- Nitpick: Pretties `?foo=${true}` expressions: `s/\?([^=]+)=\$\{true\}/\1/`

## Note

Yes, I know I'm going to have to do more magic when we harmonize the forms, and no, I didn't add the
Property Mappings to the wizard, and yes, I know I'm going to have pain with the *new* version of
the wizard. But this is a serious bug; you can't make Radius servers with *either* of the current
dialogues at the moment.

* web: streamline CaptchaStage

# What

This commit:

1. Replaces the mass of `if () { if() { if() } }` with two state tables:
  - One for `render()`
  - One for `renderBody()`

2. Breaks each Captcha out into "interactive" and "executive" versions
3. Creates a handler table for each Captcha type
4. Replaces the `.forEach` expression with a `for` loop.
5. Move `updated` to the end of the class.
6. Make captchDocument and captchaFrame constructed-on-demand with a cache.
7. Remove a lot of `@state` handlers
8. Give IframeMessageEvent its own type.
9. Removed `this.scriptElement`
10. Replaced `window.removeEventListener` with an `AbortController()`
# Why

1. **Replacing `if` trees with a state table.** The logic of the original was really hard to follow.
   With the state table, we can clearly see now that for the `render()` function, we care about the
   Boolean flags `[embedded, challenged, interactive]` and have appropriate effects for each. With
   `renderBody()`, we can see that we care about the Boolean flags `[hasError, challenged]`, and can
   see the appropriate effects for each one.

2. (and 3.) **Breaking each Captcha clause into separate handlers.** Again, the logic was convoluted,
   when what we really care about is "Does this captcha have a corresponding handler attached to
   `window`, and, if so, should we run the interactive or executive version of it?" By putting all
   of that into a table of `[name, challenge, execute]`, we can clearly see what's being handled
   when.

4. **Replacing `foreach()` with `for()`**: [You cannot use a `forEach()` with async
   expressions](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach#:~:text=does%20not%20wait%20for%20promises).
   If you need asynchronous behavior in an ordered loop, `for()` is the safest way to handle it; if
   you need asynchronous behavior from multiple promises, `Promise.allSettled(handlers.map())` is
   the way to go.

   I tried to tell if this function *meant* to run every handler it found simultaneously, or if it
   meant to test them in order; I went with the second option, breaking and exiting the loop once a
   handler had run successfully.

5. **Reordered the code a bit**. We're trying to evolve a pattern in our source code: styles,
   properties, states, internal variables, constructor, getters & setters that are not `@property()`
   or `@state()`, DOM-related lifecycle handlers, event handlers, pre-render lifecycle handlers,
   renderers, and post-render lifecycle handlers. Helper methods (including subrenderers) go above
   the method(s) they help.

6. **Constructing Elements on demand with a cache**. It is not guaranteed that we will actually need
   either of those. Constructing them on demand with a cache is both performant and cleaner.
   Likewise, I removed these from the Lit object's `state()` table, since they're constructed once
   and never change over the lifetime of an instance of `ak-stage-captcha`.

9. **Remove `this.scriptElement`**: It was never referenced outside the one function where it was used.

10. **Remove `removeEventListener()`**: `AbortController()` is a bit more verbose for small event
    handler collections, but it's considered much more reliable and much cleaner.

* Didn't save the extracted ListenerController.
@gcp-cherry-pick-bot gcp-cherry-pick-bot bot requested a review from a team as a code owner December 9, 2024 22:18
@netlify
Copy link

netlify bot commented Dec 9, 2024

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit 73a4ce4
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/67576ccf98a975000812fe79
😎 Deploy Preview https://deploy-preview-12304--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.60%. Comparing base (efac5ce) to head (73a4ce4).
Report is 1 commits behind head on version-2024.10.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           version-2024.10   #12304      +/-   ##
===================================================
- Coverage            92.67%   92.60%   -0.07%     
===================================================
  Files                  761      761              
  Lines                37953    37953              
===================================================
- Hits                 35173    35147      -26     
- Misses                2780     2806      +26     
Flag Coverage Δ
e2e 49.18% <ø> (-0.11%) ⬇️
integration 24.87% <ø> (ø)
unit 90.19% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BeryJu BeryJu merged commit 0e4b153 into version-2024.10 Dec 9, 2024
@BeryJu BeryJu deleted the cherry-pick-6426de-version-2024.10 branch December 9, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants