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

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

Open
frasdav opened this issue Apr 5, 2024 · 1 comment
Open

Comments

@frasdav
Copy link

frasdav commented Apr 5, 2024

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
Copy link
Member

baywet commented Apr 16, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants