Hello
I'd like to add the ability to retry requests transparently on token authentication failures, and wanted to check to see if it was functionality that might be accepted before going off and writing it. This is not a request to add oauth support, but functionality that could be used to easily add oauth support. The public API I see (names are probably not great) would look like:
extension Request {
public typealias RetryCallback = NSURLRequest? -> Void
public typealias RetryCheck = (NSURLRequest?, NSHTTPURLResponse?, RetryCallback) -> Void
func check(check: RetryCheck) -> Self {
}
}
This would be consumed by the client like:
request.check() {HTTPRequest, HTTPResponse, callback in
if let HTTPResponse = HTTPResponse where HTTPResponse.statusCode == 401 {
self.refreshAuthenticationToken() { token in
let newRequest = HTTPRequest.mutableCopy()
newRequest?.allHTTPHeaderFields["Authorization"] = "Bearer \(token)"
callback(newRequest)
}
}
callback(nil)
}
// Normal response handler
request.responseObject(completion)
_Background:_
Transparent token refresh is almost always desired when interacting with OAuth. This forces every request to re-implement painful token refresh that has to recapture all of the inputs and perform the logic above. It would be awesome to easily add this refresh behavior to the existing chain.
_Implementation Approach_
I believe this would be relatively easy to implement. First the operation queue would be suspended. If nil is passed to the callback, the operation queue would be resumed, and the remaining response chain would run. If a request was passed to the callback, all of the state of the request object would be cleared out and the new request would over-write the old one. The request would then be ran again with a new token, and hopefully succeed.
I was hoping to be able to create a request copy and copy over all of the response chain, but the response chain is built by grabbing the request via operation block closures, so I don't think that approach can work.
Any thoughts?
Thanks!
Brian
So, I was looking through the implementation approach above, and it looks pretty bad. A request really doesn't want to retry, it's one request.
However, it may be easy to make the responder chain copyable. Right now it appears that the NSOperationQueue that represents the responder chain grabs the request state via closures. If that relation ship was stored as arrays of ResponderChain objects that conformed to a protocol like func respondWithRequest(request: Request), a retry could move the responder chain to a new request and function as though it was one request from the API consumer perspective.
It appears that the use of the NSOperationQueue as a responder chain happens in 4 places, so this should be a relatively small change. Good chance that there's another shortfall I'm un-aware of though!
Retrying on an authentication failure is an important feature of a project I'm currently working on, too. Would love to see this built into Alamofire, if it is simple, clean, and elegant.
For what it's worth, I found @cnoon's answer on StackOverflow to be very helpful: http://stackoverflow.com/questions/28733256/alamofire-how-to-handle-errors-globally
Hi @KingOfBrian,
Thanks for putting together your thoughts on this. It's certainly an issue that a vast majority of iOS developers face these days. After looking through your suggestions, I think your looking at a greatly simplified example of what's generally happening. When a token has expired, generally that means the user hasn't used your app in a while. When they open it up, you probably have 10 different requests you need to make to sync back up. Unfortunately ALL those requests are going to hit a 401, not just one of them, and they are all going to hit it at slightly different times.
If you don't consider all the requests that are racing towards a 401, you're going to be in a world of hurt. Most OAuth servers aren't going to allow you to have 10 different access tokens all at the same time. Generally, you can create a few, but creating new ones tends to invalidate old ones. Therefore, creating new ones for each request can end up invalidating other ones that are already in flight. You can get into some really bad scenarios here.
Rather than thinking about the problem as refreshing a token for a single request, you need to think about refreshing a token for a group of requests. Because you don't know which scenario you are going to hit and you definitely need to handle both. Therefore, I'd recommend reading my much more in-depth answer that @glennrfisher pointed out (thanks @glennrfisher).
With that said, I would LOVE to be able to move my OAuth2 wrapper library out into the OSS community. Unfortunately, it requires you to move away from method chaining and into higher level, more powerful, but less flexible APIs. I haven't created an OAuth2 library yet under the Alamofire org b/c it's going to be very tricky to get right and will take a significant amount of time to build. However, that doesn't mean I'm not going to. It's certainly on my roadmap to build, but I haven't had enough time to start putting it together yet. For now, I'd recommend reading through the Stack Overflow answer and trying to roll your own using the approach and sample code documented in that thread.
Best of luck! 🍻
Thanks for the response @cnoon. I totally agree with this not being a complete retry / oath implementation, but it's easy to add to this once the response chain can be copied. I'd like to be able to gather more input from the Alamofire team as to what the desired retry implementation would look like and what the requirements are. I have seen you mention that it's in the feature backlog.
I work with a lot of projects doing consulting that all do their own abstraction on top for authentication which is frustrating, since A) Different project, different API. B) The Alamofire API is so much more expressive. I think getting some retry support into Alamofire will help a lot more than you might picture!
Here's my OAuth implementation, which has some internal abstractions, but addresses your concerns. Let me re-write it without my internal abstractions to give a better picture of what it would look like.
func reauthenticateRequest(request: Alamofire.Request) -> Alamofire.Request {
guard let HTTPRequest = request.request else { fatalError("Invalid Request") }
// swiftlint:disable force_cast
let authorizedHTTPRequest = HTTPRequest.mutableCopy() as! NSMutableURLRequest
// swiftlint:enable force_cast
if var headers = authorizedHTTPRequest.allHTTPHeaderFields, let bearerToken = self.bearerToken {
// Add the bearer token and retry
headers[Constants.Authorizaton] = Constants.Bearer(bearerToken)
authorizedHTTPRequest.allHTTPHeaderFields = headers
let authorizedRequest = Alamofire.request(authorizedHTTPRequest)
authorizedRequest.copyResponseChainFromRequest(request)
return authorizedRequest
}
else {
// There is not enough information to authenticate, return the original request, which will 401.
return request
}
}
// APIEndpoint encapsulates headers, parameters, encoding, fetch type to help cluster like API's. It's purpose is similar to URLRequestConverable, without having to define a hostname (which always has to be static storage in the URLRequestConverablePattern)
func request(endpoint: APIEndpoint, authenticated: Bool = true) -> Alamofire.Request {
let headers: Dictionary<String, String>?
let request: Alamofire.Request
let needsAuthentication: Bool
var requestIsReady: Bool = true
if let bearerToken = bearerToken where authenticated == true {
headers = [Constants.Authorizaton : Constants.Bearer(bearerToken)]
needsAuthentication = false
}
else {
headers = nil
needsAuthentication = authenticated
}
request = manager.request(configuration.baseURLString, endpoint: endpoint, headers: headers)
if needsAuthentication {
if let authorizationRequest = authenticationRequest {
// A re-authorization is in progress, retry when it's done
authorizationRequest.delegate.queue.addOperationWithBlock() {
self.reauthenticateRequest(request).resume()
}
requestIsReady = false
}
else if canReauthenticate {
// There is enough information to re-authenticate
authenticateClient() {
self.reauthenticateRequest(request).resume()
}
requestIsReady = false
}
}
else if authenticated {
// If an authenticated request has a 401, reauthenticate and retry.
request.checkForRetry() { request, completion in
if request.response?.statusCode == 401 && self.canReauthenticate {
self.authenticateClient() {
let newRequest = self.reauthenticateRequest(request)
completion(.Retry(request: newRequest))
newRequest.resume()
}
}
else {
completion(.Continue)
}
}
}
if requestIsReady {
request.resume()
}
return request
}
@KingOfBrian I can't understand the code above totally. Can you provide the whole example?
Has there been any progress on this front? I'm maintaining the p2.OAuth2 Swift library and the (Swift) 3.0 version will have a _DataLoader_, which will (or should) mostly do what's discussed here – intercept 401, re-perform authorization only once while enqueuing all other requests, retry the failed requests once.
I'd be interested to see how this could work with Alamofire without creating a dependency on either framework. Our DataLoader can be used by itself but I have not the slightest intention building this out to a sophisticated loader, that's what Alamofire is for.
@p2 Take a look at our new RequestRetrier feature in Alamofire 4.
That looks like a great entry point. Maybe a bit tricky to get queueing right (enqueue calls that all fail while they wait for the first 401 to finish authorization), but should work if sessionManager.retrier does not block while waiting for the completion block to be called.
Thanks for pointing out RequestRetrier, @jshier! That's great and will be a big help to our team. :)
Great, thanks a lot for RequestRetrier, the OAuth2 library in v3 now works seamlessly with Alamofire with less than 40 lines of code.
Most helpful comment
@p2 Take a look at our new
RequestRetrierfeature in Alamofire 4.