feat(auth): add support for MS-PASS

[Gamer92000]

MS-PASS is the proprietary authentication protocol used for e.g. OneDrive.
Because of how this protocol works I could not follow the design principles of the other auth methods 100% but tried to stay as close as possible to them.

fixes #68

[chripo]

Thank you for the PR.
I dislike the auth related changes in requests.go.
We should refactor the auth handling at all.
I need some time to think about.

[Gamer92000]

I dislike the auth related changes in requests.go.

I have to agree, the resulting code is not as straight forward anymore. I think it can be made nicer by adding some constants to the other auth types, like RecreateOnFailure to instantiate a new auth object whenever the previous authentication failed / timed out. But then it would also need some guard to prevention infinite recursions...

For the deadlock I just need to switch those lines, but you're right.

Regarding the failed test, if I'm not mistaken there is no way to do this without a race condition with this shared http.Client. Is there a reason for this shared client? If so there would need to be one giant mutex around the block changing the redirect handling, executing the request and reverting the redirect handling.

[chripo]

... I think it can be made nicer by adding some constants to the other auth types, like RecreateOnFailure to instantiate a new auth object whenever the previous authentication failed / timed out. But then it would also need some guard to prevention infinite recursions...

This may be one way to deal with this design problem. First of all, all the auth code needs to be moved/handled outside of the requests.go file. Some thoughts filed in issue #15.

For the deadlock I just need to switch those lines, but you're right.

Yes.

Regarding the failed test, if I'm not mistaken there is no way to do this without a race condition with this shared http.Client. Is there a reason for this shared client? If so there would need to be one giant mutex around the block changing the redirect handling, executing the request and reverting the redirect handling.

The original usage is to spawn clients inside go routines to avoid all mutex/concurrency issues.
Therefore, the current implementation of auth limits the usage.
I would like to avoid pulling more mutexes inside the library.

[Gamer92000]

I just had an idea. What would you think of two shared http clients? One with normal redirection and one without. This would mitigate the race condition and prevent the need to create a new client for every request.

[chripo]

I doubt it would solve the problems. It's not about redirection. The mutexes would remain because the concurrent subrequests are still there.

I'm going to see how I can rip off of auth.

[Gamer92000]

Well, all mutexes that are currently in the code would be untouched. This is purely to fix the race condition in the tests. It's caused by the client being modified to handle redirects differently at different times. So there would no longer be a race condition by having two clients and not changing one.

[chripo]

Sure, it would fix the tests, but it would also leave the code in a more complex state / mess.
Wouldn't it be?

[Gamer92000]

Please take a look at my changes.
I think even though there are more changes now in total, it still seems better than before.

[chripo]

I've reviewed your update.
I think this is not the way we should ship this feature.
I've started to refactor it and it's coming along nicely.
Could you please adopt your work to the next branch?
Please give me some feedback on what we might need to change in order to make it work.

[Gamer92000]

https://github.com/studio-b12/gowebdav/blob/next/requests.go#L42-L46

This will not work for passportAuth. Unfortunately, Microsoft is incapable of returning the correct error codes.
Instead of returning a 401, OneDrive will return a 302 with the required headers. The default HTTP client follows the redirect (which also includes the required variables). But the redirect leads to the login page which in turn returns a 200.

The www-authenticate header is only in the 302 message. Thus there is no way to reliably detect the necessity for passportAuth without using an HTTP client that catches redirects.

[chripo]

we will!
take a look inside the nullAuth. this runs first and only one time.
the Authorize method gets the client too. so we can fire up and handle a request prior the orign.
everything should be in place.

[Gamer92000]

I don't really see how this will help.
The main problem is, that in this line
https://github.com/studio-b12/gowebdav/blob/next/requests.go#L41
You will skip the WWW-Authentication Header of the Passport1.4 protocol. Thus I don't see a way for your factory to correctly identify the Passport Auth as it has no access to the headers of a response that was a redirect as the client followed the redirect.

[chripo]

if I´d not get it wrong. we could check for a redirect or compare the request and response with the path in Veriify method of nullAuth then do the partner server auth to obtain the credentials.
we could move the code into the Authorize method and make a kind of preflight request.
everything should happen in the nullAuth which is going to be replaced by a real new MS-Pass Authenticator, which we need to tigger somehow, later...

[Gamer92000]

That sounds like a possibility, but I'm not sure it's a good solution.
I'm not that familiar with the webdav specs, but can't there be a reason for a path-changing redirect ( while authenticated )?
I think the cleanest way would be to catch all redirects (at least when nullAuth is set) and only follow the redirect if no www-authenticate header is found.
But this could absolutely be done by some kind of preflight in the nullAuth's Authorize function with a custom http.Client.

[chripo]

Well, even if the specs would prohibit the redirection for authentication, the cancer thing is already doing that.
If we get redirected in nullAuth then we should send another authentication request to verify and handle the authentication anyway.
Because we keep the body, it is not necessary to spawn a new client.
The response offers the Location to be matched against the path.
Try it and see if it works.

[Gamer92000]

Please tell me that I am stupid. We have neither the www-authenticate header nor the Location header when using an unmodified http.Client.

[chripo]

Ashes on my head.
The Location stays nil it's useless.
I'll add some black magic!

[chripo]

Thanks for your patience.
Added some redirection control e3d49a5

[Gamer92000]

I adapted my changes to the new next branch.
It generally works, but I'm not sure about my Verify function. I would greatly appreciate any feedback you can give me.

[chripo]

Try doing the auth ping-pong in the Verify method if the initial auth fails.
Maybe the reauth naming is misleading you, we should it name redo.

[chripo]

we're on a good way!

[chripo]

I have tried to take it precisely.
well done!
I'm looking forward to get this merged.
Thank you very much!!

[Gamer92000]

I just noticed something. The p.inhibitRedirect approach will not work when the authenticator is used concurrently.
So I think the easiest cleanest solution would be to simply go back to the Verify function resolving the redirect.

[chripo]

we are almost there! 🚀

[chripo]

awesome!
this is a very nice implementation.
Well done.
I'll squash next, tomorrow.
Thank you for your patience!

[chripo]

Could you please squash your commits together and put them on top of next?