Skip to content

Conversation

@markwahl-msft
Copy link
Contributor

@markwahl-msft markwahl-msft commented Dec 21, 2021

In the Identity.Governance custom beta cmdlet, the cmdlet was setting the StartDateTime property by supplying a string value in UTC format. However, issue #901 highlighted that this would override the caller's start date time, and also the generated output had the incorrect time zone. So this PR addresses two problems.

First, for background, the underlying autogeneratied code contains a C# DateTime rather than DateTimeOffset for Edm.DateTimeOffset., Furthermore, the converters to JSON of a DateTime would use its ToString() method with a DateTimeFormat string ending in K (see for example 'date-time.ts' in autorest powershell for an example of that format string). Unfortunately, the semantics of the K format specifier is that the output string's time zone style varies depending on the Kind value inside the DateTime. So when a caller, in PSh, sets a DateTime of a .NET object from a string, it is not safe to assume the automatic conversion PowerShell does from a customer's string results in UTC even if the string looks like it could be UTC.. This issue is not specific to the identity.governance module; the problem can occur elsewhere in msgraph-sdk-powershell, wherever a customer sets a DateTime from a string and then that DateTime is serialized into (for example) a GET filter parameter, or a POST, PUT or PATCH request payload. When the DateTime has a Kind that is set to Unspecified, meaning that the DateTime parser earlier could not reliably guarantee that the Date and time the customer had supplied was in a UTC format, then the serialized JSON value from toJSON() is not valid OData because it is missing a time zone indicator, and that causes wierd errors to be returned by the Graph services. And if the Kind of the DateTime being serialized is Local, then the resulting JSON is not meeting the requirements of the Graph API, which state that time strings are in UTC, as waw seen in #901.

This change removes setting the start date explicitly, and instead adds an check on the customer's supplied value, to force the Kind to always be Utc if the Parse method did not detect Utc.

Copy link
Member

@peombwa peombwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good!

I've also opened a PR that ensures string to date-time conversion is always in UTC, and all DateTime properties are serialized in UTC. This will be applied to all modules.

DateTimeOffset support will be added by AutoREST.PowerShell Azure/autorest.powershell#584.

@peombwa peombwa merged commit 85fcebc into microsoftgraph:dev Jan 6, 2022
@markwahl-msft markwahl-msft deleted the mwahl-em-csd branch January 6, 2022 16:55
@peombwa peombwa mentioned this pull request Jan 19, 2022
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.

2 participants