1Password is filling inputs from multiple forms at once in Craft CMS

brandonkelly
brandonkelly
Community Member
edited August 2018 in 1Password in the Browser

In the Craft CMS Control Panel (https://craftcms.com), if you attempt to change your password, Craft will show you a modal window where you must enter your current password. The modal has its own <form> separate from the main page’s <form>, yet if you attempt to fill your current password in the modal using 1Password, it will update both the main page form (replaces the Email field with your username) and the modal’s form (fills in your current password, as expected).

We are following the guidance on https://support.1password.com/compatible-website-design/ as best as we can, however this is not possible:

To make the intention of each form element clear on password change forms, ask for the current password, the new password, and a password confirmation in that order.

There are multiple things on the page that could require you to enter your current password, in addition to changing your password. (You will also be prompted if you change the user’s email, make them an Admin, or assign new user groups / permissions to their account... basically anything that could result in permission escalation.) Therefore it made more sense to just track each of those fields, and when attempting to save the user account, if any of them have changed, we show the Current Password modal.

Current Password modal in Craft CMS

So, is there anything we can do to tell 1Password to only focus on the Current Password input in the modal window? Sortof a reverse-autofill on the main form fields?

At the least, it seems like a bug that 1Password would be autofilling multiple forms at the same time, so maybe you can look into that.

Steps to reproduce:

  1. Create a demo at https://demo.craftcms.com
  2. Save a login in 1Password for the domain (username: admin, password: password).
  3. When the email arrives, click on the link to the Control Panel, and login
  4. Click on your user account in the sidebar and click "My Account".
  5. Change your password and click Save
  6. Use 1Password to autofill your password in the Current Password modal.

Expected Behavior

Only the password input in the Current Password modal should be autofilled by 1Password.

Actual Behavior

In addition to the Current Password modal getting autofilled, the Email field in the background will also be updated to "admin".

Additional Info

A customer has reported a similar issue, which we're not able to reproduce, but sounds related to the same bug. In their case, 1Password is updating the New Password field in the main form, instead of Email.

https://github.com/craftcms/cms/issues/3207


1Password Version: 6.8.8
Extension Version: 4.7.2.90
OS Version: macOS 10.13.6 (17G65)
Sync Type: 1password.com

ref: SMM-49435-766

Comments

  • littlebobbytables
    littlebobbytables
    1Password Alumni
    edited February 2019

    Hello @brandonkelly,

    It would be great if we could limit 1Password to a single form but even in my short time at 1Password I've seen a bewildering range of designs, many with dubious choices ranging from no form or even a single form per field. I once adjusted the filling logic to ignore read-only fields only to learn from feedback that this seemingly innocuous change impacted on sites. It's almost impressive at times the variation you can see in the HTML.

    So at the moment I'm unsure how to improve matters on this page without it impacting a number of sites. I will file a bug report to see if others maybe have a moment of inspiration.

    ref: xplatform/filling-issues#243

  • brandonkelly
    brandonkelly
    Community Member

    Maybe 1Password could check for some proprietary attribute, which web developers could add if they want to explicitly tell 1P which fields to be looking at?

    For example, if a form on the page has a data-1password-focus attribute, then 1P should know to ignore any other forms on the page and only autofill fields in that one. And maybe beyond that, the individual inputs could have data-1password-username and data-1password-password attributes, so 1P is told explicitly which fields to fill, rather than relying on input names, etc.

  • littlebobbytables
    littlebobbytables
    1Password Alumni

    Greetings @brandonkelly,

    Having had the page kicking around my head for a night I suspect my own personal preference would be for us to more aggressively focus on the autocomplete attribute as the page in question does actually have a username field. It, the current and new password fields could all be marked up in this way so that 1Password sees that the real username field already contains the username and does nothing. The benefit to this approach would be the attribute could have a general benefit outside of just 1Password, making it a more compelling change to make. It would require some changes on our side as well though as I tweaked the page in my browser and we still fixated on the wrong field.

  • brandonkelly
    brandonkelly
    Community Member

    Cool. Later today we will release Craft 3.0.21 which does a better job setting autocomplete attributes:

    • The login form’s username and password inputs have autocomplete="username" and autocomplete="current-password"
    • The Current Password modal's password input has autocomplete="current-password"
    • The New Password input on the My Account page has autocomplete="new-password"
    • Other fields on the My Account page have autocomplete="off", so 1Password should know not to touch them.

    I saved out the HTML (sans <script> tags) + resources for the My Account page with the Current Password modal open, with these changes in place, so you can test. You'll probably need to save it on a web server where 1Password can do its thing.

    https://www.dropbox.com/s/omjvr1knojpj4qk/myaccount.zip?dl=0

    (Unfortunately demo sites created at demo.craftcms.com are still running Craft 2, so for now this is the only way I could get you a test page, unless you actually install Craft 3.0.21 yourself once it's released.)

  • littlebobbytables
    littlebobbytables
    1Password Alumni

    Hi @brandonkelly,

    It will sound daft but we will have to be careful about ignoring fields with autocomplete="off". You are using it correctly but so many sites tried to use it to block automated filling that even I was aware of the attribute at all browsers had already started ignoring it when sites used autocomplete="off". That said, I think that if we're intelligent about it the fact that certain fields use it correctly means we should note that for when you do use autocomplete="off".

    Now I'll avoid the rambling explanation (unless you're curious) but I noticed that the field for the email address on the page is type="text". I wonder how you might feel about using type="email". It offers free validation by the browser if I'm reading the documentation correctly and I'm thinking about more sensible inferring 1Password can do. We do support filling an input field of type email for the username but I'm thinking we should only do so if the value stored in 1Password is of the right format. That would also help ensure we do less daft filling. As it's still standard HTML I can't envisage any negative impact or worry over custom code at your end which has to be the most important aspect to consider.

  • brandonkelly
    brandonkelly
    Community Member

    We use autocomplete="off" pretty liberally in Craft because it does do a good job preventing Chrome from giving you the dropdown menu of previous values you've used in fields with the same name -- which is incredibly annoying when you are mostly managing other user accounts or CMS content :)

    But from 1Password's perspective, it does seem reasonable to ignore autocomplete="off" unless you see other fields that have different autocomplete values, suggesting the web developer actually does understand how the attribute works.

    And yes, you’re right we should use type="email" there.

  • AGAlumB
    AGAlumB
    1Password Alumni

    That makes sense. As an added bonus, type="email" should also get you a better onscreen keyboard layout on touch devices. That may be small potatoes for this use case, but it's a person pet peeve of mine. :lol:

  • brandonkelly
    brandonkelly
    Community Member

    Sorry to resurrect an old thread, but how do you think we could force 1Password to disable autofill on user management pages, where none of the fields pertain to the currently logged-in user (thus all fields would just have autocomplete="off"?

    Seems like there really needs to be some way we can tell 1Password “Trust me, I know how autocomplete works and I mean it when I say autocomplete=‘off’!”

  • jxpx777
    jxpx777
    1Password Alumni

    Hi, @brandonkelly. No worries bringing this back up. I think we have a solution for your particular instance even though we're not quite ready to embrace autocomplete="off". It looks like you have a 1Password membership, so I'm curious to know if 1Password X works better for you. It has an improvement that our regular extension doesn't yet have and it would be lovely to get confirmation that it works better for you. Let us know!

    --
    Jamie Phelps
    Code Wrangler @ 1Password
    Olympia, WA

  • brandonkelly
    brandonkelly
    Community Member

    Hey @jxpx777 , yep, 1Password X does behave correctly! Any idea when the main 1P app will be updated to follow suit?

  • AGAlumB
    AGAlumB
    1Password Alumni

    It's a work in progress, but we'll be bringing the improvements in the new filling engine to all of the apps in time. :)

  • brandonkelly
    brandonkelly
    Community Member

    Sorry to resurrect this old thread, but 1Password X has started acting up recently. Now it is showing the accounts menu on the Username field within Craft’s account settings:

    The Username field on the My Account page in Craft CMS, with the 1Password X accounts menu visible below it

    That input HTML is:

    <input class="text fullwidth" type="text" id="username" name="username" value="admin" autofocus="" autocomplete="off">
    

    Is there any new info on what we can be doing to guide 1Password (X) on whether it should (or should not) attach itself to account fields?

  • brandonkelly
    brandonkelly
    Community Member

    (ping @jxpx777 & @brenty)

  • kaitlyn
    kaitlyn
    1Password Alumni

    Hey @brandonkelly! I'm glad you brought this up again. I'm not sure I understand how the behavior changed. Based on this thread, it seems like 1Password X didn't used to offer to fill the username field in your screenshot. From a code perspective, I can't tell why it wouldn't want to fill that field. Could you explain a bit more about why you don't want 1Password X filling there?

  • brandonkelly
    brandonkelly
    Community Member

    Because it’s not a login form; it’s an account settings form. If you are editing the Username field here, it is to change it to something else. (And as an administrator, the account you’re editing may not even be your own – you could be editing someone else’s account details.)

    How can the form, or its inputs, tell 1Password to ignore it?

  • kaitlyn
    kaitlyn
    1Password Alumni

    @brandonkelly – That makes sense. Thank you for clarifying that for me! I don't know of a way to let 1Password know to ignore that specific field. I'll pass the HTML along to our developers and see if they have any ideas for us, though. Stay tuned. :)

    ref: dev/core/core#756

  • kaitlyn
    kaitlyn
    1Password Alumni

    @brandonkelly – Just wanted to check in with you. We've got a developer on the case. They're determining if there's any way for 1Password X to behave better on Craft CMS without affecting a variety of other websites in the process. One of us will reach back out when we have any news.

  • brandonkelly
    brandonkelly
    Community Member

    @kaitlyn Thanks for the update!

  • kaitlyn
    kaitlyn
    1Password Alumni

    Absolutely! :+1:

  • brandonkelly
    brandonkelly
    Community Member

    Any progress @kaitlyn?

  • ag_ana
    ag_ana
    1Password Alumni

    @brandonkelly:

    Not yet, but as kailtyn wrote, we will reach back out once we have any news. Thank you for your patience!

  • brandonkelly
    brandonkelly
    Community Member

    To follow up on this, someone recently brought up a similar bug with LastPass, and it turns out LP has a very elegant solution to this: they check for a data-lpignore="true" attribute on form inputs, and if present, the input will be completely ignored (documented here).

    Would be really awesome if 1Password / 1Password X supported something similar, e.g. data-1p-ignore.

  • kaitlyn
    kaitlyn
    1Password Alumni

    @brandonkelly – I've suggested something similar in the past. I'll bring it up to the developers once again, though, Thanks for the heads up! :chuffed:

This discussion has been closed.