Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] OpenIdConnectMessage Improperly Parses expires_in Field #2336

Closed
1 of 14 tasks
mgw854 opened this issue Sep 28, 2023 · 4 comments · Fixed by #2351
Closed
1 of 14 tasks

[Bug] OpenIdConnectMessage Improperly Parses expires_in Field #2336

mgw854 opened this issue Sep 28, 2023 · 4 comments · Fixed by #2351
Assignees
Labels
Bug Product is not functioning as expected Regression

Comments

@mgw854
Copy link

mgw854 commented Sep 28, 2023

Which version of Microsoft.IdentityModel are you using?
Note that to get help, you need to run the latest version.
Microsoft.IdentityModel.Protocols.OpenIdConnect 7.0.0

Where is the issue?

  • M.IM.JsonWebTokens
  • M.IM.KeyVaultExtensions
  • M.IM.Logging
  • M.IM.ManagedKeyVaultSecurityKey
  • M.IM.Protocols
  • M.IM.Protocols.OpenIdConnect
  • M.IM.Protocols.SignedHttpRequest
  • M.IM.Protocols.WsFederation
  • M.IM.TestExtensions
  • M.IM.Tokens
  • M.IM.Tokens.Saml
  • M.IM.Validators
  • M.IM.Xml
  • S.IM.Tokens.Jwt
  • Other (please describe)

Is this a new or an existing app?
Existing application, upgrading from 5.2.2

Repro

var message = new OpenIdConnectMessage("{ \"expires_in\": 600, \"access_token\": \"foobar\" }");
Assert.Equal(600, message.ExpiresIn);

Expected behavior
The test passes successfully

Actual behavior
ExpiresIn is null, because it doesn't deserialize a number into the string, although this is expected per https://datatracker.ietf.org/doc/html/rfc6749#section-5.1

Possible solution
Correct handling of numbers to allow conversion to strings during deserialization

@brentschmaltz
Copy link
Member

@mgw854 did you mean to write (600 with "):

Assert.Equal("600", message.ExpiresIn);

@brentschmaltz
Copy link
Member

@mgw854 i just issued this PR to fix this issue: #2351

@johncrim
Copy link

johncrim commented Oct 12, 2023

This bit us as well yesterday when we attempted to upgrade to Microsoft.IdentityModel.Protocols.OpenIdConnect 7.0.2. This is a severe bug - it's not just the expires_in field, but all JSON fields that were not of JSON type string were just dropped on the floor. Upgrading to 7.0.2 breaks authentication except in very narrow cases.

This case should have been tested, and 7.0.2 never should have been released. Is it possible to delete the release so as to not break anyone else using this dependency?

@brentschmaltz
Copy link
Member

brentschmaltz commented Oct 12, 2023

@johncrim we will have a fix in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Product is not functioning as expected Regression
Projects
None yet
4 participants