Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impersonation with async tasks #32

Closed
mattjohnsonpint opened this issue Sep 13, 2018 · 7 comments
Closed

Impersonation with async tasks #32

mattjohnsonpint opened this issue Sep 13, 2018 · 7 comments

Comments

@mattjohnsonpint
Copy link
Owner

Considering code such as:

Item item = await Impersonation.RunAsUser(credentials, LogonType.NewCredentials, async () =>
{
    using (var context = new MyDbContext())
    {
        return await context.Items.FirstOrDefaultAsync();
    }
});

While this looks correct, it appears that the task sometimes runs as if not impersonated. This is being discussed further in dotnet/corefx#24977.

I'll leave this item open for tracking, and discussion of alternatives. In the meantime, I recommend against using impersonation with asynchronous tasks.

@mattjohnsonpint
Copy link
Owner Author

Hmmm... Despite the discussion on the corefx issue, I can't repro this. I've added a test at ImpersonationTests.Impersonate_RunAsUser_Async but it passes. 😕

@Mac95
Copy link

Mac95 commented Sep 14, 2018

yes, it passes. This occured on my side when leaving the ASP.NET MVC application idle for a while. First, everything works as expected. After a while the connection to SQL-Server gets dropped and after that the error comes up. The following connection to SQL-Server is not under the impersonated user but using the ApplicationPoolIdentity (or whatever user the AppPool is using).

@EasyMilos
Copy link

Can we confirm few things. Where this doesn't work? Full .Net Framework or .Net Core only? ASP.NET or Desktop applications? In case of ASP.NET, if I remember correctly, there are few more settings that may cause you issues.

@WadeTheFade
Copy link

WadeTheFade commented Nov 30, 2018

I would just like to add that when I was using 2.0.1 I ran into this issue.

I was using async methods SQL connections to execute SQL commands within the impersonation block.

Only when multiple different users were using the service at the same time did I run into the log showing others connecting to SQL connections using other people's impersonated threads.

FWIW, this is what I was using before (simplified) that presented the issue:

using (_impersonator.LogonUser("DOMAIN", "USER","PASS", LogonType.Interactive))
{
	using (var connection = _db.SqlConnection($"Initial Catalog={DBNAME};Data Source={SERVER};Integrated Security=SSPI;Application Name=APP_{USER};Pooling=false"))
	{
		var cmd = _db.Open(connection);
		cmd.CommandText = QUERY;
		cmd.CommandTimeout = commandTimeOut;
		cmd.CommandType = commandType;

		using (var reader = await cmd.ExecuteReaderAsync())
		{
			//ExecuteReaderAsync() picked up other user's threads
		}
	}
}

Tried all things mentioned in Readme and threads (flaw in the ointment, etc). I remember trying the padding trick on the connection string, different LogonTypes, etc... I ended up just making a comment block \\NO ASYNC START and \\NO ASYNC END

I am doing a rewrite of my service now, and am upgrading to 3.0 and it looks like the issue is still around... after reading dotnet/corefx#24977 it doesn't look like its anything within this library, but the underlying nature of Impersonation.

@avivanoff
Copy link

avivanoff commented Sep 24, 2019

This might be related: aspnet/AspNetWebStack#260.

@mattjohnsonpint
Copy link
Owner Author

I hadn't noticed until now, but looks like we got WindowsIdentity.RunImpersonatedAsync with .NET 5.0.

I'll work on an update that targets .NET 5+ to address this issue and use the new API.

@mattjohnsonpint
Copy link
Owner Author

Turns out that RunImpersonatedAsync is just an overload for RunImpersonated that imposes a Task or Task<T> action. The underlying issue was resolved some time ago using AsyncLocal under the hood. No need for a separate Async API here.

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

No branches or pull requests

5 participants