Multi-targeting Katana/Kestral oAuth Middleware Trouble

[kfrancis]

Continued from a Twitter stub discussion: https://twitter.com/djbyter/status/1319339978346926081

I've created a library which multi-targets both net full and net core frameworks to allow for a common API (as much as possible) when implementing this in either platform, here: https://github.com/Clinical-Support-Systems/oneid-oauth-middleware

It works, per say, but I've had to work around some issues because it's been pretty difficult finding documentation as things have been shifting in this arena over the last few years.

Current problems:
a) CorrelationId, can't validate. I can't for the life of me understand why this can't be validated. You'll see here that I've pulled out the related code so I can step through but it is always reporting "correlation cookie and state property mismatch" and it's very unclear why. Obviously this line but during runtime, it's not clear. They look equal.

b) Due to this particular service, the tokens are massive. This is one of the biggest headaches. If I just blindly SaveTokens then I get header cookie size exceptions. I've replaced the data, but the ultimate result from the access_token endpoint is

{
    "access_token": "a token 2.74 kb in size",
    "refresh_token": "a token 1.39 kb in size",
    "scope": "openid",
    "contextSessionId": "B1B484A3CB564E26E05400144FFBA259",
    "id_token": "a token 2.21 kb in size",
    "token_type": "Bearer",
    "expires_in": 604799
}

I get that there is a limit, and annoyingly I think almost everything in these tokens is required so I can't shave it. Even if I could due to the nature of the service there is dynamic content being returned in these tokens that might continue to push it over. You'll see here that I gave up and just started throwing the access_token in session but it smells to me. Now that I'm trying to use the refresh token, I can either throw that in session as well temporarily but I would like to understand if that's "normal" or if I just misunderstand something about how this works.

The primary inspiration was https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers and then worked backwards to make it work for net full.

[blowdart]

Before an actual dev kicks in, heh, what I will say is access tokens and refresh tokens are opaque to you the consumer, they're not yours to delve into, or to extract things from. They're for you to pass to the consuming service as they have been supplied to you.

Those are flipping huge though. Feels a bit wrong, but who knows, I mean, it's Canada, could be both French and English in one :) Putting it in session feels like a reasonable thing to do, or using a reference cookie, which is, well, just putting it in session again. Have you checked with eHealth that these tokens are indeed the size they expect to return to you, just to rule out some weird error somewhere along the way?

[kfrancis]

They are the right size apparently, very very verbose.

Scrubbed access token:

{
  "sub": "(some identifier)",
  "cts": "OAUTH2_STATELESS_GRANT",
  "auth_level": 0,
  "auditTrackingId": "(something looking like a guid)",
  "iss": "(some url)",
  "tokenName": "access_token",
  "token_type": "Bearer",
  "authGrantId": "(some id)",
  "aud": [
    "(our client_id)",
    "https://provider.ehealthontario.ca"
  ],
  "nbf": 1603383952,
  "grant_type": "authorization_code",
  "scope": [
    "openid",
    "(some scope)"
  ],
  "auth_time": 1603383952,
  "realm": "/idaaspstoidc",
  "exp": 1603988752,
  "iat": 1603383952,
  "expires_in": 604800,
  "jti": "zxmKt-Ui1FXDsIMKN1EVT_NlYyE",
  "given_name": "(first name)",
  "family_name": "(last name)",
  "email": "(some email)",
  "rid": [
    "URP"
  ],
  "username": "(a username)",
  "azp": "(client_id again)",
  "idp": "2.16.840.1.113883.3.239.35.3.1",
  "contextSessionId": "B1B484A3CB564E26E05400144FFBA259",
  "uao": "2.16.840.1.113883.3.239.9:103698089424",
  "uaoType": "Organization",
  "uaoName": "(org name)",
  "api_keys": [
    "(some api key)"
  ],
  "DN": "(related to the cert)",
  "version": "1.0",
  "_profile": [
    "(something related to the scope)"
  ],
  "state": "CfDJ8AtjUWfvZcZCprnf5a-zXKNoW6OOd1S7gRaS93-mPQ1YcDfu0qZddXa3dEuS5DlUUBl4qjsRAMQ-8wsfI9XRkMOrl2qwz1Jgz1CSQyYNaFEulS-1CbpbQFhi37_QXppcmP8qXqtQ-NjmomDZ5oZrRQeCTeU25FquMAwHkuHE5XZSH1TmTI_Ft7JBhEayEy-GgwiF7s3h3xN2L9uKw-GxT2_kA3C59Wfto1IAFv7GacOFIhav5Q9jwplhuAGpu2tLpoPdurqC28xlSsAoKmygnCvrE796Ir03eSkBRyzH0-cxAkL7FKJQHu1nj6D_2_Fc5jyBqUdBJEGCExMDZH9bNd6Of2K3cjD7vB5bfufd_3fLyQJyleUxGA2Uvi55Ed6GOecVtqW_f-PJT6fCaXB3UaQ"
}

Scrubbed refresh token:

{
  "sub": "(some identifier)",
  "cts": "OAUTH2_STATELESS_GRANT",
  "auth_level": 0,
  "auditTrackingId": "4aeef6c6-99ae-4c11-bfd6-a3d3685dedc2-948971",
  "iss": "(some url)",
  "tokenName": "refresh_token",
  "authModules": "uaopicker|idaasoidcsaml",
  "token_type": "Bearer",
  "authGrantId": "6BAyyfx0fdQxkhm5lQxnGPW7eko",
  "aud": "(client_id)",
  "acr": "0",
  "nbf": 1603383952,
  "ops": "EVC7cQoHmn9lNEhgbIkThVvNBkA",
  "grant_type": "authorization_code",
  "scope": [
    "openid",
    "(some scope)"
  ],
  "auth_time": 1603383952,
  "realm": "/idaaspstoidc",
  "exp": 1603988752,
  "iat": 1603383952,
  "expires_in": 604800,
  "jti": "66fcw43bIYH_RQmk0r9ZSPy8kUg"
}

Scrubbed id token:

{
  "at_hash": "w8Fj_wTzT7grNI0Y2HglHA",
  "sub": "(same identifier)",
  "auditTrackingId": "4aeef6c6-99ae-4c11-bfd6-a3d3685dedc2-948973",
  "iss": "(some url)",
  "tokenName": "id_token",
  "rid": [
    "URP"
  ],
  "acr": "0",
  "azp": "(client_id)",
  "contextSessionId": "B1B484A3CB564E26E05400144FFBA259",
  "auth_time": 1603383952,
  "exp": 1603988752,
  "iat": 1603383952,
  "email": "(email)",
  "uao": "2.16.840.1.113883.3.239.9:103698089424",
  "serviceEntitlements": "(an inner jwt here)",
  "given_name": "Hlatechnologist",
  "aud": "(client_id again)",
  "c_hash": "6fnNPUzyU_GCxfmzb78w2w",
  "org.forgerock.openidconnect.ops": "EVC7cQoHmn9lNEhgbIkThVvNBkA",
  "s_hash": "Yerl05HJQ7Szlrpmf-6jEg",
  "phoneNumber": "000-000-0000",
  "idp": "2.16.840.1.113883.3.239.35.3.1",
  "realm": "/idaaspstoidc",
  "tokenType": "JWTToken",
  "family_name": "(last name)"
}

// serviceEntitlements inner encoded jwt
{
  "UAO": [
    {
      "type": "Organization",
      "id": "2.16.840.1.113883.3.239.9:103698089424",
      "friendName": "(org name)",
      "service": [
        {
          "name": "(service name)",
          "attribute": [
            {
              "name": "scope",
              "value": "(same scope)"
            },
            {
              "name": "_profile",
              "value": "(profile related to api usage)"
            }
          ]
        }
      ]
    }
  ]
}

Ok. If it makes sense to use session, then that's fine.

[blowdart]

OMG that's insane. But yea, session, just make sure you are matching the current user details to the session too, just in case of session highjack. Which, although unlikely because of how we generate session ID, it's still a concern.

[kfrancis]

Ok!

Just getting you two with some authority to look at this is honestly just filling my ol' developer soul.

[kfrancis]

@blowdart What does "matching the current user details to the session too" look like?

[blowdart]

Put the original username in session, check it on every request, throw if no match.

You're going beyond the oauth middleware now of course, because the oauth pieces end at giving you the token, how it's stored and retrieved is rather beyond what the authorization middleware is meant for.

[Tratcher]

On cross-targeting: I'm impressed you got this far. You're right that the stacks are quite different. I wonder how much code you're really sharing and if you'd be better off with two packages? Currently you have #if's around entire files.

Make sure to remove any 2.2 package dependencies, those are now available in the shared framework in 3.x.
https://github.com/Clinical-Support-Systems/oneid-oauth-middleware/blob/859c56758b099f0f56e996d602ac158f089495d2/src/AspNet.Security.OAuth.OneID/AspNet.Security.OAuth.OneID.csproj#L22-L23

[kfrancis]

Yep, it was pretty annoying. Things like Options, Handler and Backchannel get re-used for the most part. From an "ease of use" perspective, that's why I didn't go with two packages. I agree that who files swallowed in #if NETFULL aren't great .. first kick at the working can.

[Tratcher]

That Backchannel was pretty strange. It seems like it was only factored that way to allow you to share more code? Normally you'd have implemented that logic in the auth handler directly right?

[kfrancis]

I'm honestly not sure. It seemed the only place where I could hook into the process to handle claim_assertion and deal with things like fixing double URL encoding. Hard to find documentation with some sort of example that didn't span 4 years of changes.

[Tratcher]

Better to generate the request correctly the first time in OneIdAuthenticationHandler.ExchangeCodeAsync/AuthenticateCoreAsync than to try to fix it up after the fact in OneIdAuthenticationBackChannelHandler. That's double true with the redirect_uri encoding, don't decode something to fix a double encoding issue, avoid double encoding it to begin with.

[Tratcher]

For a) CorrelationId, you're only having this issue on Katana, it works on Core? It looks like you use the base class implementation of GenerateCorrelationId, but you customized the validation step with ValidateOurCorrelationId? Why not use the base ValidateCorrelationId? Or is that what you meant about pulling the code out for debugging?

Your code seems to be using the same StateCookie for correlation and state? These are different concepts and I think trying to merge them is what's breaking you. The correlation cookie is just a unique id for that handshake that mitigates some CSRF(?) attacks. The handshake state can flow in the OAuth state field, it doesn't need a cookie.

[kfrancis]

Yep, that's what I meant - ValidateOurCorrelationId was just so I could see what was going on.

I wasn't initially doing anything with StateCookie, but started to try and pull it out because I couldn't understand why correlation was failing. SameSite kept biting me, for one.