From 1e03d435bb28c6f1b12b10189443fcb9210b2e78 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:37:03 -0700 Subject: [PATCH 1/6] g-orchestrated: validate all present URL fields in discovery documents --- Sources/AppAuthCore/OIDServiceDiscovery.m | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/Sources/AppAuthCore/OIDServiceDiscovery.m b/Sources/AppAuthCore/OIDServiceDiscovery.m index 4d96f9db3..b0392827e 100644 --- a/Sources/AppAuthCore/OIDServiceDiscovery.m +++ b/Sources/AppAuthCore/OIDServiceDiscovery.m @@ -156,15 +156,28 @@ + (BOOL)dictionaryHasRequiredFields:(NSDictionary *)dictionary } } - // Check required URL fields are valid URLs. - NSArray *requiredURLFields = @[ + // Check that present URL-valued fields are valid URLs; absent ones are skipped because optional + // fields may be omitted. + NSArray *urlFields = @[ kIssuerKey, + kAuthorizationEndpointKey, + kDeviceAuthorizationEndpointKey, kTokenEndpointKey, - kJWKSURLKey + kUserinfoEndpointKey, + kJWKSURLKey, + kRegistrationEndpointKey, + kEndSessionEndpointKey, + kServiceDocumentationKey, + kOPPolicyURIKey, + kOPTosURIKey ]; - for (NSString *field in requiredURLFields) { - if (![NSURL URLWithString:dictionary[field]]) { + for (NSString *field in urlFields) { + id value = dictionary[field]; + if (!value) { + continue; + } + if (![value isKindOfClass:[NSString class]] || ![NSURL URLWithString:value]) { if (error) { NSString *errorText = [NSString stringWithFormat:kInvalidURLFieldErrorText, field]; *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeInvalidDiscoveryDocument From 8c7f611bbb12b85c8b043881fcc3c14da858b8ab Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:37:06 -0700 Subject: [PATCH 2/6] g-orchestrated: re-enable discovery validation tests --- UnitTests/OIDServiceDiscoveryTests.m | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/UnitTests/OIDServiceDiscoveryTests.m b/UnitTests/OIDServiceDiscoveryTests.m index 74fc15fe5..fed48e24e 100644 --- a/UnitTests/OIDServiceDiscoveryTests.m +++ b/UnitTests/OIDServiceDiscoveryTests.m @@ -70,7 +70,7 @@ - (void)encodeWithCoder:(NSCoder *)coder { static NSString *const kTestURL = @"http://www.google.com/"; /*! A string for testing an invalid URL. */ -static NSString *const kTestURLInvalid = @"abc"; +static NSString *const kTestURLInvalid = @"http://foo bar/"; /*! Field keys associated with an OpenID Connect Discovery Document. */ static NSString *const kIssuerKey = @"issuer"; @@ -337,8 +337,7 @@ - (void)testNoErrorWhenNoMissingFields { /*! @brief Tests that we get an error when the required fields are present but invalid. */ -// TODO: this test is failing -- (void)pendingTestErrorWhenMalformedFields { +- (void)testErrorWhenMalformedFields { NSError *error; // builds a discovery document with a munged field NSMutableDictionary *serviceDiscoveryDictionary = @@ -358,8 +357,7 @@ - (void)pendingTestErrorWhenMalformedFields { /*! @brief Tests that we get an error when null is passed in through JSON. */ -// TODO: this test is failing -- (void)pendingTestErrorWhenJSONNullField { +- (void)testErrorWhenJSONNullField { NSError *error; OIDServiceDiscovery *discovery = [[OIDServiceDiscovery alloc] initWithJSON:kDiscoveryDocumentNullField error:&error]; @@ -370,6 +368,24 @@ - (void)pendingTestErrorWhenJSONNullField { XCTAssertEqual(error.code, OIDErrorCodeInvalidDiscoveryDocument, @""); } +/*! @brief Tests that a present optional URL-valued field that is not a string produces an error + instead of raising. + */ +- (void)testErrorWhenOptionalURLFieldIsNotAString { + NSError *error; + NSMutableDictionary *serviceDiscoveryDictionary = + [[[self class] minimumServiceDiscoveryDictionary] mutableCopy]; + serviceDiscoveryDictionary[kOPPolicyURIKey] = @42; + + OIDServiceDiscovery *discovery = + [[OIDServiceDiscovery alloc] initWithDictionary:serviceDiscoveryDictionary error:&error]; + XCTAssertNil(discovery, @"When initializing a discovery document, it should not return an " + "instance if an optional URL field is not a string."); + XCTAssertNotNil(error, @"There should be an error indicating the discovery document is invalid."); + XCTAssertEqualObjects(error.domain, OIDGeneralErrorDomain, @""); + XCTAssertEqual(error.code, OIDErrorCodeInvalidDiscoveryDocument, @""); +} + /*! @brief Tests that the JSON String version results in a valid object. */ - (void)testJSONStringDecoding { From c1f51bdfe1d17a8bd008972bbbd077241339c59f Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:37:09 -0700 Subject: [PATCH 3/6] g-orchestrated: note discovery URL validation hardening in changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50023e728..0d244cb05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # UNRELEASED - Improved the state validation error messages to distinguish a missing `state` parameter from a mismatched one. The end session message now notes that a provider's `end_session_endpoint` may point at a legacy or SAML single-logout endpoint, which does not echo `state`. Addresses issue #956. - Fixed `OIDExternalUserAgentIOSCustomBrowser` failing the flow with "Unable to open Safari." when `canOpenURL:` returns `NO` for a URL that the system can open, for example an https URL when Safari is restricted and another browser is the default. The flow now fails only if the URL really cannot be opened. Addresses issue #980. +- Discovery documents containing a JSON `null` or a non-string value in a URL-valued field now fail with `OIDErrorCodeInvalidDiscoveryDocument` instead of raising `NSInvalidArgumentException`. URL-valued metadata fields that are present are now all checked, not only the required ones. # 3.0.0 - BREAKING: Updates made to support Xcode 27. ([#972](https://github.com/openid/AppAuth-iOS/pull/972), [#973](https://github.com/openid/AppAuth-iOS/pull/973)) From a9573b0204e81758512678a951fc248e37602f59 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:09:54 -0700 Subject: [PATCH 4/6] g-orchestrated: only require nonnull discovery URL fields to parse --- Sources/AppAuthCore/OIDServiceDiscovery.m | 35 ++++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/Sources/AppAuthCore/OIDServiceDiscovery.m b/Sources/AppAuthCore/OIDServiceDiscovery.m index b0392827e..a735cfe20 100644 --- a/Sources/AppAuthCore/OIDServiceDiscovery.m +++ b/Sources/AppAuthCore/OIDServiceDiscovery.m @@ -127,7 +127,9 @@ - (nullable instancetype)initWithDictionary:(NSDictionary *)serviceDiscoveryDict @discussion This test is not meant to provide semantic analysis of the document (eg. fields where the value @c none is not an allowed option would not cause this method to fail if their value was @c none.) We are just testing to make sure we can meet the nullability - contract we promised in the header. + contract we promised in the header. A URL-valued field that is present must be a string, + or the getters would raise. A URL field that does not parse is an error only for the + nonnull URL properties. The nullable ones return nil for it instead. */ + (BOOL)dictionaryHasRequiredFields:(NSDictionary *)dictionary error:(NSError **_Nullable)error { @@ -156,9 +158,9 @@ + (BOOL)dictionaryHasRequiredFields:(NSDictionary *)dictionary } } - // Check that present URL-valued fields are valid URLs; absent ones are skipped because optional - // fields may be omitted. - NSArray *urlFields = @[ + // Every URL-valued field that is present must be a string. NSJSONSerialization turns a JSON + // null into NSNull, and +[NSURL URLWithString:] raises on anything that is not a string. + NSArray *urlFields = @[ kIssuerKey, kAuthorizationEndpointKey, kDeviceAuthorizationEndpointKey, @@ -174,10 +176,29 @@ + (BOOL)dictionaryHasRequiredFields:(NSDictionary *)dictionary for (NSString *field in urlFields) { id value = dictionary[field]; - if (!value) { - continue; + if (value && ![value isKindOfClass:[NSString class]]) { + if (error) { + NSString *errorText = [NSString stringWithFormat:kInvalidURLFieldErrorText, field]; + *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeInvalidDiscoveryDocument + underlyingError:nil + description:errorText]; + } + return NO; } - if (![value isKindOfClass:[NSString class]] || ![NSURL URLWithString:value]) { + } + + // The nonnull URL properties must also parse. The nullable ones return nil for a value that + // does not parse, so a sloppy optional field does not reject the whole document. Keep this + // list in sync with the nonnull NSURL properties in OIDServiceDiscovery.h. + NSArray *requiredURLFields = @[ + kIssuerKey, + kAuthorizationEndpointKey, + kTokenEndpointKey, + kJWKSURLKey + ]; + + for (NSString *field in requiredURLFields) { + if (![NSURL URLWithString:(NSString *)dictionary[field]]) { if (error) { NSString *errorText = [NSString stringWithFormat:kInvalidURLFieldErrorText, field]; *error = [OIDErrorUtilities errorWithCode:OIDErrorCodeInvalidDiscoveryDocument From d82f2fb9adc6719b05708171daf477b81efb4a38 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:09:54 -0700 Subject: [PATCH 5/6] g-orchestrated: test that an unparseable optional URL field is accepted --- UnitTests/OIDServiceDiscoveryTests.m | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/UnitTests/OIDServiceDiscoveryTests.m b/UnitTests/OIDServiceDiscoveryTests.m index fed48e24e..41e2f99f9 100644 --- a/UnitTests/OIDServiceDiscoveryTests.m +++ b/UnitTests/OIDServiceDiscoveryTests.m @@ -386,6 +386,23 @@ - (void)testErrorWhenOptionalURLFieldIsNotAString { XCTAssertEqual(error.code, OIDErrorCodeInvalidDiscoveryDocument, @""); } +/*! @brief Tests that a present optional URL-valued field that does not parse is accepted, with + its property reading as nil. + */ +- (void)testNoErrorWhenOptionalURLFieldDoesNotParse { + NSError *error; + NSMutableDictionary *serviceDiscoveryDictionary = + [[[self class] minimumServiceDiscoveryDictionary] mutableCopy]; + serviceDiscoveryDictionary[kOPPolicyURIKey] = kTestURLInvalid; + + OIDServiceDiscovery *discovery = + [[OIDServiceDiscovery alloc] initWithDictionary:serviceDiscoveryDictionary error:&error]; + XCTAssertNotNil(discovery, @"An optional URL field that does not parse should not reject the " + "discovery document."); + XCTAssertNil(error, @""); + XCTAssertNil(discovery.OPPolicyURI, @""); +} + /*! @brief Tests that the JSON String version results in a valid object. */ - (void)testJSONStringDecoding { From 0accc01d0f7f64ffc912929293d92bfaabf4fe74 Mon Sep 17 00:00:00 2001 From: Worthing ~ <115107835+w-goog@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:09:54 -0700 Subject: [PATCH 6/6] g-orchestrated: narrow the discovery validation changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec0259aa..98e2ac096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ - Improved the state validation error messages to distinguish a missing `state` parameter from a mismatched one. The end session message now notes that a provider's `end_session_endpoint` may point at a legacy or SAML single-logout endpoint, which does not echo `state`. Addresses issue #956. - Fixed `OIDExternalUserAgentIOSCustomBrowser` failing the flow with "Unable to open Safari." when `canOpenURL:` returns `NO` for a URL that the system can open, for example an https URL when Safari is restricted and another browser is the default. The flow now fails only if the URL really cannot be opened. Addresses issue #980. - Fixed a crash when the user cancels sign-in on iOS, caused by `ASWebAuthenticationSession` being deallocated during its completion handler or dismiss animation. Based on [#917](https://github.com/openid/AppAuth-iOS/pull/917) by @Prash88. -- Discovery documents containing a JSON `null` or a non-string value in a URL-valued field now fail with `OIDErrorCodeInvalidDiscoveryDocument` instead of raising `NSInvalidArgumentException`. URL-valued metadata fields that are present are now all checked, not only the required ones. +- Discovery documents containing a JSON `null` or a non-string value in a URL-valued field now fail with `OIDErrorCodeInvalidDiscoveryDocument` instead of raising `NSInvalidArgumentException`. An `authorization_endpoint` that does not parse now fails the document too, like the other required URL fields. # 3.0.0 - BREAKING: Updates made to support Xcode 27. ([#972](https://github.com/openid/AppAuth-iOS/pull/972), [#973](https://github.com/openid/AppAuth-iOS/pull/973))