ramosbugs / openidconnect-rs

OpenID Connect Library for Rust
MIT License
372 stars 98 forks source link

Incorrect `alg` for EdDSA signed JWTs #156

Closed beanow-at-crabnebula closed 3 months ago

beanow-at-crabnebula commented 3 months ago

https://github.com/ramosbugs/openidconnect-rs/pull/130 adds support for EdDSA. However trying it out in practice I found an issue parsing tokens from an external issuer.

As per https://www.rfc-editor.org/rfc/rfc8037

The header should look like:

{"alg":"EdDSA"}

(In practice you'd often see typ and kid as well.)

But the library tests with a header like:

{"alg":"Ed25519","crv":"Ed25519"}

Producing errors such as:

Error("Failed to parse header JSON: Error(\"unrecognized JSON Web Algorithm `EdDSA`\", line: 1, column: 14)", line: 1, column: 1466)

Which seems like a serde_json error for parsing a token response. Guessing at least related to this serde rename.

https://github.com/ramosbugs/openidconnect-rs/blob/fcada1718118cfebfaa874e8b1920cd1dbc2b358/src/core/mod.rs#L682-L684

ramosbugs commented 3 months ago

Thanks for the bug report! Looking into this... cc: @xshadowlegendx

ramosbugs commented 3 months ago

I think you're correct. I even said "I assume we're talking about the EdDSA alg value defined in RFC 8037?" in #129 but then missed that when reviewing the PR 🤦‍♂️

I'll push a fix.