Add macOS OAuth browser support and platform integration #241
Conversation
Introduces native macOS OAuth browser integration via a new MacBrowser implementation, including native plugin, bridging code, and build instructions. Updates browser selection logic to support macOS standalone builds and refactors platform detection to treat macOS as a mobile runtime for authentication flows.
WalkthroughIntroduces a macOS native bridge (Objective‑C++) wrapping ASWebAuthenticationSession, Unity plugin metadata, and a universal dylib. Updates Unity runtime to use MacBrowser on Standalone macOS, extends browser flow and interop in iOS/macOS C# code, and centralizes mobile runtime detection via a new IsMobileRuntime method. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Unity as Unity (C#)
participant Browser as MacBrowser (C#)
participant Native as libMacBrowser.dylib (ObjC++)
participant ASWeb as ASWebAuthenticationSession
User->>Unity: Trigger Login (Standalone macOS)
Unity->>Browser: Login(url, scheme, redirect)
Browser->>Native: InitWithURL(url, scheme, callback*)
note right of Native: Stores C callback<br/>creates ASWebAuthenticationSession
Browser->>Native: Start()
Native->>ASWeb: start()
ASWeb-->>Native: completion(url?, error?)
Native-->>Browser: callback(urlStr, errorCode)
Browser->>Unity: BrowserResult(success|canceled|error)
Unity-->>User: Login result
opt Cancellation
Unity->>Browser: Cancel()
Browser->>Native: Cancel()
ASWeb-->>Native: completion(nil, canceled)
Native-->>Browser: callback(nil, canceled)
Browser->>Unity: BrowserResult(canceled)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
Assets/Thirdweb/Plugins/macOS/README.md (1)
9-15: Fix invalid clang flag and ensure consistent macOS build commands.
-ObjC++is not a valid clang flag; use-fobjc-arc(if ARC is desired). The.mmextension already selects Objective‑C++.- Keep
-framework Cocoa -framework AuthenticationServices.Apply:
-clang++ -std=c++17 -ObjC++ -fmodules -Wall -Werror -arch arm64 -framework Cocoa -framework AuthenticationServices -dynamiclib MacBrowser.mm -o /tmp/libMacBrowser_arm64.dylib -clang++ -std=c++17 -ObjC++ -fmodules -Wall -Werror -arch x86_64 -framework Cocoa -framework AuthenticationServices -dynamiclib MacBrowser.mm -o /tmp/libMacBrowser_x86_64.dylib +clang++ -std=c++17 -fobjc-arc -fmodules -Wall -Werror -arch arm64 -framework Cocoa -framework AuthenticationServices -dynamiclib MacBrowser.mm -o /tmp/libMacBrowser_arm64.dylib +clang++ -std=c++17 -fobjc-arc -fmodules -Wall -Werror -arch x86_64 -framework Cocoa -framework AuthenticationServices -dynamiclib MacBrowser.mm -o /tmp/libMacBrowser_x86_64.dylibAssets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (2)
455-462: Centralizing isMobile via IsMobileRuntime is good—verify downstream expectations.Passing macOS as “mobile” changes OAuth/SIWE URL and redirect handling. Ensure wallet flows and deep-link handling expect this on macOS.
If divergence is needed, consider a separate IsEmbeddedBrowserRuntime() or an explicit macOS flag to avoid conflating with true mobile.
601-609: Treating OSXPlayer as “mobile” is a product choice—document or gate it.Return true for macOS affects all isMobile flows. Add a short comment or a config toggle to make this intentional and adjustable.
protected virtual bool IsMobileRuntime() { - if (Application.platform == RuntimePlatform.OSXPlayer) - { - return true; - } + // Treat macOS standalone as "mobile" to use ASWebAuthenticationSession-style flows. + if (Application.platform == RuntimePlatform.OSXPlayer) return true; return Application.isMobilePlatform; }Assets/Thirdweb/Runtime/Unity/Browser/iOSBrowser.cs (4)
46-53: Dispose safety is good—consider native release to avoid leaks.If native allocates, consider exposing and calling a Release/Dispose P/Invoke to free native memory on Dispose() (in addition to Cancel()).
Can you confirm MacBrowser.mm frees its session object on cancel/completion?
80-88: Completion callback dictionary: consider cleanup after callback.You rely on Dispose to remove entries; if Dispose is delayed, the dictionary may retain callbacks longer than needed. Optionally remove in OnAuthenticationSessionCompleted after invoking the callback.
if (CompletionCallbacks.TryGetValue(session, out var callback)) { callback?.Invoke(callbackUrl, new ASWebAuthenticationSessionError((ASWebAuthenticationSessionErrorCode)errorCode, errorMessage)); + CompletionCallbacks.Remove(session); }
105-115: Use RunContinuationsAsynchronously and dispose cancellation registration.Prevents inline continuation reentrancy and avoids leaking the registration.
- _taskCompletionSource = new TaskCompletionSource<BrowserResult>(); + _taskCompletionSource = new TaskCompletionSource<BrowserResult>(TaskCreationOptions.RunContinuationsAsynchronously); ... - cancellationToken.Register(() => - { - _taskCompletionSource?.TrySetCanceled(); - }); + using var ctr = cancellationToken.Register(() => _taskCompletionSource?.TrySetCanceled());Apply similarly to the MacBrowser Login method.
151-210: MacBrowser implementation mirrors iOS cleanly—apply the same TCS/cancellation improvements.Mirror the RunContinuationsAsynchronously and disposable cancellation registration changes here too.
- _taskCompletionSource = new TaskCompletionSource<BrowserResult>(); + _taskCompletionSource = new TaskCompletionSource<BrowserResult>(TaskCreationOptions.RunContinuationsAsynchronously); ... - cancellationToken.Register(() => - { - _taskCompletionSource?.TrySetCanceled(); - }); + using var ctr = cancellationToken.Register(() => _taskCompletionSource?.TrySetCanceled());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Assets/Thirdweb/Plugins/macOS/libMacBrowser.dylibis excluded by!**/*.dylib
📒 Files selected for processing (9)
Assets/Thirdweb/Plugins/macOS.meta(1 hunks)Assets/Thirdweb/Plugins/macOS/MacBrowser.mm(1 hunks)Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.meta(1 hunks)Assets/Thirdweb/Plugins/macOS/README.md(1 hunks)Assets/Thirdweb/Plugins/macOS/README.md.meta(1 hunks)Assets/Thirdweb/Plugins/macOS/libMacBrowser.dylib.meta(1 hunks)Assets/Thirdweb/Runtime/Unity/Browser/CrossPlatformUnityBrowser.cs(1 hunks)Assets/Thirdweb/Runtime/Unity/Browser/iOSBrowser.cs(5 hunks)Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Assets/Thirdweb/Runtime/Unity/Browser/CrossPlatformUnityBrowser.cs (1)
Assets/Thirdweb/Runtime/Unity/Browser/iOSBrowser.cs (1)
MacBrowser(152-209)
Assets/Thirdweb/Runtime/Unity/Browser/iOSBrowser.cs (2)
Assets/Thirdweb/Runtime/Unity/Browser/CrossPlatformUnityBrowser.cs (1)
Task(47-50)Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (6)
Task(304-312)Task(338-343)Task(357-541)Task(543-584)Task(586-599)ThirdwebClient(241-241)
🔇 Additional comments (4)
Assets/Thirdweb/Plugins/macOS.meta (1)
1-8: LGTM — standard folder meta.Assets/Thirdweb/Plugins/macOS/README.md.meta (1)
1-7: LGTM — text asset meta is fine.Assets/Thirdweb/Runtime/Unity/Browser/CrossPlatformUnityBrowser.cs (1)
40-44: Nice: macOS routed to MacBrowser.This cleanly extends platform coverage.
Assets/Thirdweb/Runtime/Unity/Browser/iOSBrowser.cs (1)
28-34: Init guard is good.Throwing when session init returns zero avoids null deref later.
| - first: | ||
| : Any | ||
| second: | ||
| enabled: 0 | ||
| settings: | ||
| Exclude Android: 1 | ||
| Exclude Editor: 1 | ||
| Exclude Linux64: 1 | ||
| Exclude OSXUniversal: 0 | ||
| Exclude WebGL: 1 | ||
| Exclude Win: 1 | ||
| Exclude Win64: 1 | ||
| Exclude iOS: 1 | ||
| - first: |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Exclude the .mm source from macOS Standalone to avoid duplicate/accidental compilation.
Since you ship a prebuilt dylib, ensure the Objective‑C++ source isn’t included for Standalone OSX. The “Any/Exclude OSXUniversal: 0” plus “Standalone/OSXUniversal enabled: 0” mix is easy to misconfigure.
Set the .mm to be excluded on OSXUniversal at the “Any” level (and keep Standalone/OSXUniversal disabled):
- Exclude OSXUniversal: 0
+ Exclude OSXUniversal: 1Please confirm in Unity that only libMacBrowser.dylib is included in macOS builds.
Also applies to: 54-61
🏁 Script executed:
#!/bin/bash
set -e
echo "---- Lines with OSXUniversal ----"
grep -n 'OSXUniversal' Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.meta || true
echo
echo "---- Lines with Standalone ----"
grep -n 'Standalone' Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.meta || true
echo
echo "---- Full contents of MacBrowser.mm.meta ----"
nl -ba Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.metaLength of output: 339
🏁 Script executed:
#!/bin/bash
set -e
echo "---- Context around 'Exclude OSXUniversal' (lines 15–30) ----"
sed -n '15,30p' Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.meta
echo
echo "---- Context around 'Standalone: OSXUniversal' (lines 45–60) ----"
sed -n '45,60p' Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.metaLength of output: 776
Exclude MacBrowser.mm from OSXUniversal at the default level
Set Exclude OSXUniversal to 1 under the default Any settings (keep Standalone: OSXUniversal enabled: 0) to prevent the .mm source from compiling alongside the dylib.
- Exclude OSXUniversal: 0
+ Exclude OSXUniversal: 1Confirm only libMacBrowser.dylib is included in macOS builds.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - first: | |
| : Any | |
| second: | |
| enabled: 0 | |
| settings: | |
| Exclude Android: 1 | |
| Exclude Editor: 1 | |
| Exclude Linux64: 1 | |
| Exclude OSXUniversal: 0 | |
| Exclude WebGL: 1 | |
| Exclude Win: 1 | |
| Exclude Win64: 1 | |
| Exclude iOS: 1 | |
| - first: | |
| - first: | |
| : Any | |
| second: | |
| enabled: 0 | |
| settings: | |
| Exclude Android: 1 | |
| Exclude Editor: 1 | |
| Exclude Linux64: 1 | |
| Exclude OSXUniversal: 1 | |
| Exclude WebGL: 1 | |
| Exclude Win: 1 | |
| Exclude Win64: 1 | |
| Exclude iOS: 1 | |
| - first: |
🤖 Prompt for AI Agents
In Assets/Thirdweb/Plugins/macOS/MacBrowser.mm.meta around lines 14 to 27, the
default Any platform settings currently have "Exclude OSXUniversal: 0" so the
.mm source can compile alongside the dylib; update the Any section to set
"Exclude OSXUniversal" to 1 (leave any Standalone: OSXUniversal entry with
enabled: 0 as-is) so the MacBrowser.mm source is excluded at the default level;
after changing the meta, verify macOS builds only include libMacBrowser.dylib
and not the .mm source.
|
@coderabbitai create a short user-facing changelog for this |
|
Tip For best results, initiate chat on the files or code changes. @0xFirekeeper Here's a short user-facing changelog for this PR: 🍎 macOS OAuth Browser Support✨ New Features
🔧 Improvements
📦 What's Included
This update enables developers to deploy Unity applications with fully functional OAuth authentication on macOS platforms. |
Introduces native macOS OAuth browser integration via a new MacBrowser implementation, including native plugin, bridging code, and build instructions. Updates browser selection logic to support macOS standalone builds and refactors platform detection to treat macOS as a mobile runtime for authentication flows.
PR-Codex overview
This PR introduces a
MacBrowserimplementation for the Thirdweb Unity SDK, enhancing OAuth support on macOS. It includes the addition of a native bridge usinglibMacBrowser.dylib, updates to various scripts, and improvements in handling mobile platform detection.Detailed summary
MacBrowserclass for OAuth login on macOS.libMacBrowser.dylibfor native functionality.ThirdwebManagerBase.csto useIsMobileRuntime()for mobile detection.README.md.guidandfileFormatVersion.Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation