Conversation
There was a problem hiding this comment.
Pull request overview
Initial commit of the RxAuthSwift Swift Package, providing a core OAuth 2.0 + PKCE authentication manager (iOS/macOS) and an optional SwiftUI sign-in UI layer, along with build scripts and CI.
Changes:
- Add core RxAuthSwift module: configuration, PKCE helpers, OAuth manager, keychain token storage, and platform web auth sessions.
- Add RxAuthSwiftUI module: sign-in view, appearance configuration, and supporting UI components/modifiers.
- Add tests, documentation, build scripts, and a GitHub Actions CI workflow.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/build-macos.sh | macOS build + test script using SwiftPM. |
| scripts/build-ios.sh | iOS build script using xcodebuild. |
| Tests/RxAuthSwiftTests/RxAuthSwiftTests.swift | Unit tests for PKCE/config/user model/errors. |
| Sources/RxAuthSwiftUI/Styles/GlassEffectModifiers.swift | SwiftUI “glass” styling modifiers and fallbacks. |
| Sources/RxAuthSwiftUI/RxSignInView.swift | Main sign-in view wiring OAuthManager to UI. |
| Sources/RxAuthSwiftUI/RxSignInAppearance.swift | Appearance configuration model for the sign-in UI. |
| Sources/RxAuthSwiftUI/Components/SecondaryAuthButton.swift | Secondary action button component. |
| Sources/RxAuthSwiftUI/Components/PrimaryAuthButton.swift | Primary sign-in button with loading state. |
| Sources/RxAuthSwiftUI/Components/AuthErrorBanner.swift | Error banner UI with animation/haptics. |
| Sources/RxAuthSwiftUI/Components/AnimatedSecurityIcon.swift | Animated security icon component. |
| Sources/RxAuthSwiftUI/Components/AnimatedGradientBackground.swift | Animated gradient/orb background. |
| Sources/RxAuthSwiftUI/Components/AnimatedAppLogo.swift | Animated logo/icon header component. |
| Sources/RxAuthSwift/TokenStorageProtocol.swift | Protocol for access/refresh token persistence. |
| Sources/RxAuthSwift/RxAuthSwift.swift | Module entry/export convenience imports. |
| Sources/RxAuthSwift/RxAuthNotifications.swift | Notification for session expiry. |
| Sources/RxAuthSwift/RxAuthConfiguration.swift | OAuth configuration and endpoint URL construction. |
| Sources/RxAuthSwift/Platform/WebAuthSessionProvider.swift | Internal abstraction for platform web auth sessions. |
| Sources/RxAuthSwift/Platform/MacOSWebAuthSession.swift | macOS WKWebView-based authentication session. |
| Sources/RxAuthSwift/Platform/IOSWebAuthSession.swift | iOS ASWebAuthenticationSession implementation. |
| Sources/RxAuthSwift/PKCEHelper.swift | PKCE verifier/challenge generation. |
| Sources/RxAuthSwift/OAuthManager.swift | Main OAuth flow, token exchange/refresh, userinfo fetch. |
| Sources/RxAuthSwift/OAuthError.swift | Error types for OAuth/keychain operations. |
| Sources/RxAuthSwift/KeychainTokenStorage.swift | Keychain-backed token storage implementation. |
| Sources/RxAuthSwift/AuthenticationState.swift | Auth state enum and User model. |
| README.md | Usage documentation, setup, and examples. |
| Package.swift | SwiftPM package definition (targets/products/deps). |
| Package.resolved | Locked dependency resolution (swift-log). |
| .gitignore | Ignore build artifacts and local config files. |
| .github/workflows/ci.yml | CI jobs to build/test macOS and build iOS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public func refreshTokenIfNeeded() async throws { | ||
| guard let refreshToken = tokenStorage.getRefreshToken() else { | ||
| throw OAuthError.noRefreshToken | ||
| } | ||
|
|
There was a problem hiding this comment.
refreshTokenIfNeeded() refreshes any time a refresh token exists, even when the access token is still valid. With the repeating timer, this will trigger unnecessary refreshes and can cause rate limiting/refresh-token invalidation; add an early return when !tokenStorage.isTokenExpired() (and an access token exists).
| let body = [ | ||
| "grant_type=authorization_code", | ||
| "code=\(code)", | ||
| "redirect_uri=\(configuration.redirectURI)", | ||
| "client_id=\(configuration.clientID)", | ||
| "code_verifier=\(codeVerifier)", | ||
| ].joined(separator: "&") |
There was a problem hiding this comment.
This x-www-form-urlencoded token-exchange body is built via string interpolation without percent-encoding values such as code, redirect_uri, and code_verifier. Use proper form-url-encoding (e.g., URLQueryItem percent-encoding) to avoid malformed requests.
| let body = [ | |
| "grant_type=authorization_code", | |
| "code=\(code)", | |
| "redirect_uri=\(configuration.redirectURI)", | |
| "client_id=\(configuration.clientID)", | |
| "code_verifier=\(codeVerifier)", | |
| ].joined(separator: "&") | |
| var components = URLComponents() | |
| components.queryItems = [ | |
| URLQueryItem(name: "grant_type", value: "authorization_code"), | |
| URLQueryItem(name: "code", value: code), | |
| URLQueryItem(name: "redirect_uri", value: "\(configuration.redirectURI)"), | |
| URLQueryItem(name: "client_id", value: configuration.clientID), | |
| URLQueryItem(name: "code_verifier", value: codeVerifier), | |
| ] | |
| let body = components.percentEncodedQuery ?? "" |
| public enum SignInIcon: Sendable { | ||
| case systemImage(String) | ||
| case image(Image) | ||
| case assetImage(String, Bundle?) | ||
| case none |
There was a problem hiding this comment.
SignInIcon is marked Sendable but has a case that stores a SwiftUI Image, which is typically non-Sendable. This can break compilation with strict concurrency; consider replacing the Image payload with a sendable identifier (asset/system name) or dropping Sendable.
| nonisolated func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { | ||
| MainActor.assumeIsolated { | ||
| guard let scene = UIApplication.shared.connectedScenes.first as? UIWindowScene, | ||
| let window = scene.windows.first | ||
| else { |
There was a problem hiding this comment.
UIApplication/UIWindowScene are used below, but this file only imports AuthenticationServices. This will fail to compile on iOS unless you add import UIKit (inside the same #if os(iOS) guard) or otherwise avoid UIKit symbols here.
| nonisolated func webView( | ||
| _ webView: WKWebView, | ||
| decidePolicyFor navigationAction: WKNavigationAction, | ||
| decisionHandler: @escaping @MainActor @Sendable (WKNavigationActionPolicy) -> Void |
There was a problem hiding this comment.
The WKNavigationDelegate signature must match WebKit exactly. Adding @MainActor @Sendable to the decisionHandler parameter changes the closure type and can break protocol conformance/compilation; keep the original signature and hop to the main actor inside as needed.
| decisionHandler: @escaping @MainActor @Sendable (WKNavigationActionPolicy) -> Void | |
| decisionHandler: @escaping (WKNavigationActionPolicy) -> Void |
| let body = [ | ||
| "grant_type=refresh_token", | ||
| "refresh_token=\(refreshToken)", | ||
| "client_id=\(configuration.clientID)", | ||
| ].joined(separator: "&") | ||
| request.httpBody = body.data(using: .utf8) | ||
|
|
There was a problem hiding this comment.
This x-www-form-urlencoded body is built via string interpolation without percent-encoding values like refresh_token and client_id. If these contain reserved characters, the request will be malformed; build the body using proper form-url-encoding.
| let body = [ | |
| "grant_type=refresh_token", | |
| "refresh_token=\(refreshToken)", | |
| "client_id=\(configuration.clientID)", | |
| ].joined(separator: "&") | |
| request.httpBody = body.data(using: .utf8) | |
| var components = URLComponents() | |
| components.queryItems = [ | |
| URLQueryItem(name: "grant_type", value: "refresh_token"), | |
| URLQueryItem(name: "refresh_token", value: refreshToken), | |
| URLQueryItem(name: "client_id", value: configuration.clientID) | |
| ] | |
| request.httpBody = components.percentEncodedQuery?.data(using: .utf8) |
| public enum PKCEHelper: Sendable { | ||
| public static func generateCodeVerifier() -> String { | ||
| var bytes = [UInt8](repeating: 0, count: 32) | ||
| _ = SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) |
There was a problem hiding this comment.
The return status from SecRandomCopyBytes is ignored. If it fails, a weak/invalid PKCE verifier could be produced silently; check the OSStatus and fail/throw/retry instead of proceeding.
| _ = SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) | |
| let status = SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) | |
| guard status == errSecSuccess else { | |
| fatalError("Failed to generate secure random bytes for PKCE code verifier. OSStatus: \(status)") | |
| } |
| public struct RxSignInAppearance: Sendable { | ||
| public var icon: SignInIcon | ||
| public var title: String |
There was a problem hiding this comment.
RxSignInAppearance is declared Sendable, but it contains SwiftUI types (e.g., Color, and Image via SignInIcon) that are not guaranteed to satisfy strict Swift 6 sendability checks. Consider removing Sendable or storing sendable identifiers instead of SwiftUI values.
|
|
||
| @MainActor | ||
| @Observable | ||
| public final class OAuthManager: Sendable { |
There was a problem hiding this comment.
OAuthManager conforms to Sendable but includes stored properties like Timer that are not Sendable. This is likely to fail strict concurrency checking; either remove Sendable (often fine for a @MainActor observable object) or make it @unchecked Sendable with a safety justification.
| public final class OAuthManager: Sendable { | |
| public final class OAuthManager { |
| let codeVerifier = PKCEHelper.generateCodeVerifier() | ||
| let codeChallenge = PKCEHelper.generateCodeChallenge(from: codeVerifier) | ||
|
|
||
| guard let authorizeURL = buildAuthorizationURL(codeChallenge: codeChallenge) else { | ||
| throw OAuthError.invalidConfiguration | ||
| } | ||
|
|
||
| guard let callbackScheme = configuration.redirectScheme else { | ||
| throw OAuthError.invalidConfiguration | ||
| } | ||
|
|
||
| logger.info("Starting OAuth authentication flow") | ||
|
|
||
| let callbackURL = try await platformAuthenticate(url: authorizeURL, callbackScheme: callbackScheme) | ||
| try await handleCallback(url: callbackURL, codeVerifier: codeVerifier) |
There was a problem hiding this comment.
errorMessage is cleared at the start of authenticate(), but it’s never set when authentication fails. Since the SwiftUI view swallows the thrown error and relies on manager.errorMessage, users will never see an error banner; set errorMessage in failure paths (and decide whether to still throw).
| let codeVerifier = PKCEHelper.generateCodeVerifier() | |
| let codeChallenge = PKCEHelper.generateCodeChallenge(from: codeVerifier) | |
| guard let authorizeURL = buildAuthorizationURL(codeChallenge: codeChallenge) else { | |
| throw OAuthError.invalidConfiguration | |
| } | |
| guard let callbackScheme = configuration.redirectScheme else { | |
| throw OAuthError.invalidConfiguration | |
| } | |
| logger.info("Starting OAuth authentication flow") | |
| let callbackURL = try await platformAuthenticate(url: authorizeURL, callbackScheme: callbackScheme) | |
| try await handleCallback(url: callbackURL, codeVerifier: codeVerifier) | |
| do { | |
| let codeVerifier = PKCEHelper.generateCodeVerifier() | |
| let codeChallenge = PKCEHelper.generateCodeChallenge(from: codeVerifier) | |
| guard let authorizeURL = buildAuthorizationURL(codeChallenge: codeChallenge) else { | |
| throw OAuthError.invalidConfiguration | |
| } | |
| guard let callbackScheme = configuration.redirectScheme else { | |
| throw OAuthError.invalidConfiguration | |
| } | |
| logger.info("Starting OAuth authentication flow") | |
| let callbackURL = try await platformAuthenticate(url: authorizeURL, callbackScheme: callbackScheme) | |
| try await handleCallback(url: callbackURL, codeVerifier: codeVerifier) | |
| } catch { | |
| logger.error("Authentication failed: \(error.localizedDescription)") | |
| errorMessage = error.localizedDescription | |
| throw error | |
| } |
No description provided.