win_dns_record DACL permissions issue

[markatdxb]

SUMMARY

Seems that module has hardcoded the -AllowUpdateAny parameter which adds AuthenticatedUsers WRITE permissions to every new record There should be a way to disable this parameter

ISSUE TYPE

• Bug Report

COMPONENT NAME

win_dns_record

ANSIBLE VERSION

3.0.0

CONFIGURATION

OS / ENVIRONMENT

STEPS TO REPRODUCE

- name: Create database server record
  community.windows.win_dns_record:
    name: "cgyl1404p"
    type: "A"
    value: "10.1.1.1"
    zone: "amer.example.com"

EXPECTED RESULTS

DNS static records without any permissions granted to AuthenticatedUsers

ACTUAL RESULTS

DNS static records includes WRITE permission granted to AuthenticatedUsers When i create the same record manually via GUI, this permission is not there.

[markatdxb]

@johnboy2 - hello, sorry would you be able to help here?

[johnboy2]

This appears to have resulted from our use of Add-DnsServerResourceRecord and in particular its -AllowUpdateAny flag.

That particular flag has been present in this module since the very beginning, but I have no recollection whatsoever why it is there. My best guess is that it may have been a holdover from early feasibility work (albeit one that subsequently managed to elude all subsequent review).

That said, the cmdlet utterly lacks fine-grained DACL control (that one flag is pretty much all there is), so we'll have to do some testing without the flag to ensure things still behave as expected.

[markatdxb]

That -AllowUpdateAny flag actually makes sense in certain scenarios, but it would great to have it as a parameter of the module instead of always on hardcoded

[johnboy2]

While we could make that behavior optional, that would actually make me concerned with idempotence around the update case.

In particular, let's suppose we decide to add a new allow_update_any boolean option, with a default to be determined, and that an end-user decides to change that flag in update mode. The problem with this case is that the resulting resource record doesn't actually indicate its ACLs (i.e. the code would have to query those separately), which makes the whole thing more complicated.

Anyway, that makes me wonder if we shouldn't just change the behavior on security grounds; and anyone who wants the old behavior can be referred to using the win_acl module to do so explicitly. Would probably want a doc example to show that though.

@jborean93 - any thoughts here?