microsoftgraph / msgraph-sdk-go-core

Microsoft Graph SDK for Go - Core Library
https://learn.microsoft.com/graph/sdks/sdks-overview
MIT License
17 stars 9 forks source link

Role management policy rule API not accepting valid duration formats #280

Open frasdav opened 3 months ago

frasdav commented 3 months ago

When the ISODuration type is serialised, any value greater than 6 days (P6D) and less than 1 year (P1Y) results in a "week" format being used, but the API doesn't accept this format.

As an example:

package main

import (
    "context"

    "github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
    "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
    "github.com/microsoft/kiota-abstractions-go/serialization"
    graph "github.com/microsoftgraph/msgraph-sdk-go"
    "github.com/microsoftgraph/msgraph-sdk-go/models"
)

var (
    policyId string = "Group_d45f33a3-3835-4a58-baa2-9635c004e980_55d0a2d1-93b3-4913-9a52-5eb6432c5c17"
    ruleId   string = "Expiration_Admin_Eligibility"
)

func main() {

    credential, err := azidentity.NewDefaultAzureCredential(nil)
    if err != nil {
        panic(err)
    }

    client, err := graph.NewGraphServiceClientWithCredentials(credential, []string{"https://graph.microsoft.com/.default"})
    if err != nil {
        panic(err)
    }

    rule := models.NewUnifiedRoleManagementPolicyExpirationRule()

    rule.SetId(to.Ptr(ruleId))

    duration, err := serialization.ParseISODuration("P8D")
    if err != nil {
        panic(err)
    }

    rule.SetMaximumDuration(duration)

    _, err = client.Policies().
        RoleManagementPolicies().
        ByUnifiedRoleManagementPolicyId(policyId).
        Rules().
        ByUnifiedRoleManagementPolicyRuleId(ruleId).
        Patch(context.Background(), rule, nil)
    if err != nil {
        panic(err)
    }
}

..results in this:

2024/04/05 14:29:23 REQUEST
2024/04/05 14:29:23 PATCH https://graph.microsoft.com/v1.0/policies/roleManagementPolicies/Group_d45f33a3-3835-4a58-baa2-9635c004e980_55d0a2d1-93b3-4913-9a52-5eb6432c5c17/rules/Expiration_Admin_Eligibility
2024/04/05 14:29:23 Sdkversion: graph-go/1.36.0, graph-go-core/1.1.0 (hostOS=darwin; hostArch=arm64; runtimeEnvironment=go1.21.4;)
2024/04/05 14:29:23 Client-Request-Id: 52ba56db-30da-4a8c-aa25-b19eb23cc3c6
2024/04/05 14:29:23 Content-Encoding: gzip
2024/04/05 14:29:23 User-Agent: kiota-go/1.3.1
2024/04/05 14:29:23 Content-Type: application/json
2024/04/05 14:29:23 Authorization: Bearer REDACTED
2024/04/05 14:29:23 Accept: application/json
2024/04/05 14:29:23 Payload: {"id":"Expiration_Admin_Eligibility","@odata.type":"#microsoft.graph.unifiedRoleManagementPolicyExpirationRule","maximumDuration":"P1W1D"}

2024/04/05 14:29:23 RESPONSE
2024/04/05 14:29:23 Status: 400 Bad Request
2024/04/05 14:29:23 Vary: Accept-Encoding
2024/04/05 14:29:23 Strict-Transport-Security: max-age=31536000
2024/04/05 14:29:23 Request-Id: 35e61fff-3690-4d90-b289-772d469b8bbd
2024/04/05 14:29:23 Client-Request-Id: 52ba56db-30da-4a8c-aa25-b19eb23cc3c6
2024/04/05 14:29:23 X-Ms-Ags-Diagnostic: {"ServerInfo":{"DataCenter":"UK South","Slice":"E","Ring":"5","ScaleUnit":"004","RoleInstance":"LO1PEPF00002171"}}
2024/04/05 14:29:23 Date: Fri, 05 Apr 2024 13:29:23 GMT
2024/04/05 14:29:23 Content-Type: application/json
2024/04/05 14:29:23 Payload: {"error":{"code":"InvalidPolicyRule","message":"The policy rule is invalid.","innerError":{"date":"2024-04-05T13:29:23","request-id":"35e61fff-3690-4d90-b289-772d469b8bbd","client-request-id":"52ba56db-30da-4a8c-aa25-b19eb23cc3c6"}}}

You can see that P8D has been serialised as P1W1D, which the API rejects. Changing to P6D works as expected.

I was unsure where to raise this, because it's a combination of Kiota serialisation behaviour and the Graph APIs intolerance of the "W" quantifier, but thought I'd start here.

Included some version info below, let me know if you need anything else - happy to help test if required.

go 1.21

require (
    github.com/Azure/azure-sdk-for-go/sdk/azcore v1.10.0
    github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.1
    github.com/jasonjoh/msgraph-sdk-go-debug-logger v0.0.1
    github.com/microsoft/kiota-abstractions-go v1.5.6
    github.com/microsoft/kiota-http-go v1.3.1
    github.com/microsoftgraph/msgraph-sdk-go v1.36.0
    github.com/microsoftgraph/msgraph-sdk-go-core v1.1.0
)
baywet commented 2 months ago

Hi @frasdav Thanks for using the Go SDK and for reporting this. This is a case of misalignment of standards and potential implementation issue as well as far as I understand.

According to the OData ABNF an Edm.Duration is NOT and RFC 3339 Duration or an ISO 8601 Duration. And it doesn't support Year or Week. (search for durationValue =

The OAS registry defines the duration format as an RFC3339 duration

And the conversion library converts an EDM.Duration into a duration format. Which is wider than the original.

The kiota abstractions library relies on this library under the hood. Interestingly enough, although the template "allows" for P1W1D, the parsing below would not

I believe that the normalization method should be amended to use modulo (%) to ensure that when it normalizes days to weeks, it only does so when all days convert to weeks (no remaining day).

Could you open up an issue over there or even a pull request?