Skip to content

Render non-passkey-restricted WebAuthn input as visible#8830

Merged
aduth merged 2 commits intomainfrom
aduth-webauthn-input-render-always
Jul 27, 2023
Merged

Render non-passkey-restricted WebAuthn input as visible#8830
aduth merged 2 commits intomainfrom
aduth-webauthn-input-render-always

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jul 21, 2023

🛠 Summary of changes

Improves WebauthnInputComponent to avoid handling visibility in JavaScript for simple cases where the input should be visible in all JavaScript-enabled browsers.

Why?

  • So that the user doesn't experience flickering page content and layout shifts as those elements become visible.
  • Simplify and reduce size of JavaScript sent to client
  • To support future revisions where the passkey-only restriction may be lifted, thus being able to remove WebauthnInputElement JavaScript altogether

📜 Testing Plan

  1. Go to http://localhost:3000
  2. Create an account
  3. Complete account creation up to MFA selection
  4. On MFA selection screen, observe that "Security key" and "Face or touch unlock" are visible
  5. Disable JavaScript in your browser and refresh the page
  6. Observe that neither "Security key" nor "Face or touch unlock" are visible
  7. Re-enable JavaScript
  8. Add to your config/application.yml and restart your server:
    development:
      show_unsupported_passkey_platform_authentication_setup: false
    
  9. Observe that "Security key" is visible, but "Face or touch unlock" is not if you are on an unsupported device
  10. Set up both "Security key" and "Face or touch unlock" for your account, and complete account creation
  11. Click "Forget all browsers" in account sidebar and confirm prompt
  12. Sign out
  13. Sign in
  14. Click "Choose another authentication method"
  15. Observe that "Security key" and "Face or touch unlock" are visible
  16. Disable JavaScript in your browser and refresh the page
  17. Observe that neither "Security key" nor "Face or touch unlock" are visible
  18. Remove the config/application.yml snippet from Step 8 and restart your server
  19. Observe that both "Security key" and "Face or touch unlock" are visible

aduth added 2 commits July 21, 2023 12:03
changelog: User-Facing Enhancements, MFA Methods, Avoid flickering layout for WebAuthn options on authenticator selection
changelog: User-Facing Improvements, MFA Methods, Avoid flickering layout for WebAuthn options on authenticator selection
@aduth aduth marked this pull request as ready for review July 24, 2023 13:05
@aduth aduth requested a review from a team July 24, 2023 13:05
if platform? && passkey_supported_only?
{ hidden: true }
else
{ class: 'js' }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably should have clarified how this is working: We have .js and .no-js class styles which hide the content in JavaScript disabled / enabled environments respectively (applied and toggled in the base layout). So by adding js as a class here, it's how we hide the content for browsers with JavaScript disabled.

@kevinsmaster5
Copy link
Contributor

I'm having trouble validating using the steps above.
Mac Os Chrome 114

Step 9: Face or touch unlock is not visible given config is added, JavaScript is reenabled. I am normally able to use Face/Touch Unlock using my Mac fingerprint unlock button.

Step 19: Security Key and Face or Touch unlock are still not visible given removed config and disabled JavaScript.

Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@aduth aduth merged commit dd9d4cf into main Jul 27, 2023
@aduth aduth deleted the aduth-webauthn-input-render-always branch July 27, 2023 12:14
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.

4 participants