Improves the authentication flow

[chripo]

The main goal was to simplify the req method in request.go and making it easier to add more authentication methods.

All the magic went into the auth.go file.

This feature introduces an Authorizer which acts as an Authenticator factory. Under the hood it creates an authenticator shim per request, which delegates the authentication flow to our authenticators.

The authentication flow itself is broken down into Authorize' and Verify' steps to encapsulate and control complex authentication challenges.

Furthermore, the default NewAutoAuth authenticator can be overridden by a custom implementation for more control over flow and resources. The NewBacisAuth Authorizer gives us the feel of the old days.

[chripo]

Thanks @ueffel! Any thoughts on the basic strategy of the game? Is it clear or easier to gasp?

[ueffel]

The Authorizer and Authenticator look easy enough to implement for a third party. Maybe @Gamer92000 can checkout if this new API simplifies #70. Overall it looks good to me.

[ueffel]

It may be interesting to try implementing Kerberos authentication without wrapping the http.Transport (which is need with github.com/dpotapov/go-spnego) I think I will try that to check how it goes.

[ueffel]

I think there should be a way to create an Authorizer without the Basic and Digest methods, so one can just add (AddAuthenticator) the Authenticators, that are wanted.

func NewEmptyAuth() Authorizer {
    fmap := make(map[string]AuthFactory)
    az := &authorizer{fmap, sync.Mutex{}, &nullAuth{}}
    return az
}

And maybe one Authorizer that does authenticates preemptively:

func NewPreemptiveAuth(auther Authenticator) Authorizer {
    fmap := make(map[string]AuthFactory)
    az := &authorizer{fmap, sync.Mutex{}, auther}
    return az
}

Minor inconvience newPathError and newPathErrorErr could be public, so they can be reused.

Sample kerberos authentication

```go package main import ( "net/http" "os" "github.com/dpotapov/go-spnego" "github.com/studio-b12/gowebdav" ) type krb5Auth struct { sspi spnego.Provider NoCanonicalize bool } func NewKrb5Auth() *krb5Auth { return &krb5Auth{sspi: spnego.New()} } func (k *krb5Auth) Authorize(c *http.Client, rq *http.Request, method string, path string) error { return k.sspi.SetSPNEGOHeader(rq, !k.NoCanonicalize) } func (k *krb5Auth) Verify(rq *http.Request, rs *http.Response, method string, path string) (bool, error) { if rs.StatusCode == http.StatusUnauthorized { return false, &os.PathError{ Op: "Authorize", Path: path, Err: gowebdav.StatusError{Status: rs.StatusCode}, } } return false, nil } func (k *krb5Auth) Clone() gowebdav.Authenticator { auther := NewKrb5Auth() auther.NoCanonicalize = k.NoCanonicalize return auther } func (k *krb5Auth) Close() error { return nil } var _ gowebdav.Authenticator = (*krb5Auth)(nil) ``` Usage like this ```go authorizer := gowebdav.NewEmptyAuth() authorizer.AddAuthenticator("negotiate", func(rq *http.Request, rs *http.Response, method, path string) (auth gowebdav.Authenticator, err error) { return NewKrb5Auth(), nil }) ``` or ```go authorizer := gowebdav.NewPreemptiveAuth(NewKrb5Auth()) ```

[chripo]

awesome! for kerberos only auth it could shipped with a authorizer like the BasicAuth does.

func NewBasicAuth(login, secret string) Authorizer {
    return &basicAuthAuthorizer{&BasicAuth{login, secret}}
}

[ueffel]

awesome! for kerberos only auth it could shipped with a authorizer like the BasicAuth does.

func NewBasicAuth(login, secret string) Authorizer {
    return &basicAuthAuthorizer{&BasicAuth{login, secret}}
}

Yeah, but this requires far more code :innocent:.

Also I thought about what would happen, if a server offers multiple ways to authenticate (multiple Www-Authenticate headers) and the first one (what rs.Header.Get() returns) is not the one that should be taken by the client. I think right now this wouldn't work.

(Edit: wouldn't work)

[chripo]

Yeah, but this requires far more code innocent.

I LIKE U

Also I thought about what would happen, if a server offers multiple ways to authenticate (multiple Www-Authenticate headers) .... we ship this since 2014, so it works(tm) but it's a good thought. we need to sort the Authenticators and pull out all headers. PR welcome.

[chripo]

Minor inconvience newPathError and newPathErrorErr could be public, so they can be reused.

we will take care about that too.

[ueffel]

@chripo Have a look at the "next-plus" branch I tried myself at supporting multiple Www-Authenticate header but i'm not happy with the solution. Too many new struct members for the authorizer. Maybe you have an idea to make it better?

[chripo]

@ueffel indeed that a lot of changes. take a look at 9da199c this should be enough to favor the order if the server supports multiple authentication methods.

[ueffel]

take a look at 9da199c this should be enough to favor the order if the server supports multiple authentication methods.

but this still only looks at the first authentication header header := strings.ToLower(rs.Header.Get("Www-Authenticate")) instead of all of them headers := rs.Header.Values("Www-Authenticate")

[chripo]

In fact, I was looking up in the wrong docs, somehow... This needs some more work, I've got an idea.

[chripo]

Well, I'm done so far. @ueffel What do you think about the last changes?

What needs to be done to get merged:

• [x] add some auth flow / API documentation and examples of how to enroll a custom authentication (Readme)
• [ ] clarify how to deal with 3rd party authentication that pulls in dependencies
• [x] more reviews
• [x] #70 may show up with more changes, hopefully not

[chripo]

thank you again @ueffel! Absolut Fett! your tests made it awesome. I'll pull them off at the final squash / end, that you can apply them by our own to preserve ownership!!

[chripo]

@zekroTJA Do you have time for a feature review?

[zekroTJA]

@zekroTJA Do you have time for a feature review?

Yeah, I would take a look into it in the next couple of days if that fits. I am not that deep into the WebDav protocol itself but I'll take a look into it. :)

[Gamer92000]

Hey, sorry to bother you. Could you get this resolved and merged? This is needed for #70 which in turn would benefit a lot of projects downstream (e.g. rmfakecloud).