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

Consider renaming OAuthBearer #413

Closed
Tratcher opened this issue Aug 26, 2015 · 5 comments
Closed

Consider renaming OAuthBearer #413

Tratcher opened this issue Aug 26, 2015 · 5 comments
Assignees
Milestone

Comments

@Tratcher
Copy link
Member

The OAuthBearer middleware is not a generic bearer token handler, it only supports JWT's. Consider renaming this one to something more specific (JWTBearer? OAughJWT?) so that we can create a more generic one later.

@Eilon
Copy link
Member

Eilon commented Aug 27, 2015

Let's go with JwtBearer.

@kevinchalet
Copy link
Contributor

The OAuthBearer middleware is not a generic bearer token handler, it only supports JWT's.

The OAuth2 bearer middleware is not tied to JWT since it only requires a ISecurityTokenValidator (well, a list of ISecurityTokenValidator to be exact), which is also implemented by SamlSecurityTokenHandler or Saml2SecurityTokenHandler (though I'm pretty sure they are still non-finalized things ATM).

Not to mention that you can also use your own token format using the MessageReceived or SecurityTokenReceived notifications. Renaming this middleware to something JWT-specific would be a bit misleading IMHO.

@kevinchalet
Copy link
Contributor

Related (old) comments about OAuthBearerAuthenticationMiddleware's name:


I agree with @Tratcher: we really need a basic "token validation" middleware that behaves exactly like the OAuth2 bearer token middleware built in Katana, taking no dependency on the OIDC extensions.

Making SecurityTokenHandler the first class citizen in the base bearer middleware is not necessarily a bad thing, but if you replace ISecureDataFormat<AuthenticationTicket> AccessTokenFormat by a collection of security token handlers, supporting both serialization mechanisms is a must (think about the scenario where the access token created by the OIDC server is opaque and uses a data protector).

Personally, I'd rename the existing OAuth2 bearer middleware to TokenAuthenticationMiddleware and extract all the new OAuth2/OIDC greatness you plan to add to a separate package. OAuthBearerAuthenticationMiddleware has been a very confusing name for most people (visit #owin on JabbR if you're not convinced), so I think it's really time to fix that.

#112 (comment)


@brentschmaltz @Tratcher all these changes are great and we definitely need them (the OIDC configuration discovery is just amazing). But we also need a basic token validation middleware that behaves exactly like the existing OAuth2 bearer middleware and allows full pluggability without relying on complicated and OIDC-specific things. Here's my suggestion:

  • Rename the existing OAuth2 bearer middleware to Microsoft.AspNet.Security.Tokens.TokenAuthenticationMiddleware: there's definitely nothing OAuth2/OIDC-specific with OAuthBearerAuthenticationMiddleware in its current form (almost identical to Katana 3) and this led to a terrible confusion. In Average Joe's mind, it should be clear that this middleware can be used with tokens issued by non-OAuth2 authorization servers: for instance, you must be able to plug in a ISecureDataFormat that relies on the data protection stuff or that communicates with a remote server to validate an opaque token that cannot be verified locally. That's also the place where I'd create the ISecureDataFormat -> SecurityTokenHandler adapter.
  • Move your new OIDC-stuff to a new package and a new middleware subclassing TokenAuthenticationMiddleware... what about Microsoft.AspNet.Security.Tokens.OpenIdConnect?

#112 (comment)

@Tratcher
Copy link
Member Author

Tratcher commented Sep 1, 2015

@PinpointTownes I'm told SAML can't be used for bearer auth because the tokens are too long. As for hooking MessageReceived and reading your own token, you'd still have to override most of the validation notifications as well so at that point you might as well use a different middleware.

@kevinchalet
Copy link
Contributor

True, but this limitation is caused by the headers length limits configured by the host, not by the OAuth2 bearer middleware. The same error may occur with a JWT access token containing too many claims.

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

3 participants