Custom Authentication Mechanisms for WebDAV and APIs #26742

joneug · 2016-11-29T20:59:50Z

Description

The authentication mechanisms for securing WebDAV and APIs were extended to allow 3rd-party apps to provide additional authentication backends. In the WebDAV interface the dispatching of the OCA\DAV\Connector\Sabre::authInit event was added. For securing the API the interface IAuthModule was added. Apps can register their implementation of this interface in the info.xml file.

Related Issue

#10400

Motivation and Context

This, for example, allows the implementation of Bearer Authentication with OAuth 2.0 (see OAuth 2.0 app).

How Has This Been Tested?

The OAuth2 app's PHPUnit tests have been run on Travis CI with the following configurations:

PHP versions: 5.6, 7
Core branches: master
Databases: PostgreSQL, MySQL, SQLite

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

mention-bot · 2016-11-29T20:59:52Z

@joneug, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975 to be a potential reviewer.

CLAassistant · 2016-11-29T20:59:53Z

All committers have signed the CLA.

DeepDiver1975 · 2016-11-29T23:30:15Z

@PhilippSchaffrath @Peter-Prochaska

DeepDiver1975 · 2016-12-14T10:37:53Z

apps/dav/lib/Connector/Sabre/AbstractBearer.php

+ * Most of the logic is handled, implementors just need to worry about
+ * the determineUsername method.
+ */
+abstract class AbstractBearer implements BackendInterface {


this class already exists in sabre dav - no need to add it here. THX

guruz · 2016-12-21T15:00:08Z

Linking #24995

joneug · 2017-01-24T13:43:09Z

@PhilippSchaffrath @DeepDiver1975 @butonic Please review the changes. Looking forward to your ideas.

joneug · 2017-01-24T17:23:11Z

I think the problem with the build failing in Jenkins is that the OAuth 2.0 app was added as a Submodule which requires additional commands for cloning. Would you like to use submodules or should I include the OAuth 2.0 app's files directly?

User Hooks are currently not triggered because of the missing password

joneug · 2017-02-14T19:41:21Z

The dependencies between the core and the oauth2 app are now fully eliminated.

We need some plugin mechanism to this

I added IAuthModule as suggested.

And classes implementing this interface can be loaded via info.xml - basically just like the TwoFactor plugins

I updated the oauth2 app according to this. The AuthModule class is registered in info.xml under auth-modules.

And within the UserSession instead if calling tryTokenLogin, tryBearerLogin, trySessionLogin ....
We just loop over all registered modules

I added the function tryAuthModuleLogin that implements this loop. I don't see a way of replacing tryTokenLogin with this loop as tokens are not implemented with an app. What do you think?

# Conflicts: # lib/private/legacy/api.php

joneug · 2017-03-02T16:32:54Z

I changed the title of the PR, because now OAuth 2.0 is fully capsulated in the app and this PR addresses the more general problem of adding 3rd-party authentication backends.

Furthermore, I added the function getUserPassword to the IAuthModule interface in order to be able to trigger the hooks. Where to get the password from should not concern this PR.

As a side note: in the OAuth 2.0 app an empty string is returned because we are not handling any passwords at all. Consequently, only master key encryption is working, but this restriction exists when using Shibboleth, too.

Lastly, I added loading of additional AuthBackends for the webdav interface.

joneug · 2017-03-02T16:37:47Z

Could you please review the changes?

Additionally, considering a backport to stable9.1 would be nice since the changes are relatively small and there is high interest form our side in using this mechanism for implementing OAuth 2.0 in ownCloud 9.1.

DeepDiver1975 · 2017-03-06T14:44:22Z

lib/private/User/Session.php

+	 */
+	public function tryAuthModuleLogin(IRequest $request) {
+		/** @var IAppManager $appManager */
+		$appManager = OC::$server->query('AppManager');


please inject appManager as parameter in the ctor

I tried to inject the AppManager, but when changing the registration of UserSession in Server.php (see here) I get the following error:

PHP Fatal error: Maximum function nesting level of '256' reached

Inside the closure for the registerService function, $appManager = $c->getAppManager(); is called over and over again. Any ideas?

okay then let's keep it this way

DeepDiver1975 · 2017-03-06T14:45:18Z

lib/private/User/Session.php

+						}
+
+						/** @var IAuthModule $authModule */
+						$authModule = OC::$server->query($class);


adding a type check?

if ($authModule instanceof IAuthModule) {

DeepDiver1975 · 2017-03-08T23:33:39Z

hmmm ... two tests are failing on oracle ....

23:15:18 There were 2 failures:
23:15:18 
23:15:18 1) Test\Files\External\Service\GlobalStoragesServiceTest::testUpdateStorage with data set #1 (array('mountpoint', 'identifier:\Test\Files\Extern...ackend', 'identifier:\Auth\Mechanism', array('value1', 'value2', 'testPassword'), array('user1', 'user2'), array(), 15))
23:15:18 Failed asserting that two arrays are equal.
23:15:18 --- Expected
23:15:18 +++ Actual
23:15:18 @@ @@
23:15:18  Array (
23:15:18 -    0 => 'user1'
23:15:18 -    1 => 'user2'
23:15:18 +    0 => 'user2'
23:15:18 +    1 => 'user1'
23:15:18  )
23:15:18 
23:15:18 /var/lib/jenkins/workspace/owncloud-core_core_PR-26742-JCGXUYPGPX2CBJQVJBQBPP3QLMW2OPW6JKKJVSVR77ZQ5M6LTFKQ/tests/lib/Files/External/Service/GlobalStoragesServiceTest.php:185
23:15:18 
23:15:18 2) Test\Files\External\Service\GlobalStoragesServiceTest::testUpdateStorage with data set #3 (array('mountpoint', 'identifier:\Test\Files\Extern...ackend', 'identifier:\Auth\Mechanism', array('value1', 'value2', 'testPassword'), array('user1', 'user2'), array('group1', 'group2'), 15))
23:15:18 Failed asserting that two arrays are equal.
23:15:18 --- Expected
23:15:18 +++ Actual
23:15:18 @@ @@
23:15:18  Array (
23:15:18 -    0 => 'user1'
23:15:18 -    1 => 'user2'
23:15:18 +    0 => 'user2'
23:15:18 +    1 => 'user1'
23:15:18  )
23:15:18

joneug · 2017-03-09T06:34:17Z

I pulled the changes in master branch and now the Jenkins build was successful. But the travis build failed:

$ sh -c "if [ '$TEST_DAV' = '1' ]; then bash apps/dav/tests/travis/$TC/install.sh; fi"
bash: apps/dav/tests/travis/selenium/install.sh: No such file or directory

The command "sh -c "if [ '$TEST_DAV' = '1' ]; then bash apps/dav/tests/travis/$TC/install.sh; fi"" failed and exited with 127 during .

Are there errors in the build script (see changes of commit cac897b)?

DeepDiver1975 · 2017-03-09T08:02:46Z

I pulled the changes in master branch and now the Jenkins build was successful. But the travis build failed:

I'd say this is unrelated ..... let's merge

DeepDiver1975 · 2017-03-09T08:03:49Z

Thanks a lot! @joneug and your team! Great job!

joneug · 2017-03-09T08:09:33Z

What do you think about backporting to 9.1?

Additionally, considering a backport to stable9.1 would be nice since the changes are relatively small and there is high interest form our side in using this mechanism for implementing OAuth 2.0 in ownCloud 9.1.

DeepDiver1975 · 2017-03-09T08:10:42Z

What do you think about backporting to 9.1?

@PVince81 what's your opionion? THX

joneug · 2017-03-13T08:46:36Z

I created a backport in PR #27370.

lock · 2019-08-03T18:02:44Z

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.

joneug added 2 commits November 28, 2016 17:13

Add if-statement for switching auth backends

be3bcba

Implement determineUsername function

e23a570

DeepDiver1975 added this to the 9.2 milestone Nov 29, 2016

DeepDiver1975 added 3 - To Review 2 - Developing and removed 3 - To Review labels Nov 29, 2016

Update PHPDoc

45983f1

DeepDiver1975 mentioned this pull request Dec 7, 2016

Browser-based client login #24794

Closed

Update code for changes in OAuth 2.0 app

76a37fa

DeepDiver1975 reviewed Dec 14, 2016

View reviewed changes

Replace own AbstractBearer implementation with that of sabre/dav

14cabab

joneug added 5 commits December 23, 2016 10:33

Add OAuth 2.0 App as submodule

c1833bb

Update comment in webdav.php

43f12b0

Improve if-statement for the auth backend

38b2062

Update code for changes in OAuth 2.0 app

b9cd079

Add OAuth2Test

253c827

joneug changed the title ~~[WIP] OAuth 2.0 for WebDAV~~ OAuth 2.0 for WebDAV Jan 24, 2017

Update OAuth2Test

3d6dc7e

joneug added 6 commits January 29, 2017 15:51

Merge branch 'master' of https://github.com/owncloud/core into dav-oauth

ae46539

Add OAuth 2.0 login mechanism to Session.php for use in api.php

7f5fe29

Update loginWithOAuth2 function

190ebab

User Hooks are currently not triggered because of the missing password

Update OAuth 2.0 App

0962285

Improve tearDown function in OAuth2Test

58949f6

Refactor OAuth2Test

bfa9c04

joneug added 2 commits February 14, 2017 20:27

Update PHPDoc in IAuthModule

480173d

Update PHPDoc for auth function

574ce23

joneug added 3 commits February 23, 2017 20:22

Merge branch 'master' of https://github.com/owncloud/core into dav-oauth

213ccd7

# Conflicts: # lib/private/legacy/api.php

Add getUserPassword function in IAuthModule

c78e854

Add loading of additional AuthBackends for webdav interface

945f6bc

joneug changed the title ~~OAuth 2.0 for WebDAV and APIs~~ Custom Authentication Mechanisms for WebDAV and APIs Mar 2, 2017

DeepDiver1975 requested changes Mar 6, 2017

View reviewed changes

Add type check for IAuthModule

cde90f6

Merge branch 'master' of https://github.com/owncloud/core into dav-oauth

b378b45

DeepDiver1975 approved these changes Mar 9, 2017

View reviewed changes

DeepDiver1975 merged commit d35a58d into owncloud:master Mar 9, 2017

mmattel mentioned this pull request Mar 9, 2017

README.md update necessary owncloud/oauth2#25

Closed

joneug mentioned this pull request Mar 13, 2017

[stable9.1] Custom Authentication Mechanisms for WebDAV and APIs #27370

Merged

9 tasks

joneug mentioned this pull request Mar 13, 2017

Integration of ownCloud and Moodle can be found in other repositories owncloud-archive/moodle#1

Closed

This was referenced Mar 22, 2017

Authentication based in token (OAuth2) owncloud/ios-legacy#716

Closed

Authentication based in token (OAuth2) owncloud/android#1724

Closed

Implement token-based authentication with opening URL in external browser owncloud/client#4798

Closed

butonic mentioned this pull request Nov 19, 2018

<auth-modules> not documented in info.xml owncloud/docs#297

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom Authentication Mechanisms for WebDAV and APIs #26742

Custom Authentication Mechanisms for WebDAV and APIs #26742

joneug commented Nov 29, 2016 •

edited

Loading

mention-bot commented Nov 29, 2016

CLAassistant commented Nov 29, 2016 •

edited

Loading

DeepDiver1975 commented Nov 29, 2016

DeepDiver1975 Dec 14, 2016

guruz commented Dec 21, 2016

joneug commented Jan 24, 2017

joneug commented Jan 24, 2017

joneug commented Feb 14, 2017

joneug commented Mar 2, 2017 •

edited

Loading

joneug commented Mar 2, 2017 •

edited

Loading

DeepDiver1975 Mar 6, 2017

joneug Mar 7, 2017

joneug Mar 8, 2017

DeepDiver1975 Mar 8, 2017

DeepDiver1975 Mar 6, 2017

joneug Mar 7, 2017

DeepDiver1975 commented Mar 8, 2017

joneug commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

joneug commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

joneug commented Mar 13, 2017

lock bot commented Aug 3, 2019

Custom Authentication Mechanisms for WebDAV and APIs #26742

Custom Authentication Mechanisms for WebDAV and APIs #26742

Conversation

joneug commented Nov 29, 2016 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

mention-bot commented Nov 29, 2016

CLAassistant commented Nov 29, 2016 • edited Loading

DeepDiver1975 commented Nov 29, 2016

DeepDiver1975 Dec 14, 2016

Choose a reason for hiding this comment

guruz commented Dec 21, 2016

joneug commented Jan 24, 2017

joneug commented Jan 24, 2017

joneug commented Feb 14, 2017

joneug commented Mar 2, 2017 • edited Loading

joneug commented Mar 2, 2017 • edited Loading

DeepDiver1975 Mar 6, 2017

Choose a reason for hiding this comment

joneug Mar 7, 2017

Choose a reason for hiding this comment

joneug Mar 8, 2017

Choose a reason for hiding this comment

DeepDiver1975 Mar 8, 2017

Choose a reason for hiding this comment

DeepDiver1975 Mar 6, 2017

Choose a reason for hiding this comment

joneug Mar 7, 2017

Choose a reason for hiding this comment

DeepDiver1975 commented Mar 8, 2017

joneug commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

joneug commented Mar 9, 2017

DeepDiver1975 commented Mar 9, 2017

joneug commented Mar 13, 2017

lock bot commented Aug 3, 2019

joneug commented Nov 29, 2016 •

edited

Loading

CLAassistant commented Nov 29, 2016 •

edited

Loading

joneug commented Mar 2, 2017 •

edited

Loading

joneug commented Mar 2, 2017 •

edited

Loading