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

Second authentication factor #13787

Merged
merged 15 commits into from Nov 1, 2017

Conversation

Projects
None yet
3 participants
@nijel
Member

nijel commented Oct 30, 2017

Generic framework for supporting second authentication factor.

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

@nijel nijel self-assigned this Oct 30, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Oct 31, 2017

Codecov Report

Merging #13787 into master will increase coverage by 0.1%.
The diff coverage is 78.48%.

@@            Coverage Diff            @@
##           master   #13787     +/-   ##
=========================================
+ Coverage   53.91%   54.01%   +0.1%     
=========================================
  Files         488      494      +6     
  Lines       82314    82659    +345     
=========================================
+ Hits        44382    44652    +270     
- Misses      37932    38007     +75

codecov bot commented Oct 31, 2017

Codecov Report

Merging #13787 into master will increase coverage by 0.1%.
The diff coverage is 78.48%.

@@            Coverage Diff            @@
##           master   #13787     +/-   ##
=========================================
+ Coverage   53.91%   54.01%   +0.1%     
=========================================
  Files         488      494      +6     
  Lines       82314    82659    +345     
=========================================
+ Hits        44382    44652    +270     
- Misses      37932    38007     +75

@nijel nijel changed the title from WIP: Second authentication factor to Second authentication factor Oct 31, 2017

nijel added some commits Oct 30, 2017

Add generic interface for second authentication factor
Signed-off-by: Michal Čihař <michal@cihar.com>
Add support for HOTP and TOTP authentication
This supports Google Authenticator and similar applications.

Issue #6197

Signed-off-by: Michal Čihař <michal@cihar.com>
Add configuration for second authentication factor
Signed-off-by: Michal Čihař <michal@cihar.com>
Add documentation for second authentication factor
Signed-off-by: Michal Čihař <michal@cihar.com>
Add support for FIDO U2F authentication
Signed-off-by: Michal Čihař <michal@cihar.com>
Simplify second factor auth API
We now only pass SecondFactor object to plugins, not individual
parameters.

Signed-off-by: Michal Čihař <michal@cihar.com>
Test check method for simple second factor
Signed-off-by: Michal Čihař <michal@cihar.com>
Share and test code for listing backends
Signed-off-by: Michal Čihař <michal@cihar.com>
Share code for error report in second factor
...and test it.

Signed-off-by: Michal Čihař <michal@cihar.com>
Test registration and authentication for FIDO U2F
Signed-off-by: Michal Čihař <michal@cihar.com>
Properly update FIDO U2F counter on login
Signed-off-by: Michal Čihař <michal@cihar.com>
Improved FIDO U2F error reporting
Signed-off-by: Michal Čihař <michal@cihar.com>
Install optional dependencies on Scrutinizer
Signed-off-by: Michal Čihař <michal@cihar.com>
Properly test second factor check
The default code does session caching, we want to avoid it in the tests.

Signed-off-by: Michal Čihař <michal@cihar.com>
Share code for getting server URL and use it in 2FA as well
This way multiple phpMyAdmin installations can be identified.

Signed-off-by: Michal Čihař <michal@cihar.com>

@nijel nijel added this to the 4.8.0 milestone Nov 1, 2017

@nijel nijel merged commit 45d1924 into phpmyadmin:master Nov 1, 2017

5 checks passed

DCO All commits have a DCO sign-off from the author
Scrutinizer Analysis: 2 new issues, 65 updated code elements – Tests: passed
Details
codecov/patch 78.48% of diff hit (target 53.91%)
Details
codecov/project 54.01% (+0.1%) compared to 069cb02
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nijel nijel deleted the nijel:second-factor branch Nov 1, 2017

@ibennetch

This comment has been minimized.

Show comment
Hide comment
@ibennetch

ibennetch Nov 2, 2017

Member

In my experience, this is mostly called "two factor authentication" rather than "second authentication factor" — is there any objection to me adjusting the documentation?

Member

ibennetch commented Nov 2, 2017

In my experience, this is mostly called "two factor authentication" rather than "second authentication factor" — is there any objection to me adjusting the documentation?

@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Nov 2, 2017

Member

I was unsure about naming - the problem is that there is 2FA (Two factor authentication) and U2F (Universal second factor), which we now support both. Wikipedia now calls this Multi-factor authentication.

Member

nijel commented Nov 2, 2017

I was unsure about naming - the problem is that there is 2FA (Two factor authentication) and U2F (Universal second factor), which we now support both. Wikipedia now calls this Multi-factor authentication.

nijel added a commit that referenced this pull request Nov 2, 2017

Changelog entry for #6197 and #13787
Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Nov 2, 2017

Member

Anyway looking at other tools implementing this, indeed "Two-factor authentication" is the most widely used naming, so I've changed to it in 5acd64f.

Member

nijel commented Nov 2, 2017

Anyway looking at other tools implementing this, indeed "Two-factor authentication" is the most widely used naming, so I've changed to it in 5acd64f.

@ibennetch

This comment has been minimized.

Show comment
Hide comment
@ibennetch

ibennetch Nov 4, 2017

Member

This is very nice, thank you.

Member

ibennetch commented Nov 4, 2017

This is very nice, thank you.

@pwallner

This comment has been minimized.

Show comment
Hide comment
@pwallner

pwallner Nov 22, 2017

Not working, at settings I see
Two-factor authentication is not available, please install optional dependencies to enable authentication backends.

Did a git clone and then composer update

pwallner commented Nov 22, 2017

Not working, at settings I see
Two-factor authentication is not available, please install optional dependencies to enable authentication backends.

Did a git clone and then composer update

@ibennetch

This comment has been minimized.

Show comment
Hide comment
@ibennetch

ibennetch Nov 22, 2017

Member

@pwallner Since the dependencies are categorized as suggested packages in Composer, a composer update doesn't install them automatically.

The only way I know to do this is to add them to the Composer requirements (through composer require pragmarx/google2fa bacon/bacon-qr-code, and also adding samyoul/u2f-php-server if you need FIDO U2F) . That tells Composer to add those as required dependencies, then it installs them for you. That should fix your problem.

Member

ibennetch commented Nov 22, 2017

@pwallner Since the dependencies are categorized as suggested packages in Composer, a composer update doesn't install them automatically.

The only way I know to do this is to add them to the Composer requirements (through composer require pragmarx/google2fa bacon/bacon-qr-code, and also adding samyoul/u2f-php-server if you need FIDO U2F) . That tells Composer to add those as required dependencies, then it installs them for you. That should fix your problem.

@pwallner

This comment has been minimized.

Show comment
Hide comment
@pwallner

pwallner Nov 23, 2017

Thx for help.

2 Problems:

  1. If I activate Google auth, everything goes well during configuration, but when I logout and login, no Code request appears and I can login without google auth?!
  2. If I go to "Settings" with Chrome, I cannot select anything, because there is only one line which shows all options but without ratio buttons and without Button to send the request. If I hack the template <option name="2fa_configure"> then everything is shown, but I cannot use U2F, because if I connect the USB U2F device, the device isn't recognized (LED isn't flashing). On other sites (e.g. gmail) the usb U2F device is working...

pwallner commented Nov 23, 2017

Thx for help.

2 Problems:

  1. If I activate Google auth, everything goes well during configuration, but when I logout and login, no Code request appears and I can login without google auth?!
  2. If I go to "Settings" with Chrome, I cannot select anything, because there is only one line which shows all options but without ratio buttons and without Button to send the request. If I hack the template <option name="2fa_configure"> then everything is shown, but I cannot use U2F, because if I connect the USB U2F device, the device isn't recognized (LED isn't flashing). On other sites (e.g. gmail) the usb U2F device is working...
@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Nov 23, 2017

Member

Apparently this needs more testing in other browsers. Anyway can you please report these issues separately (one problem per issue) instead of using merged pull request for this? This would be way easier to track...

1 sounds strange, let's deal with it in #13828.

2 is fixed in 602d07d

Member

nijel commented Nov 23, 2017

Apparently this needs more testing in other browsers. Anyway can you please report these issues separately (one problem per issue) instead of using merged pull request for this? This would be way easier to track...

1 sounds strange, let's deal with it in #13828.

2 is fixed in 602d07d

nijel added a commit that referenced this pull request Nov 23, 2017

Remove things not needed for u2f
Issue #13787

Signed-off-by: Michal Čihař <michal@cihar.com>

nijel added a commit that referenced this pull request Nov 23, 2017

Fix markup in two factor auth configuration
Issue #13787

Signed-off-by: Michal Čihař <michal@cihar.com>
@ibennetch

This comment has been minimized.

Show comment
Hide comment
@ibennetch

ibennetch Nov 23, 2017

Member

Also, we've updated the documentation about this at https://docs.phpmyadmin.net/en/latest/two_factor.html

Member

ibennetch commented Nov 23, 2017

Also, we've updated the documentation about this at https://docs.phpmyadmin.net/en/latest/two_factor.html

@pwallner

This comment has been minimized.

Show comment
Hide comment
@pwallner

pwallner Nov 25, 2017

1 see #13828 . It doesn't ask for google auth during login
2 is fixed, led flash and device is registered, but I can login with user and pass only, no need of U2F, see #13829

pwallner commented Nov 25, 2017

1 see #13828 . It doesn't ask for google auth during login
2 is fixed, led flash and device is registered, but I can login with user and pass only, no need of U2F, see #13829

@pwallner

This comment has been minimized.

Show comment
Hide comment
@pwallner

pwallner Jan 5, 2018

Back from vacation. Did a git pull and composer update and now my FIDO U2F and google auth is working. But now I have another question:

  1. Is it possible to activate both (U2F and google auth), because maybe I only have access to one of both
  2. Is it possible to implement backup codes if I loose my google auth or U2F? Otherwise I cannot login anymore

Thx

pwallner commented Jan 5, 2018

Back from vacation. Did a git pull and composer update and now my FIDO U2F and google auth is working. But now I have another question:

  1. Is it possible to activate both (U2F and google auth), because maybe I only have access to one of both
  2. Is it possible to implement backup codes if I loose my google auth or U2F? Otherwise I cannot login anymore

Thx

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