HTTP BASIC AUTH + CSRF protection => failed XHR requests

[stephane-martin]

Hi, i just spent my night tracking down this one...
Owncloud : 5.0.10
HTTP Basic authentication (with Apache 2.4, and LDAP as backend)
Browser : not important
OS : not important

Symptoms : when i try to upload file with web interface in "File app", or when i try to submit a new bookmark in "bookmark app", action fails with this message : "session has expired, reload the page".

I figured out that a new session is regenerated each time i request a page in owncloud, with a different session id. Thus requesttoken is not maintained through different requests.

After a few hours of digging, i figured that OC::tryBasicAuthLogin calls OC_User::isLoggedIn. And OC_User calls session_regenerate_id...

This behaviour is OK for usual form login : we want session to be reinitialized after form login. But NOT with HTTP basic auth !

Fix is quite trivial when you know the problem: just add two lines to tryBasicAuthLogin in lib/base.php

protected static function tryBasicAuthLogin() {
if (!isset($_SERVER["PHP_AUTH_USER"])
|| !isset($_SERVER["PHP_AUTH_PW"])
) {
return false;
}
OC_App::loadApps(array('authentication'));
if (!OC_User::isLoggedIn()) { <----- HERE !!!!
if (OC_User::login($_SERVER["PHP_AUTH_USER"], $_SERVER["PHP_AUTH_PW"])) {
//OC_Log::write('core',"Logged in with HTTP Authentication", OC_Log::DEBUG);
OC_User::unsetMagicInCookie();
$_SERVER['HTTP_REQUESTTOKEN'] = OC_Util::callRegister();
}
}
return true;
}

Regards,
Stephane

[Niduroki]

@LukasReschke @DeepDiver1975 @tanghus

[DeepDiver1975]

Thanks a lot for digging into this. 
Can I ask you to submit a pull request? 

Thx

[stephane-martin]

Im not really familiar with git. How should i do that ? First clone
owncloud/core on github, then commit to my clone, then make pull request to
official repository ?

Stephane

On Mon, Aug 26, 2013 at 8:08 AM, Thomas Müller notifications@github.comwrote:

Thanks a lot for digging into this.
Can I ask you to submit a pull request?

Thx

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4574#issuecomment-23245073
.

[stephane-martin]

Did it.

Thanks,
Stephane

On Mon, Aug 26, 2013 at 3:13 PM, stephane martin stef.martin@gmail.comwrote:

Im not really familiar with git. How should i do that ? First clone
owncloud/core on github, then commit to my clone, then make pull request to
official repository ?

Stephane

On Mon, Aug 26, 2013 at 8:08 AM, Thomas Müller notifications@github.comwrote:

Thanks a lot for digging into this.
Can I ask you to submit a pull request?

Thx

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4574#issuecomment-23245073
.

[karlitschek]

Thanks. Could you send us a contributor agreement or make you change available under the MIT license? https://owncloud.org/about/contributor-agreement/

[stephane-martin]

Just to be sure, a noob question : do i have to do something else so that my patch is included in owncloud ? Thanks!

[DeepDiver1975]

@houbaastef I just reverted this pull request - you require two 👍 from two different reviewers. http://owncloud.org/code-reviews-on-github/

I'm sorry for this move but we all have to follow these rules.

Please don't take any offense from this action and I kindly ask you to resubmit your pull request - THX

[PVince81]

Any update on this ? Does it still happen in 6.0.2 ?

@houbaastef can you resubmit your PR as requested ?
Thanks!

[PVince81]

@josh4trunks does this relate in any way to #7852 ?

[tanghus]

Look s like this one, #7852 and #8021 are trying to accomplish the same goal using different means.
@Kondou-ger @josh4trunks @DeepDiver1975

[josh4trunks]

1. Yup, both of our commits don't overwrite cookie login with basic auth if cookie auth already exists.
2. Mine also allows login when wrong basic auth credential are on the browser (by removing the the check that is no longer needed after the change 1)
3. Mine also allows logout when correct basic auth credentials are on the browser by adding a cookie to ignore the current basic auth user.

I would test #8021 but I have a feeling it wont work in case 2 and 3 above.

[stephane-martin]

Exact.

Regards,
Stephane
Le 3 avr. 2014 16:10, "josh4trunks" notifications@github.com a écrit :

1. Yup, both of our commits don't overwrite cookie login with basic
   auth if cookie auth already exists.
2. Mine also allows login when wrong basic auth credential are on the
   browser (by removing the the check that is no longer needed after the
   change 1)
3. Mine also allows logout when correct basic auth credentials are on
   the browser by adding a cookie to ignore the current basic auth user.

I would test #8021 #8021 but I
have a feeling it wont work in case 2 and 3 above.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4574#issuecomment-39455193
.

[josh4trunks]

I believe this can be closed, #7852 addressed this issue, along with a few other cases.

[PVince81]

Ok, closing.

@houbaastef if you think that something has been missed, feel free to reopen.