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

do not allow client password logins if token auth is enforced or 2FA … #24811

Merged
merged 2 commits into from
May 25, 2016

Conversation

ChristophWurst
Copy link
Contributor

@ChristophWurst ChristophWurst commented May 24, 2016

…is enabled

fixes #24779
fixes #24768

TODO

  • Check if instance enforces token auth (default for new installations)
  • Check if 2FA is enforced for the user
  • Add config to config.sample.php

Tested with DAV and OCS, works as expected.

*/
public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider) {
public function __construct(IUserManager $manager, ISession $session, ITimeFactory $timeFacory, $tokenProvider, IConfig $config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

$tokenProvider has no type hint, intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, because null in injected during setup

@ChristophWurst
Copy link
Contributor Author

@PVince81 @nickvergessen @schiesbn @rullzer @LukasReschke test & review please :-)

Tested also with LDAP, seems to work 🙊

@ChristophWurst ChristophWurst force-pushed the client-login-token-2fa branch from 1fd8191 to 4939f35 Compare May 24, 2016 15:53
@@ -362,6 +362,9 @@ public function install($options) {
$config->setAppValue('core', 'installedat', microtime(true));
$config->setAppValue('core', 'lastupdatedat', microtime(true));

// Disable strict token auth by default
$config->setSystemValue('token_auth_enforced', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove that, let it rely on the default value being false anyway

@ChristophWurst ChristophWurst force-pushed the client-login-token-2fa branch from 4939f35 to 91f4708 Compare May 24, 2016 15:56
@PVince81
Copy link
Contributor

Looks good 👍

@PVince81
Copy link
Contributor

Second review ? @rullzer @nickvergessen

@ChristophWurst ChristophWurst force-pushed the client-login-token-2fa branch from 91f4708 to a922957 Compare May 24, 2016 16:02
@rullzer
Copy link
Contributor

rullzer commented May 24, 2016

Awesome!
Tested and works over here.
Code looks good.

👍

@ChristophWurst
Copy link
Contributor Author

Failing test looks unrelated

16:27:55 There was 1 failure:
16:27:55 
16:27:55 1) OCA\Files_Sharing\Controllers\ShareControllerTest::testShowShare
16:27:55 Failed asserting that two objects are equal.
16:27:55 --- Expected
16:27:55 +++ Actual
16:27:55 @@ @@
16:27:55          'previewEnabled' => true
16:27:55 +        'previewMaxX' => null
16:27:55 +        'previewMaxY' => null
16:27:55      )
16:27:55      'renderAs' => 'base'
16:27:55      'appName' => 'files_sharing'
16:27:55      'headers' => Array (...)
16:27:55      'cookies' => Array ()
16:27:55      'status' => 200
16:27:55      'lastModified' => null
16:27:55      'ETag' => null
16:27:55      'contentSecurityPolicy' => OCP\AppFramework\Http\ContentSecurityPolicy Object (...)
16:27:55  )

@PVince81
Copy link
Contributor

@ChristophWurst likely due to this PR: #24346

Will rerun the tests on master and make tickets so we can fix these today.

@PVince81 PVince81 merged commit 5f1ddf1 into master May 25, 2016
@PVince81 PVince81 deleted the client-login-token-2fa branch May 25, 2016 07:07
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants