Add support for irrelevant object in OpenIdConnectConfigurationSerializer#2408
Closed
tehho wants to merge 3 commits into
Closed
Add support for irrelevant object in OpenIdConnectConfigurationSerializer#2408tehho wants to merge 3 commits into
tehho wants to merge 3 commits into
Conversation
This stops the reader from ending prematurely just because a } was reached.
| } | ||
| } | ||
|
|
||
| if (JsonPrimitives.IsReaderAtTokenType(ref reader, JsonTokenType.StartObject, true) ) |
Contributor
There was a problem hiding this comment.
Skipping doesn't seem to be the right thing here.
Reading should handle an unknown object.
There was a problem hiding this comment.
I did some local tests. Looks like the end of object check needs to be moved from the end to the first thing after the read operation
while (reader.Read())
{
if (IfJsonSerializerPrimitives.IsReaderAtTokenType(ref reader, JsonTokenType.EndObject, false))
{
break;
}
Looks like all the unrecognized entries are put in AdditionalData storage.
There was a problem hiding this comment.
The tests covering additional data could be something like this.
public static string JsonAllValues =
@"{ ""acr_values_supported"" : [""acr_value1"", ""acr_value2"", ""acr_value3""],
""authorization_endpoint"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/authorize"",
""frontchannel_logout_session_supported"": ""true"",
""frontchannel_logout_supported"": ""true"",
""additional_data_section_start"":""start"",
""pushed_authorization_request_endpoint"": ""https://additionaldata/oauth/v2/oauth-authorize/par"",
""backchannel_authentication_endpoint_auth_methods_supported"": [""client_secret_post"", ""client_secret_basic"", ""client_secret_jwt""],
""tls_client_certificate_bound_access_tokens"": false,
""backchannel_logout_session_supported"": true,
""backchannel_authentication_request_signing_alg_values_supported"": [""PS256"", ""ES256""],
""mtls_endpoint_aliases"": {
""token_endpoint"": ""https://additionaldata/oauth/v2/oauth-token"",
""revocation_endpoint"": ""https://additionaldata/oauth/v2/oauth-revoke"",
""introspection_endpoint"": ""https://additionaldata/oauth/v2/oauth-introspect"",
""pushed_authorization_request_endpoint"": ""https://additionaldata/oauth/v2/oauth-authorize/par"",
""userinfo_endpoint"": ""https://additionaldata/oauth/v2/oauth-userinfo"",
""backchannel_authentication_endpoint"": ""https://additionaldata/token-service/oauth-backchannel-authentication""
},
""revocation_endpoint_auth_signing_alg_values_supported"": [""HS512""],
""backchannel_token_delivery_modes_supported"": [""poll""],
""revocation_endpoint_auth_methods_supported"": [""client_secret_post"", ""client_secret_basic"", ""client_secret_jwt""],
""additional_data_section_end"":""end"",
""check_session_iframe"":""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/checksession"",
""claims_locales_supported"" : [ ""claim_local1"", ""claim_local2"", ""claim_local3"" ],
""claims_parameter_supported"" : true,
""claims_supported"": [ ""sub"", ""iss"", ""aud"", ""exp"", ""iat"", ""auth_time"", ""acr"", ""amr"", ""nonce"", ""email"", ""given_name"", ""family_name"", ""nickname"" ],
""claim_types_supported"" : [ ""Normal Claims"", ""Aggregated Claims"", ""Distributed Claims"" ],
""display_values_supported"" : [ ""displayValue1"", ""displayValue2"", ""displayValue3"" ],
""end_session_endpoint"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/logout"",
""grant_types_supported"" : [""authorization_code"",""implicit""],
""http_logout_supported"" : true,
""id_token_encryption_alg_values_supported"" : [""RSA1_5"", ""A256KW""],
""id_token_encryption_enc_values_supported"" : [""A128CBC-HS256"",""A256CBC-HS512""],
""id_token_signing_alg_values_supported"" : [""RS256""],
""introspection_endpoint"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/introspect"",
""introspection_endpoint_auth_methods_supported"" : [""client_secret_post"",""private_key_jwt""],
""introspection_endpoint_auth_signing_alg_values_supported"" : [""ES192"", ""ES256""],
""issuer"" : ""https://sts.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/"",
""jwks_uri"" : ""JsonWebKeySet.json"",
""logout_session_supported"" : true,
""microsoft_multi_refresh_token"" : true,
""op_policy_uri"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/op_policy_uri"",
""op_tos_uri"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/op_tos_uri"",
""request_object_encryption_alg_values_supported"" : [""A192KW"", ""A256KW""],
""request_object_encryption_enc_values_supported"" : [""A192GCM"",""A256GCM""],
""request_object_signing_alg_values_supported"" : [""PS256"", ""PS512""],
""request_parameter_supported"" : true,
""request_uri_parameter_supported"" : true,
""require_request_uri_registration"" : true,
""response_modes_supported"" : [""query"", ""fragment"",""form_post""],
""response_types_supported"" : [""code"",""id_token"",""code id_token""],
""service_documentation"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/service_documentation"",
""scopes_supported"" : [""openid""],
""subject_types_supported"" : [""pairwise""],
""token_endpoint"" : ""https://login.windows.net/d062b2b0-9aca-4ff7-b32a-ba47231a4002/oauth2/token"",
""token_endpoint_auth_methods_supported"" : [""client_secret_post"",""private_key_jwt""],
""token_endpoint_auth_signing_alg_values_supported"" : [""ES192"", ""ES256""],
""ui_locales_supported"" : [""hak-CN"", ""en-us""],
""userinfo_endpoint"" : ""https://login.microsoftonline.com/add29489-7269-41f4-8841-b63c95564420/openid/userinfo"",
""userinfo_encryption_alg_values_supported"" : [""ECDH-ES+A128KW"",""ECDH-ES+A192KW""],
""userinfo_encryption_enc_values_supported"" : [""A256CBC-HS512"", ""A128CBC-HS256""],
""userinfo_signing_alg_values_supported"" : [""ES384"", ""ES512""]
}";
public static OpenIdConnectConfiguration FullyPopulatedWithKeys
{
get
{
var config = Default;
config.JsonWebKeySet = DataSets.JsonWebKeySet1;
config.AdditionalData["microsoft_multi_refresh_token"] = true;
config.AdditionalData["backchannel_logout_session_supported"] = true;
config.AdditionalData["additional_data_section_start"] = "start";
config.AdditionalData["additional_data_section_end"] = "end";
// .the.rest.of.additional.data
config.SigningKeys.Add(KeyingMaterial.RsaSecurityKey1);
config.SigningKeys.Add(KeyingMaterial.RsaSecurityKey2);
config.SigningKeys.Add(KeyingMaterial.X509SecurityKey1);
config.SigningKeys.Add(KeyingMaterial.X509SecurityKey2);
return config;
}
}
When failing, a test report would look like this:
Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.OpenIdConnectConfigurationRetrieverTests.FromJson
Source: OpenIdConnectConfigurationRetrieverTests.cs line 53
Duration: 252 ms
Message:
Microsoft.IdentityModel.TestUtils.TestException : CompareAllPublicProperties: Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectConfiguration
AdditionalData:
(dictionary1.Count != dictionary2.Count: 12, 4)
dictionary1[key] ! found in dictionary2. key: pushed_authorization_request_endpoint
dictionary1[key] ! found in dictionary2. key: backchannel_authentication_endpoint_auth_methods_supported
dictionary1[key] ! found in dictionary2. key: tls_client_certificate_bound_access_tokens
dictionary1[key] ! found in dictionary2. key: backchannel_authentication_request_signing_alg_values_supported
dictionary1[key] ! found in dictionary2. key: mtls_endpoint_aliases
dictionary1[key] ! found in dictionary2. key: revocation_endpoint_auth_signing_alg_values_supported
dictionary1[key] ! found in dictionary2. key: backchannel_token_delivery_modes_supported
dictionary1[key] ! found in dictionary2. key: revocation_endpoint_auth_methods_supported
There was a problem hiding this comment.
Interesting, seems like such an easy fix, but the tests keep breaking no matter what - the roundtrip tests break because on writing all the indentations are dropped.
Anyways if it helps anyone here are my experiments:
95aeb9b
brentschmaltz
suggested changes
Nov 21, 2023
brentschmaltz
left a comment
brentschmaltz
left a comment
Contributor
There was a problem hiding this comment.
We will need some test cases.
|
Is there a way to access these "irrelevant" objects? It will be useful for those who don't think they're irrelevant. |
Author
|
Got a test case and this pr does not fix it. Do you want me to commit only the test or just close this pr? |
Author
|
@microsoft-github-policy-service agree |
Contributor
|
@tehho i am not seeing any source code changes here. |
Contributor
|
@tehho are you still working on this PR? Thanks. |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add support for object in OpenIdConnectConfigurationSerializer
Summary of the changes (Less than 80 chars)
Description
Error when reading .well-known/openid-configuration with an nested object.
Returns on first JsonTokenType.EndObject instead of the one at end of file.
Fixes #2407