Handle PKCE method selection (#253)

src/oidcc_authorization.erl

@@ -15,20 +15,14 @@
 -export([create_redirect_url/2]).
 
 -export_type([error/0]).
--export_type([pkce/0]).
 -export_type([opts/0]).
 
--type pkce() :: #{challenge := binary(), method := binary()}.
-%% Configure PKCE for authorization
-%%
-%% See [https://datatracker.ietf.org/doc/html/rfc7636#section-4.3]
-
 -type opts() ::
     #{
         scopes => oidcc_scope:scopes(),
         state => binary(),
         nonce => binary(),
-        pkce => pkce() | undefined,
+        pkce_verifier => binary(),
         redirect_uri := uri_string:uri_string(),
         url_extension => oidcc_http_util:query_params()
     }.
@@ -42,7 +36,8 @@
 %%   <li>`scopes' - list of scopes to request (defaults to `[<<"openid">>]')</li>
 %%   <li>`state' - state to pass to the provider</li>
 %%   <li>`nonce' - nonce to pass to the provider</li>
-%%   <li>`pkce' - pkce arguments to pass to the provider</li>
+%%   <li>`pkce_verifier' - pkce verifier (random string), see
+%%     [https://datatracker.ietf.org/doc/html/rfc7636#section-4.1]</li>
 %%   <li>`redirect_uri' - redirect target after authorization is completed</li>
 %%   <li>`url_extension' - add custom query parameters to the authorization url</li>
 %% </ul>
@@ -105,22 +100,56 @@ redirect_params(#oidcc_client_context{client_id = ClientId} = ClientContext, Opt
             | maps:get(url_extension, Opts, [])
         ],
     QueryParams1 = maybe_append(<<"state">>, maps:get(state, Opts, undefined), QueryParams),
-    QueryParams2 =
-        maybe_append(<<"nonce">>, maps:get(nonce, Opts, undefined), QueryParams1),
-    QueryParams3 = append_code_challenge(maps:get(pkce, Opts, undefined), QueryParams2),
+    QueryParams2 = maybe_append(<<"nonce">>, maps:get(nonce, Opts, undefined), QueryParams1),
+    QueryParams3 = append_code_challenge(
+        maps:get(pkce_verifier, Opts, none), QueryParams2, ClientContext
+    ),
     QueryParams4 = oidcc_scope:query_append_scope(
         maps:get(scopes, Opts, [openid]), QueryParams3
     ),
     attempt_request_object(QueryParams4, ClientContext).
 
--spec append_code_challenge(
-    Pkce :: pkce() | undefined, QueryParams :: oidcc_http_util:query_params()
-) ->
-    oidcc_http_util:query_params().
-append_code_challenge(#{challenge := Challenge, method := Method}, QueryParams) ->
-    [{<<"code_challenge">>, Challenge}, {<<"code_challenge_method">>, Method} | QueryParams];
-append_code_challenge(undefined, QueryParams) ->
-    QueryParams.
+-spec append_code_challenge(PkceVerifier, QueryParams, ClientContext) ->
+    oidcc_http_util:query_params()
+when
+    PkceVerifier :: binary() | none,
+    QueryParams :: oidcc_http_util:query_params(),
+    ClientContext :: oidcc_client_context:t().
+append_code_challenge(none, QueryParams, _ClientContext) ->
+    QueryParams;
+append_code_challenge(CodeVerifier, QueryParams, ClientContext) ->
+    #oidcc_client_context{provider_configuration = ProviderConfiguration} = ClientContext,
+    #oidcc_provider_configuration{code_challenge_methods_supported = CodeChallengeMethodsSupported} =
+        ProviderConfiguration,
+    case CodeChallengeMethodsSupported of
+        undefined ->
+            QueryParams;
+        Methods when is_list(Methods) ->
+            case
+                {
+                    lists:member(<<"S256">>, CodeChallengeMethodsSupported),
+                    lists:member(<<"plain">>, CodeChallengeMethodsSupported)
+                }
+            of
+                {true, _PlainSupported} ->
+                    CodeChallenge = base64:encode(crypto:hash(sha256, CodeVerifier), #{
+                        mode => urlsafe, padding => false
+                    }),
+                    [
+                        {"code_challenge", CodeChallenge},
+                        {"code_challenge_method", <<"S256">>}
+                        | QueryParams
+                    ];
+                {false, true} ->
+                    [
+                        {"code_challenge", CodeVerifier},
+                        {"code_challenge_method", <<"plain">>}
+                        | QueryParams
+                    ];
+                {false, false} ->
+                    QueryParams
+            end
+    end.
 
 -spec maybe_append(Key, Value, QueryParams) -> QueryParams when
     Key :: unicode:chardata(),

src/oidcc_token.erl

@@ -93,11 +93,9 @@
 %%   <li>`scope' - {@link oidcc_scope:scopes()}</li>
 %% </ul>
 
--type pkce() :: #{verifier := binary()}.
-
 -type retrieve_opts() ::
     #{
-        pkce => pkce(),
+        pkce_verifier => binary(),
         nonce => binary() | any,
         scope => oidcc_scope:scopes(),
         refresh_jwks => oidcc_jwt_util:refresh_jwks_for_unknown_kid_fun(),
@@ -111,7 +109,9 @@
 %% <h2>Fields</h2>
 %%
 %% <ul>
-%%   <li>`pkce' - PKCE verification options</li>
+%%   <li>`pkce_verifier' - pkce verifier (random string previously given to
+%%     `oidcc_authorization'), see
+%%     [https://datatracker.ietf.org/doc/html/rfc7636#section-4.1]</li>
 %%   <li>`nonce' - Nonce to check</li>
 %%   <li>`scope' - Scope to store with the token</li>
 %%   <li>`refresh_jwks' - How to handle tokens with an unknown `kid'.
@@ -301,7 +301,7 @@ retrieve(AuthCode, ClientContext, Opts) ->
     case lists:member(<<"authorization_code">>, GrantTypesSupported) of
         true ->
 
-            Pkce = maps:get(pkce, Opts, undefined),
+            PkceVerifier = maps:get(pkce_verifier, Opts, none),
             QsBody =
                 [{<<"grant_type">>, <<"authorization_code">>},
                 {<<"code">>, AuthCode},
@@ -311,7 +311,7 @@ retrieve(AuthCode, ClientContext, Opts) ->
                 extra_meta => #{issuer => Issuer, client_id => ClientId}},
 
             maybe
-                {ok, Token} ?= retrieve_a_token(QsBody, Pkce, ClientContext, Opts, TelemetryOpts, true),
+                {ok, Token} ?= retrieve_a_token(QsBody, PkceVerifier, ClientContext, Opts, TelemetryOpts, true),
                 extract_response(Token, ClientContext, Opts)
             end;
         false ->
@@ -378,7 +378,7 @@ refresh(RefreshToken, ClientContext, Opts) ->
                 extra_meta => #{issuer => Issuer, client_id => ClientId}},
 
             maybe
-                {ok, Token} ?= retrieve_a_token(QueryString1, undefined, ClientContext, Opts, TelemetryOpts, true),
+                {ok, Token} ?= retrieve_a_token(QueryString1, none, ClientContext, Opts, TelemetryOpts, true),
                 {ok, TokenRecord} ?=
                     extract_response(Token, ClientContext, maps:put(nonce, any, Opts)),
                 case TokenRecord of
@@ -464,7 +464,7 @@ jwt_profile(Subject, ClientContext, Jwk, Opts) ->
                 extra_meta => #{issuer => Issuer, client_id => ClientId}},
 
             maybe
-                {ok, Token} ?= retrieve_a_token(QueryString1, undefined, ClientContext, Opts, TelemetryOpts, false),
+                {ok, Token} ?= retrieve_a_token(QueryString1, none, ClientContext, Opts, TelemetryOpts, false),
                 {ok, TokenRecord} ?= extract_response(Token, ClientContext, maps:put(nonce, any, Opts)),
                 case TokenRecord of
                     #oidcc_token{id = none} ->
@@ -519,7 +519,7 @@ client_credentials(ClientContext, Opts) ->
                 extra_meta => #{issuer => Issuer, client_id => ClientId}},
 
             maybe
-                {ok, Token} ?= retrieve_a_token(QueryString1, undefined, ClientContext, Opts, TelemetryOpts, true),
+                {ok, Token} ?= retrieve_a_token(QueryString1, none, ClientContext, Opts, TelemetryOpts, true),
                 extract_response(Token, ClientContext, maps:put(nonce, any, Opts))
             end;
         false ->
@@ -756,16 +756,18 @@ verify_missing_required_claims(Claims) ->
             {error, {missing_claim, MissingClaim, Claims}}
     end.
 
--spec retrieve_a_token(QsBodyIn, Pkce, ClientContext, Opts, TelemetryOpts, AuthenticateClient) ->
+-spec retrieve_a_token(
+    QsBodyIn, PkceVerifier, ClientContext, Opts, TelemetryOpts, AuthenticateClient
+) ->
     {ok, map()} | {error, error()}
 when
     QsBodyIn :: oidcc_http_util:query_params(),
-    Pkce :: pkce() | undefined,
+    PkceVerifier :: binary() | none,
     ClientContext :: oidcc_client_context:t(),
     Opts :: retrieve_opts() | refresh_opts(),
     TelemetryOpts :: oidcc_http_util:telemetry_opts(),
     AuthenticateClient :: boolean().
-retrieve_a_token(QsBodyIn, Pkce, ClientContext, Opts, TelemetryOpts, AuthenticateClient) ->
+retrieve_a_token(QsBodyIn, PkceVerifier, ClientContext, Opts, TelemetryOpts, AuthenticateClient) ->
     #oidcc_client_context{provider_configuration = Configuration, client_jwks = ClientJwks} =
         ClientContext,
     #oidcc_provider_configuration{token_endpoint = TokenEndpoint,
@@ -774,7 +776,7 @@ retrieve_a_token(QsBodyIn, Pkce, ClientContext, Opts, TelemetryOpts, Authenticat
 
     Header0 = [{"accept", "application/jwt, application/json"}],
 
-    Body0 = add_pkce(QsBodyIn, Pkce),
+    Body0 = add_pkce_verifier(QsBodyIn, PkceVerifier),
 
     MaybeAuthMethod = case AuthenticateClient of
         true -> select_preferred_auth(SupportedAuthMethods);
@@ -797,7 +799,7 @@ retrieve_a_token(QsBodyIn, Pkce, ClientContext, Opts, TelemetryOpts, Authenticat
                 {ok, TokenResponse}
             else
                 {error, auth_method_not_possible} ->
-                    retrieve_a_token(QsBodyIn, Pkce, ClientContext#oidcc_client_context{
+                    retrieve_a_token(QsBodyIn, PkceVerifier, ClientContext#oidcc_client_context{
                         provider_configuration = Configuration#oidcc_provider_configuration{
                             token_endpoint_auth_methods_supported =
                                 SupportedAuthMethods -- [atom_to_binary(AuthMethod)]
@@ -969,13 +971,13 @@ add_authentication(
 ) ->
     {ok, {QsBodyList, Header}}.
 
--spec add_pkce(QueryList, Pkce) -> oidcc_http_util:query_params() when
+-spec add_pkce_verifier(QueryList, PkceVerifier) -> oidcc_http_util:query_params() when
     QueryList :: oidcc_http_util:query_params(),
-    Pkce :: pkce() | undefined.
-add_pkce(BodyQs, #{verifier := CV}) ->
-    [{<<"code_verifier">>, CV} | BodyQs];
-add_pkce(BodyQs, undefined) ->
-    BodyQs.
+    PkceVerifier :: binary() | none.
+add_pkce_verifier(BodyQs, none) ->
+    BodyQs;
+add_pkce_verifier(BodyQs, PkceVerifier) ->
+    [{<<"code_verifier">>, PkceVerifier} | BodyQs].
 
 -spec signed_client_assertion(ClientContext, Jwk) -> {ok, binary()} | {error, error()} when
     Jwk :: jose_jwk:key(),

test/oidcc_authorization_test.erl

@@ -14,6 +14,12 @@ create_redirect_url_test() ->
     {ok, Configuration} = oidcc_provider_configuration:decode_configuration(
         jose:decode(ValidConfigString)
     ),
+    PkcePlainConfiguration = Configuration#oidcc_provider_configuration{
+        code_challenge_methods_supported = [<<"plain">>]
+    },
+    NoPkceConfiguration = Configuration#oidcc_provider_configuration{
+        code_challenge_methods_supported = undefined
+    },
 
     Jwks = jose_jwk:from_pem_file(PrivDir ++ "/test/fixtures/jwk.pem"),
 
@@ -24,6 +30,12 @@ create_redirect_url_test() ->
 
     ClientContext =
         oidcc_client_context:from_manual(Configuration, Jwks, ClientId, <<"client_secret">>),
+    PkcePlainClientContext =
+        oidcc_client_context:from_manual(
+            PkcePlainConfiguration, Jwks, ClientId, <<"client_secret">>
+        ),
+    NoPkceClientContext =
+        oidcc_client_context:from_manual(NoPkceConfiguration, Jwks, ClientId, <<"client_secret">>),
 
     BaseOpts =
         #{
@@ -50,15 +62,16 @@ create_redirect_url_test() ->
                 url_extension => [{<<"test">>, <<"id">>}, {<<"other">>, <<"green">>}]
             }
         ),
-    Opts5 =
-        maps:merge(BaseOpts, #{pkce => #{challenge => <<"foo">>, method => <<"plain">>}}),
+    Opts5 = maps:merge(BaseOpts, #{pkce_verifier => <<"foo">>}),
 
     {ok, Url1} = oidcc_authorization:create_redirect_url(ClientContext, BaseOpts),
     {ok, Url2} = oidcc_authorization:create_redirect_url(ClientContext, Opts1),
     {ok, Url3} = oidcc_authorization:create_redirect_url(ClientContext, Opts2),
     {ok, Url4} = oidcc_authorization:create_redirect_url(ClientContext, Opts3),
     {ok, Url5} = oidcc_authorization:create_redirect_url(ClientContext, Opts4),
     {ok, Url6} = oidcc_authorization:create_redirect_url(ClientContext, Opts5),
+    {ok, Url7} = oidcc_authorization:create_redirect_url(PkcePlainClientContext, Opts5),
+    {ok, Url8} = oidcc_authorization:create_redirect_url(NoPkceClientContext, Opts5),
 
     ExpUrl1 =
         <<"https://my.provider/auth?scope=openid&response_type=code&client_id=client_id&redirect_uri=https%3A%2F%2Fmy.server%2Freturn&test=id">>,
@@ -81,9 +94,17 @@ create_redirect_url_test() ->
     ?assertEqual(ExpUrl5, iolist_to_binary(Url5)),
 
     ExpUrl6 =
-        <<"https://my.provider/auth?scope=openid&code_challenge=foo&code_challenge_method=plain&response_type=code&client_id=client_id&redirect_uri=https%3A%2F%2Fmy.server%2Freturn&test=id">>,
+        <<"https://my.provider/auth?scope=openid&code_challenge=LCa0a2j_xo_5m0U8HTBBNBNCLXBkg7-g-YpeiGJm564&code_challenge_method=S256&response_type=code&client_id=client_id&redirect_uri=https%3A%2F%2Fmy.server%2Freturn&test=id">>,
     ?assertEqual(ExpUrl6, iolist_to_binary(Url6)),
 
+    ExpUrl7 =
+        <<"https://my.provider/auth?scope=openid&code_challenge=foo&code_challenge_method=plain&response_type=code&client_id=client_id&redirect_uri=https%3A%2F%2Fmy.server%2Freturn&test=id">>,
+    ?assertEqual(ExpUrl7, iolist_to_binary(Url7)),
+
+    ExpUrl8 =
+        <<"https://my.provider/auth?scope=openid&response_type=code&client_id=client_id&redirect_uri=https%3A%2F%2Fmy.server%2Freturn&test=id">>,
+    ?assertEqual(ExpUrl8, iolist_to_binary(Url8)),
+
     ok.
 
 create_redirect_url_with_request_object_test() ->