azuread_conditional_access_policy: support includeAuthenticationContextClassReferences and applicationFilter of conditions#1534
Conversation
5641f20 to
3887987
Compare
…lter of azuread_conditional_access_policy.conditions resource
3887987 to
68b2904
Compare
| * `included_user_actions` - (Optional) A list of user actions to include. Supported values are `urn:user:registerdevice` and `urn:user:registersecurityinfo`. Cannot be specified with `included_applications`. One of `included_applications` or `included_user_actions` must be specified. | ||
| * `filter` - (Optional) A `filter` block as described below. | ||
| * `included_applications` - (Optional) A list of application IDs the policy applies to, unless explicitly excluded (in `excluded_applications`). Can also be set to `All`, `None` or `Office365`. Cannot be specified with `included_user_actions`. One of `included_applications`, `included_user_actions` or `included_authentication_context_class_references` must be specified. | ||
| * `included_authentication_context_class_references` - (Optional) A list of authentication context class reference to include. Supported values are `c1` through `c99`. |
There was a problem hiding this comment.
I see in the API docs that it only supports IDs up to c25,
In fact, the official doc states that IDs can be used up to c99.
jackofallops
left a comment
There was a problem hiding this comment.
Hi @kenchan0130 - before we do a full review, can you add acceptance test coverage for the new properties and validation conditions?
Thanks
This provider does not yet have a terraform resource to create an authentication context. So unfortunately I am unable to add a test on the attributes. |
|
I have nothing valuable to add... but I just wanted to cheer you guys on as I'm hoping to leverage this appFilter as well lol thanks for your work on it! |
|
Hi Team, can you please review this PR. We will need support for : included_authentication_context_class_references |
|
hi, @kenchan0130 @jackofallops do you need any support to finish the PR and merge the feature? |
Could you please give me your opinion on this? |
Please let me know if you need more information concerning this matter. |
|
The |
FIX: #882 #1318
Related to #1357