diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be94c79a..9a397dec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Unreleased - Add SwiftUI + Swift Package Manager sample app under `Examples/Example-iOS_Swift-SPM`. ([#952](https://github.com/openid/AppAuth-iOS/pull/952)) -- Removed external browser (Safari) fallback from `OIDExternalUserAgentIOS`. If `ASWebAuthenticationSession` fails to start (e.g., Guided Access is enabled), the authorization flow now fails with an error instead of opening an external browser. +- Removed external browser (Safari) fallback from `OIDExternalUserAgentIOS`. If `ASWebAuthenticationSession` fails to start (e.g., Guided Access isenabled), the authorization flow now fails with an error instead of opening an external browser. +- Added `resumeExternalUserAgentFlowWithURL:error:` to `OIDExternalUserAgentSession` protocol. This method returns errors via an out-parameter instead of throwing `NSException` in invalid-state scenarios. The previous `resumeExternalUserAgentFlowWithURL:` method is deprecated and forwards to the new method. Note that, as a side effect of this forwarding, the deprecated method no longer raises `NSException` (with name `OIDOAuthExceptionInvalidAuthorizationFlow`) on invalid state — it now returns `NO` silently. Migrate to the `resumeExternalUserAgentFlowWithURL:error:` variant to inspect the cause. ([#955](https://github.com/openid/AppAuth-iOS/pull/955)) +- Added `OIDErrorCodeURLMismatch` and `OIDErrorCodeInvalidAuthorizationFlow` error codes. ([#955](https://github.com/openid/AppAuth-iOS/pull/955)) # 2.0.0 - Raise minimum supported iOS version to iOS 12. ([#918](https://github.com/openid/AppAuth-iOS/pull/918)) diff --git a/Examples/Example-iOS_ObjC-Carthage/Source/AppDelegate.m b/Examples/Example-iOS_ObjC-Carthage/Source/AppDelegate.m index 716270381..f646eee17 100644 --- a/Examples/Example-iOS_ObjC-Carthage/Source/AppDelegate.m +++ b/Examples/Example-iOS_ObjC-Carthage/Source/AppDelegate.m @@ -49,7 +49,7 @@ - (BOOL)application:(UIApplication *)app options:(NSDictionary *)options { // Sends the URL to the current authorization flow (if any) which will process it if it relates to // an authorization response. - if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url]) { + if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url error:nil]) { _currentAuthorizationFlow = nil; return YES; } diff --git a/Examples/Example-iOS_ObjC/Source/AppDelegate.m b/Examples/Example-iOS_ObjC/Source/AppDelegate.m index cbf861d4a..d00f1a25a 100644 --- a/Examples/Example-iOS_ObjC/Source/AppDelegate.m +++ b/Examples/Example-iOS_ObjC/Source/AppDelegate.m @@ -49,7 +49,7 @@ - (BOOL)application:(UIApplication *)app options:(NSDictionary *)options { // Sends the URL to the current authorization flow (if any) which will process it if it relates to // an authorization response. - if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url]) { + if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url error:nil]) { _currentAuthorizationFlow = nil; return YES; } diff --git a/Examples/Example-iOS_Swift-Carthage/Source/AppDelegate.swift b/Examples/Example-iOS_Swift-Carthage/Source/AppDelegate.swift index 070c3d71a..e530753fb 100644 --- a/Examples/Example-iOS_Swift-Carthage/Source/AppDelegate.swift +++ b/Examples/Example-iOS_Swift-Carthage/Source/AppDelegate.swift @@ -32,9 +32,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate { func application(_ app: UIApplication, open url: URL, options: [UIApplicationOpenURLOptionsKey : Any] = [:]) -> Bool { - if let authorizationFlow = self.currentAuthorizationFlow, authorizationFlow.resumeExternalUserAgentFlow(with: url) { - self.currentAuthorizationFlow = nil - return true + // Inspecting the error lets you distinguish a benign URL mismatch + // (the URL belongs to another handler) from an unexpected condition + // such as no pending flow, which previously surfaced as an NSException. + if let authorizationFlow = self.currentAuthorizationFlow { + do { + try authorizationFlow.resumeExternalUserAgentFlow(with: url) + self.currentAuthorizationFlow = nil + return true + } catch { + print("Authorization flow could not handle URL: \(error.localizedDescription)") + } } return false diff --git a/Examples/Example-iOS_Swift-SPM/Example/AppDelegate.swift b/Examples/Example-iOS_Swift-SPM/Example/AppDelegate.swift index a1838b2cd..adb459ee9 100644 --- a/Examples/Example-iOS_Swift-SPM/Example/AppDelegate.swift +++ b/Examples/Example-iOS_Swift-SPM/Example/AppDelegate.swift @@ -24,9 +24,17 @@ class AppDelegate: NSObject, UIApplicationDelegate { var currentAuthorizationFlow: OIDExternalUserAgentSession? func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any] = [:]) -> Bool { - if let authorizationFlow = self.currentAuthorizationFlow, authorizationFlow.resumeExternalUserAgentFlow(with: url) { - self.currentAuthorizationFlow = nil - return true + // Inspecting the error lets you distinguish a benign URL mismatch + // (the URL belongs to another handler) from an unexpected condition + // such as no pending flow, which previously surfaced as an NSException. + if let authorizationFlow = self.currentAuthorizationFlow { + do { + try authorizationFlow.resumeExternalUserAgentFlow(with: url) + self.currentAuthorizationFlow = nil + return true + } catch { + print("Authorization flow could not handle URL: \(error.localizedDescription)") + } } return false diff --git a/Examples/Example-macOS/Source/AppDelegate.m b/Examples/Example-macOS/Source/AppDelegate.m index 7e5a8068b..b5de912de 100644 --- a/Examples/Example-macOS/Source/AppDelegate.m +++ b/Examples/Example-macOS/Source/AppDelegate.m @@ -49,7 +49,7 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)replyEvent { NSString *URLString = [[event paramDescriptorForKeyword:keyDirectObject] stringValue]; NSURL *URL = [NSURL URLWithString:URLString]; - [_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:URL]; + [_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:URL error:nil]; } @end diff --git a/README.md b/README.md index c9f44a41c..edbd54632 100644 --- a/README.md +++ b/README.md @@ -380,9 +380,16 @@ authorization session (created in the previous session): options:(NSDictionary *)options { // Sends the URL to the current authorization flow (if any) which will // process it if it relates to an authorization response. - if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url]) { + // Handling the error lets you filter for conditions that require + // logging, such as an unexpected state + // (OIDErrorCodeInvalidAuthorizationFlow). Benign mismatches + // (OIDErrorCodeURLMismatch) are kept silent. + NSError *error = nil; + if ([_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:url error:&error]) { _currentAuthorizationFlow = nil; return YES; + } else if (error.code == OIDErrorCodeInvalidAuthorizationFlow) { + NSLog(@"Authorization flow could not handle URL: %@", error.localizedDescription); } // Your additional URL handling (if any) goes here. @@ -397,11 +404,20 @@ func application(_ app: UIApplication, open url: URL, options: [UIApplicationOpenURLOptionsKey : Any] = [:]) -> Bool { // Sends the URL to the current authorization flow (if any) which will - // process it if it relates to an authorization response. - if let authorizationFlow = self.currentAuthorizationFlow, - authorizationFlow.resumeExternalUserAgentFlow(with: url) { - self.currentAuthorizationFlow = nil - return true + // process it if it relates to an authorization response. Handling the + // error lets you filter for conditions that require logging, such as + // an unexpected state (OIDErrorCodeInvalidAuthorizationFlow). Benign + // mismatches (OIDErrorCodeURLMismatch) are kept silent. + if let authorizationFlow = self.currentAuthorizationFlow { + do { + try authorizationFlow.resumeExternalUserAgentFlow(with: url) + self.currentAuthorizationFlow = nil + return true + } catch let error as NSError where error.code == OIDErrorCodeInvalidAuthorizationFlow.rawValue { + print("Authorization flow could not handle URL: \(error.localizedDescription)") + } catch { + // Benign mismatch (e.g. OIDErrorCodeURLMismatch): fall through. + } } // Your additional URL handling (if any) diff --git a/Sources/AppAuth/iOS/OIDExternalUserAgentCatalyst.m b/Sources/AppAuth/iOS/OIDExternalUserAgentCatalyst.m index d6771b3e9..4c10c154d 100644 --- a/Sources/AppAuth/iOS/OIDExternalUserAgentCatalyst.m +++ b/Sources/AppAuth/iOS/OIDExternalUserAgentCatalyst.m @@ -89,7 +89,7 @@ - (BOOL)presentExternalUserAgentRequest:(id)request } strongSelf->_webAuthenticationVC = nil; if (callbackURL) { - [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL]; + [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL error:nil]; } else { NSError *safariError = [OIDErrorUtilities errorWithCode:OIDErrorCodeUserCanceledAuthorizationFlow diff --git a/Sources/AppAuth/iOS/OIDExternalUserAgentIOS.m b/Sources/AppAuth/iOS/OIDExternalUserAgentIOS.m index 95acdc16c..eb8c3f08d 100644 --- a/Sources/AppAuth/iOS/OIDExternalUserAgentIOS.m +++ b/Sources/AppAuth/iOS/OIDExternalUserAgentIOS.m @@ -114,7 +114,7 @@ - (BOOL)presentExternalUserAgentRequest:(id)request } strongSelf->_webAuthenticationVC = nil; if (callbackURL) { - [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL]; + [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL error:nil]; } else { NSError *safariError = [OIDErrorUtilities errorWithCode:OIDErrorCodeUserCanceledAuthorizationFlow diff --git a/Sources/AppAuth/macOS/OIDExternalUserAgentMac.m b/Sources/AppAuth/macOS/OIDExternalUserAgentMac.m index d1e08f902..184767668 100644 --- a/Sources/AppAuth/macOS/OIDExternalUserAgentMac.m +++ b/Sources/AppAuth/macOS/OIDExternalUserAgentMac.m @@ -97,7 +97,7 @@ - (BOOL)presentExternalUserAgentRequest:(id)request } strongSelf->_webAuthenticationSession = nil; if (callbackURL) { - [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL]; + [strongSelf->_session resumeExternalUserAgentFlowWithURL:callbackURL error:nil]; } else { NSError *safariError = [OIDErrorUtilities errorWithCode:OIDErrorCodeUserCanceledAuthorizationFlow diff --git a/Sources/AppAuth/macOS/OIDRedirectHTTPHandler.m b/Sources/AppAuth/macOS/OIDRedirectHTTPHandler.m index 3d0d765d8..5f7c4813b 100644 --- a/Sources/AppAuth/macOS/OIDRedirectHTTPHandler.m +++ b/Sources/AppAuth/macOS/OIDRedirectHTTPHandler.m @@ -126,7 +126,7 @@ - (void)stopHTTPListener { - (void)HTTPConnection:(HTTPConnection *)conn didReceiveRequest:(HTTPServerRequest *)mess { // Sends URL to AppAuth. CFURLRef url = CFHTTPMessageCopyRequestURL(mess.request); - BOOL handled = [_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:(__bridge NSURL *)url]; + BOOL handled = [_currentAuthorizationFlow resumeExternalUserAgentFlowWithURL:(__bridge NSURL *)url error:nil]; // Stops listening to further requests after the first valid authorization response. if (handled) { diff --git a/Sources/AppAuthCore/OIDAuthorizationService.m b/Sources/AppAuthCore/OIDAuthorizationService.m index cc749a3f9..069a97eed 100644 --- a/Sources/AppAuthCore/OIDAuthorizationService.m +++ b/Sources/AppAuthCore/OIDAuthorizationService.m @@ -120,9 +120,21 @@ - (BOOL)shouldHandleURL:(NSURL *)URL { return [[self class] URL:URL matchesRedirectionURL:_request.redirectURL]; } +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-implementations" - (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL { + return [self resumeExternalUserAgentFlowWithURL:URL error:nil]; +} +#pragma clang diagnostic pop + +- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL error:(NSError *_Nullable *_Nullable)error { // rejects URLs that don't match redirect (these may be completely unrelated to the authorization) if (![self shouldHandleURL:URL]) { + if (error) { + *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeURLMismatch + underlyingError:nil + description:@"URL does not match the expected redirect URI."]; + } return NO; } @@ -130,24 +142,28 @@ - (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL { // checks for an invalid state if (!_pendingauthorizationFlowCallback) { - [NSException raise:OIDOAuthExceptionInvalidAuthorizationFlow - format:@"%@", OIDOAuthExceptionInvalidAuthorizationFlow, nil]; + if (error) { + *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeInvalidAuthorizationFlow + underlyingError:nil + description:@"There is no pending authorization flow to resume."]; + } + return NO; } OIDURLQueryComponent *query = [[OIDURLQueryComponent alloc] initWithURL:URL]; - NSError *error; + NSError *errorLocal; OIDAuthorizationResponse *response = nil; // checks for an OAuth error response as per RFC6749 Section 4.1.2.1 if (query.dictionaryValue[OIDOAuthErrorFieldError]) { - error = [OIDErrorUtilities OAuthErrorWithDomain:OIDOAuthAuthorizationErrorDomain + errorLocal = [OIDErrorUtilities OAuthErrorWithDomain:OIDOAuthAuthorizationErrorDomain OAuthResponse:query.dictionaryValue underlyingError:nil]; } // no error, should be a valid OAuth 2.0 response - if (!error) { + if (!errorLocal) { response = [[OIDAuthorizationResponse alloc] initWithRequest:_request parameters:query.dictionaryValue]; @@ -161,14 +177,14 @@ - (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL { response.state, response]; response = nil; - error = [NSError errorWithDomain:OIDOAuthAuthorizationErrorDomain + errorLocal = [NSError errorWithDomain:OIDOAuthAuthorizationErrorDomain code:OIDErrorCodeOAuthAuthorizationClientError userInfo:userInfo]; } } [_externalUserAgent dismissExternalUserAgentAnimated:YES completion:^{ - [self didFinishWithResponse:response error:error]; + [self didFinishWithResponse:response error:errorLocal]; }]; return YES; @@ -253,19 +269,36 @@ - (BOOL)shouldHandleURL:(NSURL *)URL { matchesRedirectionURL:_request.postLogoutRedirectURL]; } +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-implementations" - (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL { + return [self resumeExternalUserAgentFlowWithURL:URL error:nil]; +} +#pragma clang diagnostic pop + +- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL + error:(NSError *_Nullable *_Nullable)error { // rejects URLs that don't match redirect (these may be completely unrelated to the authorization) if (![self shouldHandleURL:URL]) { + if (error) { + *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeURLMismatch + underlyingError:nil + description:@"URL does not match the expected redirect URI."]; + } return NO; } // checks for an invalid state if (!_pendingEndSessionCallback) { - [NSException raise:OIDOAuthExceptionInvalidAuthorizationFlow - format:@"%@", OIDOAuthExceptionInvalidAuthorizationFlow, nil]; + if (error) { + *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeInvalidAuthorizationFlow + underlyingError:nil + description:@"There is no pending authorization flow to resume."]; + } + return NO; } - NSError *error; + NSError *responseError; OIDEndSessionResponse *response = nil; OIDURLQueryComponent *query = [[OIDURLQueryComponent alloc] initWithURL:URL]; @@ -282,13 +315,13 @@ - (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL { response.state, response]; response = nil; - error = [NSError errorWithDomain:OIDOAuthAuthorizationErrorDomain + responseError = [NSError errorWithDomain:OIDOAuthAuthorizationErrorDomain code:OIDErrorCodeOAuthAuthorizationClientError userInfo:userInfo]; } [_externalUserAgent dismissExternalUserAgentAnimated:YES completion:^{ - [self didFinishWithResponse:response error:error]; + [self didFinishWithResponse:response error:responseError]; }]; return YES; diff --git a/Sources/AppAuthCore/OIDError.h b/Sources/AppAuthCore/OIDError.h index 5131f0ad4..256244b4b 100644 --- a/Sources/AppAuthCore/OIDError.h +++ b/Sources/AppAuthCore/OIDError.h @@ -151,6 +151,15 @@ typedef NS_ENUM(NSInteger, OIDErrorCode) { /*! @brief The ID Token did not pass validation (e.g. issuer, audience checks). */ OIDErrorCodeIDTokenFailedValidationError = -15, + + /*! @brief The URL does not match the expected redirect URI for this session. + */ + OIDErrorCodeURLMismatch = -16, + + /*! @brief The redirect URL was received, but the session has no pending callback to deliver it + to (the callback was already invoked or the session was cancelled). + */ + OIDErrorCodeInvalidAuthorizationFlow = -17, }; /*! @brief Enum of all possible OAuth error codes as defined by RFC6749 diff --git a/Sources/AppAuthCore/OIDExternalUserAgentSession.h b/Sources/AppAuthCore/OIDExternalUserAgentSession.h index 3b886a6c3..7bb080c58 100644 --- a/Sources/AppAuthCore/OIDExternalUserAgentSession.h +++ b/Sources/AppAuthCore/OIDExternalUserAgentSession.h @@ -51,7 +51,26 @@ NS_ASSUME_NONNULL_BEGIN @remarks Has no effect if called more than once, or after a @c cancel message was received. @return YES if the passed URL matches the expected redirect URL and was consumed, NO otherwise. */ -- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL; +- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL __deprecated_msg("Use resumeExternalUserAgentFlowWithURL:error: instead"); + +/*! @brief Clients should call this method with the result of the external user-agent code flow if + it becomes available. This is the preferred replacement for the deprecated version. + @param URL The redirect URL invoked by the server. + @param error On failure, an NSError describing why the URL was not handled. Pass NULL if you do + not need the error. + @discussion When the URL represented a valid response, implementations should clean up any + left-over UI state from the request, for example by closing the + \SFSafariViewController or loopback HTTP listener if those were used. The completion block + of the pending request should then be invoked. + Two specific error cases: (1) OIDErrorCodeURLMismatch when the URL does not match the + expected redirect, (2) OIDErrorCodeInvalidAuthorizationFlow when no pending authorization + flow exists. + @remarks Has no effect if called more than once, or after a @c cancel message was received. + @return YES if the passed URL matches the expected redirect URL and was consumed. + NO if the URL did not match (\@c OIDErrorCodeURLMismatch) or no authorization flow + was pending (\@c OIDErrorCodeInvalidAuthorizationFlow). + */ +- (BOOL)resumeExternalUserAgentFlowWithURL:(NSURL *)URL error:(NSError *_Nullable *_Nullable)error; /*! @brief @c OIDExternalUserAgent or clients should call this method when the external user-agent flow failed with a non-OAuth error. diff --git a/UnitTests/OIDRPProfileCode.m b/UnitTests/OIDRPProfileCode.m index 1c37b1f30..afc381d8c 100644 --- a/UnitTests/OIDRPProfileCode.m +++ b/UnitTests/OIDRPProfileCode.m @@ -64,7 +64,7 @@ - (BOOL)presentExternalUserAgentRequest:(id )reques NSDictionary* headers = [(NSHTTPURLResponse *)response allHeaderFields]; NSString *location = [headers objectForKey:@"Location"]; NSURL *url = [NSURL URLWithString:location]; - [session resumeExternalUserAgentFlowWithURL:url]; + [session resumeExternalUserAgentFlowWithURL:url error:nil]; }] resume]; return YES;