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

[nordes]

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

[Nordes]

Please do a pull request

…

On Wed, Oct 10, 2018, 10:35 PM AdamDurani ***@***.***> wrote: I have a fix on a local branch, can i have access to create a branch? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAbQbA2s2preJy3BgdT_ttZ3-YiaC3C4ks5ujfe7gaJpZM4XVWkX> .

[AdamDurani]

I need access to PR. From: Nordes Ménard-Lamarre <[email protected]> Sent: 11 October 2018 13:07 To: Nordes/IdentityServer4.LdapExtension <[email protected]> Cc: Adam Durani <[email protected]>; Author <[email protected]> Subject: Re: [Nordes/IdentityServer4.LdapExtension] null ref exception thrown when displayname and/or username are not provided (#9) Please do a pull request

On Wed, Oct 10, 2018, 10:35 PM AdamDurani ***@***.******@***.***>> wrote: I have a fix on a local branch, can i have access to create a branch? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAbQbA2s2preJy3BgdT_ttZ3-YiaC3C4ks5ujfe7gaJpZM4XVWkX> .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#9 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ApC8YPnni2AVPMSEvNLoTxae4tmh9Q9Yks5ujzRMgaJpZM4XVWkX>.

…

--

______________________________________________________________________ This email has been scanned for malware by Sophos UTM 9.

______________________________________________________________________

[Nordes]

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]

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]

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

[ttutko]

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 [email protected]
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]

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]

I have same problem. @Nordes Please fix it.

[Nordes]

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