Codecov Report

All modified and coverable lines are covered by tests ✅

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

@@             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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

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

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 ✔️
2. The app has to be compatible with all popular TOTP apps, like Google Authenticator, by default. Seems this is the case right now ✔️
3. Configuration for the secret/length and the algorithm must not be done by the user. This has to be an admin setting. ❌
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.

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 ✔️
• migration needs to be tested on MySQL/MariaDB throws error Fixed ✔️
• l10n

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

@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.

@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.

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.

@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.

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