-
Notifications
You must be signed in to change notification settings - Fork 16
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
LFT-1285 - Ignore use=enc JWKs in JWKS parsing #364
Changes from 2 commits
102be6f
dc0abd2
0ec887f
ae7e2a3
38f020c
d77cd67
14ecd11
96b0e84
90d852f
3d91c22
ae36031
b0bc922
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
using System; | ||
#nullable enable | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using D2L.Security.OAuth2.Keys.Default; | ||
using Newtonsoft.Json; | ||
|
@@ -49,91 +51,169 @@ public virtual DateTime? ExpiresAt { | |
/// <returns>A JWK DTO</returns> | ||
public abstract object ToJwkDto(); | ||
|
||
|
||
/// <summary> | ||
/// Deserialize a JWK | ||
/// </summary> | ||
/// <param name="json">The json JWK</param> | ||
/// <returns>A <see cref="JsonWebKey"/></returns> | ||
public static JsonWebKey FromJson( string json ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maintaining this API + behaviour for people that call this directly (> 0 in our code) |
||
if( !TryParseJsonSigningKey( json, out var result, out var error, out var e, out _ ) ) { | ||
throw new JsonWebKeyParseException( error, e ); | ||
} | ||
|
||
return result; | ||
} | ||
|
||
public static bool TryParseJsonSigningKey( | ||
string json, | ||
[NotNullWhen( true )] | ||
out JsonWebKey? key, | ||
[NotNullWhen( false )] | ||
out string? error, | ||
[NullWhen( true )] | ||
out Exception? exception, | ||
out bool useEncKey | ||
) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly changed to TryX from throwing, but also added this separate |
||
Dictionary<string, object> data; | ||
try { | ||
data = JsonConvert.DeserializeObject<Dictionary<string, object>>( json ); | ||
} catch( JsonReaderException e ) { | ||
throw new JsonWebKeyParseException( "error deserializing JSON web key string", e ); | ||
result = null; | ||
error = "error deserializing JSON web key string"; | ||
exception = e; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "use" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'use' parameter in JSON web key" ); | ||
result = null; | ||
error = "missing 'use' parameter in JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( data[ "use" ].ToString() != "sig" ) { | ||
string msg = String.Format( "invalid 'use' value in JSON web key: {0}", data[ "use" ] ); | ||
throw new JsonWebKeyParseException( msg ); | ||
result = null; | ||
error = "invalid 'use' value in JSON web key: " + data[ "use" ]; | ||
exception = null; | ||
useEncKey = true; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "kty" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'kty' parameter in JSON web key" ); | ||
result = null; | ||
error = "missing 'kty' parameter in JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "kid" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'kid' parameter in JSON web key" ); | ||
result = null; | ||
error = "missing 'kid' parameter in JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
string id = data[ "kid" ].ToString(); | ||
DateTime? expiresAt = null; | ||
if( data.ContainsKey( "exp" ) ) { | ||
if( !long.TryParse( data[ "exp" ].ToString(), out long ts ) ) { | ||
string msg = String.Format( "invalid 'exp' value in JSON web key: {0}", data[ "exp" ] ); | ||
throw new JsonWebKeyParseException( msg ); | ||
result = null; | ||
error = "invalid 'exp' value in JSON web key: " + data[ "exp" ]; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
expiresAt = DateTimeHelpers.FromUnixTime( ts ); | ||
} | ||
|
||
switch( data[ "kty" ].ToString() ) { | ||
case "RSA": | ||
if( !data.ContainsKey( "n" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'n' parameter in RSA JSON web key" ); | ||
result = null; | ||
error = "missing 'n' parameter in RSA JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "e" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'e' parameter in RSA JSON web key" ); | ||
result = null; | ||
error = "missing 'e' parameter in RSA JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( HasRsaPrivateKeyMaterial( data ) ) { | ||
throw new JsonWebKeyParseException( "RSA JSON web key has private key material" ); | ||
result = null; | ||
error = "RSA JSON web key has private key material"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
return new RsaJsonWebKey( | ||
result = new RsaJsonWebKey( | ||
id: id, | ||
expiresAt: expiresAt, | ||
n: data[ "n" ].ToString(), | ||
e: data[ "e" ].ToString() | ||
); | ||
|
||
error = null; | ||
exception = null; | ||
useEncKey = false; | ||
|
||
return true; | ||
|
||
case "EC": | ||
if( !data.ContainsKey( "crv" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'crv' parameter in EC JSON web key" ); | ||
result = null; | ||
error = "missing 'crv' parameter in EC JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "x" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'x' parameter in EC JSON web key" ); | ||
result = null; | ||
error = "missing 'x' parameter in EC JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
if( !data.ContainsKey( "y" ) ) { | ||
throw new JsonWebKeyParseException( "missing 'y' parameter in EC JSON web key" ); | ||
result = null; | ||
error = "missing 'y' parameter in EC JSON web key"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
} | ||
|
||
return new EcDsaJsonWebKey( | ||
result = new EcDsaJsonWebKey( | ||
id: id, | ||
expiresAt: expiresAt, | ||
curve: data[ "crv" ].ToString(), | ||
x: data[ "x" ].ToString(), | ||
y: data[ "y" ].ToString() | ||
); | ||
|
||
error = null; | ||
exception = null; | ||
useEncKey = false; | ||
return true; | ||
|
||
default: | ||
string msg = String.Format( "'{0}' is not a supported JSON eb key type", data[ "kty" ] ); | ||
throw new JsonWebKeyParseException( msg ); | ||
result = null; | ||
error = $"'{data["kty"]}' is not a supported JSON web key type"; | ||
exception = null; | ||
useEncKey = false; | ||
return false; | ||
|
||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filtering could apply more generally but for now I just want to do the minimal thing to get the integrations working. I think the API needs a redesign overall.