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

null ref exception thrown when displayname and/or username are not provided #9

Closed
AdamDurani opened this issue Oct 10, 2018 · 9 comments
Labels
Bug🐞 Something isn't working Missing Details 🤷

Comments

@AdamDurani
Copy link

I have a fix on a local branch, can i have access to create a branch?

@Nordes
Copy link
Owner

Nordes commented Oct 11, 2018 via email

@AdamDurani
Copy link
Author

AdamDurani commented Oct 12, 2018 via email

@Nordes
Copy link
Owner

Nordes commented Nov 6, 2018

Sorry for late reply,

Pull requests can be done from your own repository. You first need to fork, then be on the latest version + do your changes and then pull... normally it will shows into the pull request list.

@Nordes
Copy link
Owner

Nordes commented Nov 8, 2018

Please, also give a full detail for your user properties. A user without a username is a bit problematic ;).

I will require the steps to reproduce locally. Thank you.

@Nordes Nordes added Missing Details 🤷 Bug🐞 Something isn't working labels Nov 8, 2018
@Nordes
Copy link
Owner

Nordes commented Nov 10, 2018

FYI: I received pullrequest from peoeple who don't have any access.

@ttutko
Copy link
Contributor

ttutko commented Dec 15, 2018

I just ran into this very thing after spending many hours thinking I was screwing up the appsettings configuration. The bug is in the SetBaseDetails method of ActiveDirectoryAppUser.cs. If DisplayName attribute is not present on the Active Directory user, then NullReferenceException is thrown when StringValue is accessed on line 130. A similar issue could happen with Username on line 131.

The steps to reproduce for me were:

  1. Stand up Windows Server 2019 Core instance (performed using a Hyper-V VM)
  2. Promote to domain controller
  3. Create a user using Powershell: New-ADUser -Name "testuser" -GivenName Test -SurName User -SamAccountName testuser -UserPrincipalName testuser@mydomain.local
  4. Set password for user using Powershell: Set-ADAccountPassword "CN=testuser,CN=users,dc=mydomain,dc=local" -Reset -NewPassword (ConvertTo-SecureString -AsPlainText "mypassword" -Force
  5. Enable the account: Enable-ADAccount -Identity testuser
  6. Attempt to login using the Sample project and you will simply get a Invalid Username/Password.

I have not created a PR on github in the past but I can look at doing so.

@ttutko
Copy link
Contributor

ttutko commented Dec 15, 2018

Note: My PR does not handle UserName attribute being null. I intentionally avoided changing that because if UserName is null then SubjectId won't be set and I think there will be a whole lot of other problems. Might be best to split that out into a separate issue and perhaps just update the returned error message (or at least log something better) that indicates something like "Required attribute 'UserName' is missing from the user's ActiveDirectory attributes."

@dmitrydvm
Copy link

I have same problem. @Nordes Please fix it.

@Nordes Nordes closed this as completed in #20 Mar 5, 2019
@Nordes
Copy link
Owner

Nordes commented Mar 5, 2019

Should now be available through Nuget.
https://www.nuget.org/packages/IdentityServer.LdapExtension/2.1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug🐞 Something isn't working Missing Details 🤷
Projects
None yet
Development

No branches or pull requests

4 participants