Basic Auth is broken - again...

[tanghus]

This is most likely not the 'proper' way to do it, but it works.

Please test Basic Auth whenever refactoring the code. I don't know how many time is has been broken.

@icewind1991 @DeepDiver1975 @karlitschek

[karlitschek]

is it intended that you commented out redirectToDefaultPage ?

[tanghus]

is it intended that you commented out redirectToDefaultPage ?

Yes, it went into a never ending loop otherwise. When commented out it reaches the destination URL.

[tanghus]

Actually since basic auth is mainly (only?) used for non-browser access, maybe we should call OC_Util::callRegister() in tryBasicAuthLogin(), as it only called in the template otherwise.
I don't know if it would have any security implications, @LukasReschke ?

[bantu]

The ticket description indicates there should be functional tests for HTTP auth now. ;)

[tanghus]

It's basically untestable, and the login logic is - absent... I' trying to make heads and tails out of it.

[tanghus]

Nope, without a total redesign this is the only way I can get basic auth to work, without breaking normal login, cookie validation and routing.

Cookie validation is already broken in master - #854 really needs to be fixed.

Anyways, until a redesign of the startup procedure, please consider this patch.

@karlitschek @icewind1991 @LukasReschke @DeepDiver1975

[MTGap]

3rdparty got changed in the last commit.

Should we organize a review of base.php to clean up the login stuff? I don't want to do it alone.

[tanghus]

3rdparty got changed in the last commit.

Thanks, hadn't noticed.

Should we organize a review of base.php to clean up the login stuff?

Maybe we should create an issue summarizing all the issues in OC::handleRequest(); and separate them into more manageable parts? Find out what pitfalls there are etc.

I don't want to do it alone.

No one should have that burden alone ;)

[tanghus]

But if we are going to rewrite anyway, I think we should consider a design like SabreDAV, instantiating a server object and inject plugins for dealing with authentication, handling requests etc.

[icewind1991]

But if we are going to rewrite anyway, I think we should consider a design like SabreDAV, instantiating a server object and inject plugins for dealing with authentication, handling requests etc.

👍

[DeepDiver1975]

@tanghus should the comment be moved as well? Just asking

[tanghus]

Yes it probably should ;) This hack was mostly a PoC what would work with the current base.php - a rewrite would be better but might not be feasible before the next major release? If not, tryBasicAuth() should also call OC_Util::callRegister(); if the request should be used for anything else than simple GETs.
Should I update this PR for something that can be work, or should we prioritize a rewrite like @icewind1991 described in https://gist.github.com/icewind1991/5685072 for oC 6?

[DeepDiver1975]

OC5 and master are broken - right? -> fix it
The rewrite is too far away from my understanding.

[tanghus]

@DeepDiver1975 Yes, oC5 is broken too. Pushed changes for master.

[tanghus]

Any 👎 or 👍 for this temporary fix? A full rewrite is only in the far horizon.

[icewind1991]

👍

[tanghus]

@MTGap @karlitschek @DeepDiver1975 what say you?

[DeepDiver1975]

👍

[DeepDiver1975]

@tanghus oc5 is still broken - right? Can you please backport this? THX

[tanghus]

oc5 is still broken - right? Can you please backport this? THX

@DeepDiver1975 I deleted 2500 git msgs when I got back from vacation, so never saw this :)
Yes, stable5 seems to be broken as well. I'll see if I can backport this.