Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Claims density #105

Closed
Tratcher opened this issue Dec 8, 2014 · 10 comments
Closed

Claims density #105

Tratcher opened this issue Dec 8, 2014 · 10 comments

Comments

@Tratcher
Copy link
Member

Tratcher commented Dec 8, 2014

Since compression was removed from the cookie serializer (#79) there's some concern that claims are too large.

Consider using more terse claim types such as these: http://openid.net/specs/openid-connect-core-1_0.html#StandardClaims

@brentschmaltz
Copy link
Contributor

The size has a potential to increase as the industry is moving to adding 'role' claims to tokens.
The ClaimsIdentity creation algorithms attempt to normalize between SAML and Jwt tokens. Where a claim type was used in first in SAML, the JWT token handler maps the 'terse' name to a long name.

The goal is to allow: IsInRole('admin') to be independent of protocol or tokentype . If a user is ONLY using jwt's, then the JwtSecurityTokenHandler.InboundClaimTypeMap (static) can be cleared and only the short names will be used.

@brockallen
Copy link

"f a user is ONLY using jwt's, then the JwtSecurityTokenHandler.InboundClaimTypeMap (static) can be cleared and only the short names will be used."

@brentschmaltz it seems odd that if you're only using JWTs that one must change the default behavior to the JWT security token handler to use the actual claim types that are in the JWT. Of course it's too late (and this thread is probably not the right place for this feedback), but making the JWT security token handler try to map to the WS-* claims types (and the ClaimsTypes WIF class' constants) is a mistake.

@blowdart
Copy link
Member

And the more "correct" fix would be reference tokens, with an in memory or in database store. That would work for everything, rather than just caring about JWT or SAML.

@brentschmaltz
Copy link
Contributor

@brockallen @blowdart there seem to be two issues: size and why should mapping even happen. The mapping could be off by default and a simple switch turns it on.

We could co-ordinate on the two fixes for a simpler model.
AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#95

@brockallen
Copy link

I find myself typing this far too often:

JwtSecurityTokenHandler.InboundClaimTypeMap = new Dictionary<string, string>();

So I'd love for the OIDC claim types to take precedence. Rather, I'd just prefer for whatever claims a token contains to be the claims presented.

@leastprivilege
Copy link
Contributor

Yea - just give me the claims in the token and not some Microsoft/legacy representation.

@jods4
Copy link

jods4 commented Oct 20, 2015

What is the status of this?
The issues related to cookie size have been closed but I don't think the issue is actually fixed and there's no plan/ticket we early adopters can publicly track?

We're using Cookies auth. and this is becoming an issue very fast. We use lots of roles to give fine grain access to features. The repeated serialized Claim type (for Roles) makes our cookies grow quickly. And soon we'll reach size where browsers, proxies and servers are not liking the big cookies and block them. And I think that having a 5K overhead on every AJAX request is a bit excessive.

@blowdart said a possible fix would be usage of reference tokens, which is certainly a solution. What about that?

@Tratcher
Copy link
Member Author

@jods4 he was referring to an existing feature where you can store the principal on the server and only send a small reference cookie to the client. See https://github.com/aspnet/Security/blob/dev/samples/CookieSessionSample/Startup.cs#L26. This isn't enabled by default as you need somewhere to store them, and where you store them depends on how many servers you're running.

@jods4
Copy link

jods4 commented Oct 20, 2015

@Tratcher ah ok, some misunderstanding here.

I like my servers stateless, so it would be nice to be able to avoid that.

From the code I have the impression that if the ticket is not in the session anymore the user would have to log in again? Same thing if the server is restarted?

What I thought was that @blowdart suggested replacing the long, repeated claim type strings by tokens. It would be easy to at least define tokens for default claim types and massively reduce the size for common cases. (For custom claims, they would be serialized as string like today. If they are many, an API to register them -- and assign a token -- would be required.)

So what is the ASP.NET team position about this issue? Is the answer "use sessions" or are you going to do something to reduce the serialized size? If yes, can we track that effort somewhere?

BTW if the answer is "use session", you should think through the experience for new developpers. As this is not the default, it is not a "pit of success" at all. Most people will not even think about what is serialized inside the cookie and what its size may be... until they fail.

@jods4
Copy link

jods4 commented Oct 20, 2015

Thinking about this on my way back home:
Having to declare tokens for custom claims types is not convenient nor elegant.

The serialization could simply benefit from having a dictionary at its start. It's a basic but effective compression method (but not subject to the attacks mentioned in the compression issue):

  1. Write the number of claim types, for each claim type it's name (index is implicit).
  2. Then for each claim simply write the claim type index.

This would massively reduce the ticket size and most certainly solve this issue for reasonable claim uses (for unreasonable ones, I think more drastic methods are required, such saving the claims on the server-side with the session option).

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

No branches or pull requests

6 participants