From f17772c1a59fbc76fb3f981bfc2ac33cb1ffe690 Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Tue, 8 Sep 2026 23:27:09 +0530 Subject: [PATCH 1/3] T 1551265: Support public OAuth2 clients without a client secret --- CHANGELOG.md | 6 +- .../OAuth2ClientConfiguration.swift | 40 +++++- .../OAuth2RequestPipelineMiddleware.swift | 8 +- .../OAuth2TokenGrantStrategy.swift | 11 +- .../Auth/OAuth2ClientConfigurationTests.swift | 14 ++ .../OAuth2PublicClientTokenGrantTests.swift | 130 ++++++++++++++++++ ...OAuth2RequestPipelineMiddlewareTests.swift | 37 +++++ .../Auth/OAuth2TokenStoreTests.swift | 20 +++ 8 files changed, 258 insertions(+), 8 deletions(-) create mode 100644 Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 1437fcda..2325c81f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,11 @@ - None #### Enhancements -- None +- Support public OAuth2 clients that hold no client secret. `OAuth2ClientConfiguration` gains a + secret-less initializer (`init(publicClientIdentifier:environment:guestUsername:guestPassword:)`) + and an `isPublicClient` flag. A public client sends its `client_id` in the token grant body and + omits the `Authorization: Basic` header across every grant type; a confidential client is + unchanged. #### Bug Fixes - None diff --git a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift index e44cfa6f..31489800 100644 --- a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift +++ b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift @@ -14,9 +14,17 @@ public struct OAuth2ClientConfiguration: Equatable { /// The OAuth2 client identifier public var clientIdentifier: String - /// The OAuth2 client secret + /// The OAuth2 client secret. Empty for a public client (see `isPublicClient`); do not assign a + /// non-empty secret to a public client, as it will not be sent. public var clientSecret: String + /// Whether the client is public, i.e. it holds no client secret. A public client sends its + /// `client_id` in the token request body and omits the `Authorization: Basic` header across every + /// grant type, and supports only bearer client-level authorization (a client-level basic request + /// fails, as there is no secret to send). A confidential client (the default) is unchanged. Set + /// only through the public-client initializer. + public private(set) var isPublicClient: Bool = false + /// The guest user's username, if one exists or is needed for client-level authorization public var guestUsername: String? @@ -44,4 +52,34 @@ public struct OAuth2ClientConfiguration: Equatable { self.guestPassword = guestPassword self.environment = environment } + + /// Creates a new OAuth2ClientConfiguration for a public client, i.e. one that holds no client secret. + /// The client authenticates by sending its `client_id` in the token request body and omits the + /// `Authorization: Basic` header. + /// - Parameters: + /// - publicClientIdentifier: The OAuth2 client identifier + /// - environment: The OAuth2 server application environment that the client communicates with + /// - guestUsername: The guest user's username, if one exists or is needed for client-level authorization + /// - guestPassword: The guest user's password, if one exists or is needed for client-level authorization + public init(publicClientIdentifier: String, + environment: OAuth2ServerEnvironment, + guestUsername: String? = nil, + guestPassword: String? = nil) { + self.init(clientIdentifier: publicClientIdentifier, + clientSecret: "", + environment: environment, + guestUsername: guestUsername, + guestPassword: guestPassword) + self.isPublicClient = true + } + + /// The `BasicToken` for client authentication, or `nil` for a public client, which authenticates + /// with `client_id` in the request body instead. Reading through `isPublicClient` keeps a public + /// client from ever emitting a Basic header, even if a secret was mistakenly assigned. + var basicToken: BasicToken? { + guard !isPublicClient else { + return nil + } + return BasicToken(username: clientIdentifier, password: clientSecret) + } } diff --git a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift index 0ad38f67..0cc9cd49 100644 --- a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift +++ b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift @@ -239,10 +239,12 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { } } else { - // Apply basic header logger.verbose("Client doesn't require a bearer token. Proceeding with a basic token...") - let basicToken = BasicToken(username: clientConfiguration.clientIdentifier, - password: clientConfiguration.clientSecret) + guard let basicToken = clientConfiguration.basicToken else { + logger.warn("Client-level basic authorization was requested for a public client, which has no secret") + completion(.error(OAuth2Error.clientFailure(nil, nil))) + return + } makeRequestByApplyingAuthorizationHeader(to: request, with: basicToken, completion: completion) diff --git a/Sources/Conduit/Auth/TokenGrants/OAuth2TokenGrantStrategy.swift b/Sources/Conduit/Auth/TokenGrants/OAuth2TokenGrantStrategy.swift index 5f2b36b2..59af6103 100644 --- a/Sources/Conduit/Auth/TokenGrants/OAuth2TokenGrantStrategy.swift +++ b/Sources/Conduit/Auth/TokenGrants/OAuth2TokenGrantStrategy.swift @@ -31,8 +31,6 @@ extension OAuth2TokenGrantStrategy { grantType: String, additionalGrantParameters: [String: Any], requestSerializer: RequestSerializer = JSONRequestSerializer()) throws -> URLRequest { - let basicToken = BasicToken(username: clientConfiguration.clientIdentifier, password: clientConfiguration.clientSecret) - let requestBuilder = HTTPRequestBuilder(url: clientConfiguration.environment.tokenGrantURL) var parameters: [String: Any] = [ "grant_type": grantType @@ -42,6 +40,11 @@ extension OAuth2TokenGrantStrategy { parameters["scope"] = scope } + // Set before merging additionalGrantParameters so a grant-supplied client_id still wins. + if clientConfiguration.isPublicClient { + parameters["client_id"] = clientConfiguration.clientIdentifier + } + for additionalParameter in additionalGrantParameters { parameters[additionalParameter.key] = additionalParameter.value } @@ -51,7 +54,9 @@ extension OAuth2TokenGrantStrategy { requestBuilder.serializer = requestSerializer do { var request = try requestBuilder.build() - request.setValue(basicToken.authorizationHeaderValue, forHTTPHeaderField: "Authorization") + if let basicToken = clientConfiguration.basicToken { + request.setValue(basicToken.authorizationHeaderValue, forHTTPHeaderField: "Authorization") + } return request } catch let error { diff --git a/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift b/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift index 9863cd69..e9900e0c 100644 --- a/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift @@ -13,6 +13,20 @@ class OAuth2ClientConfigurationTests: XCTestCase { let environment = OAuth2ServerEnvironment(scope: "baz", tokenGrantURL: URL(fileURLWithPath: "baz.com")) + func testConfidentialClientByDefault() { + let configuration = OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "bar", + environment: environment) + XCTAssertFalse(configuration.isPublicClient) + XCTAssertEqual(configuration.clientSecret, "bar") + } + + func testPublicClientHasNoSecret() { + let configuration = OAuth2ClientConfiguration(publicClientIdentifier: "foo", environment: environment) + XCTAssertTrue(configuration.isPublicClient) + XCTAssertEqual(configuration.clientIdentifier, "foo") + XCTAssertTrue(configuration.clientSecret.isEmpty) + } + func testEquality() { XCTAssertEqual(OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "bar", environment: environment), diff --git a/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift b/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift new file mode 100644 index 00000000..e02426ef --- /dev/null +++ b/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift @@ -0,0 +1,130 @@ +// +// OAuth2PublicClientTokenGrantTests.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import XCTest +@testable import Conduit + +/// A public client sends `client_id` in the body and no `Authorization: Basic` header; a confidential +/// client sends the header and no `client_id`. Pinned across every grant type. +class OAuth2PublicClientTokenGrantTests: XCTestCase { + + let clientIdentifier = "herp" + let clientSecret = "derp" + + private func environment() throws -> OAuth2ServerEnvironment { + OAuth2ServerEnvironment(tokenGrantURL: try URL(absoluteString: "https://httpbingo.org/get")) + } + + private func confidentialConfiguration() throws -> OAuth2ClientConfiguration { + OAuth2ClientConfiguration(clientIdentifier: clientIdentifier, clientSecret: clientSecret, + environment: try environment()) + } + + private func publicConfiguration() throws -> OAuth2ClientConfiguration { + OAuth2ClientConfiguration(publicClientIdentifier: clientIdentifier, environment: try environment()) + } + + private func bodyAndHeaders(of request: URLRequest) throws -> ([String: String], [String: String]) { + guard let body = request.httpBody, + let bodyParameters = AuthTestUtilities.deserialize(urlEncodedParameterData: body), + let headers = request.allHTTPHeaderFields else { + throw OAuth2Error.internalFailure + } + return (bodyParameters, headers) + } + + private func assertPublicShape(_ request: URLRequest, file: StaticString = #file, line: UInt = #line) throws { + let (bodyParameters, headers) = try bodyAndHeaders(of: request) + XCTAssertEqual(bodyParameters["client_id"], clientIdentifier, file: file, line: line) + XCTAssertNil(headers["Authorization"], file: file, line: line) + } + + private func assertConfidentialShape(_ request: URLRequest, file: StaticString = #file, line: UInt = #line) throws { + let (bodyParameters, headers) = try bodyAndHeaders(of: request) + XCTAssertNil(bodyParameters["client_id"], file: file, line: line) + XCTAssertEqual(headers["Authorization"]?.contains("Basic"), true, file: file, line: line) + } + + // MARK: - Authorization code + + func testAuthorizationCodeGrantPublicClient() throws { + let sut = OAuth2AuthorizationCodeTokenGrantStrategy(code: "hunter2", redirectURI: "x-oauth2-myapp://authorize", + clientConfiguration: try publicConfiguration()) + try assertPublicShape(try sut.buildTokenGrantRequest()) + } + + func testAuthorizationCodeGrantConfidentialClient() throws { + let sut = OAuth2AuthorizationCodeTokenGrantStrategy(code: "hunter2", redirectURI: "x-oauth2-myapp://authorize", + clientConfiguration: try confidentialConfiguration()) + try assertConfidentialShape(try sut.buildTokenGrantRequest()) + } + + // MARK: - Refresh token + + func testRefreshTokenGrantPublicClient() throws { + let sut = OAuth2RefreshTokenGrantStrategy(refreshToken: "refresh-abc", clientConfiguration: try publicConfiguration()) + let request = try sut.buildTokenGrantRequest() + try assertPublicShape(request) + let (bodyParameters, _) = try bodyAndHeaders(of: request) + XCTAssertEqual(bodyParameters["grant_type"], "refresh_token") + XCTAssertEqual(bodyParameters["refresh_token"], "refresh-abc") + } + + func testRefreshTokenGrantConfidentialClient() throws { + let sut = OAuth2RefreshTokenGrantStrategy(refreshToken: "refresh-abc", clientConfiguration: try confidentialConfiguration()) + let request = try sut.buildTokenGrantRequest() + try assertConfidentialShape(request) + let (bodyParameters, _) = try bodyAndHeaders(of: request) + XCTAssertEqual(bodyParameters["grant_type"], "refresh_token") + XCTAssertEqual(bodyParameters["refresh_token"], "refresh-abc") + } + + // MARK: - Extension grant (delegation) + + func testExtensionGrantPublicClient() throws { + var sut = OAuth2ExtensionTokenGrantStrategy(grantType: "delegation", clientConfiguration: try publicConfiguration()) + sut.tokenGrantRequestAdditionalBodyParameters = ["assertion": "123abc"] + let request = try sut.buildTokenGrantRequest() + try assertPublicShape(request) + let (bodyParameters, _) = try bodyAndHeaders(of: request) + XCTAssertEqual(bodyParameters["grant_type"], "delegation") + XCTAssertEqual(bodyParameters["assertion"], "123abc") + } + + func testExtensionGrantConfidentialClient() throws { + var sut = OAuth2ExtensionTokenGrantStrategy(grantType: "delegation", clientConfiguration: try confidentialConfiguration()) + sut.tokenGrantRequestAdditionalBodyParameters = ["assertion": "123abc"] + try assertConfidentialShape(try sut.buildTokenGrantRequest()) + } + + // MARK: - Password grant + + func testPasswordGrantPublicClient() throws { + let sut = OAuth2PasswordTokenGrantStrategy(username: "user", password: "pass", + clientConfiguration: try publicConfiguration()) + try assertPublicShape(try sut.buildTokenGrantRequest()) + } + + func testPasswordGrantConfidentialClient() throws { + let sut = OAuth2PasswordTokenGrantStrategy(username: "user", password: "pass", + clientConfiguration: try confidentialConfiguration()) + try assertConfidentialShape(try sut.buildTokenGrantRequest()) + } + + // MARK: - Client credentials grant + + func testClientCredentialsGrantPublicClient() throws { + let sut = OAuth2ClientCredentialsTokenGrantStrategy(clientConfiguration: try publicConfiguration()) + try assertPublicShape(try sut.buildTokenGrantRequest()) + } + + func testClientCredentialsGrantConfidentialClient() throws { + let sut = OAuth2ClientCredentialsTokenGrantStrategy(clientConfiguration: try confidentialConfiguration()) + try assertConfidentialShape(try sut.buildTokenGrantRequest()) + } + +} diff --git a/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift b/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift index 499af730..86366639 100644 --- a/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift @@ -353,5 +353,42 @@ class OAuth2RequestPipelineMiddlewareTests: XCTestCase { XCTAssertEqual(firstHeader, header) } } + + func testAppliesBasicHeaderForConfidentialClientLevelBasicAuthorization() throws { + let authorization = OAuth2Authorization(type: .basic, level: .client) + let sut = OAuth2RequestPipelineMiddleware(clientConfiguration: try makeValidClientConfiguration(), + authorization: authorization, tokenStorage: OAuth2TokenMemoryStore()) + let request = try makeDummyRequest() + + let completionExpectation = expectation(description: "completion handler executed") + sut.prepareForTransport(request: request) { result in + XCTAssertEqual(result.value?.allHTTPHeaderFields?["Authorization"]?.contains("Basic"), true) + completionExpectation.fulfill() + } + + waitForExpectations(timeout: 2) + } + + func testFailsClientLevelBasicAuthorizationForPublicClient() throws { + let environment = OAuth2ServerEnvironment(scope: "all the things", + tokenGrantURL: try URL(absoluteString: "http://localhost:5000/oauth2/issue/token")) + let publicConfiguration = OAuth2ClientConfiguration(publicClientIdentifier: validClientID, environment: environment) + let authorization = OAuth2Authorization(type: .basic, level: .client) + let sut = OAuth2RequestPipelineMiddleware(clientConfiguration: publicConfiguration, + authorization: authorization, tokenStorage: OAuth2TokenMemoryStore()) + let request = try makeDummyRequest() + + let completionExpectation = expectation(description: "completion handler executed") + sut.prepareForTransport(request: request) { result in + guard let error = result.error, case OAuth2Error.clientFailure = error else { + XCTFail("Expected a clientFailure for a public client requesting client-level basic authorization") + return + } + XCTAssertNil(result.value?.allHTTPHeaderFields?["Authorization"]) + completionExpectation.fulfill() + } + + waitForExpectations(timeout: 2) + } } // swiftlint:enable type_body_length diff --git a/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift b/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift index 4ea86abc..aacb1f43 100644 --- a/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift @@ -41,6 +41,26 @@ class OAuth2TokenStoreTests: XCTestCase { XCTAssertNil(storedToken) } + /// The token identifier must not depend on the client secret or on whether the client is public, + /// so switching a client to public resolves the same stored token and does not log the user out. + func testTokenIdentifierIgnoresSecretAndPublicFlag() { + let environment = OAuth2ServerEnvironment(scope: "foos", tokenGrantURL: URL(fileURLWithPath: "yay")) + let authorization = OAuth2Authorization(type: .bearer, level: .user) + let store = OAuth2TokenMemoryStore() + + let confidential = OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "bar", + environment: environment) + let differentSecret = OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "changed", + environment: environment) + let publicClient = OAuth2ClientConfiguration(publicClientIdentifier: "foo", environment: environment) + + let confidentialIdentifier = store.tokenIdentifierFor(clientConfiguration: confidential, authorization: authorization) + XCTAssertEqual(store.tokenIdentifierFor(clientConfiguration: differentSecret, authorization: authorization), + confidentialIdentifier) + XCTAssertEqual(store.tokenIdentifierFor(clientConfiguration: publicClient, authorization: authorization), + confidentialIdentifier) + } + private class CustomStore: OAuth2TokenStore { var tokenMap: [String: OAuth2Token] = [:] var tokenLocks: [String: Date] = [:] From d23c0e35771a23bcdbb13302214b3b0c7df4e007 Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Wed, 9 Sep 2026 20:16:37 +0530 Subject: [PATCH 2/3] T 1551265: Fail public client-level bearer without guest credentials; tighten docs and tests --- CHANGELOG.md | 11 ++- .../OAuth2ClientConfiguration.swift | 30 +++++--- .../OAuth2RequestPipelineMiddleware.swift | 16 ++-- .../Auth/OAuth2ClientConfigurationTests.swift | 13 ++++ ...dlewareClientLevelAuthorizationTests.swift | 73 +++++++++++++++++++ .../OAuth2PublicClientTokenGrantTests.swift | 20 +++++ ...OAuth2RequestPipelineMiddlewareTests.swift | 38 ---------- .../Auth/OAuth2TokenStoreTests.swift | 3 +- 8 files changed, 145 insertions(+), 59 deletions(-) create mode 100644 Tests/ConduitTests/Auth/OAuth2MiddlewareClientLevelAuthorizationTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 2325c81f..de17ce09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,14 @@ #### Enhancements - Support public OAuth2 clients that hold no client secret. `OAuth2ClientConfiguration` gains a secret-less initializer (`init(publicClientIdentifier:environment:guestUsername:guestPassword:)`) - and an `isPublicClient` flag. A public client sends its `client_id` in the token grant body and - omits the `Authorization: Basic` header across every grant type; a confidential client is - unchanged. + and an `isPublicClient` flag. A public client identifies itself with `client_id` in the token grant + body and omits the `Authorization: Basic` header across every grant type; a confidential client is + unchanged. Client-level authorization for a public client needs guest credentials: client-level + basic and the `client_credentials` grant both need a secret and fail with + `OAuth2Error.internalFailure` (RFC 6749 §4.4). A public client using `authorization_code` has no + code-to-client binding, so the authorization server must enforce PKCE (RFC 8252 §6); pass + `code_challenge` via `OAuth2AuthorizationRequest.additionalParameters` and `code_verifier` via + `tokenGrantRequestAdditionalBodyParameters`. #### Bug Fixes - None diff --git a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift index 31489800..573c72e6 100644 --- a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift +++ b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift @@ -18,11 +18,12 @@ public struct OAuth2ClientConfiguration: Equatable { /// non-empty secret to a public client, as it will not be sent. public var clientSecret: String - /// Whether the client is public, i.e. it holds no client secret. A public client sends its - /// `client_id` in the token request body and omits the `Authorization: Basic` header across every - /// grant type, and supports only bearer client-level authorization (a client-level basic request - /// fails, as there is no secret to send). A confidential client (the default) is unchanged. Set - /// only through the public-client initializer. + /// Whether the client is public, i.e. it holds no client secret. A public client identifies itself + /// with `client_id` in the token request body and omits the `Authorization: Basic` header across + /// every grant type. Client-level authorization needs guest credentials (password grant); client-level + /// basic and the `client_credentials` grant both need a secret and fail with + /// `OAuth2Error.internalFailure`. A confidential client (the default) is unchanged. Set only through + /// the public-client initializer. public private(set) var isPublicClient: Bool = false /// The guest user's username, if one exists or is needed for client-level authorization @@ -54,13 +55,22 @@ public struct OAuth2ClientConfiguration: Equatable { } /// Creates a new OAuth2ClientConfiguration for a public client, i.e. one that holds no client secret. - /// The client authenticates by sending its `client_id` in the token request body and omits the - /// `Authorization: Basic` header. + /// The client identifies itself with `client_id` in the token request body and omits the + /// `Authorization: Basic` header; `client_id` is identification, not authentication (RFC 6749 §3.2.1). + /// + /// A public client using the `authorization_code` grant has no code-to-client binding, so the + /// authorization server must enforce PKCE (RFC 8252 §6): send `code_challenge` through + /// `OAuth2AuthorizationRequest.additionalParameters` and `code_verifier` through + /// `OAuth2AuthorizationCodeTokenGrantStrategy.tokenGrantRequestAdditionalBodyParameters`. + /// + /// Client-level authorization requires guest credentials: without them the `client_credentials` + /// grant would be sent with no credential at all, which RFC 6749 §4.4 reserves for confidential + /// clients, so the request fails with `OAuth2Error.internalFailure` instead. /// - Parameters: /// - publicClientIdentifier: The OAuth2 client identifier /// - environment: The OAuth2 server application environment that the client communicates with - /// - guestUsername: The guest user's username, if one exists or is needed for client-level authorization - /// - guestPassword: The guest user's password, if one exists or is needed for client-level authorization + /// - guestUsername: The guest user's username, required for client-level authorization + /// - guestPassword: The guest user's password, required for client-level authorization public init(publicClientIdentifier: String, environment: OAuth2ServerEnvironment, guestUsername: String? = nil, @@ -73,7 +83,7 @@ public struct OAuth2ClientConfiguration: Equatable { self.isPublicClient = true } - /// The `BasicToken` for client authentication, or `nil` for a public client, which authenticates + /// The `BasicToken` for client authentication, or `nil` for a public client, which identifies itself /// with `client_id` in the request body instead. Reading through `isPublicClient` keeps a public /// client from ever emitting a Basic header, even if a secret was mistakenly assigned. var basicToken: BasicToken? { diff --git a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift index 0cc9cd49..b589afb1 100644 --- a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift +++ b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift @@ -229,9 +229,7 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { self.tokenStorage.store(token: newToken, for: self.clientConfiguration, with: self.authorization) - self.makeRequestByApplyingAuthorizationHeader(to: request, - with: newToken, - completion: completion) + self.makeRequestByApplyingAuthorizationHeader(to: request, with: newToken, completion: completion) } Auth.Migrator.notifyTokenPostFetchHooksWith(client: self.clientConfiguration, authorizationLevel: self.authorization.level, @@ -242,12 +240,10 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { logger.verbose("Client doesn't require a bearer token. Proceeding with a basic token...") guard let basicToken = clientConfiguration.basicToken else { logger.warn("Client-level basic authorization was requested for a public client, which has no secret") - completion(.error(OAuth2Error.clientFailure(nil, nil))) + completion(.error(OAuth2Error.internalFailure)) return } - makeRequestByApplyingAuthorizationHeader(to: request, - with: basicToken, - completion: completion) + makeRequestByApplyingAuthorizationHeader(to: request, with: basicToken, completion: completion) } } @@ -262,6 +258,12 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { logger.verbose("Guest user credentials exist. Attempting password grant...") authenticationStrategy = OAuth2PasswordTokenGrantStrategy(username: username, password: password, clientConfiguration: clientConfiguration) } + else if clientConfiguration.isPublicClient { + // The client_credentials grant is reserved for confidential clients (RFC 6749 §4.4). + logger.warn("Client-level bearer authorization was requested for a public client without guest credentials") + completion(.error(OAuth2Error.internalFailure)) + return + } else { logger.verbose("Guest user credentials do not exist. Attempting client_credentials grant...") authenticationStrategy = OAuth2ClientCredentialsTokenGrantStrategy(clientConfiguration: clientConfiguration) diff --git a/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift b/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift index e9900e0c..1a515ef4 100644 --- a/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2ClientConfigurationTests.swift @@ -27,6 +27,19 @@ class OAuth2ClientConfigurationTests: XCTestCase { XCTAssertTrue(configuration.clientSecret.isEmpty) } + func testPublicClientCarriesGuestCredentials() { + let configuration = OAuth2ClientConfiguration(publicClientIdentifier: "foo", environment: environment, + guestUsername: "u", guestPassword: "p") + XCTAssertEqual(configuration.guestUsername, "u") + XCTAssertEqual(configuration.guestPassword, "p") + } + + func testPublicAndConfidentialClientsWithSameIdentifierAreNotEqual() { + XCTAssertNotEqual(OAuth2ClientConfiguration(publicClientIdentifier: "foo", environment: environment), + OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "", + environment: environment)) + } + func testEquality() { XCTAssertEqual(OAuth2ClientConfiguration(clientIdentifier: "foo", clientSecret: "bar", environment: environment), diff --git a/Tests/ConduitTests/Auth/OAuth2MiddlewareClientLevelAuthorizationTests.swift b/Tests/ConduitTests/Auth/OAuth2MiddlewareClientLevelAuthorizationTests.swift new file mode 100644 index 00000000..420fa5ae --- /dev/null +++ b/Tests/ConduitTests/Auth/OAuth2MiddlewareClientLevelAuthorizationTests.swift @@ -0,0 +1,73 @@ +// +// OAuth2MiddlewareClientLevelAuthorizationTests.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import XCTest +@testable import Conduit + +/// Client-level authorization with no stored token: a confidential client applies its Basic header, +/// while a public client has no secret and must fail before any request is built. +class OAuth2MiddlewareClientLevelAuthorizationTests: XCTestCase { + + private let clientIdentifier = "test_client" + + private func environment() throws -> OAuth2ServerEnvironment { + OAuth2ServerEnvironment(scope: "all the things", + tokenGrantURL: try URL(absoluteString: "http://localhost:5000/oauth2/issue/token")) + } + + private func confidentialConfiguration() throws -> OAuth2ClientConfiguration { + OAuth2ClientConfiguration(clientIdentifier: clientIdentifier, clientSecret: "test_secret", environment: try environment()) + } + + private func publicConfiguration() throws -> OAuth2ClientConfiguration { + OAuth2ClientConfiguration(publicClientIdentifier: clientIdentifier, environment: try environment()) + } + + private func makeDummyRequest() throws -> URLRequest { + let requestBuilder = HTTPRequestBuilder(url: try URL(absoluteString: "https://httpbingo.org/post")) + requestBuilder.method = .GET + return try requestBuilder.build() + } + + private func prepare(clientConfiguration: OAuth2ClientConfiguration, + authorization: OAuth2Authorization) throws -> Result { + let sut = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, tokenStorage: OAuth2TokenMemoryStore()) + var outcome: Result? + let completionExpectation = expectation(description: "completion handler executed") + sut.prepareForTransport(request: try makeDummyRequest()) { result in + outcome = result + completionExpectation.fulfill() + } + waitForExpectations(timeout: 2) + return try XCTUnwrap(outcome) + } + + func testConfidentialClientLevelBasicAppliesBasicHeader() throws { + let result = try prepare(clientConfiguration: try confidentialConfiguration(), + authorization: OAuth2Authorization(type: .basic, level: .client)) + XCTAssertEqual(result.value?.allHTTPHeaderFields?["Authorization"]?.contains("Basic"), true) + } + + func testPublicClientLevelBasicFailsWithInternalFailure() throws { + let result = try prepare(clientConfiguration: try publicConfiguration(), + authorization: OAuth2Authorization(type: .basic, level: .client)) + guard let error = result.error, case OAuth2Error.internalFailure = error else { + return XCTFail("Expected an internalFailure: a public client has no secret to send") + } + } + + func testPublicClientLevelBearerWithoutGuestCredentialsFailsWithInternalFailure() throws { + // The client_credentials grant is reserved for confidential clients (RFC 6749 §4.4). + let result = try prepare(clientConfiguration: try publicConfiguration(), + authorization: OAuth2Authorization(type: .bearer, level: .client)) + guard let error = result.error, case OAuth2Error.internalFailure = error else { + return XCTFail("Expected an internalFailure instead of an unauthenticated client_credentials grant") + } + } + +} diff --git a/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift b/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift index e02426ef..7f66de45 100644 --- a/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2PublicClientTokenGrantTests.swift @@ -127,4 +127,24 @@ class OAuth2PublicClientTokenGrantTests: XCTestCase { try assertConfidentialShape(try sut.buildTokenGrantRequest()) } + // MARK: - Invariants + + func testPublicClientNeverSendsBasicHeaderEvenWithAnAssignedSecret() throws { + var configuration = try publicConfiguration() + configuration.clientSecret = "leaked-from-somewhere" + XCTAssertNil(configuration.basicToken) + + let sut = OAuth2ClientCredentialsTokenGrantStrategy(clientConfiguration: configuration) + try assertPublicShape(try sut.buildTokenGrantRequest()) + } + + func testGrantSuppliedClientIdentifierWinsForPublicClient() throws { + var sut = OAuth2ExtensionTokenGrantStrategy(grantType: "delegation", clientConfiguration: try publicConfiguration()) + sut.tokenGrantRequestAdditionalBodyParameters = ["client_id": "grant-supplied"] + + let (bodyParameters, headers) = try bodyAndHeaders(of: try sut.buildTokenGrantRequest()) + XCTAssertEqual(bodyParameters["client_id"], "grant-supplied") + XCTAssertNil(headers["Authorization"]) + } + } diff --git a/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift b/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift index 86366639..a204db0e 100644 --- a/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2RequestPipelineMiddlewareTests.swift @@ -31,7 +31,6 @@ struct CustomRefreshTokenGrantStrategyFactory: OAuth2RefreshStrategyFactory { } } -// swiftlint:disable type_body_length class OAuth2RequestPipelineMiddlewareTests: XCTestCase { let validClientID = "test_client" @@ -354,41 +353,4 @@ class OAuth2RequestPipelineMiddlewareTests: XCTestCase { } } - func testAppliesBasicHeaderForConfidentialClientLevelBasicAuthorization() throws { - let authorization = OAuth2Authorization(type: .basic, level: .client) - let sut = OAuth2RequestPipelineMiddleware(clientConfiguration: try makeValidClientConfiguration(), - authorization: authorization, tokenStorage: OAuth2TokenMemoryStore()) - let request = try makeDummyRequest() - - let completionExpectation = expectation(description: "completion handler executed") - sut.prepareForTransport(request: request) { result in - XCTAssertEqual(result.value?.allHTTPHeaderFields?["Authorization"]?.contains("Basic"), true) - completionExpectation.fulfill() - } - - waitForExpectations(timeout: 2) - } - - func testFailsClientLevelBasicAuthorizationForPublicClient() throws { - let environment = OAuth2ServerEnvironment(scope: "all the things", - tokenGrantURL: try URL(absoluteString: "http://localhost:5000/oauth2/issue/token")) - let publicConfiguration = OAuth2ClientConfiguration(publicClientIdentifier: validClientID, environment: environment) - let authorization = OAuth2Authorization(type: .basic, level: .client) - let sut = OAuth2RequestPipelineMiddleware(clientConfiguration: publicConfiguration, - authorization: authorization, tokenStorage: OAuth2TokenMemoryStore()) - let request = try makeDummyRequest() - - let completionExpectation = expectation(description: "completion handler executed") - sut.prepareForTransport(request: request) { result in - guard let error = result.error, case OAuth2Error.clientFailure = error else { - XCTFail("Expected a clientFailure for a public client requesting client-level basic authorization") - return - } - XCTAssertNil(result.value?.allHTTPHeaderFields?["Authorization"]) - completionExpectation.fulfill() - } - - waitForExpectations(timeout: 2) - } } -// swiftlint:enable type_body_length diff --git a/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift b/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift index aacb1f43..dd8b2543 100644 --- a/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2TokenStoreTests.swift @@ -42,7 +42,8 @@ class OAuth2TokenStoreTests: XCTestCase { } /// The token identifier must not depend on the client secret or on whether the client is public, - /// so switching a client to public resolves the same stored token and does not log the user out. + /// so switching a client to public resolves the same stored token. Whether the server still accepts + /// that token is the server's policy, not the store's. func testTokenIdentifierIgnoresSecretAndPublicFlag() { let environment = OAuth2ServerEnvironment(scope: "foos", tokenGrantURL: URL(fileURLWithPath: "yay")) let authorization = OAuth2Authorization(type: .bearer, level: .user) From e3a03e6f5e075353a0693a1e762808151e7d9071 Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Wed, 9 Sep 2026 23:49:16 +0530 Subject: [PATCH 3/3] T 1551265: Narrow the public-client docs to what the middleware enforces --- CHANGELOG.md | 7 ++++--- .../OAuth2ClientConfiguration.swift | 20 ++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de17ce09..b515d3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,10 @@ secret-less initializer (`init(publicClientIdentifier:environment:guestUsername:guestPassword:)`) and an `isPublicClient` flag. A public client identifies itself with `client_id` in the token grant body and omits the `Authorization: Basic` header across every grant type; a confidential client is - unchanged. Client-level authorization for a public client needs guest credentials: client-level - basic and the `client_credentials` grant both need a secret and fail with - `OAuth2Error.internalFailure` (RFC 6749 §4.4). A public client using `authorization_code` has no + unchanged. Client-level authorization for a public client through `OAuth2RequestPipelineMiddleware` + needs guest credentials: client-level basic, and client-level bearer without guest credentials, fail + with `OAuth2Error.internalFailure` rather than sending a request with no credential + (RFC 6749 §4.4). A public client using `authorization_code` has no code-to-client binding, so the authorization server must enforce PKCE (RFC 8252 §6); pass `code_challenge` via `OAuth2AuthorizationRequest.additionalParameters` and `code_verifier` via `tokenGrantRequestAdditionalBodyParameters`. diff --git a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift index 573c72e6..32dda723 100644 --- a/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift +++ b/Sources/Conduit/Auth/Models/Configuration/OAuth2ClientConfiguration.swift @@ -20,10 +20,10 @@ public struct OAuth2ClientConfiguration: Equatable { /// Whether the client is public, i.e. it holds no client secret. A public client identifies itself /// with `client_id` in the token request body and omits the `Authorization: Basic` header across - /// every grant type. Client-level authorization needs guest credentials (password grant); client-level - /// basic and the `client_credentials` grant both need a secret and fail with - /// `OAuth2Error.internalFailure`. A confidential client (the default) is unchanged. Set only through - /// the public-client initializer. + /// every grant type. Client-level authorization through `OAuth2RequestPipelineMiddleware` needs guest + /// credentials (password grant): client-level basic, and client-level bearer without guest credentials, + /// fail with `OAuth2Error.internalFailure`. A confidential client (the default) is unchanged. Set only + /// through the public-client initializer. public private(set) var isPublicClient: Bool = false /// The guest user's username, if one exists or is needed for client-level authorization @@ -63,14 +63,16 @@ public struct OAuth2ClientConfiguration: Equatable { /// `OAuth2AuthorizationRequest.additionalParameters` and `code_verifier` through /// `OAuth2AuthorizationCodeTokenGrantStrategy.tokenGrantRequestAdditionalBodyParameters`. /// - /// Client-level authorization requires guest credentials: without them the `client_credentials` - /// grant would be sent with no credential at all, which RFC 6749 §4.4 reserves for confidential - /// clients, so the request fails with `OAuth2Error.internalFailure` instead. + /// Client-level authorization through `OAuth2RequestPipelineMiddleware` requires guest credentials: + /// without them the middleware would have to send a `client_credentials` grant with no credential, + /// which RFC 6749 §4.4 reserves for confidential clients, so it fails with + /// `OAuth2Error.internalFailure` instead. Constructing `OAuth2ClientCredentialsTokenGrantStrategy` + /// directly is not guarded. /// - Parameters: /// - publicClientIdentifier: The OAuth2 client identifier /// - environment: The OAuth2 server application environment that the client communicates with - /// - guestUsername: The guest user's username, required for client-level authorization - /// - guestPassword: The guest user's password, required for client-level authorization + /// - guestUsername: The guest user's username; `nil` unless client-level authorization is used, which requires it + /// - guestPassword: The guest user's password; `nil` unless client-level authorization is used, which requires it public init(publicClientIdentifier: String, environment: OAuth2ServerEnvironment, guestUsername: String? = nil,