aspnet / Antiforgery

[Archived] AntiForgery token feature for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
78 stars 40 forks source link

Allow customization of ClaimUid extraction #166

Closed mattvperry closed 6 years ago

mattvperry commented 7 years ago

In the event that the claims which you look for in the current identity (sub, nameid, upn) don't exist you fallback on hashing all claims. All claims most likely includes expiry/issuance dates which change frequently. The end result being that antiforgery tokens which last across token refreshes will be wrongly invalidated.

My suggestion would be to add a setting which describes an alternate claim to use for uniquely identifying a principal.

I ran into this since my service uses an odd set of claims, none of which are the ones you are looking for.

Eilon commented 7 years ago

@blowdart / @Tratcher / @rynowak - thoughts on adding an option for this?

rynowak commented 7 years ago

This is possible to do with pubternal today: https://github.com/aspnet/Antiforgery/blob/dev/src/Microsoft.AspNetCore.Antiforgery/Internal/IClaimUidExtractor.cs

This would be easy to clean up and document if we think it's valuable. I believe this was possible in System.Web

mattvperry commented 7 years ago

Yeah, overriding that was how I worked around the issue. However, IMO, depending on the shape of anything in the .Internal namespaces seems like a code smell since you guys probably feel more comfortable changing things at will in there.

rynowak commented 7 years ago

Yeah, overriding that was how I worked around the issue. However, IMO, depending on the shape of anything in the .Internal namespaces seems like a code smell since you guys probably feel more comfortable changing things at will in there.

Yeah this is exactly correct. We want you to be able to unblock yourself, but over time APIs will migrate from .Internal to the mainline areas as we get requests for improvements.

If you find more cases like this feel free to give us feedback. It's definitely intended that we polish and move .Internal code into public areas if there is a good reason for you to use it. 👍

aspnet-hello commented 6 years ago

This issue was moved to aspnet/Home#2412