Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Provisioning Engine does not create groups in sub sites #202

Closed
lanegoolsby opened this issue Nov 18, 2015 · 18 comments
Closed

Provisioning Engine does not create groups in sub sites #202

lanegoolsby opened this issue Nov 18, 2015 · 18 comments

Comments

@lanegoolsby
Copy link
Contributor

In ObjectSiteSecurity.cs there is an explicit check for if the current site is a sub site or the root web and if its a sub site to not apply any of the settings defined in the section. Why is this? If a sub site isn't inheriting permissions then there shouldn't be an exclusion from creating groups for that site.

    public override bool WillProvision(Web web, ProvisioningTemplate template)
    {
        if (!_willProvision.HasValue)
        {
            _willProvision = (template.Security.AdditionalAdministrators.Any() ||
                template.Security.AdditionalMembers.Any() ||
                template.Security.AdditionalOwners.Any() ||
                template.Security.AdditionalVisitors.Any() ||
                template.Security.SiteGroups.Any()) &&
                !web.IsSubSite();
        }

        return _willProvision.Value;

    }
@nickvdheuvel
Copy link
Contributor

Will this be fixed? Or do I need to create custom code to make it possible to create groups in subsites (when permission inheritance is turned off)?

I suppose the following code (replacement for line 10) will fix this issue (not tested yet):
(!web.IsSubSite() || web.HasUniqueRoleAssignments)

@PaoloPia
Copy link
Contributor

Hi,
the welcome page business logic is under reconstruction. Probably with the next monthly release it will be fixed. Thus, I will close this issue, waiting for the next master release. Thanks for your feedback and commitment.

@lanegoolsby
Copy link
Contributor Author

Hi @PaoloPia, this issue does not deal with welcome pages. This issue has to do with creating groups for sub sites that aren't inheriting permissions from the root web. Please reopen it. I just checked ObjectSiteSecurity.cs and the IF's in question still exist.

After speaking to some coworkers they are under the impression that the reason for the IF is because creating groups at a sub web is not a best practice but I don't quite understand the reasoning for that.

Regardless, if a site template includes groups but is being provisioned as a sub web then the groups need to be created, either at the web level or at the site level. Either adding a check to see if the web inherits permission from the parent site and creating them there if it does or automatically creating the groups at the site level and then setting the site to use those permissions.

I am actually working having to implement some of this anyways, so if you can provide guidance on if creating the groups at the sub web level is acceptable or if groups should only be created at the root web then I would be happy to add my changes and issue a PR.

@PaoloPia
Copy link
Contributor

Oops ... I guess I wrote the comment in the wrong issue :) ... I'm sorry. Thanks for letting me know. We will come back with some feedbacks and guidance, as soon as possible.

@PaoloPia PaoloPia reopened this Feb 22, 2016
@lanegoolsby
Copy link
Contributor Author

Thanks Paola. I have the following questions on this topic

  1. Are groups created on sub sites not inheriting permissions considered an anti-pattern?
  2. Should custom permission levels also be created at the root web only?
  3. Should groups created at the root web automatically be granted read access to the root web (so they can query root web content such as the Welcome Page or other content in the Site Assets library for root web)?

@nickvdheuvel
Copy link
Contributor

My view on the topic:

  1. There should be an attribute at RoleAssigment to set the breaking of role inheritance (default false) like:
    <pnp:RoleAssignment Principal="My SharePoint Group" RoleDefinition="My Role Definition" BreakRoleInheritance="true" />
  2. Permission levels (or roles / role definitions) are created at the site collection level. The role assignments should be at the site level (root and sub).
  3. No, not automatically. This should only happen if you give the group the permissions explicitly with RoleAssignment or associate the group to a default group (Owner, Member, Visitor).

@lanegoolsby
Copy link
Contributor Author

My thoughts for number 1 in Nick's reply would be to have the BreakRoleInheritance attribute on the pnp:RoleAssignments element instead. No need to specify it for each of the groups IMO.

@nickvdheuvel
Copy link
Contributor

<pnp:RoleAssignments BreakRoleInheritance="true"> is better indeed.

@lanegoolsby
Copy link
Contributor Author

I submitted PR #473 to handle the creation of the groups. The suggestion from Nick about adding the BreakRoleInheritance option to the schema will take me a bit longer to get completed.

@nickvdheuvel
Copy link
Contributor

Alternative solution: Define role inheritance in pnp:Security with RoleInheritanceattribute (default false). If false, keep the role inheritance. If truebreak role inheritance and create defined default groups and set them as associated groups.

Examples to preserve role inheritance:

<pnp:Security RoleInheritance="true">
  <!-- XML just like you would do before -->
</pnp:Security>
<pnp:Security>
  <!-- XML just like you would do before -->
</pnp:Security>

Examples to break role inheritance and define default (associated) groups:

<pnp:Security RoleInheritance="false">
  <pnp:AssociatedGroups>
    <pnp:AssociatedGroup Name="Owner" />
    <pnp:AssociatedGroup Name="Member" />
    <pnp:AssociatedGroup Name="Visitor" />
  </pnp:AssociatedGroups>
</pnp:Security>

Breaks role inheritance and create all default groups and assign them as the corresponding associated groups (AssociatedOwnerGroup, AssociatedMemberGroup, AssociatedVisitorGroup).

<pnp:Security RoleInheritance="false">
  <pnp:AssociatedGroups>
    <pnp:AssociatedGroup Name="Owner" />
  </pnp:AssociatedGroups>
</pnp:Security>

Breaks role inheritance and creates only the default Owner group and assigns it as the corresponding associated group (AssociatedOwnerGroup).

<pnp:Security RoleInheritance="false">
  <pnp:AssociatedGroups>
    <pnp:AssociatedGroup Name="Owner" />
    <pnp:AssociatedGroup Name="Member" />
  </pnp:AssociatedGroups>
</pnp:Security>

Breaks role inheritance and creates only the default Owner group and default Member group and assigns them as the corresponding associated groups (AssociatedOwnerGroup, AssociatedMemberGroup).

@lanegoolsby
Copy link
Contributor Author

I actually did start down the path of putting the RoleInheritance attribute on the Security element. I ran into issues with the schema XSD's though that caused me to backtrack a bit (see pnp/PnP-Provisioning-Schema#79 for more detail if curious).

I find your idea of AssociatedGroup interesting. That would negate the need for the additional Boolean I mention in the issue above. Question though, how would those elements be different than the AdditionalAdministrator, AdditionalMembers, and AdditionalVisitors elements, or could those existing elements be used in the same manner to avoid having to change the schema definition?

@nickvdheuvel
Copy link
Contributor

For semantical reasons I created the pnp:AssociatedGroups element, but pnp:Additional***S could be used as well to implicitly state that the defined default group should be created.

If we use the pnp:Additional***S way, the pnp:User in pnp:Additional***S should not be mandatory, so empty groups are allowed. As far as I know it is currently not allowed to define pnp:Additional***S elements without defining at least one pnp:User child element.

@hyankov
Copy link
Contributor

hyankov commented Jun 24, 2016

@haroldvandekamp
Copy link

What is the status around this issue?
Is there an option available using the PnP:PowerShell or the PnP:Core provisioning template route to creates SharePoint groups for subsites when the subsite doesn't inherits site permissions?

@hyankov
Copy link
Contributor

hyankov commented Sep 12, 2016

@haroldvandekamp - that's exactly my point. If the subsite has 'broken permissions', it should be OK to modify the root site by provisioning the groups. However, currently the only way to this is via extension, refer to my post above.

@erwinvanhunen
Copy link
Member

I'm currently working on the the implementation. The moment I merge the update you will receive a notification on this thread.

@haroldvandekamp
Copy link

Sounds great! Looking forward to the implementation @erwinvanhunen

@erwinvanhunen
Copy link
Member

Implemented it in the dev branch. See PR #752

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

No branches or pull requests

6 participants