keepassxreboot / keepassxc-browser

KeePassXC Browser Extension
GNU General Public License v3.0
1.76k stars 187 forks source link

Can't fill TOTP on app.clickup.com #1649

Closed AdrianSkar closed 2 years ago

AdrianSkar commented 2 years ago

Expected Behavior

Autofill TOTP fields or manually after using the keyboard shortcut.

Current Behavior

Upon autofill/when clicking the autofill button the extension shows the "No TOTP found" dialog: image

When using the keyboard shortcut nothing happens except a logged error in the console: image image

Debug info

KeePassXC - 2.7.1 KeePassXC-Browser - 1.7.12 Operating system: Windows 10 Browser: Firefox 102

varjolintu commented 2 years ago

Click the extension toolbar icon, choose your credentials and fill TOTP again. This should be fixed in the current develop branch, so give it a try if you can, thanks.

AdrianSkar commented 2 years ago

Click the extension toolbar icon, choose your credentials and fill TOTP again. This should be fixed in the current develop branch, so give it a try if you can, thanks.

Thank you @varjolintu. I tried that already (and now again just in case) but had no luck. Their TOTP page shows 6 fields to be filled (one for each number) but admits manually pasting the whole sequence at once. I'm not sure if that might be the issue in this case.

image image

varjolintu commented 2 years ago

Selecting segmented TOTP fields with Custom Login Fields is currently not supported, but the extension should support it directly. Seems those fields are not regognized. Could you copy/paste the form/div's code that hold the input fields so I can take a look at why those are not identified?

AdrianSkar commented 2 years ago

Of course:

<form class="cu-form cu-onboarding__form cu-onboarding__form_totp">
    <div class="cu-form__row validate-code">
        <div class="cu-form__label"> Authentication code </div>
        <cu-code-input class="cu-form__field validate-field"><input maxlength="1" type="text" inputmode="numeric"
                pattern="[0-9]*" class="cu-form__input validate-input ng-star-inserted" data-next-index="1"
                kpxc-defined="totp"><input maxlength="1" type="text" inputmode="numeric" pattern="[0-9]*"
                class="cu-form__input validate-input ng-star-inserted" data-next-index="2" data-last-index="0"><input
                maxlength="1" type="text" inputmode="numeric" pattern="[0-9]*"
                class="cu-form__input validate-input ng-star-inserted" data-next-index="3" data-last-index="1"><input
                maxlength="1" type="text" inputmode="numeric" pattern="[0-9]*"
                class="cu-form__input validate-input ng-star-inserted" data-next-index="4" data-last-index="2"><input
                maxlength="1" type="text" inputmode="numeric" pattern="[0-9]*"
                class="cu-form__input validate-input ng-star-inserted" data-next-index="5" data-last-index="3"><input
                maxlength="1" type="text" inputmode="numeric" pattern="[0-9]*"
                class="cu-form__input validate-input ng-star-inserted" data-last-index="4">
            <!---->
        </cu-code-input>
        <!---->
    </div><button type="submit" class="login-page-new__main-form-button" disabled="">
        <div class="login-page-new__main-form-button-text ng-star-inserted"> Verify </div>
        <!---->
        <!---->
    </button>
    <div class="cu-onboarding__footer-link center ng-star-inserted">
        <!----><a queryparamshandling="preserve" href="https://app.clickup.com/login">Logout</a>
    </div>
    <!---->
</form>
varjolintu commented 2 years ago

The fields are identified correctly. Does the TOTP fill if you do it manually from the context menu?

AdrianSkar commented 2 years ago

The fields are identified correctly. Does the TOTP fill if you do it manually from the context menu?

No, I've tried context menu, keyboard shortcut and the fill icon that appears on top of it. The only way I can access currently is taking the value from the KeepassXC entry itself.

varjolintu commented 2 years ago

Make sure you've cleared all Custom Login Field data from the site before trying the TOTP fill.

AdrianSkar commented 2 years ago

Make sure you've cleared all Custom Login Field data from the site before trying the TOTP fill.

Email and password are prompted on a previous page (https://app.clickup.com/login as opposed to https://app.clickup.com/login/totp). I tried w/o custom fields, with all fields set and with only TOTP set but no luck.

jurgenhaas commented 2 years ago

I'm having the same issue with Drupal sites that have 2FA enabled. The username and password are recognized on login form. Then, a second form for the TOTP is called on a different path, but the field is not recognized. Not even with the context menu or keyboard shortcut. That field won't get filled.

Then I tried defining custom fields, but that makes things even worse. When I configure the TOTP field on the second form, it does work alright, but then the user name field on the first form gets recognized as the TOTP field as well, although it is the user name field.

I then exported the settings from this plugin and checked, how the custom fields are defined, and that gave me a glue what's wrong:

    "https://www.example.com": {
      "totp": [
        "/html/body/div/div/div/div[2]/div/div/main/section/form/fieldset/input",
        "INPUT text code "
      ],
      "fields": []
    }

The identifier /html/body/div/div/div/div[2]/div/div/main/section/form/fieldset/input is the same for all the input elements in both forms. The 3 fields look like this:

# On https://www.example.com/user/login: username and password
<input autocorrect="none" autocapitalize="none" spellcheck="false" autofocus="autofocus" data-drupal-selector="edit-name" aria-describedby="edit-name--description" type="text" id="edit-name" name="name" value="" size="60" maxlength="60" class="required form-control" required="required" aria-required="true">
<input data-drupal-selector="edit-pass" aria-describedby="edit-pass--description" type="password" id="edit-pass" name="pass" size="60" maxlength="128" class="required form-control" required="required" aria-required="true">

# On https://www.example.com/tfa/...: TOTP
<input autocomplete="off" autofocus="autofocus" data-drupal-selector="edit-code" aria-describedby="edit-code--description" type="text" id="edit-code" name="code" value="" size="60" maxlength="128" class="required form-control" required="required" aria-required="true">

The context /html/body/div/div/div/div[2]/div/div/main/section/form/fieldset is the same on both forms for all the input field.

My suggestion: each input field always has a name attribute, using that in addition to the DOM-path would solve the problem.

While on it, I wonder why the TOTP field is not recognized here in the first place. What attributes would have to be added from the Drupal site to make this work out of the box?

varjolintu commented 2 years ago

@jurgenhaas It's not 100% sure that input fields have a name attribute. Some pages have, some don't. Thanks for the TOTP field code. I can check what's wrong with it. The manual fill from context menu should work though.

However, this issue is a separate from the original one. A different site too. Drupal is already mentioned in https://github.com/keepassxreboot/keepassxc-browser/issues/1087.

jurgenhaas commented 2 years ago

Thanks @varjolintu for your quick response. I saw the other issue as well but drupal.org is slightly different from other Drupal sites. But you're right, it is most likely related.

To answer your question: no, context menu doesn't work, neither on any of my Drupal sites, nor on drupal.org

Please let me know if I can help with testing or anything else on either of the 2 issues. I'm also connected with the maintainers of the TFA module for Drupal, so if we need to adjust anything there, this should be achievable.

varjolintu commented 2 years ago

Maybe it's easiest if I create a Drupal account and test this.

jurgenhaas commented 2 years ago

@varjolintu I could also provide you with an account on one of my Drupal sites, if that makes testing easier. Please see also my comment in #1087 where I got it to work with an easy hack.

varjolintu commented 2 years ago

@jurgenhaas A ready account would be more than helpful. Contact me on Matrix for details.

jurgenhaas commented 2 years ago

Any progress on this one?

varjolintu commented 2 years ago

@jurgenhaas Haven't had time to look at it yet.

varjolintu commented 2 years ago

I made a one fix for it that allows filling the TOTP using the context menu. So far it's the best I can do.

jurgenhaas commented 2 years ago

Thanks @varjolintu are you also planning to tag a release for that?

varjolintu commented 2 years ago

@jurgenhaas Coming for the next version (1.8.3). A milestone will work as a temporary tag. I usually push the real tag just before an actual release.