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

OC 8 + HTTP Basic Auth + File upload = "Token expired, please reload page" Re-Appeared!! #14469

Closed
t0mcat1337 opened this issue Feb 24, 2015 · 25 comments · Fixed by #14471
Closed
Assignees
Labels
Milestone

Comments

@t0mcat1337
Copy link

Hi everybody,

just updated to the last stable OC 8 and this Bug from OC 6.x / OC7.0.x reappeared:

1.) After logging in with Apache's Basic Auth the user is correctly logged in to OC
2.) Uploading a file fails, the progress bar completely gets filled but then this message appears:
"Token expired, please reload page"
and the file isn't there.

Exactly the same behaviour was described in

#4574
and
#7852

In OC6/7.0x the solution from #4574 always worked. But as the code structure has heavily changed I coultn't find the correct file / line to try, if this still would work.

Any hints?

Thx in advance!

@t0mcat1337 t0mcat1337 changed the title OC 8 + HTTP Basic Auth + "Token expired, please reload page" Re-Appeared!! OC 8 + HTTP Basic Auth + File upload = "Token expired, please reload page" Re-Appeared!! Feb 24, 2015
@LukasReschke
Copy link
Member

Where do you authenticate against? Which endpoint? If you take a look at the source to you see something like data-requesttoken and does it has a value?

@t0mcat1337
Copy link
Author

Just at this moment I found the line:

lib/base.php:

if (!self::checkUpgrade(false)
&& !$systemConfig->getValue('maintenance', false)
&& !\OCP\Util::needUpgrade()) {
// For logged-in users: Load everything
if(OC_User::isLoggedIn()) { <------------------ ??
OC_App::loadApps();
} else {
// For guests: Load only authentication, filesystem and logging
OC_App::loadApps(array('authentication'));
OC_App::loadApps(array('filesystem', 'logging'));
if (!OC_User::isLoggedIn()) { <------------------------------ HERE-------------------
\OC_User::tryBasicAuthLogin();
} <----------------------HERE ----------------------------------
}
}

When inserting the "HERE" lines, file uploading works... when deleting them again (as is in origin) the error appears....
But I don't really understand why this works as "OC_User::isLoggedIn" is already checked in the former outer if.... (the Line wirh "??")

Anyway... with this little hack, everything works again...

@t0mcat1337
Copy link
Author

@LukasReschke : Just now saw your answer... The HTTP Basic Auth is against LDAP, the same one, which is configured in OC8

@t0mcat1337
Copy link
Author

Yes, there is a "data-requesttoken" in the head-tag and it has a value... doesn't matter, if the lines are inserted to lib/base.php or not...

@LukasReschke
Copy link
Member

Against which endpoint are you authenticating? – How does the login request look like?

@t0mcat1337
Copy link
Author

what do u mean with "endpoint"?

@LukasReschke
Copy link
Member

How do you login? What are the exact steps that you follow to authenticate?

@t0mcat1337
Copy link
Author

1.) Enter the URL
2.) The HTTP Basic Auth window appears (as the Apache is configured to use HTTP Basic Auth for this URL)
3.) Enter my credentials as needed to authenticate against the LDAP Backend
4.) OC8 opens with the correct user

@LukasReschke
Copy link
Member

@blizzz When looking at the custom patch in #14469 (comment) it looks to me like LDAP is not yet initialized in that state and thus it does not work? - Would it make sense to initialize the authentication apps before somewhere?

@t0mcat1337
Copy link
Author

Found this out:
WITHOUT my changes in lib/base.php:
1.) Open OC8 -> requesttoken is, lets say "AAAAAAA"
2.) Change Directory by clicking on it -> requesttoken changes to, lets say "BBBBBBBB"

WITH my changes:
1.) Open OC8 -> requesttoken is, lets say "AAAAAAA"
2.) Change Directory by clicking on it -> requesttoken remains "AAAAAAA"

So it seems, WITHOUT my changes, the requesttoken is renewed at each and every request even it is in the same session. This explains, why the upload fails, as the AJAX request for the upload includes the first token (to stay with the first example: "AAAAAA", checked that in Firebug), whereas the new token "BBBBBB" is the valid one...

@blizzz
Copy link
Contributor

blizzz commented Feb 24, 2015

@LukasReschke OC_User::isLoggedIn() does not interact with user backends, only with the user session (which checks $_SESSION).

@LukasReschke
Copy link
Member

Sure. But it does also a query to \OC_User::userExists – let me try to get some LDAP setup again and verify this.

@blizzz
Copy link
Contributor

blizzz commented Feb 24, 2015

@LukasReschke oh, i have seen that spot. This explains why it works. Then yes, we'd need to load auth-apps earlier.

@LukasReschke
Copy link
Member

In earlier versions we loaded the apps already there:

OC_App::loadApps(array('authentication'));
sigh

@LukasReschke
Copy link
Member

Let me put it somewhere on top of base.php – I doubt that there are cases where it is uneeded?

@blizzz
Copy link
Contributor

blizzz commented Feb 24, 2015

Public (guest) pages?

@LukasReschke
Copy link
Member

I think even then it makes sense to load them in case user list has to get loaded somehow?

@blizzz
Copy link
Contributor

blizzz commented Feb 24, 2015

I doubt… But you never know. We'd be on the safe side.

@t0mcat1337
Copy link
Author

Did a quick test with @LukasReschke 's comment about this:

OC_App::loadApps(array('authentication'));

I added these "HERE" lines to lib/private/user.php:

    public static function isLoggedIn() {
            if (\OC::$server->getSession()->get('user_id') !== null && self::$incognitoMode === false) {
                    OC_App::loadApps(array('authentication')); <--------------------------- HERE
                    self::setupBackends(); <------------------------- HERE
                    return self::userExists(\OC::$server->getSession()->get('user_id'));
            }

            return false;
    }

as it obviously was in the earlier version.

Then I reverted my change to "lib/base.php" so it is the original one now

Witht that file uploading works, too

@blizzz
Copy link
Contributor

blizzz commented Feb 24, 2015

Thanks for veryfying @t0mcat1337

LukasReschke added a commit that referenced this issue Feb 24, 2015
The current code path may trigger situations where the LDAP application is not yet loaded and thus problems with the authentication appeared.

In previous versions of ownCloud the authentication mechanism manually loaded these apps which is why this affects ownCloud 8 and master only for my knowledge. (certainly not 6, maybe 7)

Backport to 8 might be something to consider.

Fixes #14469
@LukasReschke
Copy link
Member

I created a somewhat cleaner patch in #14471 - @t0mcat1337 could you test this and report back whether it works on the PR as well?

@t0mcat1337
Copy link
Author

And what was to expect... the requesttoken remains the same in all requests, too...

@t0mcat1337
Copy link
Author

@LukasReschke : As I have to leave work now I can test the patch during the next few hours... anyway thx a lot for this REALLY quick assistance ;)

@LukasReschke
Copy link
Member

Sure. Thanks for testing and reporting issues back and your investigation (especially that you tried some patches yourself, that is far more than the usual bug reporter does). Helped a lot to understand the issue 😄

If you stumble upon any other bug please don't hesitate to file an issue. (though this is authentication related and more urgent than some little tiny nitpick :))

@DeepDiver1975 DeepDiver1975 added this to the 8.0.1-next-maintenance milestone Feb 25, 2015
LukasReschke added a commit that referenced this issue Feb 25, 2015
The current code path may trigger situations where the LDAP application is not yet loaded and thus problems with the authentication appeared.

In previous versions of ownCloud the authentication mechanism manually loaded these apps which is why this affects ownCloud 8 and master only for my knowledge. (certainly not 6, maybe 7)

Backport to 8 might be something to consider.

Fixes #14469
@LukasReschke
Copy link
Member

Patch will be include within 8.0.1 - thanks for reporting issues back.

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

Successfully merging a pull request may close this issue.

4 participants