Skip to content

2FA setup during login#15304

Merged
rullzer merged 1 commit into
masterfrom
enh/2fa_setup_at_login
May 17, 2019
Merged

2FA setup during login#15304
rullzer merged 1 commit into
masterfrom
enh/2fa_setup_at_login

Conversation

@rullzer
Copy link
Copy Markdown
Member

@rullzer rullzer commented Apr 30, 2019 •
edited by ChristophWurst
Loading

Hacky totp branch: nextcloud/twofactor_totp#550

Fixes #12268

  • Make sure the setup during login routes are only available when 2FA is not yet setup.

@rullzer rullzer added the 2. developing Work in progress label Apr 30, 2019
@rullzer rullzer added this to the Nextcloud 17 milestone Apr 30, 2019
@rullzer rullzer mentioned this pull request Apr 30, 2019
Merged
1 task
tcitworld
tcitworld reviewed Apr 30, 2019
View reviewed changes
Comment thread core/Controller/TwoFactorChallengeController.php Outdated
rullzer
rullzer commented May 9, 2019
View reviewed changes
Comment thread .htaccess Outdated
@rullzer

This comment has been minimized.

Sign in to view
@rullzer

This comment has been minimized.

Sign in to view
@rullzer
Copy link
Copy Markdown
Member Author

rullzer commented May 13, 2019

I tested the part from @ChristophWurst (thanks for finishing up!) good from my PoV.

@rullzer
Copy link
Copy Markdown
Member Author

rullzer commented May 13, 2019

Ah. One more thing that came to mind. Should we have an admin setting if 2FA during login is allowed?

@ChristophWurst
Copy link
Copy Markdown
Member

ChristophWurst commented May 13, 2019

Ah. One more thing that came to mind. Should we have an admin setting if 2FA during login is allowed?

Fine by me, though low priority. This can be added in a follow-up PR 😉

@georgehrke
Copy link
Copy Markdown
Member

georgehrke commented May 16, 2019 •
edited
Loading

Details ![336B494C-30D0-4E1D-BFBF-A057E578358E](https://user-images.githubusercontent.com/1250540/57849898-d9142f80-77dc-11e9-8e63-595e35adea68.png)

GET http://nextcloud.local/apps/twofactor_totp/js/main-login-setup.js?v=1466e706-0 net::ERR_ABORTED 404 (Not Found)

@ChristophWurst Is this maybe related to your js cleanup?

@ChristophWurst
Copy link
Copy Markdown
Member

ChristophWurst commented May 16, 2019 •
edited
Loading

@ChristophWurst Is this maybe related to your js cleanup?

Were you on nextcloud/twofactor_totp#550 and did you build the app?

@georgehrke
Copy link
Copy Markdown
Member

georgehrke commented May 16, 2019 •
edited
Loading

yes and no. Required Krankerl and there are no macOS builds afaik?

@ChristophWurst
Copy link
Copy Markdown
Member

ChristophWurst commented May 16, 2019

yes and no. Required Krankerl and there are no macOS builds afaik?

Yes, good point. I'm building it for OSX on CI but there are no binary releases (yet). Let me package that for you: twofactor_totp.tar.gz

@georgehrke
Copy link
Copy Markdown
Member

georgehrke commented May 16, 2019

Yes, good point. I'm building it for OSX on CI but there are no binary releases (yet).

That would be awesome. I was just about to simply wrap it in a docker container.

georgehrke
georgehrke approved these changes May 16, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@georgehrke georgehrke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Works

From a usability pov i would probably drop the second time, that you have to enter your code.

MorrisJobke
MorrisJobke approved these changes May 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and worked 👍

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 17, 2019
@rullzer
Allow 2FA to be setup on first login
579162d 
Once 2FA is enforced for a user and they have no 2FA setup yet this will
now prompt them with a setup screen. Given that providers are enabled
that allow setup then.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@rullzer rullzer force-pushed the enh/2fa_setup_at_login branch from aef0b4e to 579162d Compare May 17, 2019 08:13
@rullzer rullzer merged commit 528eb1b into master May 17, 2019
@rullzer rullzer deleted the enh/2fa_setup_at_login branch May 17, 2019 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tcitworld tcitworld tcitworld left review comments

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@georgehrke georgehrke georgehrke approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@nickvergessen nickvergessen Awaiting requested review from nickvergessen nickvergessen is a code owner

Assignees

@rullzer rullzer

@ChristophWurst ChristophWurst

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement feature: authentication

Projects

None yet

Milestone

Nextcloud 17

Development

Successfully merging this pull request may close these issues.

Possibility to set up 2FA on login

5 participants

@rullzer @ChristophWurst @georgehrke @MorrisJobke @tcitworld