-
Notifications
You must be signed in to change notification settings - Fork 518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MacCatalyst] Added Default Entitlements for MacCatalyst projects #18669
[MacCatalyst] Added Default Entitlements for MacCatalyst projects #18669
Conversation
…titlement that allows for using developer tools when developing blazor apps. For release, entitlements required by Apple to publish to the App Store. Also added unit test to check for entitlements when project is created.
|
</PropertyGroup> | ||
|
||
<ItemGroup Condition="'$(EnableDefaultMacCatalystDebugEntitlements)' == 'True' and '$(Configuration)' == 'Debug'"> | ||
<CustomEntitlements Include="com.apple.security.get-task-allow" Type="boolean" Value="true" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being that this isn't part of the Release configuration and only Debug, and that this is something that Xcode adds in itself to enable web debugging, wouldn't this be fine? I'll add an assert in the unit test to make sure it's not being passed into the release configuration.
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(EnableDefaultMacCatalystReleaseEntitlements)' == 'True' and '$(Configuration)' == 'Release'"> | ||
<CustomEntitlements Include="com.apple.security.app-sandbox" Type="boolean" Value="true" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 since it allows sandboxing on catalyst apps: https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_security_app-sandbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that Xcode also automatically adds this entitlement for apps that run on MacOS: https://developer.apple.com/documentation/uikit/mac_catalyst/creating_a_mac_version_of_your_ipad_app
@rolfbjarne Should app-sandbox get the same treatment as network.client? MAUI wanted this in our sdk.. can you publish Mac Catalyst apps in the Mac App Store without this entitlement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you publish Mac Catalyst apps in the Mac App Store without this entitlement?
No: "To distribute a macOS app through the Mac App Store, you must enable the App Sandbox capability."
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
var executable = GetNativeExecutable (platform, appPath); | ||
var foundEntitlements = TryGetEntitlements (executable, out var entitlements); | ||
if (configuration == "Release") { | ||
Assert.IsTrue (foundEntitlements, "Found in Release"); | ||
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.app-sandbox")?.Value, "com.apple.security.app-sandbox enlistment was not found."); | ||
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.network.client")?.Value, "com.apple.security.network.client enlistment was not found."); | ||
} else if (configuration == "Debug") { | ||
Assert.IsTrue (foundEntitlements, "Found in Debug"); | ||
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.get-task-allow")?.Value, "com.apple.security.get-task-allow enlistment was not found."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i understand this format is consistent with the other tests.. but i wonder if it may be better to have a separate test per config? it might be a bit more verbose but in the future if there are more entitlements added it might be easier to keep track of and the test failure will speak for itself
otherwise lgtm
var executable = GetNativeExecutable (platform, appPath); | |
var foundEntitlements = TryGetEntitlements (executable, out var entitlements); | |
if (configuration == "Release") { | |
Assert.IsTrue (foundEntitlements, "Found in Release"); | |
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.app-sandbox")?.Value, "com.apple.security.app-sandbox enlistment was not found."); | |
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.network.client")?.Value, "com.apple.security.network.client enlistment was not found."); | |
} else if (configuration == "Debug") { | |
Assert.IsTrue (foundEntitlements, "Found in Debug"); | |
Assert.IsTrue (entitlements!.Get<PBoolean> ("com.apple.security.get-task-allow")?.Value, "com.apple.security.get-task-allow enlistment was not found."); | |
} | |
var executable = GetNativeExecutable (platform, appPath); | |
TryGetEntitlements (executable, out var entitlements); | |
if(entitlements is null) | |
Assert.Fail("no entitlements found"); | |
// debug test | |
Assert.IsTrue (entitlements.Get<PBoolean> ("com.apple.security.app-sandbox")?.Value, "com.apple.security.app-sandbox enlistment was not found."); | |
// release test | |
Assert.IsTrue (entitlements.Get<PBoolean> ("com.apple.security.app-sandbox")?.Value, "com.apple.security.app-sandbox enlistment was not found."); | |
Assert.IsTrue (entitlements.Get<PBoolean> ("com.apple.security.network.client")?.Value, "com.apple.security.network.client enlistment was not found."); |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from other comments, LGTM! Cool work!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Added missing character. Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ Generator diffGenerator diff is empty Pipeline on Agent |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11.5) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻✅ All tests on macOS M1 - Mac Ventura (13.0) passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: simulator tests. 🎉 All 235 tests passed 🎉 Tests counts✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
…ects (xamarin#18669)" This reverts commit 1b016fa.
…ects (xamarin#18669)" This reverts commit 1b016fa.
…r MacCatalyst projects" (#19171) Reverts #18669 per discussion in MAUI about sdk defaults. See dotnet/maui#16355 for more information. Consult teams thread for more information: https://teams.microsoft.com/l/message/19:989ffa44998147aca4ceaf7482967668@thread.skype/1696009594958?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=0056f60b-301f-43ac-bbcf-f356d3c42c92&parentMessageId=1696009594958&teamName=VS%20Client%20Experiences&channelName=MAUI%20??&createdTime=1696009594958 (MSFT only) Backport of #19125 Co-authored-by: dustin-wojciechowski <dustin.wojciechowski@microsoft.com>
…nts for MacCatalyst projects" (#19172) Reverts #18669 per discussion in MAUI about sdk defaults. See dotnet/maui#16355 for more information. Consult teams thread for more information: https://teams.microsoft.com/l/message/19:989ffa44998147aca4ceaf7482967668@thread.skype/1696009594958?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=0056f60b-301f-43ac-bbcf-f356d3c42c92&parentMessageId=1696009594958&teamName=VS%20Client%20Experiences&channelName=MAUI%20??&createdTime=1696009594958 (MSFT only) Backport of #19125 Co-authored-by: dustin-wojciechowski <dustin.wojciechowski@microsoft.com>
Added default entitlements for MacCatalyst projects. For Debug, an entitlement that allows for using developer tools when developing blazor apps. For release, entitlements required by Apple to publish to the App Store.
Also added unit test to check for entitlements when project is created.
Fixes #18344