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

Validate Context.User.Identity before accessing it #872

Merged
merged 1 commit into from Jan 20, 2015

Conversation

Projects
None yet
2 participants
@grzmiel
Contributor

grzmiel commented Jan 13, 2015

The Context.User.Identity can be null, if there is custom implementation. We should validate it to prevent NullReferenceException.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 13, 2015

Member

I'm assuming you were getting a null reference exception here?

Member

avanderhoorn commented Jan 13, 2015

I'm assuming you were getting a null reference exception here?

@avanderhoorn avanderhoorn self-assigned this Jan 13, 2015

@avanderhoorn avanderhoorn added this to the vNext milestone Jan 13, 2015

@grzmiel

This comment has been minimized.

Show comment
Hide comment
@grzmiel

grzmiel Jan 13, 2015

Contributor

Yes, since I'm using custom User Identity.

Contributor

grzmiel commented Jan 13, 2015

Yes, since I'm using custom User Identity.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 16, 2015

Member

@grzmiel What type do you inherit from for your custom Identity? Wondering if there is a better type we can cast to...

Member

avanderhoorn commented Jan 16, 2015

@grzmiel What type do you inherit from for your custom Identity? Wondering if there is a better type we can cast to...

@grzmiel

This comment has been minimized.

Show comment
Hide comment
@grzmiel

grzmiel Jan 18, 2015

Contributor

I'm inheriting from IPrincipal interface (http://msdn.microsoft.com/en-us/library/system.security.principal.iprincipal(v=vs.110).aspx). To be honest, I was wondering if I could extract getting client identity behind the interface (e.g. IClientIdentityProvider, with method GetClientIdentity(HttpRequestBase) ), and inject it through Service Provider. This would allow me and other to easily override the way the client identity is provided. What do you think of this solution?

Contributor

grzmiel commented Jan 18, 2015

I'm inheriting from IPrincipal interface (http://msdn.microsoft.com/en-us/library/system.security.principal.iprincipal(v=vs.110).aspx). To be honest, I was wondering if I could extract getting client identity behind the interface (e.g. IClientIdentityProvider, with method GetClientIdentity(HttpRequestBase) ), and inject it through Service Provider. This would allow me and other to easily override the way the client identity is provided. What do you think of this solution?

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 20, 2015

Member

That should work, only thing is that we aren't really investing that much more into v1.x at the moment. With v2 it will have this style of extension out of the box, but obviously its missing form v1. That said, v2 is still a little bit away and if you are interested in doing the work for the extension, we would happy help out and bring it in for release. Let me know what you think. At the very least this PR can be merged in.

Member

avanderhoorn commented Jan 20, 2015

That should work, only thing is that we aren't really investing that much more into v1.x at the moment. With v2 it will have this style of extension out of the box, but obviously its missing form v1. That said, v2 is still a little bit away and if you are interested in doing the work for the extension, we would happy help out and bring it in for release. Let me know what you think. At the very least this PR can be merged in.

avanderhoorn added a commit that referenced this pull request Jan 20, 2015

Merge pull request #872 from grzmiel/validate-identity
Validate Context.User.Identity before accessing it

@avanderhoorn avanderhoorn merged commit 98fdcac into Glimpse:master Jan 20, 2015

@grzmiel grzmiel deleted the grzmiel:validate-identity branch Apr 22, 2015

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