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 updateAllowedCombinations to AuthenticationStrengthPoliciesClient #257

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

alexwilcox9
Copy link
Contributor

It turns out you can't update the allowedCombinations field using the normal update endpoint. You'll get the following error

AuthenticationStrengthPoliciesClient.BaseClient.Patch(): unexpected status 405 with OData error: methodNotAllowed: Combinations can only be updated using updateAllowedCombinations action.: Combinations can only be updated
│ using updateAllowedCombinations action.

Instead there is a specific endpoint you must use documented here:
https://learn.microsoft.com/en-us/graph/api/authenticationstrengthpolicy-updateallowedcombinations?view=graph-rest-1.0&tabs=http

Currently I'm passing through the whole AuthenticationStrengthPolicy object but technically it should only be the allowedCombinations field without the display name etc.
If it's preferred I can create an object that only has allowedCombinations for use here but wasn't sure if that was needed given that Graph seems to ignore the extra fields

Copy link
Owner

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

Hi @alexwilcox9, thanks for this. I think we should build a new struct with the appropriate fields, since the API could at any time start throwing errors with the extra fields. We can still receive an AuthenticationStrengthPolicy as a method argument, and just pull out the ID and AllowedCombinations fileds. Other than that, this looks great 👍

msgraph/authentication_strength_policy.go Outdated Show resolved Hide resolved
@manicminer manicminer added enhancement New feature or request package/msgraph labels Aug 14, 2023
@alexwilcox9
Copy link
Contributor Author

Thanks for the feedback @manicminer, I've updated the code to create a new AuthenticationStrengthPolicy with only the allowedCombinations field set.

Is this sufficient or should I create a new struct in models.go for use here?

Copy link
Owner

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

@alexwilcox9 Sorry for the delay, I think this is fine to construct a request like this and we don't need to define a new exported struct type for it. Thanks for this!

@manicminer manicminer added this to the v0.64.0 milestone Sep 25, 2023
@manicminer manicminer merged commit c04c5dd into manicminer:main Sep 25, 2023
2 checks passed
manicminer added a commit that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package/msgraph
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants