[GH-ISSUE #509] OAuthSwift creates errors reusing NSURLErrorDomain instead of its' own domain, OAuthSwiftError.Domain #336

Closed
opened 2026-03-03 16:47:48 +03:00 by kerem · 1 comment
Owner

Originally created by @nataliachodelski on GitHub (Jan 17, 2019).
Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/509

Description:

I encountered this issue when a token refresh with Microsoft Azure ActiveDirectory failed due to a password change by the user.

In this situation, the OAuthSwift library returned a 400 error code. The error was correctly parsed to add the OAuth error information into its' userInfo but the error was returned with domain NSURLErrorDomain rather than OAuthSwiftError.

Example error as returned by line 183 of OAuthSwiftHTTPRequest.swift (line number references the current origin/master codebase file):

"Error Domain=NSURLErrorDomain Code=400 UserInfo={NSLocalizedDescription=AADSTS50173: The provided grant has expired due to it being revoked. The user might have changed or reset their password. The grant was issued on '2019-01-17T19:44:33.6456875Z' and the TokensValidFrom date for this user is '2019-01-17T20:07:18.0000000Z' \nTrace ID: 757e5b97-110a-42b4-9b84-4bb0743f0700 \nCorrelation ID: 983947ea-53bf-4b30-bd06-87856a48f1ea \nTimestamp: 2019-01-17 21:50:40Z}


I believe this is an issue with the OAuthSwift because NSURLErrorDomain is one of Apple's Foundation error domains for NSURLSession. The expected possible error codes are already defined listed in NSURLError.h, and do NOT include a code 400.

My understanding is that returning an error using NSURLErrorDomain is bad practice and possibly is a bug, as developers do not expect a non-apple library to reuse Apple's own error codes for custom errors.

Best practice for error handling in Libraries/frameworks is for developers to create their own custom error domains and codes for their libraries and publish the error domains and codes with the documentation.

This approach has generally been followed in the OAuthSwift library in OAuthSwiftError.swift, however the particular error I referenced is being created by OAuthSwift in a way that violates best practices. I believe the errors passed out of the OAuthSwift library SHOULD always have the domain OAuthSwiftError, since that is the whole point of defining an error domain and codes for this library to use in OAuthSwiftError.swift.

--

Re: fixing this issue:

  • I'd like to try fixing it (though happy to take suggestions from others re whether or how this should be addressed).
  • I will create pull request with my suggested fix for review by the other contributors.

OAuth Provider? (Twitter, Github, ..):

Microsoft Azure Active Directory.
Using OAuth2.0 with the ExchangeActiveSync API

OAuth Version:

  • Version 1
  • Version 2

OS (Please fill the version) :

  • iOS :
  • OSX :
  • TVOS :
  • WatchOS :

Installation method:

  • Carthage
  • CocoaPods
  • Manually

Library version:

  • head
  • [x ] v1.2.1
  • v1.2 (Swift 4.0)
  • v1.0.0
  • v0.6
  • other: (Please fill in the version you are using.)

Xcode version:

-[x] 10.1

  • 9.3 (Swift 4.1)

  • 9.0 (Swift 4.0)

  • 9.0 (Swift 3.2)

  • 8.x (Swift 3.x)

  • 8.0 (Swift 2.3)

  • 7.3.1

  • other: (Please fill in the version you are using.)

  • objective c

  • swift

Originally created by @nataliachodelski on GitHub (Jan 17, 2019). Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/509 ### Description: I encountered this issue when a token refresh with Microsoft Azure ActiveDirectory failed due to a password change by the user. In this situation, the OAuthSwift library returned a 400 error code. The error was correctly parsed to add the OAuth error information into its' `userInfo` but the error was returned with domain `NSURLErrorDomain` rather than `OAuthSwiftError`. Example error as returned by line 183 of OAuthSwiftHTTPRequest.swift (line number references the current origin/master codebase file): > `"Error Domain=NSURLErrorDomain Code=400 UserInfo={NSLocalizedDescription=AADSTS50173: The provided grant has expired due to it being revoked. The user might have changed or reset their password. The grant was issued on '2019-01-17T19:44:33.6456875Z' and the TokensValidFrom date for this user is '2019-01-17T20:07:18.0000000Z' \nTrace ID: 757e5b97-110a-42b4-9b84-4bb0743f0700 \nCorrelation ID: 983947ea-53bf-4b30-bd06-87856a48f1ea \nTimestamp: 2019-01-17 21:50:40Z}` --- I believe this is an issue with the OAuthSwift because `NSURLErrorDomain` is one of Apple's Foundation error domains for NSURLSession. The expected possible error codes are already defined listed in `NSURLError.h`, and do NOT include a code 400. My understanding is that returning an error using `NSURLErrorDomain` is bad practice and possibly is a bug, as developers do not expect a non-apple library to reuse Apple's own error codes for custom errors. [Best practice for error handling in Libraries/frameworks is for developers to create their own custom error domains and codes for their libraries and publish the error domains and codes with the documentation. ](https://stackoverflow.com/questions/1779034/nserror-domains-custom-domains-conventions-and-best-practices) This approach has generally been followed in the OAuthSwift library in `OAuthSwiftError.swift`, however the particular error I referenced is being created by OAuthSwift in a way that violates best practices. I believe the errors passed out of the OAuthSwift library SHOULD always have the domain `OAuthSwiftError`, since that is the whole point of defining an error domain and codes for this library to use in OAuthSwiftError.swift. -- #### Re: fixing this issue: - I'd like to try fixing it (though happy to take suggestions from others re whether or how this should be addressed). - I will create pull request with my suggested fix for review by the other contributors. --- ### OAuth Provider? (Twitter, Github, ..): Microsoft Azure Active Directory. Using OAuth2.0 with the ExchangeActiveSync API ### OAuth Version: - [ ] Version 1 - [X] Version 2 ### OS (Please fill the version) : - [x] iOS : - [ ] OSX : - [ ] TVOS : - [ ] WatchOS : ### Installation method: - [ ] Carthage - [X] CocoaPods - [ ] Manually ### Library version: - [ ] head - [x ] v1.2.1 - [ ] v1.2 (Swift 4.0) - [ ] v1.0.0 - [ ] v0.6 - [ ] other: (Please fill in the version you are using.) ### Xcode version: -[x] 10.1 - [ ] 9.3 (Swift 4.1) - [ ] 9.0 (Swift 4.0) - [ ] 9.0 (Swift 3.2) - [ ] 8.x (Swift 3.x) - [ ] 8.0 (Swift 2.3) - [ ] 7.3.1 - [ ] other: (Please fill in the version you are using.) - [ ] objective c - [x] swift
kerem 2026-03-03 16:47:48 +03:00
Author
Owner

@phimage commented on GitHub (Feb 21, 2019):

You are right the domain is wrong

We put http status code in the error, maybe a domain exist about that but not sure because I often import in my project a custom enum to list http errors code

Google use it own domain "Error Domain=com.google.HTTPStatus Code=400"
So "OAuthSwiftError.HTTPStatus" could be used

I we change somethings
a code to check

public var isExpiredToken: Bool {
        guard self.domain == NSURLErrorDomain else {
            return false
        }
<!-- gh-comment-id:465985722 --> @phimage commented on GitHub (Feb 21, 2019): You are right the domain is wrong We put http status code in the error, maybe a domain exist about that but not sure because I often import in my project a custom enum to list http errors code Google use it own domain "Error Domain=com.google.HTTPStatus Code=400" So "OAuthSwiftError.HTTPStatus" could be used I we change somethings a code to check ```swift public var isExpiredToken: Bool { guard self.domain == NSURLErrorDomain else { return false } ```
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/OAuthSwift#336
No description provided.