pnp / modernization

All modernization tooling and guidance
http://aka.ms/sppnp-modernize
MIT License
156 stars 86 forks source link

On-Prem add user mapping capability #161

Closed pkbullock closed 5 years ago

pkbullock commented 5 years ago

Related to #154

Category

[ ] Bug [X] Enhancement

Expected or Desired Behavior

Add source to target user/domain mapping capability for transform in scenarios where the source and target environments are not AD sync'd and reside on different domains.

This is primiarily to restore the capability for item level permission copy in transform

jansenbe commented 5 years ago

Assigned both of us @pkbullock...let's see after the June release how to best implement this.

pkbullock commented 5 years ago

See comments in #135

jansenbe commented 5 years ago

https://docs.microsoft.com/en-us/sharepointmigration/create-a-user-mapping-file-for-data-content-migration

jansenbe commented 5 years ago

Hey @pkbullock ,

Have been thinking about this, here are some thoughts on the possible implementation...are there things to add/change in your view?

Behavior

Default account mapping logic When we see either a SID or SamAccountName then we query an LDAP server to try to find the respective UPN value for that AD object. If found then the UPN will be used in the target, if not this on-premises account will not be taken over (create log entry to make this visible). See https://github.com/SharePoint/PnP-Transformation/blob/master/InfoPath/Migration/PeoplePickerRemediation.Console/PeoplePickerRemediation.Console/PeoplePickerRemediation.cs#L613 for code reuse options

Mapping via file Use the mapping format described in https://docs.microsoft.com/en-us/sharepointmigration/create-a-user-mapping-file-for-data-content-migration. (To check: do we always have a SID?) to map accounts. Accounts that cannot be mapped will be dropped. (create log entry to make this visible)

Possible places to integrate account mapping

pkbullock commented 5 years ago

Hi @jansenbe,

been thinking about this although it makes sense to align the mapping to the SPMT, the one thing I didn't like about the tool is that format, its difficult to read and maintain. I haven't found a point yet where I can feed that back. You would have to search for the user for the SID each time you wanted to tweak the mapping file to ensure you are working on the correct user.

The SID is a primary key in AD, so we should always have one, including groups. This in theory, should also work with older versions of SharePoint, we just have to make sure that the age of AD itself isn't an issue with the code that checks the SID/SamAccountName, im sure if wil be fine tho.

Paul

pkbullock commented 5 years ago

Gonna make a start on this one based on the above requirements, prob not going to make it for Oct release though.

jansenbe commented 5 years ago

Great news :-)

If you want to discuss with me then let me know...back in Europe now. About the previous discussion on format: don't feel obliged to follow SPMT..using a SID is indeed not really intuitive.

jansenbe commented 5 years ago

@pkbullock : Will move this one to November release, no need to rush this one in (unless you feel confident about it and are able to get things in by end of Thursday).

pkbullock commented 5 years ago

Hi @jansenbe got the bare bones of this working, just thinking, about Azure AD groups, if user specifies this in the mapping, shall include resolution of groups to SharePoint? Specially around the item level permissions option.

Just checked, i can specify a AD group in the classic UI, advanced screens. I will look at adding this.

Aiming to get a drop in the next week or so.

pkbullock commented 5 years ago

hi @jansenbe i am facing a scenario i testing where if the location of executing of the transform is not on the domain e.g. SME computer (Workgroup/Other Domain) connected to SharePoint On-Premises (e.g. Contoso Domain) the contoso domain would require credentials to connect to search directory for UPN. However, in theory, I could use the source context connection username and password, the only flaw is the password must be a string converting from secure string (easy to do) which i am not happy in doing, because it would mean the password would be in memory.

Thus, i'm going to bake in a block to stop the default UPN search if this scenario exists.

When i submit the PR, perhaps a short discussion on what this does and get some feedback. Perhaps have a preview cycle for a month to allow others with complex scenarios to test.

jansenbe commented 5 years ago

I'm getting excited to test, and yes, let's have a meeting to discuss

jansenbe commented 5 years ago

Merged :-)