two fixes with client JWK signing (#302)

* fix: include `kid` in signature if present in key

The `oidcc-client-test` validates that the "kid" value is present, so we
include it in the signature if it's available.

* fix: pass through `client_jwks` when no HMAC algs are supported

Came across this working with the `private_key_jwt` auth in the
Conformance Suite.

src/oidcc_authorization.erl

@@ -207,14 +207,14 @@ attempt_request_object(QueryParams, #oidcc_client_context{
     SigningJwks =
         case oidcc_jwt_util:client_secret_oct_keys(SigningAlgSupported, ClientSecret) of
             none ->
-                Jwks;
+                JwksWithClientJwks;
             SigningOctJwk ->
                 oidcc_jwt_util:merge_jwks(JwksWithClientJwks, SigningOctJwk)
         end,
     EncryptionJwks =
         case oidcc_jwt_util:client_secret_oct_keys(EncryptionAlgSupported, ClientSecret) of
             none ->
-                Jwks;
+                JwksWithClientJwks;
             EncryptionOctJwk ->
                 oidcc_jwt_util:merge_jwks(JwksWithClientJwks, EncryptionOctJwk)
         end,

src/oidcc_jwt_util.erl

@@ -4,6 +4,8 @@
 %%%-------------------------------------------------------------------
 -module(oidcc_jwt_util).
 
+-feature(maybe_expr, enable).
+
 -include_lib("jose/include/jose_jwk.hrl").
 -include_lib("jose/include/jose_jws.hrl").
 -include_lib("jose/include/jose_jwt.hrl").
@@ -173,32 +175,37 @@ merge_jwks(Left, Right) ->
 sign(_Jwt, _Jwk, []) ->
     {error, no_supported_alg_or_key};
 sign(Jwt, Jwk, [Algorithm | RestAlgorithms]) ->
-    Jws = jose_jws:from_map(#{<<"alg">> => Algorithm}),
-    SigningCallback = fun
-        (#jose_jwk{fields = #{<<"use">> := <<"sig">>}} = Key) ->
-            try
-                {_Jws, Token} = jose_jws:compact(jose_jwt:sign(Key, Jws, Jwt)),
-                {ok, Token}
-            catch
-                error:not_supported -> error;
-                error:{not_supported, _Alg} -> error;
-                %% Some Keys crash if a public key is provided
-                error:function_clause -> error
-            end;
-        (#jose_jwk{} = Key) when Algorithm == <<"none">> ->
-            try
-                {_Jws, Token} = jose_jws:compact(jose_jwt:sign(Key, Jws, Jwt)),
-                {ok, Token}
-            catch
-                error:not_supported -> error;
-                error:{not_supported, _Alg} -> error
-            end;
-        (_Key) ->
-            error
-    end,
-    case evaluate_for_all_keys(Jwk, SigningCallback) of
-        {ok, Token} -> {ok, Token};
-        error -> sign(Jwt, Jwk, RestAlgorithms)
+    maybe
+        #jose_jws{fields = JwsFields} = Jws0 ?= jose_jws:from_map(#{<<"alg">> => Algorithm}),
+        SigningCallback = fun
+            (#jose_jwk{fields = #{<<"use">> := <<"sig">>} = Fields} = Key) ->
+                %% add the kid field to the JWS signature if present
+                KidField = maps:with([<<"kid">>], Fields),
+                Jws = Jws0#jose_jws{fields = maps:merge(KidField, JwsFields)},
+                try
+                    {_Jws, Token} = jose_jws:compact(jose_jwt:sign(Key, Jws, Jwt)),
+                    {ok, Token}
+                catch
+                    error:not_supported -> error;
+                    error:{not_supported, _Alg} -> error;
+                    %% Some Keys crash if a public key is provided
+                    error:function_clause -> error
+                end;
+            (#jose_jwk{} = Key) when Algorithm == <<"none">> ->
+                try
+                    {_Jws, Token} = jose_jws:compact(jose_jwt:sign(Key, Jws0, Jwt)),
+                    {ok, Token}
+                catch
+                    error:not_supported -> error;
+                    error:{not_supported, _Alg} -> error
+                end;
+            (_Key) ->
+                error
+        end,
+        {ok, Token} ?= evaluate_for_all_keys(Jwk, SigningCallback),
+        {ok, Token}
+    else
+        _ -> sign(Jwt, Jwk, RestAlgorithms)
     end.
 
 %% @private

test/oidcc_authorization_test.erl

@@ -334,6 +334,79 @@ create_redirect_url_with_request_object_and_max_clock_skew_test() ->
     ?assert(ClientNbf < os:system_time(seconds) - 5),
     ok.
 
+create_redirect_url_with_request_object_no_hmac_test() ->
+    PrivDir = code:priv_dir(oidcc),
+
+    {ok, ValidConfigString} = file:read_file(PrivDir ++ "/test/fixtures/example-metadata.json"),
+    {ok, #oidcc_provider_configuration{} = Configuration0} = oidcc_provider_configuration:decode_configuration(
+        jose:decode(ValidConfigString)
+    ),
+
+    Configuration = Configuration0#oidcc_provider_configuration{
+        request_parameter_supported = true,
+        request_object_signing_alg_values_supported = [
+            <<"RS256">>
+        ],
+        request_object_encryption_alg_values_supported = [
+            <<"RSA1_5">>,
+            <<"RSA-OAEP">>,
+            <<"RSA-OAEP-256">>,
+            <<"RSA-OAEP-384">>,
+            <<"RSA-OAEP-512">>,
+            <<"ECDH-ES">>,
+            <<"ECDH-ES+A128KW">>,
+            <<"ECDH-ES+A192KW">>,
+            <<"ECDH-ES+A256KW">>,
+            <<"A128KW">>,
+            <<"A192KW">>,
+            <<"A256KW">>,
+            <<"A128GCMKW">>,
+            <<"A192GCMKW">>,
+            <<"A256GCMKW">>,
+            <<"dir">>
+        ],
+        request_object_encryption_enc_values_supported = [
+            <<"A128CBC-HS256">>,
+            <<"A192CBC-HS384">>,
+            <<"A256CBC-HS512">>,
+            <<"A128GCM">>,
+            <<"A192GCM">>,
+            <<"A256GCM">>
+        ]
+    },
+
+    ClientId = <<"client_id">>,
+    ClientSecret = <<"">>,
+
+    Jwks = jose_jwk:to_public(jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem")),
+
+    ClientJwks0 = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"),
+    ClientJwks = ClientJwks0#jose_jwk{fields = #{<<"use">> => <<"sig">>}},
+
+    RedirectUri = <<"https://my.server/return">>,
+
+    ClientContext =
+        oidcc_client_context:from_manual(Configuration, Jwks, ClientId, ClientSecret, #{
+            client_jwks => ClientJwks
+        }),
+
+    {ok, Url} = oidcc_authorization:create_redirect_url(ClientContext, #{
+        redirect_uri => RedirectUri
+    }),
+
+    #{query := QueryString} = uri_string:parse(Url),
+    QueryParams0 = uri_string:dissect_query(QueryString),
+    QueryParams1 = lists:map(
+        fun({Key, Value}) -> {list_to_binary(Key), list_to_binary(Value)} end, QueryParams0
+    ),
+    QueryParams = maps:from_list(QueryParams1),
+
+    SignedToken = maps:get(<<"request">>, QueryParams),
+
+    {true, _, _} = jose_jwt:verify(ClientJwks, SignedToken),
+
+    ok.
+
 create_redirect_url_with_invalid_request_object_test() ->
     PrivDir = code:priv_dir(oidcc),
 

test/oidcc_token_test.erl

@@ -798,7 +798,9 @@ auth_method_private_key_jwt_test() ->
         }),
 
     ClientJwk0 = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"),
-    ClientJwk = ClientJwk0#jose_jwk{fields = #{<<"use">> => <<"sig">>}},
+    ClientJwk = ClientJwk0#jose_jwk{
+        fields = #{<<"kid">> => <<"private_kid">>, <<"use">> => <<"sig">>}
+    },
 
     ClientContext = oidcc_client_context:from_manual(Configuration, Jwk, ClientId, ClientSecret, #{
         client_jwks => ClientJwk
@@ -838,6 +840,14 @@ auth_method_private_key_jwt_test() ->
                 #jose_jws{alg = {_, 'RS256'}}, ClientAssertionJws
             ),
 
+            #jose_jws{fields = ClientAssertionJwsFields} = ClientAssertionJws,
+            ?assertMatch(
+                #{
+                    <<"kid">> := <<"private_kid">>
+                },
+                ClientAssertionJwsFields
+            ),
+
             ?assertMatch(
                 #jose_jwt{
                     fields = #{