cloudant / CDTDatastore

Cloudant Sync iOS datastore library.
Apache License 2.0
175 stars 53 forks source link

Documentation for adding credentials #302

Closed pokstad closed 7 years ago

pokstad commented 8 years ago

The documentation states that passwords should be encoded in NSURLs:

// username/password can be Cloudant API keys
NSString *s = @"https://username:password@username.cloudant.com/my_database";

This is not possible with complex passwords that contain special delimiter tokens, such as the @ (at) symbol which is used to end the password or : (colon) which is used to end the username or / (backslash) which is used to end the protocol and start the username. NSURL does not have any methods to set the username and password with appropriate encoding to avoid this issue. Ultimately, these credentials will be added as basic authentication header in base64 encoding. Instead of adding them to the NSURL, they should be added directly to the headers to avoid parsing issues. This can be done with the HTTP interceptors:

func basicAuthHeaderInBase64() -> String? {
    if let username = fetchUsername(),
       let password = fetchPassword() {
        let userPasswordString = "\(username):\(password)"
        let userPasswordData = userPasswordString.dataUsingEncoding(NSUTF8StringEncoding)
        let base64EncodedCredential = userPasswordData!.base64EncodedStringWithOptions(
            .Encoding64CharacterLineLength)
        return "Basic \(base64EncodedCredential)"
    }
    return nil
}

func interceptRequestInContext(context: CDTHTTPInterceptorContext) -> CDTHTTPInterceptorContext {
    if let authString = basicAuthHeaderInBase64() {
        context.request.setValue(
            authString,
            forHTTPHeaderField: "Authorization")
    } else {
        // anonymous access requires us to remove authorization header
        context.request.setValue(nil, forHTTPHeaderField: "Authorization")
    }
    return context
}

Ideally, the NSURLSessionConfiguration object should be customized via delegate method to add basic auth:

var credential: NSURLCredential {
       return cred = NSURLCredential(
                user: self.username,
                password: self.password,
                persistence: .ForSession)
}    

var protectionSpace: NSURLProtectionSpace {
    return NSURLProtectionSpace(
            host: self.url.host,
            port: self.port,
            protocol: self.url.scheme,
            realm: nil,
            authenticationMethod: NSURLAuthenticationMethodHTTPBasic)
}

@objc func customiseNSURLSessionConfiguration(config: NSURLSessionConfiguration) {
    if let c = credential, p = protectionSpace {
        config.URLCredentialStorage?.setCredential(c, forProtectionSpace: p)
    }
}

While testing this method I found that two different replications from two different users initiated in the same protection space will not run as expected. This is probably because the URLCredentialStorage object is a shared object between all URLSessions (not verified). Each time a replication is run, it will pick a default credential from the protection space and NOT the last credential set. In order to avoid authentication errors, a work around is to remove ALL credentials for a protection space before adding the credentials desired (relevant stack overflow question). However, this ends up modifying the keychain which is not ideal if you are depending on the keychain to store your user credentials.

Another solution is to set the default credential for a protection space. However, this will not work if you want to be able to use anonymous credentials.

A more ideal solution is to use the NSURLSession delegate to handle authentication challenges. This way, based on your app's specific logic you can choose which credential to retrieve from the credential store and use that instead. Currently, I cannot find anywhere in the library that exposes the NSURLSession delegate property for this customization.

rhyshort commented 8 years ago

It sounds like we have a couple of issues here. The way we accept credentials into the replicator isn't perfect, and since we strip them from the URL to use with _session endpoint with CouchDB, we should probably expose this as a username and password properties, with the URL stripping as a way of maintaining compatibility.

I don't think exposing the delegate challenge is the right thing to do, since hiding it away means we can switch out the implementation without a breaking change in the event it is needed, we should do some thinking on it and see what we come up with.

I think there are a few things need to here:

tomblench commented 8 years ago

Firstly, : can't be used in usernames for basic auth because of the way it's encoded over the network:

userid      = *<TEXT excluding ":">

(https://www.ietf.org/rfc/rfc2617.txt)

But of course you should be able to use any other characters and tell our library that you want to use these.

Maybe we should do what some of our other libraries do and allow for username/password in URL or as extra configuration, with the latter over-riding the former. I still think it's useful to have these in the URL but I guess we have stored up trouble for ourselves by pretending we can transparently 'upgrade' the user from basic auth to cookie auth.

As a case in point, I can access local couch with username and password both of @@@ using curl http://%40%40%40:%40%40%40@localhost:5984/_all_dbs. But this doesn't seem to work in our library, presumably because cookie auth has no idea about percent encoding.