All posts in .NET

SharePoint Custom Solution Crashes IIS Worker Process (w3wp.exe) – Part 2

In Part 1 of this series, I discussed an issue we were having in one of our SharePoint 2013 farms and how I determined the issue was occurring because of a set of event receivers acting on the library.  In this post, I will discuss the code being used and what the final result was determined to be.  Stick around, it’s not what you think.

To be terribly honest nothing jumped out at me while looking over the code.  The initial review of the code indicated the issue could be around where the event receiver was trying to determine if the user adding the file was a member of the site owner group.  The original code was:

private void ProcessItemIfAuthorized(SPItemEventProperties properties, string errorMessage, bool isItemAdding)
{
    bool isFile;
    bool isSiteOwner = false;

    //Get the list of groups user is a part of.  Find if a part of site Owners
    SPSecurityManager secMgr = new SPSecurityManager();            
    List<string> spGroupsInWeb = secMgr.GetSPGroupsInWeb(properties.Web);

    //Loop through each group in the web.  If the group is the owner group, check to see if user exists within.
    foreach(string spGroup in spGroupsInWeb)
    {
        if(spGroup.Contains("<NAME OF OWNER GROUP>"))
        {
            isSiteOwner = secMgr.CheckIfUserInSPGroup(properties.Web, spGroup, secMgr.ParseUserIDFromClaim(properties.UserLoginName));
        }
    }

This seems pretty straight forward, but when “CheckIfUserInSPGroup” is called things aren’t quite as kosher.

public bool CheckIfUserInSPGroup(SPWeb spWeb, string groupUserName, string userName)
{
    bool maxReached = false;
    bool existsInGroup = false;

    if (!existsInGroup)
    {
        SPSecurity.RunWithElevatedPrivileges(delegate()
        {
            using (SPSite spSite = new SPSite(spWeb.Site.ID))
            {
                using (SPWeb currentWeb = spSite.OpenWeb(spWeb.ID))
                {
                    foreach (SPPrincipalInfo accountInfo in SPUtility.GetPrincipalsInGroup(currentWeb, groupUserName, int.MaxValue - 1, out maxReached))
                    {
                        //check to see if the account we are looking at is a nested group.  If so call the function again to dig deeper.
                        if (accountInfo.PrincipalType == SPPrincipalType.SecurityGroup)
                        {
                            existsInGroup = this.CheckIfUserInSPGroup(spWeb, accountInfo.LoginName, userName);

                            if (existsInGroup)
                            {
                                break;
                            }
                        }
                        else
                        {
                            if (this.ParseUserIDFromClaim(accountInfo.LoginName) == userName)
                            {
                                existsInGroup = true;
                                break;
                            }
                        }
                    }
                }
            }
        });

Again, normally not a huge issue, except that best practices state that you shouldn’t instantiate SPSite, SPWeb or SPList objects within an Event Receiver.  The reason for this is it causes extra database calls (more information here: https://msdn.microsoft.com/en-us/library/office/ee724407(v=office.14).aspx).  I thought this could be the culprit, but wasn’t convinced.  If this was the issue, why does it work fine for years and then suddenly stop working?  The reason the code is instantiating the SPSite and SPWeb object is it is used elsewhere in the solution and could be called by users who do not have the required access.  The same goes for the event receiver.  If I do not have access to control security groups in the site, I get an UnauthorizedAccessException.

So I thought, why not just use that.  We can safely assume that if the UnauthorizedAccessException error is thrown, the user is not in the Owners group.  So I updated the code with a try\catch (why one wasn’t already being used I don’t know) and added some logic into the catch.  Not generally the best method, but when used for targeted exceptions I believe acceptable IMHO.

//Loop through each group in the web.  If the group is the owner group, check to see if user exists within.
foreach(string spGroup in spGroupsInWeb)
{
    if(spGroup.Contains("<NAME OF OWNER GROUP>"))
    {
        try
        {
            isSiteOwner = secMgr.CheckIfUserInSPGroupEvntRcvr(properties.Web, spGroup, secMgr.ParseUserIDFromClaim(properties.UserLoginName));                        
        }
        //If authorization exception is fired, then we can assume the user is NOT in the owners group
        catch(UnauthorizedAccessException exNotAuthorized)
        {
            SPDiagnosticsService.Local.WriteTrace
                (
                    0,
                    new SPDiagnosticsCategory("App Security", TraceSeverity.High, EventSeverity.ErrorCritical),
                    TraceSeverity.High,
                    exNotAuthorized.ToString(),
                    string.Empty
                );

            isSiteOwner = false;
        }
        catch(Exception ex)
        {
            SPDiagnosticsService.Local.WriteTrace
                (
                    0,
                    new SPDiagnosticsCategory("App Security", TraceSeverity.High, EventSeverity.ErrorCritical),
                    TraceSeverity.High,
                    ex.ToString(),
                    string.Empty
                );

            //have to assume they don't have necassary access so files aren't added or deleted upon error
            isSiteOwner = false;
        }
    }
}

I also created a new method for the event receiver to call.  I couldn’t modify the existing one, as it contained valid logic to handle users without access and was being used elsewhere.

        public bool CheckIfUserInSPGroupEvntRcvr(SPWeb spWeb, string groupUserName, string userName)
        {
            bool maxReached = false;
            bool existsInGroup = false;

            if (!existsInGroup)
            {
                foreach (SPPrincipalInfo accountInfo in SPUtility.GetPrincipalsInGroup(spWeb, groupUserName, int.MaxValue - 1, out maxReached))
                {
                    //check to see if the account we are looking at is a nested group.  If so call the function again to dig deeper.
                    if (accountInfo.PrincipalType == SPPrincipalType.SecurityGroup)
                    {
                        existsInGroup = this.CheckIfUserInSPGroupEvntRcvr(spWeb, accountInfo.LoginName, userName);

                        if (existsInGroup)
                        {
                            break;
                        }
                    }
                    else
                    {
                        if (this.ParseUserIDFromClaim(accountInfo.LoginName) == userName)
                        {
                            existsInGroup = true;
                            break;
                        }
                    }
                }
            }

            return existsInGroup;
        }

So I moved the code into Pre-Prod and tried it out.  No change.  Still hanging, throwing errors and crashing the app pool.

Next step was to install Visual Studio into Pre-Prod and attach to the IIS Worker process.  I followed the code until it got into the newly created CheckIfUserInSPGroupEvntRcvr method.  There it stayed.  It kept looping through the AD users and groups within the SharePoint group.  As it was looping I watched the worker process memory usage grow and grow until it finally crashed again.  This didn’t make any sense as there are NOT that many users in these groups.

The Cause of it All

I took a look at the ownership group for the site I was testing with.  Like most (not all) of our project sites, it contained an AD group that contains our project team.  Let’s call that group All-Project.  All-Project had about a dozen users within it, however, there was an anomaly.  It also contained the Owners group from another project site.  This was an oddity.  I took a look at the Owner group and it also contained the same All-Project group. There was the culprit.

As you can see in the code above, it is designed for nested groups, so if the code hits a group it digs down to see if the nested group contains the user.  Because this Owner group was added (in error I found out while trying to figure out why it was there) to the All-Projects group, the code would dig into All-Projects then to the Owners group, from there back into the All-Projects group and then back into the Owners group… see where I am going with this?  By adding that single group to the All-Projects group in error an infinite recursion loop was created in the code.

The Final Fix

So the final fix was not an environmental change or a code modification.  It was simply to remove the Owners group from the All-Projects group.  Once that was done, the original code functioned as designed.  If this becomes a regular occurrence I will have to update the code to handle such an event, but in this case, I didn’t.  The farm is in containment (no further development short of break\fix) and the issue has not occurred for two years before this.  I hope the steps I documented in this blog series helps others out.

 

Thanks for reading!

Stop Looping through Sites and Lists in SharePoint There are Better Ways

I recently received a request at my client site to review a custom built feature that was no longer responding\functioning properly within our SharePoint environment. The feature was designed to copy selected files from a project site to the team site of the group whom would be taking over daily operations of the project once the solution was in production. Once copied the documents were declared as records within our Record Management System. The process was to select the files to be moved and then from the ribbon select the option to set the destination. This is the part that was not working.

Read more

SharePoint User\Group columns and People Picker Controls

So recently I had to perform some custom code that would add an entry from a people picker control to a user\group column and back again (on another form).  Going from the people picker control to SharePoint is really easy (the code snippet’s below are obviously vb.net.  I am trying to keep most of my examples on this site in C#, but this particular client I wrote this for was a VB shop at the time).


Protected Sub btnSave_Onclick(sender As Object, e As System.EventArgs) Handles btnSave.Click
'I use a dictionary here as I find it a very efficient method for holding all
'my data columns and their values (I have a lot more than shown here)
Dim dataToInsert As Dictionary(Of String, String) = New Dictionary(Of String, String)
Dim spControl As SharePointControl = New SharePointControl 'SharePointControl is a custom class for all my SP data access

If (ppManager.ResolvedEntities.Count > 0) Then
Dim manager As PickerEntity = TryCast(ppManager.ResolvedEntities(0), PickerEntity)
dataToInsert.Add(AttendanceSupportStrings.ManagerColumn, manager.Key)
Else
lblMessage.Text = "Manager field cannot be blank"
Exit Sub
End If
'...
'do other validation and value storage here
'...
Try
spControl.AddListItem(SPContext.Current.Site, dataToInsert)
Catch ex as exception
lblMessage.Text = (String.Format("An error occurred adding this entry.  Error returned: {0}", ex.Message))
lblMessage.Visible = True
End Try

 

Next, cast the value to an SPUser type and update the list. Please note: I realize some of you may be wondering why I am not going directly from the people picker to SPUser.  The reason being is that in most cases you are updating more than one field at a time and I wanted to show you a method (at least the one I use) to store columns of multiple types

'Use the dictionary key as the column name and the value as the column value
Sub AddListItem(sPSite As SPSite, dataToInsert As Dictionary(Of String, String))

Try
Using Web As SPWeb = sPSite.OpenWeb
Dim splist As SPList = Web.Lists(AttendanceSupportStrings.AttendanceListName)
Dim listItem As SPListItem = splist.AddItem()

For Each dictEntry As KeyValuePair(Of String, String) In dataToInsert
Select Case (dictEntry.Key)
Case AttendanceSupportStrings.ManagerColumn
listItem(dictEntry.Key) = Me.GetEmployeeByID(sPSite, dictEntry.Value)
'...
'More conditional updates here
'...
End Select
Next
listItem.Update()
End Using
Catch......

Private Function GetEmployeeByID(sPSite As SPSite, userName As String) As SPUser

Dim serviceContext As SPServiceContext
serviceContext = SPServiceContext.GetContext(sPSite)
Dim profileManager As New UserProfileManager(serviceContext)

If (profileManager.UserExists(userName)) Then
Dim spUser As SPUser = sPSite.OpenWeb.EnsureUser(userName)
Return spUser
Else
Throw New UserNotFoundException("This user is not found in SharePoint.  For the workflow process to proceed ensure user has valid email")
End If

End Function

 

That may seem like a lot of code just to upload, but remember it was written to handle more than just the one column. It also illustrates a method to upload an entire list row’s worth of data.

Now to go from a User Column to a people picker is quite a bit less code because I am only doing it one column at a time and access the column directly.  I am not building a dictionary to hold different field types nor updating a number of fields at once.  However, I do think that going from a User column to a People Picker is a bit more convoluted.  You actually first cast the user column into a SPUser object, then you put the user object’s login name into a Picker Entity.  You then add the picker entity to an ArrayList and finally you then add the array list to the picker object.

If (Not spListItem(AttendanceSupportStrings.UnionRepColumn) Is Nothing) Then
Dim unionContact As PickerEntity = New PickerEntity()
Dim unionList As ArrayList = New ArrayList()
Dim userUnion As SPUser = New SPFieldUserValue(spListItem.Web, spListItem(AttendanceSupportStrings.UnionRepColumn).ToString).User

unionContact.Key = userUnion.LoginName
unionList.Add(unionContact)
ppHRCContact.UpdateEntities(unionList)

End If

 

So hopefully this illustrates a method that can be used to take a value from a People Picker in a custom form up to SharePoint and then back again.