From 5609e3a07098bb08171af5edffa1e9a8f693ff33 Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Wed, 12 Sep 2018 11:45:05 +0200 Subject: [PATCH 1/8] Do not remove refresh_token on server errors This prevents the user to be logged out with `.noRefreshToken` when the next refresh token request is triggered in case the error was a server error and not a client error. --- Sources/Flows/OAuth2.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index 70702664..d0ea5af0 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -366,9 +366,14 @@ open class OAuth2: OAuth2Base { do { let data = try response.responseData() let json = try self.parseRefreshTokenResponseData(data) - if response.response.statusCode >= 400 { + switch response.response.statusCode { + case 400..<500: self.clientConfig.refreshToken = nil throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") + case 500...599: + throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") + default: + break } self.logger?.debug("OAuth2", msg: "Did use refresh token for access token [\(nil != self.clientConfig.accessToken)]") callback(json, nil) From 6713930d3da2fa3c203bf4612b7ede9a163bcf8e Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Wed, 12 Sep 2018 13:01:54 +0200 Subject: [PATCH 2/8] Updated the comment to reflect the changes --- Sources/Flows/OAuth2.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index d0ea5af0..7cd9bf14 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -352,7 +352,7 @@ open class OAuth2: OAuth2Base { /** If there is a refresh token, use it to receive a fresh access token. - If the request returns an error, the refresh token is thrown away. + If the request returns an client error, the refresh token is thrown away. - parameter params: Optional key/value pairs to pass during token refresh - parameter callback: The callback to call after the refresh token exchange has finished From abb62290a004211dc783ab8a7d90a103459d041d Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Fri, 14 Sep 2018 09:26:54 +0200 Subject: [PATCH 3/8] Added specific client error and removed setting the token to nil --- Sources/Base/OAuth2Error.swift | 2 ++ Sources/Flows/OAuth2.swift | 11 +++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Sources/Base/OAuth2Error.swift b/Sources/Base/OAuth2Error.swift index 50bed17f..8e68db3f 100644 --- a/Sources/Base/OAuth2Error.swift +++ b/Sources/Base/OAuth2Error.swift @@ -82,6 +82,8 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { /// There is no delegate associated with the password grant flow instance. case noPasswordGrantDelegate + case clientError(Int) + // MARK: - Request errors diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index 7cd9bf14..4f09d6a9 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -352,7 +352,7 @@ open class OAuth2: OAuth2Base { /** If there is a refresh token, use it to receive a fresh access token. - If the request returns an client error, the refresh token is thrown away. + Does not remove the refresh_token in case of a failure. For client errors (400..<500), the callback will provide the status code in the .clientError(Int) - parameter params: Optional key/value pairs to pass during token refresh - parameter callback: The callback to call after the refresh token exchange has finished @@ -367,13 +367,12 @@ open class OAuth2: OAuth2Base { let data = try response.responseData() let json = try self.parseRefreshTokenResponseData(data) switch response.response.statusCode { + case 500: + throw OAuth2Error.serverError case 400..<500: - self.clientConfig.refreshToken = nil - throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") - case 500...599: - throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") + throw OAuth2Error.clientError(response.response.statusCode) default: - break + throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") } self.logger?.debug("OAuth2", msg: "Did use refresh token for access token [\(nil != self.clientConfig.accessToken)]") callback(json, nil) From b23124638224376e5e799dab11ca8571acd71735 Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Fri, 14 Sep 2018 15:15:11 +0200 Subject: [PATCH 4/8] Added description for the clientError --- Sources/Base/OAuth2Error.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Base/OAuth2Error.swift b/Sources/Base/OAuth2Error.swift index 8e68db3f..71c17a4b 100644 --- a/Sources/Base/OAuth2Error.swift +++ b/Sources/Base/OAuth2Error.swift @@ -275,6 +275,8 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { return message ?? "The requested scope is invalid, unknown, or malformed." case .serverError: return "The authorization server encountered an unexpected condition that prevented it from fulfilling the request." + case .clientError(let statusCode): + return "The authorization server returned \(statusCode)" case .temporarilyUnavailable(let message): return message ?? "The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server." case .invalidGrant(let message): From 9109c6c9cf256ec5986d773c6c1737d7f420af6b Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Tue, 18 Sep 2018 09:55:53 +0200 Subject: [PATCH 5/8] Added ServerErrorWithCode and updated comments --- Sources/Base/OAuth2Error.swift | 9 +++++++-- Sources/Flows/OAuth2.swift | 11 ++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/Sources/Base/OAuth2Error.swift b/Sources/Base/OAuth2Error.swift index 71c17a4b..2d986185 100644 --- a/Sources/Base/OAuth2Error.swift +++ b/Sources/Base/OAuth2Error.swift @@ -82,8 +82,11 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { /// There is no delegate associated with the password grant flow instance. case noPasswordGrantDelegate - case clientError(Int) + /// Generic client error 400<..500 + case clientErrorWithStatus(Int) + /// Generic server error <=501 + case serverErrorWithStatus(Int) // MARK: - Request errors @@ -275,7 +278,9 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { return message ?? "The requested scope is invalid, unknown, or malformed." case .serverError: return "The authorization server encountered an unexpected condition that prevented it from fulfilling the request." - case .clientError(let statusCode): + case .serverErrorWithStatus(let statusCode): + return "The authorization server encountered an unexpected condition, returning \(statusCode)" + case .clientErrorWithStatus(let statusCode): return "The authorization server returned \(statusCode)" case .temporarilyUnavailable(let message): return message ?? "The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server." diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index 4f09d6a9..4c7ece5e 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -352,7 +352,7 @@ open class OAuth2: OAuth2Base { /** If there is a refresh token, use it to receive a fresh access token. - Does not remove the refresh_token in case of a failure. For client errors (400..<500), the callback will provide the status code in the .clientError(Int) + Does not remove the refresh_token in case of a failure. For client errors (4xx), the callback will provide the status code in the .clientErrorWithStatus(Int). For server errors (5xx), the callback provides the status code in .serverErrorWithStatus(Int) - parameter params: Optional key/value pairs to pass during token refresh - parameter callback: The callback to call after the refresh token exchange has finished @@ -366,11 +366,12 @@ open class OAuth2: OAuth2Base { do { let data = try response.responseData() let json = try self.parseRefreshTokenResponseData(data) - switch response.response.statusCode { - case 500: - throw OAuth2Error.serverError + let statusCode = response.response.statusCode + switch statusCode { case 400..<500: - throw OAuth2Error.clientError(response.response.statusCode) + throw OAuth2Error.clientErrorWithStatus(statusCode) + case 500...599: + throw OAuth2Error.serverErrorWithStatus(statusCode) default: throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") } From 5cc44a267110f6ab15ec2a891732ef636e9b1b07 Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Mon, 17 Sep 2018 10:10:06 +0200 Subject: [PATCH 6/8] Updated contributors list --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 7a65758e..e6f8cc55 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -3,6 +3,7 @@ Contributors Contributors to the codebase, in reverse chronological order: +- Catalina Turlea @catalinaturlea - Maxime Le Moine, @MaximeLM - Seb Skuse, @sebskuse - David Hardiman, @dhardiman From d66420580a6a32bae4ab81807ed6391b482c7c52 Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Mon, 17 Sep 2018 13:47:42 +0200 Subject: [PATCH 7/8] Parse to json only in the case of a successful response --- Sources/Flows/OAuth2.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index 4c7ece5e..715af2e7 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -364,19 +364,20 @@ open class OAuth2: OAuth2Base { perform(request: post) { response in do { - let data = try response.responseData() - let json = try self.parseRefreshTokenResponseData(data) let statusCode = response.response.statusCode switch statusCode { case 400..<500: throw OAuth2Error.clientErrorWithStatus(statusCode) case 500...599: throw OAuth2Error.serverErrorWithStatus(statusCode) + case 200..<300: + let data = try response.responseData() + let json = try self.parseRefreshTokenResponseData(data) + self.logger?.debug("OAuth2", msg: "Did use refresh token for access token [\(nil != self.clientConfig.accessToken)]") + callback(json, nil) default: throw OAuth2Error.generic("Failed with status \(response.response.statusCode)") } - self.logger?.debug("OAuth2", msg: "Did use refresh token for access token [\(nil != self.clientConfig.accessToken)]") - callback(json, nil) } catch let error { self.logger?.debug("OAuth2", msg: "Error refreshing access token: \(error)") From 7fe2e96e1b6ce88fcbd8db3234afdfadd22ec546 Mon Sep 17 00:00:00 2001 From: Catalina Turlea Date: Tue, 2 Oct 2018 17:16:50 +0200 Subject: [PATCH 8/8] Removed client error and added json parsing for 4xx errors --- Sources/Base/OAuth2Error.swift | 7 +------ Sources/Flows/OAuth2.swift | 7 +++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Sources/Base/OAuth2Error.swift b/Sources/Base/OAuth2Error.swift index 2d986185..ef992677 100644 --- a/Sources/Base/OAuth2Error.swift +++ b/Sources/Base/OAuth2Error.swift @@ -82,10 +82,7 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { /// There is no delegate associated with the password grant flow instance. case noPasswordGrantDelegate - /// Generic client error 400<..500 - case clientErrorWithStatus(Int) - - /// Generic server error <=501 + /// Generic server error 5xx case serverErrorWithStatus(Int) // MARK: - Request errors @@ -280,8 +277,6 @@ public enum OAuth2Error: Error, CustomStringConvertible, Equatable { return "The authorization server encountered an unexpected condition that prevented it from fulfilling the request." case .serverErrorWithStatus(let statusCode): return "The authorization server encountered an unexpected condition, returning \(statusCode)" - case .clientErrorWithStatus(let statusCode): - return "The authorization server returned \(statusCode)" case .temporarilyUnavailable(let message): return message ?? "The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server." case .invalidGrant(let message): diff --git a/Sources/Flows/OAuth2.swift b/Sources/Flows/OAuth2.swift index 715af2e7..e549c05c 100644 --- a/Sources/Flows/OAuth2.swift +++ b/Sources/Flows/OAuth2.swift @@ -352,7 +352,7 @@ open class OAuth2: OAuth2Base { /** If there is a refresh token, use it to receive a fresh access token. - Does not remove the refresh_token in case of a failure. For client errors (4xx), the callback will provide the status code in the .clientErrorWithStatus(Int). For server errors (5xx), the callback provides the status code in .serverErrorWithStatus(Int) + Does not remove the refresh_token in case of a failure. For server errors (5xx), the callback provides the status code in .serverErrorWithStatus(Int) - parameter params: Optional key/value pairs to pass during token refresh - parameter callback: The callback to call after the refresh token exchange has finished @@ -366,11 +366,10 @@ open class OAuth2: OAuth2Base { do { let statusCode = response.response.statusCode switch statusCode { - case 400..<500: - throw OAuth2Error.clientErrorWithStatus(statusCode) case 500...599: throw OAuth2Error.serverErrorWithStatus(statusCode) - case 200..<300: + // 2xx and 4xx responses should contain valid json data which can be parsed + case 200..<300, 400..<500: let data = try response.responseData() let json = try self.parseRefreshTokenResponseData(data) self.logger?.debug("OAuth2", msg: "Did use refresh token for access token [\(nil != self.clientConfig.accessToken)]")