feat: introducing configurable options

[ernolf]

• Solves: #26
• Related: #1324
• Related PR: #1546
• Background: https://help.nextcloud.com/t/lenght-of-totp-key/169226

This is a better, safer approach to introducing configurable options as initially suggested in #1546

This pull request has changed significantly from the initial approach. Please go to https://github.com/nextcloud/twofactor_totp/pull/1549#issuecomment-2278845895 for the description of the current, latest state.

This approach has been significantly improved and abandoned as it was here: To ensure that users are not locked out by admin settings, in this approach the values ​​for Token Length and Hash Algorithm can be set individually by each user:

New secrets are always created with the default values ​​because these are supported in every TOTP app. If you use a high-quality TOTP app like Aegis, you can then adjust the values afterwards. This adjustment must be applied identical both here in the server and in the TOTP app. The administrator still has the option to set the length of the secrte via occ though, as originally suggested in #1546 and described in the README.md:

---

Finally, the favicon URL is integrated into the setup URL (QR code) so that apps like FreeOTP that support it can use it as an icon for the account:

• Solves: #1407

ernolf

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 34.24%. Comparing base (78be10e) to head (c388d42). Report is 1 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1549 +/- ## ============================================ - Coverage 42.63% 34.24% -8.40% - Complexity 101 132 +31 ============================================ Files 19 20 +1 Lines 326 476 +150 ============================================ + Hits 139 163 +24 - Misses 187 313 +126 ``` | [Flag](https://app.codecov.io/gh/nextcloud/twofactor_totp/pull/1549/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nextcloud) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/nextcloud/twofactor_totp/pull/1549/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nextcloud) | `34.24% <ø> (-8.40%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nextcloud#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ernolf]

This approach has been significantly improved and abandoned as it was here:

[ernolf]

It has been questioned whether giving users the option to set the token length and hash algorithm might unnecessarily complicate things for them. It was considered whether these values should be predefined by the admin, like the secret length. In that case, the URL transmitted with the QR code would need to include these values. While the URL can indeed include the token length and hash algorithm along with the secret and issuer, very few TOTP apps actually take these parameters into account. For reference, see the description of the Key-Uri-Format in the Google Authenticator wiki.

In my opinion, it is more important that TOTP works out-of-the-box with ALL TOTP apps. Therefore, it has been designed so that each new secret is ALWAYS set up with the default values, ensuring immediate compatibility with ALL TOTP apps. Users of advanced TOTP apps, such as Aegis, which allow customization of token length and hash algorithm settings, can now take advantage of these options individually.

It is crucial that the values set for token length and/or hash algorithm in the UI are exactly mirrored in the TOTP app for the account. Besides the potential to increase security through a more advanced hash algorithm or a longer token length, the token can also be shortened to as few as 4 digits if desired. Technically, it is even possible to shorten the token length to 1, though this would be too easily compromised. However, a token length of 4 or 5 is still secure, as an attacker would not expect varying token lengths.

The warning message is intentionally very generic and can/should certainly be improved, just like the final buttons. This was primarily about the functionality. The graphics and localization now need to be adjusted to fit the overall look of the Security Page, where, in my opinion, there is still much to improve since everyone seems to be doing their own thing.

ernolf

[ChristophWurst]

I have discussed the feature with the Nextcloud GmbH design lead and product management.

Here are some key aspects to consider

1. The system has to prevent breaking of existing setups. That was the case with the first version. It seems impossible right now :heavy_check_mark:
2. The app has to be compatible with all popular TOTP apps, like Google Authenticator, by default. Seems this is the case right now :heavy_check_mark:
3. Configuration for the secret/length and the algorithm must not be done by the user. This has to be an admin setting. :x:
   4. There has to be a button for advanced settings. In the advanced settings, there is a warning that changing the defaults will limit the number of compatible mobile apps. Possible suggestion: add a list of configuration combinations that are known to work with a specified list of mobile apps.

It has been questioned whether giving users the option to set the token length and hash algorithm might unnecessarily complicate things for them. It was considered whether these values should be predefined by the admin, like the secret length. In that case, the URL transmitted with the QR code would need to include these values. While the URL can indeed include the token length and hash algorithm along with the secret and issuer, very few TOTP apps actually take these parameters into account. For reference, see the description of the Key-Uri-Format in the Google Authenticator wiki.

I'd say include anything in the QR code string that an app could pick up, as long as it's ignored by the others and doesn't cause compatibility issues.

[ernolf]

3. Configuration for the secret/length and the algorithm must not be done by the user. This has to be an admin setting. ❌

There are a total of three values ​​that this PR is about:

• Secret length can be changed by the admin and only affects newly created secrets. Secrets that have already been created are not affected. ~What still needs to be tested is whether there are any restrictions on this length on the part of Totp client apps.~ :heavy_check_mark: If the admin does not specify otherwise, the length of 32 characters recommended by the RFC applies.

• OTP tokenlength Some apps also allow an OTP tokenlength of 8 digits ~but most apps simply ignore this value when it is transmitted in the URL (QR code).~ ~If this value were to be set by the admin, only very few TOTP apps would remain that would still work.~

  EDIT: All apps I have tested, meanwhile support different number of digits to set as token length. This means that a default in a range from 6 - 8 digits set by the admin can be implemented. :heavy_check_mark:

• Hash algorithm This is set to sha1 by default. ~The simple, most common apps do not provide any way to change this, either with a setting option in the corresponding account in the app or by means of URL (QR code) transfer.~ ~So if the algorithm to be used is set to a different one by the admin, there are only a few Totp apps (I personally only know Aegis) that can handle this and still work.~

  EDIT: All apps I have tested, meanwhile support the three algorithms available by twofactor_totp. This means that a default to be set by the admin can be implemented. :heavy_check_mark:

I'd say include anything in the QR code string that an app could pick up, as long as it's ignored by the others and doesn't cause compatibility issues.

~Unfortunately, this ignores reality and the way how the TOTP functionality works.~ If an OTP token length or a hash algorithm that differs from the default is transmitted in the QR code, then it cannot be determined whether these values ​​were adopted by the user's app or ignored. If they are adopted, then Totp works, if they are ignored, then Totp does not work. So unfortunately we have to limit the url (QR-Code) to the lowest common denominator. ~In other words, exactly as it is currently in use.~

EDIT: I am working on a solution to satisfy it that way. :heavy_check_mark:

4. There has to be a button for advanced settings. In the advanced settings, there is a warning that changing the defaults will limit the number of compatible mobile apps. Possible suggestion: add a list of configuration combinations that are known to work with a specified list of mobile apps.

I think that's exactly what we should focus on here. I already mentioned that I only wrote something generic. Maybe a small project wiki page could even be created where supported apps can be listed.

ernolf

[ernolf]

Is it possible to use our Vue components for the elements

Yes @jancborchardt, of course!

This is my first project where I'm going down to the UI level and I have no prior knowledge in these areas. I've had to teach myself practically every step up to this point, but with the Vue components I simply lack the practice and the knowledge of where exactly I need to look. But I'm looking forward, to learning that too :wink:

[jancborchardt]

@ernolf cool, no problem – then it's best to do it in a separate follow-up so it's not a blocker for this pull request. :)

[ernolf]

4. There has to be a button for advanced settings. In the advanced settings, there is a warning that changing the defaults will limit the number of compatible mobile apps. Possible suggestion: add a list of configuration combinations that are known to work with a specified list of mobile apps.

This approach has been significantly improved and abandoned as it was here:

What about this:

@ChristophWurst?

[github-actions[bot]]

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[ernolf]

README.md:

---

no TOTP set up:

---

Click Enable:

---

Click "Advanced Settings":

---

Enter invalid secret:

The colors (here yelow, blue and red) are defined in the css of your theme:

    .warning-message {
        color: var(--color-warning);
    }

    .instruction-message {
        color: var(--color-info);
    }

    .error-message {
        color: var(--color-error);
    }

---

If any changes made, "Recreate QR-Code with custom settings" becomes clickable:

---

Once clicked, a new QR Code is created and can be scanned:

---

After verification with an OTP:

---

The "Advanced Settings" button for subsequent changes is removed now, since it became redundant as it is much easier and safer to apply custom settings via the setup where it can be tested immediately with the OTP.

---

If the QR-code is scanned with Free-OTP or Free-OTP+, the actual favicon wil be used as icon for the account.

---

Since this PR makes it possible to enter a custom secret, #1439 could be closed too.

TODO:

• migration needs to be tested on ~postgreSQL~ Done :heavy_check_mark:
• migration needs to be tested on ~MySQL/MariaDB throws error~ Fixed :heavy_check_mark:
• l10n

[mniehren]

Would there also be the possibility to set the secret and the other new options over the occ command on console ?

[ernolf]

@mniehren Would there also be the possibility to set the secret and the other new options over the occ command on consol

I've already thought about it, but I wanted to do it in a PR that follows this one. It would build on the functions created here. I also know of use cases where the admin creates the secrets and should be able to prevent the user from deleting or regenerating them. So I wanted to think about it in detail again so that it doesn't end up being a premature birth.

[mniehren]

@ernolf Great to hear, that's also my use case. The administrator should create the secrets and the user should not be able to modify or delete them, nor should he be able to turn off TOTP. Also the possibility to force TOTP for a particular user, but not for all users, would be great.

[ChristophWurst]

Just tested again. Impressive work! I'm afraid this goes a bit too far with the complexity. There are advanced settings for the user. This was defined as not acceptable

3. Configuration for the secret/length and the algorithm must not be done by the user. This has to be an admin setting. ❌

Could you find a compromise where only the admin needs to know about the advanced options?

There are two use cases where I can see stronger options enforced 1) Home user instance for family/friends 2) Big corporation

In both cases it's likely that the person setting up Nextcloud with TOTP could also tell the users which TOTP app they should use, an fine-tune the settings for the best default that work with the app.

[ernolf]

@ChristophWurst There are advanced settings for the user. This was defined as not acceptable

3. Configuration for the secret/length and the algorithm must not be done by the user. This has to be an admin setting. ❌

I have completely removed the previous approach, in which the settings could be changed by the user after the secret was created. I fully accepted the concerns about leaving these setting options to the user as they were, as it was possible for a user to render their TOTP unusable and, in the worst case, lock themselves out of their cloud.

Everything is now set up in such a way that the default values ​​set by the admin can be safely adopted for every app and it is impossible for the user to adopt incorrect settings, as every setting combination must first be authenticated with an OTP. There are also no dead ends or endless loops in which a user could get lost without being able to find their way out.

The only thing that is missing in my opinion is a "Cancel" button, which practically does the same thing as reloading the page without TOTP being activated.

The validity period of the one-time passwords can only be changed in a few apps and then again to a different extent from app to app. For this reason, I have refrained from setting a value by the admin, because the default value of 30 seconds is the only value that is supported by all devices. So if the admin is to set this and he sets a TTL of 15 seconds, for example, then as far as I know there is only one app that supports this and I am sure that a large number of admins whould be overwhelmed by this. That is why the only practical solution seemed to me to be the one presented in this approach. The concept of "Advanced Settings" is actually familiar to everyone and should not confuse anyone. Especially since it is clearly described in detail what it is about. I would find it very unfortunate if these options, which can now be set individually upon activation, were to be restricted. What I could offer, however, would be to make the visibility/availability of the Advanced Settings button unlockable by the admin. Something like this:

occ config:app:set --value=yes -- twofactor_totp enable_advanced_settings

and if not explicitly enabled, then it is off.

As you can see from the comments, there are also use cases where the user should not be able to see the settings at all, i.e. should not be able to deactivate their TOTP or create a new secret themselves. I imagine making this possible with an occ command and an additional column in the twofactor_totp_secrets database table. In such a case, the admin can then set all individual settings and provide the users with their own preconfigured TOTP devices and switch the visibility on and off individually for each user.

[ernolf]

Hi @jancborchardt, thank you for your reply

• Ideal would be to just have 2 settings:
  • "Default (SHA-1)" setting for compatibility with all OTP apps
  • And then the "Enhanced security" setting which uses a better algo with some preset lengths which is also compatible with at

As far as I know, all TOTP apps meanwhile support all three hash algorithms supported by twofactor_totp. Therefore, this is a setting that the admin can set to a default in my solution. If the admin does not set anything, SHA1 applies. The latter is important to ensure that secrets that have already been set up in the past continue to work, because when the database is migrated, these fields for existing secrets are pre-filled with the default, the previous hash algorithm SHA1.

A second default value that the admin can set is the length of the secret. If the admin does not set anything, the standard length of 32 characters applies, which corresponds exactly to the depth of 160 bits recommended by the RFC. The RFC does not allow secrets below 128 bits. For a long time, however, only a 16-character secret was used, which corresponds to a bit depth of 80. So for a long time twofactor_totp did not even meet the RFC requirements. This is why I have now set the lower limit that the admin can set for the secret to 26 characters, which corresponds to a depth of 130 bits. And I also capped it at 128 characters (640 bits). Secrets in this range are supported by all apps I know of and therefore this default can be defined by the admin as well.

Then there are two more values:

1. The length of the one-time password

   As far as I know, this can vary between 6 and 8 in every app. In a few apps, however, it can also be shortened to 1 and extended to 10 characters.

   However, since only an OTP token length of 6, 7 or 8 (as far as I know) is supported by all apps, I also made this a default value that can be set by the admin within that range (6-8).

2. The validity period of the one-time password

   This is 30 seconds by default. There are many apps that support a longer validity period, but not all apps support it and there are some apps that support a shorter period. Among those that only support the 30 seconds period is Google Authenticator, which is used by a lot of users. And to guarantee that the admin can't exclude anyone and every app continues to work out of the box without any prior knowledge, I have kept this default value of 30 seconds coded in.

So much for what the admin can set.

• And it should only be an admin setting as mentioned

There are apps that support other one-time password token lengths outside of these options and allow other validity periods for the one-time passwords. For example, it is very convenient to work with one-time passwords of 4 digits that are valid for 15 seconds. And so that those who want to use this option also have the opportunity to do so, I have now developed the Advanced Settings, which allow the user to set this on an idiot-proof interface that does not allow the user to set or activate a single thing incorrectly. These settings cannot be predefined by the admin because they go into areas that many apps do not support and are also based on the personal preferences and security requirements of the individual user.

I also looked again into the original issue #26 to check what the requirements and issues are.

It's no longer just about #26, because that's already been resolved as described in the first paragraph of this answer. It's also about #1439 and I've also solved #1407. The rest are features that I wished for a long time and used so far by hacking the code. I am not the type of user that creates feature requests, I create features if I want them and have thought about it a lot. That is what you get

• Even if it is behind a feature flag, I would like for the interface to be nice and easy. Otherwise it’s basically just going to be a feature that very few people use, but we need to maintain since it’s in the main app.

Is that a subtle hint to fork my solution? The interface is intentionally designed in such a way that it is absolutely impossible to do anything wrong. Neither incorrect, too short or too long secrets can be adopted, nor can twofactor_totp be activated incorrectly in any other way, which guarantees that twofactor_totp can ALWAYS be activated in just a few steps, but never incorrectly. A large part of the complexity of the code is due to this highest security concept and idiot-proofing. Another is that there are actually two interfaces: once when logging in for the first time, when no second factor has been activated yet, and once from the personal settings. Both interfaces have different CSS, so the first is centered and the second is left-aligned, etc. It is very likely that areas of the code can be simplified. I taught myself every single step of my solution. I have never done frontend development before. So I would be overjoyed if someone who knows vue and can do this off the cuff could help me simplify the code so that it remains maintainable.

• If we want to have it as a "we are so secure" point (which is nice), it shouldn’t be behind a feature flag either.

Very good! We agree on that point!

Here I have a view of the setup window that is shown when you first log in, once with the advanced settings closed

and once with the advanced settings opened

I have shortened the labels to make it look simpler. I have also used the same names that are usually used in the TOTP apps so that a clear assignment is possible, although this is not necessary since everything is transmitted with the QR code.

For an extended explanation of the labels and the associated select fields, I have made a :title that is displayed as a tooltip when you mouse over:

example:

            <label :class="{ centered: center }"
                for="digits"
                :title="t('twofactor_totp', 'OTP token length')"
                @mouseleave="onMouseLeave">
                {{ t('twofactor_totp', 'Digits') }}
            </label>

that looks like this:

So this is getting very complex now,

Before you get too fixated on "It's all too complex", I would suggest that I demonstrate the feature to you at the Nextcloud conference so that you can "feel" it.

---

ernolf