From a7df723459625ad54b0aa1d5bb6a21ca7ce3db26 Mon Sep 17 00:00:00 2001 From: Kevin McKee Date: Fri, 13 Jun 2025 12:50:55 -0700 Subject: [PATCH 1/5] WIP for Option 1 - looking at code coverage and ease of configuration and use for consuming clients. --- README.md | 5 +- Sources/OAuthKit/Views/OAWebView.swift | 10 ++-- .../OAuthKit/Views/OAWebViewCoordinator.swift | 6 +-- Tests/OAuthKitTests/OAWebViewTests.swift | 46 ++++++++++++++++--- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index fb948e6..4cb9242 100644 --- a/README.md +++ b/README.md @@ -25,15 +25,18 @@ import SwiftUI @main struct OAuthApp: App { + let oauth: OAuth = .init(.main) + /// Build the scene body var body: some Scene { WindowGroup { ContentView() } + .environment(\.oauth, oauth) WindowGroup(id: "oauth") { - OAWebView() + OAWebView(oauth: oauth) } } } diff --git a/Sources/OAuthKit/Views/OAWebView.swift b/Sources/OAuthKit/Views/OAWebView.swift index a3989fb..17f14af 100644 --- a/Sources/OAuthKit/Views/OAWebView.swift +++ b/Sources/OAuthKit/Views/OAWebView.swift @@ -14,12 +14,14 @@ import WebKit @MainActor public struct OAWebView { - @Environment(\.oauth) - var oauth: OAuth + let oauth: OAuth let view = WKWebView() - /// Public Initializer. - public init() { } + /// Initializer with the speciifed oauth object, + /// - Parameter oauth: the oauth object to use + public init(oauth: OAuth = .init(.main)) { + self.oauth = oauth + } public func makeWebView(context: Context) -> WKWebView { view.navigationDelegate = context.coordinator diff --git a/Sources/OAuthKit/Views/OAWebViewCoordinator.swift b/Sources/OAuthKit/Views/OAWebViewCoordinator.swift index cd82810..d9ec6a8 100644 --- a/Sources/OAuthKit/Views/OAWebViewCoordinator.swift +++ b/Sources/OAuthKit/Views/OAWebViewCoordinator.swift @@ -55,10 +55,6 @@ public class OAWebViewCoordinator: NSObject { } // Exchange the code for a token oauth.token(provider: provider, code: code) - case .clientCredentials: - break - case .deviceCode: - break case .pkce(let pkce): // Verify the state guard state == pkce.state else { @@ -70,7 +66,7 @@ public class OAWebViewCoordinator: NSObject { } // Exchange the code for a token along with the pkce validation data oauth.token(provider: provider, code: code, pkce: pkce) - case .refreshToken: + case .clientCredentials, .deviceCode, .refreshToken: break } } diff --git a/Tests/OAuthKitTests/OAWebViewTests.swift b/Tests/OAuthKitTests/OAWebViewTests.swift index 9de3f98..d997c36 100644 --- a/Tests/OAuthKitTests/OAWebViewTests.swift +++ b/Tests/OAuthKitTests/OAWebViewTests.swift @@ -25,10 +25,12 @@ final class OAWebViewTests { }() let oauth: OAuth + let webView: OAWebView /// Initializer. init() async throws { oauth = .init(.module) + webView = .init(oauth: oauth) oauth.urlSession = urlSession } @@ -42,23 +44,53 @@ final class OAWebViewTests { #expect(environmentOAuth == oauth) } - /// Tests the OAWebViewCoordinator coordinator policy decision. - /// TODO: This is fairly limited at the moment because we will receive errors - /// about accessing the `oauth` environment outside of being installed on a view. - /// Needs more investigation for testing SwiftUI views. + /// Tests the OAWebViewCoordinator coordinator policy decisions. @Test("Coordinator Policy Decisons") func whenCoordinatorDecidingPolicy() async throws { - let webView: OAWebView = .init() + // 1) Bad Request Expectations let coordinator: OAWebViewCoordinator = webView.makeCoordinator() + #expect(coordinator.oauth == oauth) let wkWebView = webView.view var urlRequest: URLRequest = .init(url: URL(string: "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/codefiesta/OAuthKit")!) urlRequest.url = nil - let navigationAction: WKNavigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) - let policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) + var navigationAction: WKNavigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) + var policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) #expect(policy == .cancel) + + let provider = oauth.providers[0] + + // 2) Authorization Code Expectations + let state: String = .secureRandom() + let code: String = .secureRandom() + + oauth.authorize(provider: provider, grantType: .authorizationCode(state)) + coordinator.update(state: oauth.state) + var urlString = provider.redirectURI! + "?code=\(code)&state=\(state)" + urlRequest = .init(url: URL(string: urlString)!) + + navigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) + policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) + #expect(policy == .allow) + + // 3) PKCE Expectations + let pkce: OAuth.PKCE = .init() + oauth.authorize(provider: provider, grantType: .pkce(pkce)) + coordinator.update(state: oauth.state) + urlString = provider.redirectURI! + "?code=\(code)&state=\(pkce.state)" + urlRequest = .init(url: URL(string: urlString)!) + + navigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) + policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) + #expect(policy == .allow) } + +// @Test("Coordinator State Updates") +// func whenCoordinatorHandlesUpdates() async throws { +// let coordinator: OAWebViewCoordinator = webView.makeCoordinator() +// coordinator.update(state: <#T##OAuth.State#>) +// } } #endif From a0a8cbf129e065a20a6e653c03d5f3e6c372cf4c Mon Sep 17 00:00:00 2001 From: Kevin McKee Date: Fri, 13 Jun 2025 13:28:52 -0700 Subject: [PATCH 2/5] Removing the default parameter as it seems like it could introduce confusion for clients. --- Sources/OAuthKit/Views/OAWebView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/OAuthKit/Views/OAWebView.swift b/Sources/OAuthKit/Views/OAWebView.swift index 17f14af..8ad5c30 100644 --- a/Sources/OAuthKit/Views/OAWebView.swift +++ b/Sources/OAuthKit/Views/OAWebView.swift @@ -19,7 +19,7 @@ public struct OAWebView { /// Initializer with the speciifed oauth object, /// - Parameter oauth: the oauth object to use - public init(oauth: OAuth = .init(.main)) { + public init(oauth: OAuth) { self.oauth = oauth } From e51c22932414ab9e1a0389fd975d3f74f4748db2 Mon Sep 17 00:00:00 2001 From: Kevin McKee Date: Fri, 13 Jun 2025 13:29:31 -0700 Subject: [PATCH 3/5] Cleanup --- Tests/OAuthKitTests/OAWebViewTests.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Tests/OAuthKitTests/OAWebViewTests.swift b/Tests/OAuthKitTests/OAWebViewTests.swift index d997c36..09f8e45 100644 --- a/Tests/OAuthKitTests/OAWebViewTests.swift +++ b/Tests/OAuthKitTests/OAWebViewTests.swift @@ -86,11 +86,5 @@ final class OAWebViewTests { policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) #expect(policy == .allow) } - -// @Test("Coordinator State Updates") -// func whenCoordinatorHandlesUpdates() async throws { -// let coordinator: OAWebViewCoordinator = webView.makeCoordinator() -// coordinator.update(state: <#T##OAuth.State#>) -// } } #endif From 17887d77cafa25728bf4a0d31284512ad3a398bb Mon Sep 17 00:00:00 2001 From: Kevin McKee Date: Sun, 15 Jun 2025 12:14:19 -0700 Subject: [PATCH 4/5] Test to check state mismatches don't allow token requests to happen. --- Tests/OAuthKitTests/OAWebViewTests.swift | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Tests/OAuthKitTests/OAWebViewTests.swift b/Tests/OAuthKitTests/OAWebViewTests.swift index 09f8e45..d5b24d2 100644 --- a/Tests/OAuthKitTests/OAWebViewTests.swift +++ b/Tests/OAuthKitTests/OAWebViewTests.swift @@ -86,5 +86,41 @@ final class OAWebViewTests { policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) #expect(policy == .allow) } + + /// Tests to make sure the coordinator doesn't being requesting access tokens when we've detected state mismatches. + @Test("Coordinator Detects Mismatched States") + func whenCoordinatorDetectsMismatchedStates() async throws { + + // 1) Bad Request Expectations + let coordinator: OAWebViewCoordinator = webView.makeCoordinator() + #expect(coordinator.oauth == oauth) + let wkWebView = webView.view + + var urlRequest: URLRequest = .init(url: URL(string: "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/codefiesta/OAuthKit")!) + let provider = oauth.providers[0] + + // 2) Authorization Code Expectations + let state: String = .secureRandom() + let code: String = .secureRandom() + + oauth.authorize(provider: provider, grantType: .authorizationCode(state)) + let urlString = provider.redirectURI! + "?code=\(code)&state=ABC-123" + urlRequest = .init(url: URL(string: urlString)!) + var navigationAction: WKNavigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) + var policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) + #expect(policy == .allow) + #expect(oauth.state != .requestingAccessToken(provider)) + + // 3) PKCE Expectations + let pkce: OAuth.PKCE = .init() + oauth.authorize(provider: provider, grantType: .pkce(pkce)) + coordinator.update(state: oauth.state) + + navigationAction = OAuthTestWKNavigationAction(urlRequest: urlRequest) + policy = await coordinator.webView(wkWebView, decidePolicyFor: navigationAction) + #expect(policy == .allow) + #expect(oauth.state != .requestingAccessToken(provider)) + + } } #endif From d9529b7ceeccf804c4eb486e64a721c27fd0725e Mon Sep 17 00:00:00 2001 From: Kevin McKee Date: Sun, 15 Jun 2025 12:58:16 -0700 Subject: [PATCH 5/5] README update --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 61b0b1d..8c2c356 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ import SwiftUI @main struct OAuthApp: App { + /// The observable oauth object let oauth: OAuth = .init(.main) /// Build the scene body