Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: make TOTP mandatory in Nextcloud #41

Closed
bitking opened this issue Aug 22, 2016 · 72 comments
Closed

Feature request: make TOTP mandatory in Nextcloud #41

bitking opened this issue Aug 22, 2016 · 72 comments

Comments

@bitking
Copy link

bitking commented Aug 22, 2016

Hi there,

Please make it possible to make TOTP mandatory for groups and in general.

Thanks!

@ChristophWurst
Copy link
Member

Hi @bitking,
thanks for your feature request.

How should that look like technically and/or on the user interface?

@bitking
Copy link
Author

bitking commented Aug 22, 2016

When an admin goes to the admin page, there should be an extra tab for twofactor_totp app with the option for additional settings. There it should be able to enforce TOTP for certain groups or for everyone but then exclude certain groups like it is with other owncloud/nextcloud apps as well..

Perhaps when the user logs in for the first time he should get redirected to a page where he has to scan the TOTP code and agree with the fact that he needs it to log in.

If the user never logged in he should still be able to login in not using TOTP to get the process described above.

Sorry cant help you with the technical aspect.

@ChristophWurst
Copy link
Member

I see, thanks for the explanation

@sandbergcs
Copy link

Hello,

I second this. We are going to be using this in a HIPAA environment and we have to have two factor enabled. Perhaps you can get a link generated that they can use instead of the scanner.

@ChristophWurst
Copy link
Member

@bitking @sandbergcs @trockenasche @cyann @Finkregh is anybody of you willing to help implement this feature?

@sandbergcs
Copy link

Hello, my company is willing to contribute financially if the rest of the members of this thread are willing to as well. I am not sure exactly what you would have to do to make this change.

@gmalone
Copy link

gmalone commented Sep 6, 2016

Regardless of the path to it, OC should have native integration of the two-factor option. Most apps that retain user information and content in the near future will out of necessity and logic offer 2-factor auth, else be viewed as not secure enough.

@ChristophWurst
Copy link
Member

Regardless of the path to it, OC should have native integration of the two-factor option. Most apps that retain user information and content in the near future will out of necessity and logic offer 2-factor auth, else be viewed as not secure enough.

2FA is natively integrated into ownCloud/Nextcloud. See http://blog.wuc.me/2016/05/30/adding-two-factor-auth-to-owncloud.html for some background information.

@gmalone
Copy link

gmalone commented Sep 6, 2016

TOTP works well except for when I log into my desktop on Ubuntu and the OC desktop sync client asks me to log in. When TOTP is on, authenticating the sync client always fails and keeps asking for authentication. Turning TOTP off allows the sync client to authenticate. Basically, the desktop sync client auth function does not seem to have hooks into the TOTP, so maybe this is an issue for that client and not TOTP... dunno.

Looking at the OC logs, I see the failed client sync authentication event below.

Side note, I was reasonably alarmed at seeing my full password in plain text in the log! (Gulp!) Are the OC/TOTP pws not being saved and handled in an encrypted manner?

Thoughts? And thanks for your efforts to help make OC more secure.

Ok here's a log event when authentication fails to work in desktop sync client and TOTP is 'on':

Exception: {"Message":"HTTP\/1.1 401 Unauthorized","Exception":"OCA\\DAV\\Connector\\Sabre\\Exception\\PasswordLoginForbidden","Code":0,"Trace":"#0 \/home\/datazens\/public_html\/owncloud\/3rdparty\/sabre\/dav\/lib\/DAV\/Auth\/Backend\/AbstractBasic.php(105): OCA\\DAV\\Connector\\Sabre\\Auth->validateUserPass('** my username **', '****** my password in plain text! ******')\n#1 \/home\/datazens\/public_html\/owncloud\/apps\/dav\/lib\/Connector\/Sabre\/Auth.php(242): Sabre\\DAV\\Auth\\Backend\\AbstractBasic->check(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#2 \/home\/datazens\/public_html\/owncloud\/apps\/dav\/lib\/Connector\/Sabre\/Auth.php(146): OCA\\DAV\\Connector\\Sabre\\Auth->auth(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#3 \/home\/datazens\/public_html\/owncloud\/3rdparty\/sabre\/dav\/lib\/DAV\/Auth\/Plugin.php(166): OCA\\DAV\\Connector\\Sabre\\Auth->check(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#4 [internal function]: Sabre\\DAV\\Auth\\Plugin->beforeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#5 \/home\/datazens\/public_html\/owncloud\/3rdparty\/sabre\/event\/lib\/EventEmitterTrait.php(105): call_user_func_array(Array, Array)\n#6 \/home\/datazens\/public_html\/owncloud\/3rdparty\/sabre\/dav\/lib\/DAV\/Server.php(446): Sabre\\Event\\EventEmitter->emit('beforeMethod', Array)\n#7 \/home\/datazens\/public_html\/owncloud\/3rdparty\/sabre\/dav\/lib\/DAV\/Server.php(248): Sabre\\DAV\\Server->invokeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#8 \/home\/datazens\/public_html\/owncloud\/apps\/dav\/appinfo\/v1\/webdav.php(56): Sabre\\DAV\\Server->exec()\n#9 \/home\/datazens\/public_html\/owncloud\/remote.php(164): require_once('\/home\/datazens\/...')\n#10 {main}","File":"\/home\/datazens\/public_html\/owncloud\/apps\/dav\/lib\/Connector\/Sabre\/Auth.php","Line":132,"User":false}

@ChristophWurst
Copy link
Member

TOTP works well except for when I log into my desktop on Ubuntu and the OC desktop sync client asks me to log in. When TOTP is on, authenticating the sync client always fails and keeps asking for authentication. Turning TOTP off allows the sync client to authenticate. Basically, the desktop sync client auth function does not seem to have hooks into the TOTP, so maybe this is an issue for that client and not TOTP... dunno.

This is intended. Simply create an app-password on your personal settings page and use that to configure your client. 2FA would be useless if an attacker could use your login password to sync files, wouldn't it 😉

Side note, I was reasonably alarmed at seeing my full password in plain text in the log! (Gulp!) Are the OC/TOTP pws not being saved and handled in an encrypted manner?

That should not happen, so maybe that's a bug in whatever server component you use. Please search for existing issues upstream (oc core or nc server). Thanks.

If there are any further questions/problems, please open a separate issue ;-)

@gmalone
Copy link

gmalone commented Sep 6, 2016

Brilliant. Thanks for the background link to 2FA... I will study... great info there. Also, thanks for the clarification re: setting an app password to resolve the desktop sync client issue. I appreciate your time,

@maximelovino
Copy link

Hi,

we recently setup nextcloud at our company, and that was one of the sore points. We need to be able to enforce the usage of TOTP on a per group/per user way.

In the meantime, we have setup a python script that checks the mySQL DB and reports to us on a daily basis which users aren't using TOTP, and we tell them to use it. But an integrated function would be very helpful, the way @bitking explained.

I can try to help if needed

@boppy
Copy link

boppy commented Nov 6, 2016

Okay, that's a great idea. I put some thoughts (and bytes) on it.

The biggest problem I thought of is the fact that a new user has to get his auth code. So he should be in a secure environment while getting the QR code. Current solution is that he get's it after the first login - right above the box to enter the digits:

first login view

I thought of three operation modes:

  • Default Mode | Each user can decide whether to use TOTP or not
  • Whitelist Mode | Nobody is forced to TOTP, except for mentioned groups/users
  • Blacklist Mode | Everybody is forced to TOTP, except for mentioned groups/users

Last thing is a selector for groups and users by using the sharee-api for selecting blacklist/whitelist entries.

I have an early alpha where basic blacklisting works. But as I'm totally new to NextCloud it took me some time to reach that point ;-)

I already made some basic work. I created the DB table, did the routes and set up the functions. Next I'll do some JS to make the selector work. Afterwards I'll try to get the DB thing working. But there's more todo. I just copied the code to create the QR from the original source. But since the needed functions are not present, the first-login-qr hat just a dirty-dev-dummy-name (it's working nevertheless), and I think there will be a better way to implement the admin-changes. They are just appended to the SettingsController. With no checks!

I will try to keep repo at https://github.com/boppy/twofactor_totp/tree/force-2fa updated and will create a push request once it's up working.

@boppy
Copy link

boppy commented Nov 6, 2016

That was a good sunday ;). Finalized the Admin-View and nearly all logical parts.

admin view

But I struggle with the oc/nc structure. Two things where I really would appreciate any hint:

  • Database Stuff (lib/C/SettingsController :145 and :158, lib/P/TotpProvider :151 and settings/admin.php :8)
  • Getting the current defaults (lib/P/TotpProvider :48). Freshly initialized OC_Defaults defaults to its defaults 🎩

But all endpoints are ready to rock. The selected users/groups are send to the server (pipe-separated as in oc-core-code), stored users/groups will be written back (if the array is filled) and rendered correctly. Login code is also complete with the 3 modes. I renamed 2 of them. Because 🐰.

Perhaps I should spend some time getting a better understanding of that app structure. Would love to finalize that. ;)

[edit +30m]
There is a mistake in the TotpProvider:136 where the displayname is used instead of the userid.
So it won't work... 😢

@GitHubUser4234
Copy link

@boppy : Just wanted to say I support you in spirit, would be great to see your work becoming finalized :)

@ChristophWurst
Copy link
Member

@boppy thanks a lot, this looks very promising! Feel free to open a PR, then I can give you some feedback on the changes 🚀

@maximelovino
Copy link

First of all, thanks a lot to the people who are working on this issue.

I would just like to add a small request as well. We should be able from the users view or admin to reset the TOTP for a given user. One of our users with TOTP activated got his phone stolen the other day and couldn't login anymore. So i had to remove his secret manually from the nextcloud database and that worked, but if we manage TOTP as admin with the enforcement etc, we should be able to do that as well.

@ChristophWurst
Copy link
Member

@boppy
Copy link

boppy commented Nov 8, 2016

See Christophs Article on 2FA where you can find the occ command to disable a 2FA:

 ./occ twofactor:disable florian

But I'm with you that this should be done in the GUI. But we seem to be close to a point where a 2FA-GUI is needed... ;) See referenced dicussions.

@maximelovino
Copy link

@ChristophWurst @boppy Thank you for your help, I hadn't looked into the occ commands. Keep up the good work 👍

@GitHubUser4234
Copy link

./occ twofactor:disable florian

@boppy How would the behavior of your implementation be for users with disabled 2FA, e.g. in Opt-Out Mode? Thanks.

@boppy
Copy link

boppy commented Nov 9, 2016

@GitHubUser4234 In my opinion that is a system call, so OCC acts above all app configs. It's like that one black hat guy... It has _the_ force! ;-)

But nevertheless: That OCC might not be needed if there is an interface where you can enable and disable that function per user/group... We'll see where that road ends.

[edit +15h]
potential road ending suggestion

@boppy
Copy link

boppy commented Nov 15, 2016

[...] Feel free to open a PR, then I can give you some feedback on the changes 🚀

@ChristophWurst: would be great - or is the discussion about the general topic blocking this one?

@ChristophWurst ChristophWurst changed the title Feature request: make TOTP mandatory in owncloud Feature request: make TOTP mandatory in Nextcloud Nov 16, 2016
@urkle
Copy link

urkle commented Nov 20, 2017

We are a customer and are wanting this feature implemented.. However last I dug into the status it seems that the core Nextcloud team didn't know exactly how they wanted to expose functionality in the core to allow for this feature to work nicely as expected. So how can I get a meeting together with someone to hash out exactly how things need to be modified so that it can be implemented by someone (possibly even me)

@bitking
Copy link
Author

bitking commented Dec 5, 2017

Well i can tell you the way i see it functionally that is no problem. Need someone to make it and maintain it though.

@ghost

This comment has been minimized.

@jakimfett
Copy link

Okay.
Done waiting.
As of today, I'm digging into the source code to get this feature request moved along.

@FreeMinded

This comment has been minimized.

@OpenAai

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@vasilko

This comment has been minimized.

@r2evans

This comment has been minimized.

@vasilko

This comment has been minimized.

@skluthe

This comment has been minimized.

@r2evans

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@r2evans

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@GoetheG

This comment has been minimized.

@andhoy

This comment has been minimized.

@DanielWeeber

This comment has been minimized.

@vasilko

This comment has been minimized.

@ChristophWurst
Copy link
Member

Please see nextcloud/server#2348 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests