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

GetEnvironmentUserName is returning the incorrect result for ASP .NET applications because ENV_USER_NAME is not being defined as expected #12

Closed
DavidR91 opened this issue Feb 16, 2017 · 9 comments · Fixed by #13

Comments

@DavidR91
Copy link

(This is against 2.1.1 net452 from NuGet)

When ENV_USER_NAME is defined (for the full .NET framework), Environment.UserName is supposed to be used. This would result in accurate reporting of application pool identities for ASP.NET applications.

That is not the case however: ENV_USER_NAME doesn't seem to have been defined at build time, and I have confirmed this with DotPeek:

 private static string GetEnvironmentUserName()
    {
      string environmentVariable1 = Environment.GetEnvironmentVariable("USERDOMAIN");
      string environmentVariable2 = Environment.GetEnvironmentVariable("USERNAME");
      if (string.IsNullOrWhiteSpace(environmentVariable1))
        return environmentVariable2;
      return string.Format("{0}\\{1}", (object) environmentVariable1, (object) environmentVariable2);
    }

This results in inaccurate username information for ASP.NET applications: when the application pools run as a different user, this will consistently report SYSTEM or Network Service (i.e. MACHINENAME$)

I am guessing that perhaps this section of project.json is not being honoured or working as expected?

  "net4.5": {
      "define": ["ENV_USER_NAME"]
    },
@nblumhardt
Copy link
Member

Sounds like a bug, thanks for the report.

@DavidR91
Copy link
Author

I may be able to PR this later - I strongly suspect it is just a case of needing to use "net452": { "define":... rather than .net4.5 in the project.json, but I'll need to set up my machine to build + confirm this

(It does seem as though the framework names in project.json have fluctuated a lot, which would explain the bug)

@adamchester
Copy link
Member

Maybe you could confirm if you accidentally referenced or disassembled the netstandard1.3 assembly instead of the net45 assembly?

@adamchester
Copy link
Member

Ahh maybe it's supposed to be net45 without the dot

@skomis-mm
Copy link
Contributor

It seems define section should be enclosed within the build section 😮

@skomis-mm skomis-mm mentioned this issue Feb 21, 2017
nblumhardt added a commit that referenced this issue Feb 21, 2017
@nblumhardt
Copy link
Member

@DavidR91
Copy link
Author

DavidR91 commented Mar 21, 2017

Is there any chance of seeing this fix go into a stable release any time soon? (Rather than pre-release?)

This information is 50% of what this enricher provides, and the use cases affected are quite broad (pretty much anything running as a service or with any kind of sophisticated identity/impersonation will be getting dud results)

(- unless you were actively waiting on my feedback for the prelease ver?)

@DavidR91
Copy link
Author

If it was my feedback you were waiting on, I can now confirm that this fix (2.1.2-dev-00731) definitely works

@nblumhardt
Copy link
Member

@DavidR91 done! Thanks for the nudge.

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

Successfully merging a pull request may close this issue.

4 participants