Fix OpenIdConnectTokenEndpointResponse to return the actual OpenIdConnectMessage

[kevinchalet]

OpenIdConnectTokenEndpointResponse manually creates an incomplete OpenIdConnectMessage by extracting a few parameters from the JSON object instead of extracting all the parameters. It should be fixed to flow the complete message.

Currently, this prevent the new SaveTokensAsClaims feature from correctly storing the refresh token: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectTokenEndpointResponse.cs#L17-L23

/cc @Tratcher @tushargupta51

[Tratcher]

👍

[kevinchalet]

Ideally, we should kill (or at least internalize) OpenIdConnectTokenEndpointResponse and directly expose both OpenIdConnectMessage and JObject in the context classes where it is currently used.

[brentschmaltz]

@PinpointTownes @Tratcher perhaps a solution here is to add a constructor to OIDCMessage that takes a JObject and 'retire' OIDCTEResponse. OIDCMessage, unknown or new properties could be surfaced in the dictionary from the Parameters property.

[Tratcher]

Sounds good.

[kevinchalet]

@brentschmaltz really nice 👍

[Tratcher]

Depends on AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#276 which was just merged today.

[kevinchalet]

Note that the new JObject support in OpenIdConnectMessage is limited to simple properties (it won't work with array or object properties)

[tushargupta51]

@PinpointTownes All the properties in OpenIdConnectMessage are strings (except RequestType). The array properties like acr_values or ui_locales are returned as space separated string from the endpoint. I tested with this json string and it worked:
string json2 = @"{
""tag"":""value1"",
""tagObj"": {
""tag3"": value3,
""tag2"": value2
}
}";
The object is converted to json string when using ToString() method.

[kevinchalet]

@tushargupta51 sure, but most people won't expect having to manually deserialize the parameter value using JObject or JsonConvert. This obviously makes complex claims (like address in a userinfo response) harder to use from the final developer's perspective.

[tushargupta51]

@PinpointTownes With the current state of OpenIdConnectMessage, I don't think one can store arrays or objects directly, right? The dictionary that is available maps a string key to string value. I am trying to understand the issue that you are hinting is with the JObject ctor or with the class itself?

[kevinchalet]

@PinpointTownes With the current state of OpenIdConnectMessage, I don't think one can store arrays or objects directly, right? The dictionary that is available maps a string key to string value.

Yup, and it works great (I'm a huge fan of this OpenIdConnectMessage primitive 😄)

I am trying to understand the issue that you are hinting is with the JObject ctor or with the class itself?

Issues come when you want to play with arrays or objects properties. Have you considered switching to object (rather than string) values or directly exposing the JObject for more flexibility?