To better support a TOTP application on the save device as TFA setup, allow the TOTP seed to be easily copied from the form.

Original bug report:

I am at https://www.drupal.org/user/107701/security/tfa/app-setup

The code field is greyed out. Why can't I copy it?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

coltrane’s picture

Title: The TFA code can't be copied » Allow TOTP seed code to be copied
Category: Bug report » Feature request
Issue summary: View changes

My original assumption was people wouldn't need to copy and paste the seed, that you'd used the QR code. Since then I've learned that people have lots of different ways of generating TOTP codes and the original assumption is incorrect.

Updating to a feature request to allow code to be copied.

greggles’s picture

Title: Allow TOTP seed code to be copied » Make it easier for TOTP seed code to be copied

FWIW, I used chrome and it copies fine. I guess safari is the same.

Firefox does let you select it but does not let you copy it...

joachim’s picture

Title: Make it easier for TOTP seed code to be copied » Make it possible for TOTP seed code to be copied on all browsers
Category: Feature request » Bug report

It doesn't work on FF -- therefore it's a bug.

greggles’s picture

Title: Make it possible for TOTP seed code to be copied on all browsers » Make it easier for TOTP seed code to be copied on all browsers
Category: Bug report » Feature request

Pedantic hat

It works fine to copy the code in FF:

1. Using the browser built in "copy" is only one way to duplicate the string. You could also read and type to duplicate the information.
2. View source, double click the text, use the browser built-in "copy" feature.

Pragmatic hat

In my experience, changing the Category of an issue doesn't help move the issue forward unless you have provided a compelling argument. I don't find #3 to be compelling.

How about instead of arguing about category you post a patch? In my experience that *does* help move the issue forward dramatically more.

joachim’s picture

> 1. Using the browser built in "copy" is only one way to duplicate the string. You could also read and type to duplicate the information.

That's a PITA and error-prone.

> 2. View source, double click the text, use the browser built-in "copy" feature.

Which is what I did. And most users on d.org will be savvy enough to know to do that. I don't imagine all users of this module on other sites will though.

> In my experience, changing the Category of an issue doesn't help move the issue forward unless you have provided a compelling argument. I don't find #3 to be compelling.

When something's broken, it's a bug. As a user it's incredibly frustrating to have my bug reports turned into feature requests and tasks, because it makes me feel the problems I'm encountering aren't being taken seriously.

greggles’s picture

As I'm sure you've experienced as a maintainer it's incredibly frustrating to have someone flipflop the category because it makes it feel like they care more about semantics than progress.

You got a positive response within 14 minutes. How many other queues give that kind of feedback and indication that the issue is worthwhile?

If you want the issue to be taken "more seriously", provide a patch. Otherwise, after a massive sprint to get this in shape to deploy on d.o, I imagine you can understand that it might take more than a day to find time to prioritize this issue.

banviktor’s picture

Status: Active » Needs review
FileSize
507 bytes

The problem here is that FF doesn't enable selection of disabled elements. I think their reason for this is the way they handle focus of HTML elements. FF requires focus for selection, Chrome doesn't, and by the HTML standards disabled elements can't have focus.

FF recommends setting readonly instead of disabled for elements that should be copiable. The Drupal Form API documentation doesn't show the way to do it (manually setting the #attributes tag to have the readonly element instead of using disabled would bypass Drupal's security checks that do not allow disabled elements to be returned with modified data - this is on line 2075 of form.inc in Drupal 7.39). However I could find in the code that setting #allow_focus on a #disabled element would make it readonly (form.inc line 2060), so this is the correct way of doing it, but it's not yet documented on the Form API page.

One downside though is that it doesn't look that "disabled" as the background is set to white by default on most browsers for readonly elements. I could add some explicit styling, but not sure whether that would be a nice solution.

Here's the patch, and I will file an issue for the Form API page soon.

  • coltrane committed 8546879 on 7.x-1.x authored by banviktor
    Issue #2483529 by banviktor: Make it easier for TOTP seed code to be...
coltrane’s picture

Status: Needs review » Fixed

Thanks for the research @banviktor! I confirmed this works on FF and have committed it. The input field having a white background is acceptable.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.