-
Notifications
You must be signed in to change notification settings - Fork 92
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
Enhance Intune Win32 App Lifecycle Automation with Improved programmatic Auth and upload Retry Logic #162
base: master
Are you sure you want to change the base?
Conversation
Feat remove msal.ps requirement
Fix: missing content-type header
…nvert-null-to-type-systemdatetime Fix: error invalid argument cannot convert null to type system.datetime
and finalization
back to regular retries
…o-azure-storage-blob 6 sporadic upload chunk failure to azure storage blob
…p-creation-from-json 8 sporadic errors during win32 app creation from json
I believe the upload retry logic is a badly needed feature of IntuneWin32App since Intune gets unpredictable too often. Especially for automation scenarios. So good job on implementing that Tim. I'm now testing specifically this part of the PR but all the uploads end up with:
As it turns out there's a problem with a cast in this line
Looking at the original it looks like you just lost a call to the
And honestly I believe a cast to datetime is redundant here since $TokenExpireMinutes = [System.Math]::Round(($Global:AccessToken.ExpiresOn.UtcDateTime - $UTCDateTime).TotalMinutes) |
On my end, I got errors when using the original:
Once I removed I'll test your below suggestion to see if that works for me as well:
|
My end goal is Github Actions as well - I already have an instance of Intune App Factory running there. But I'm testing all the code changes in Windows Sandbox, that's where I got the error above. Not sure why the statement behaves differently for us, but it could be due to different locales and date/time formats. |
region agnostic
10 datetime isnt locale safe
region agnostic
…x-test 10b datetime isnt locale safe fix test
@obuolinis Are you able to test this latest change? It is working well on everything I can test it on. I'd appreciate it if you could try it when you have time. |
Next up, I need to add retry logic to almost all calls to MS. It's actually fairly frequent that I get request timeouts, and simply waiting a few seconds fixes it. |
Managed to test it just now. It does work for me as well, both in Github and within Windows Sandbox. However, I'm really puzzled about this "black magic" trick you went for :) IntuneWin32App/Private/Invoke-AzureStorageBlobUpload.ps1 Lines 57 to 62 in 75901a5
Why did you have to convert a DateTimeOffset to String and then parse back to DateTimeOffset? I don't think DateTimeOffset is culture-dependent. In my eyes token expiration calculation here has got to be just a simple subtraction of either two Datetimes or two Datetimeoffsets, whatever the preference. If you like Datetimeoffset, then PS C:\> $AccessToken.ExpiresOn.GetType()
IsPublic IsSerial Name BaseType
-------- -------- ---- --------
True True DateTimeOffset System.ValueType If you go for Datetime, like in the original, then there's PS C:\> $AccessToken.ExpiresOn.UtcDateTime.GetType()
IsPublic IsSerial Name BaseType
-------- -------- ---- --------
True True DateTime System.ValueType Did you test any of the above? |
Summary
Overview:
This pull request introduces several key changes aimed at enhancing the reliability, flexibility, and automation capabilities (e.g., GitHub Actions, etc.) of the Intune Win32 app lifecycle management using modern authentication (MSAL.PS is not recommended). The changes primarily address issues related to token handling, authentication flow, Azure Storage blob upload processes, and error handling. Designed to automate via Azure Service Principal auth.
Key Changes:
Enhanced Token Handling and Authentication Flow:
New-ClientCredentialsAccessToken
function now calculates and adds anExpiresOn
property to the token object for easier expiration checks.Test-AccessToken
function now uses theExpiresOn
property to determine token expiration, enhancing reliability.Connect-MSIntuneGraph
: Improved error handling and dynamic installation of theMSAL.PS
module if still required for other auth methods.ExpiresOn
property directly.Content-Type Header Addition:
content-type
header to the REST request to ensure correct handling of the request body during upload finalization.Fixing
System.DateTime
Error:System.DateTime
usage in Azure blob upload processes.Retry Logic and SAS URI Renewal:
Module Configuration:
MSAL.PS
from the required modules to support environments where it is dynamically loaded.This PR also introduces retry logic to improve the robustness of the Add-IntuneWin32App function, in hopes to help mitigate some transient errors in the following potential areas: (see Issue #8)
Detailed Changes:
Token Handling Enhancements:
ExpiresOn
property.ExpiresOn
property directly.Azure Storage Blob Upload:
Module Configuration:
MSAL.PS
from the required modules to allow dynamic loading.Error Handling Improvements:
Add-IntuneWin32App: (see Issue #8) Adds retry logic to the following:
Impact:
These improvements are expected to significantly enhance the automation capabilities of the Intune Win32 app lifecycle management process. They address previous limitations and errors, making the system more robust against issues like authentication, throttling / rate limitations, blob upload issues, and some other minor fixes.
Testing:
I have tested this using a fully automated GitHub Actions Intune app management lifecycle workflow, and locally on up to date PS 7. If others can help test other scenarios I would appreciate the help.
Example of the fix working successfully during a GitHub Action (throttling / network / etc issue):
Please review the changes and provide feedback. Thank you!