Skip to content
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

Add fallback to 24h time format when MSI authentication fails due to date format mismatch #1177

Closed
wants to merge 1 commit into from

Conversation

vladkun
Copy link

@vladkun vladkun commented Nov 1, 2019

Addressing this bug - #1135
Proper solution for the problem would be to use unix timestamps instead of some locale-dependent string representation of date for expires_on field, but since it would be breaking change for existing users, I propose to add fallback logic to allow MSI work on Linux machines. Tests include actual responses from MSI endpoints.

@msftclas
Copy link

msftclas commented Nov 1, 2019

CLA assistant check
All CLA requirements met.

@ulvii
Copy link
Contributor

ulvii commented Nov 4, 2019

Hi @walk0r ,
Thank you for the contribution.
We are currently waiting for Azure side changes that might affect the behavior of the driver, we will revisit your PR once the changes are official.

@peterbae
Copy link
Contributor

Hi @walk0r, thanks for your contribution. I've contacted the Azure team and confirmed that the String format for MSI expiry time being different across region/Azure Web App platform is a bug on Azure's side, and no changes should be necessary from the driver's end once the fix is merged in around Feb/Mar timeline. In the meantime, I've provided our driver with a temporary fix for this solution in the original issue #1135. I'll keep this PR open for now, and I will look to close it once the Azure fix is deployed and confirms that the driver doesn't need any code changes.

@vladkun
Copy link
Author

vladkun commented Feb 12, 2020

@peterbae sure, I'm ok with it.

@vladkun vladkun closed this Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants