introduce StringOrBool to workaround illegal implementation

[Justice4Joffrey]

This is a kludge I'm using to work with Apple OAuth. They send string bools for these fields, sadly.

It's not really a merge-worthy fix, but if you can suggest a more appropriate way of integrating this (I'm sure it's a large use-case for your consumers of this library), I'm happy to throw it together.

[ramosbugs]

hey, thanks for the PR!

In order to avoid introducing a breaking change, I'd suggest only changing the Deserialize implementation, rather than the actual field type. Also, for this crate to remain standards-compliant by default, relaxations like this should be behind a non-default feature flag.

For this change, let's follow the example of the accept-rfc3339-timestamps feature flag:

openidconnect-rs/src/types.rs

Lines 1192 to 1216 in 769f796

	pub mod serde_utc_seconds {
	use crate::types::Timestamp;
	use chrono::{DateTime, Utc};
	use serde::{Deserialize, Deserializer, Serialize, Serializer};
	pub fn deserialize<'de, D>(deserializer: D) -> Result<DateTime<Utc>, D::Error>
	where
	D: Deserializer<'de>,
	{
	let seconds: Timestamp = Deserialize::deserialize(deserializer)?;
	super::timestamp_to_utc(&seconds).map_err(|_| {
	serde::de::Error::custom(format!(
	"failed to parse `{}` as UTC datetime (in seconds)",
	seconds
	))
	})
	}
	pub fn serialize<S>(v: &DateTime<Utc>, serializer: S) -> Result<S::Ok, S::Error>
	where
	S: Serializer,
	{
	super::utc_to_seconds(v).serialize(serializer)
	}
	}

Internally, it uses a Timestamp type that deserializes from either an integer or a string depending on whether the feature flag is enabled:

openidconnect-rs/src/types.rs

Lines 1004 to 1010 in 769f796

	#[derive(Debug, Deserialize, Serialize)]
	#[serde(untagged)]
	pub(crate) enum Timestamp {
	Seconds(serde_json::Number),
	#[cfg(feature = "accept-rfc3339-timestamps")]
	Rfc3339(String),
	}

In this case, however, we should only need to specify a deserialize_with annotation since the underlying type (bool) already matches the Serialize implementation we want (vs. needing to convert a chrono::DateTime back to seconds).

[Justice4Joffrey]

Thanks for the pointers. Might be some stylistic things to clear up, but think this is largely what you were suggesting.

Let me know!

[codecov]

Codecov Report

Merging #84 (ac12554) into main (d676faa) will decrease coverage by 0.07%.
The diff coverage is 53.33%.

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
- Coverage   72.28%   72.21%   -0.08%     
==========================================
  Files          16       16              
  Lines        3753     3779      +26     
==========================================
+ Hits         2713     2729      +16     
- Misses       1040     1050      +10     

Impacted Files	Coverage Δ
src/claims.rs	37.03% <0.00%> (ø)
src/macros.rs	17.36% <14.28%> (+0.04%)	⬆️
src/types.rs	69.83% <71.42%> (+0.12%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ramosbugs]

thanks! looks ready to merge once the 1.45.0 build passes. thanks for adding a test! feel free to ignore the codecov target

[ramosbugs]

This has now been released in 2.4.0!