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.

Sign up for GitHub

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

Jump to bottom

2FA setup during login #15304

Merged
merged 1 commit into from
May 17, 2019
Merged

2FA setup during login #15304

merged 1 commit into from
May 17, 2019

Conversation

rullzer
Copy link
Member

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

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
rullzer
rullzer commented May 9, 2019
View reviewed changes
.htaccess Outdated Show resolved Hide resolved
@rullzer

This comment has been minimized.

Sign in to view
@rullzer

This comment has been minimized.

Sign in to view
@ChristophWurst ChristophWurst added this to TO REVIEW (max 4 PRs) in Christoph's Tasks via automation May 10, 2019
@rullzer
Copy link
Member Author

rullzer commented May 13, 2019

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

@rullzer
Copy link
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
Member

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
Member

georgehrke commented May 16, 2019 •
edited

![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
Member

ChristophWurst commented May 16, 2019 •
edited

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

georgehrke commented May 16, 2019 •
edited

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

@ChristophWurst
Copy link
Member

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
Member

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

Christoph's Tasks automation moved this from TO REVIEW (max 4 PRs) to TO INTEGRATE May 17, 2019
MorrisJobke
MorrisJobke approved these changes May 17, 2019
View reviewed changes
Copy link
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 merged commit 528eb1b into master May 17, 2019
Christoph's Tasks automation moved this from TO INTEGRATE to DONE 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

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@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
No open projects
Christoph's Tasks
DONE
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