From be5921028c64fa8c4ed72234ca874009ad52f162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jimmy=20Z=C3=B6ger?= Date: Thu, 29 Aug 2024 16:53:35 +0200 Subject: [PATCH 01/10] no-release: Fix id_token version but skip the release process --- src/id_token.app.src | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/id_token.app.src b/src/id_token.app.src index 43c5847..0dc45eb 100644 --- a/src/id_token.app.src +++ b/src/id_token.app.src @@ -1,6 +1,6 @@ {application, id_token, [{description, "An OTP application to validate Id tokens"}, - {vsn, "0.1.1"}, + {vsn, git}, {registered, []}, {mod, {id_token_app, []}}, {applications, From 4ea5d9d05d8a0965e914a3e41192dbf130c65e20 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:18:31 +0100 Subject: [PATCH 02/10] Simplify maintenance of Elvis linter rules --- .gitignore | 1 - .tool-versions | 1 + Makefile | 29 +--------------------- elvis.config | 65 +++----------------------------------------------- rebar.config | 5 +++- 5 files changed, 9 insertions(+), 92 deletions(-) create mode 100644 .tool-versions diff --git a/.gitignore b/.gitignore index 7ec2064..69ace0d 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,3 @@ _build rebar3.crashdump *~ .edts -.elvis diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 0000000..9045e20 --- /dev/null +++ b/.tool-versions @@ -0,0 +1 @@ +erlang 25.3.2.13 diff --git a/Makefile b/Makefile index a86ebbb..0676e54 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,3 @@ -ELVIS_IN_PATH := $(shell elvis --version 2> /dev/null) -ELVIS_LOCAL := $(shell .elvis/_build/default/bin/elvis --version 2> /dev/null) - all: compile compile: @@ -33,29 +30,5 @@ unlock: lock: rebar3 lock -elvis: -ifdef ELVIS_IN_PATH - elvis git-branch origin/HEAD -V -else ifdef ELVIS_LOCAL - .elvis/_build/default/bin/elvis git-branch origin/HEAD -V -else - $(MAKE) compile_elvis - .elvis/_build/default/bin/elvis git-branch origin/HEAD -V -endif - elvis_rock: -ifdef ELVIS_IN_PATH - elvis rock -else ifdef ELVIS_LOCAL - .elvis/_build/default/bin/elvis rock -else - $(MAKE) compile_elvis - .elvis/_build/default/bin/elvis rock -endif - -compile_elvis: - git clone https://github.com/inaka/elvis.git .elvis && \ - cd .elvis && \ - rebar3 compile && \ - rebar3 escriptize && \ - cd .. + rebar3 lint diff --git a/elvis.config b/elvis.config index 4bdf35a..20671ee 100644 --- a/elvis.config +++ b/elvis.config @@ -1,74 +1,15 @@ %% -*- erlang -*- [ {elvis, [ {config, - [ #{dirs => [ "src/*" - , "src" + [ #{dirs => [ "src" ], filter => "*.erl", - ignore => [], - ruleset => [{elvis_style, state_record_and_type, disable}], - rules => [ {elvis_text_style, line_length, - #{ limit => 100, - skip_comments => false - }} - , {elvis_text_style, no_tabs} - , {elvis_text_style, no_trailing_whitespace} - , {elvis_style, macro_module_names} - , {elvis_style, nesting_level, - #{ level => 3, - ignore => [ - ] - }} - , {elvis_style, god_modules, - #{ limit => 25, - ignore => [ - ] - }} - , {elvis_style, no_nested_try_catch, - #{ignore => [ - ] - }} - , {elvis_style, invalid_dynamic_call, - #{ignore => [ - ] - }} - , {elvis_style, used_ignored_variable} - , {elvis_style, no_behavior_info} - , {elvis_style, module_naming_convention, - #{ ignore => [], - regex => "^([a-z][a-z0-9]*_?)([a-z0-9]*_?)*$" - }} - , {elvis_style, function_naming_convention, - #{ regex => "^([a-z][a-z0-9]*_?)([a-z0-9]*_?)*$" - }} - , {elvis_style, variable_naming_convention, - #{ regex => "^_?([A-Z][0-9a-zA-Z_]*)$" - }} - , {elvis_style, state_record_and_type} - , {elvis_style, no_spec_with_records} - , {elvis_style, dont_repeat_yourself, - #{ min_complexity => 25, - ignore => [ - ] - }} - ] + ruleset => erl_files }, #{dirs => [ "test" ], filter => "*.erl", - rules => [ {elvis_text_style, line_length, - #{ limit => 100, - skip_comments => false - }} - , {elvis_text_style, no_tabs} - , {elvis_text_style, no_trailing_whitespace} - , {elvis_style, macro_module_names} - , {elvis_style, no_debug_call, - #{ignore => [ prop_pubkeys_storage, - prop_id_token_sign - ] - }} - ] + ruleset => erl_files } ] } diff --git a/rebar.config b/rebar.config index 0549c46..a4c25b3 100644 --- a/rebar.config +++ b/rebar.config @@ -8,7 +8,10 @@ {apps, [id_token]} ]}. -{project_plugins, [rebar3_proper]}. +{project_plugins, [ + {rebar3_proper, "0.12.1"}, + {rebar3_lint, "3.2.6"} +]}. {profiles, [{test, [ From 72f075121eeb6418573f81a82191c904f59c0446 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:18:45 +0100 Subject: [PATCH 03/10] rebar3_lint: fix for consistent_variable_casing --- src/id_token_jws.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/id_token_jws.erl b/src/id_token_jws.erl index 39d7faa..31fb854 100644 --- a/src/id_token_jws.erl +++ b/src/id_token_jws.erl @@ -73,11 +73,11 @@ generate_key_for(Alg, Options) -> _ -> generate_rsa_key(Alg, Options) end, Jwk0 = jose_jwk:from_key(Key), - Jwk = Jwk0#jose_jwk{fields = #{<<"kid">> => kid(Options), + JWK = Jwk0#jose_jwk{fields = #{<<"kid">> => kid(Options), <<"use">> => <<"sig">>, <<"iat">> => iat(Options)}}, - {_, PublicKeyMap} = jose_jwk:to_public_map(Jwk), - {Jwk, PublicKeyMap}. + {_, PublicKeyMap} = jose_jwk:to_public_map(JWK), + {JWK, PublicKeyMap}. extract_kid(IdToken) -> Protected = jose_jwt:peek_protected(IdToken), @@ -91,7 +91,7 @@ extract_kid(IdToken) -> validate_signature(Key, IdToken) -> case jose_jwt:verify(Key, IdToken) of - {true, #jose_jwt{fields = Claims}, _Jws} -> + {true, #jose_jwt{fields = Claims}, _JWS} -> {ok, Claims}; {false, _, _} -> {error, invalid_signature} From f31d582eab145c166d3341eda333c64f6c2b99c5 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:19:23 +0100 Subject: [PATCH 04/10] rebar3_lint: fix for no_trailing_whitespace --- src/id_token_jws.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/id_token_jws.erl b/src/id_token_jws.erl index 31fb854..88b393d 100644 --- a/src/id_token_jws.erl +++ b/src/id_token_jws.erl @@ -4,7 +4,7 @@ [generate_key_for/1, generate_key_for/2, sign/2, sign/3, validate/1, validate/2, - extract_kid/1]). + extract_kid/1]). -ignore_xref(?API_CALLS). -export(?API_CALLS). From 1d10109cf5d886b31ffc7dc2c0115f61f071dd92 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:21:45 +0100 Subject: [PATCH 05/10] rebar3_lint: "fix" for no_block_expressions A proper fix would probably be to not avoid it, but 1. that'd change code, which is not our priority here, 2. that might not be easy to achieve when macros are involved --- elvis.config | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/elvis.config b/elvis.config index 20671ee..8bbc3e0 100644 --- a/elvis.config +++ b/elvis.config @@ -4,12 +4,16 @@ [ #{dirs => [ "src" ], filter => "*.erl", - ruleset => erl_files + ruleset => erl_files, + rules => [ {elvis_style, no_block_expressions, disable} + ] }, #{dirs => [ "test" ], filter => "*.erl", - ruleset => erl_files + ruleset => erl_files, + rules => [ {elvis_style, no_block_expressions, disable} + ] } ] } From b9a7b409cf6569a23aa8a21e9e9ed4d4290f9a09 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:22:33 +0100 Subject: [PATCH 06/10] rebar3_lint: fix for used_ignored_variable --- test/prop_id_token_jwt.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/prop_id_token_jwt.erl b/test/prop_id_token_jwt.erl index 4d4b7a4..966b51f 100644 --- a/test/prop_id_token_jwt.erl +++ b/test/prop_id_token_jwt.erl @@ -38,7 +38,7 @@ prop_valid_signature() -> end). prop_invalid_signature() -> - ?FORALL({{Jwk, _PublicKeyMap}, {OtherJwk, OtherPublicKeyMap}, Claims}, + ?FORALL({{Jwk, PublicKeyMap}, {OtherJwk, OtherPublicKeyMap}, Claims}, {key_pair(), key_pair(), jwt_claims()}, begin #jose_jwk{fields = OtherFields} = OtherJwk, @@ -49,7 +49,7 @@ prop_invalid_signature() -> end). prop_no_matching_key() -> - ?FORALL({[{Jwk, _PublicKeyMap} | OtherKeys], Claims}, + ?FORALL({[{Jwk, PublicKeyMap} | OtherKeys], Claims}, {non_empty(list(key_pair())), jwt_claims()}, begin Jwt = id_token_jws:sign(Claims, Jwk), From fbc814ed3423d811b20d31a87c3f0ed907f8a95b Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:23:38 +0100 Subject: [PATCH 07/10] rebar3_lint: fix for operator_spaces --- test/prop_id_token_sign.erl | 2 +- test/prop_pubkeys_storage.erl | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/prop_id_token_sign.erl b/test/prop_id_token_sign.erl index b9ebdc4..e7c7355 100644 --- a/test/prop_id_token_sign.erl +++ b/test/prop_id_token_sign.erl @@ -27,7 +27,7 @@ prop_test() -> {History, State, Result} = run_commands(?MODULE, Cmds), id_token_sign:stop(), ?WHENFAIL(io:format("History: ~p\nState: ~p\nResult: ~p\n", - [History,State,Result]), + [History, State, Result]), aggregate(command_names(Cmds), Result =:= ok)) end). diff --git a/test/prop_pubkeys_storage.erl b/test/prop_pubkeys_storage.erl index 2b0ac18..76d2c2a 100644 --- a/test/prop_pubkeys_storage.erl +++ b/test/prop_pubkeys_storage.erl @@ -23,7 +23,7 @@ prop_test() -> {History, State, Result} = run_commands(?MODULE, Cmds), id_token_pubkeys_storage:stop(), ?WHENFAIL(io:format("History: ~p\nState: ~p\nResult: ~p\n", - [History,State,Result]), + [History, State, Result]), aggregate(command_names(Cmds), Result =:= ok)) end). @@ -37,9 +37,9 @@ initial_state() -> %% @doc List of possible commands to run against the system command(_State) -> frequency([{1, {call, id_token_pubkeys_storage, get_all, []}}, - {1, {call, id_token_pubkeys_storage, get,[kid()]}}, - {3, {call, id_token_pubkeys_storage, put,[key()]}}, - {1, {call, id_token_pubkeys_storage, delete,[kid()]}} + {1, {call, id_token_pubkeys_storage, get, [kid()]}}, + {3, {call, id_token_pubkeys_storage, put, [key()]}}, + {1, {call, id_token_pubkeys_storage, delete, [kid()]}} ]). %% @doc Determines whether a command should be valid under the @@ -84,7 +84,7 @@ key() -> end). kid() -> - ?LET(Int, integer(1,10), integer_to_binary(Int)). + ?LET(Int, integer(1, 10), integer_to_binary(Int)). %%%_* Emacs ============================================================ %%% Local Variables: From 7493914924bdac616e5a5275775a987e577aabf9 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:25:11 +0100 Subject: [PATCH 08/10] rebar3_lint: "fix" for no_debug_call Instead of an actual fix we work around it by allowing it; on the other hand I'm not sure why it exists in the code but it's not our priority to check that now --- elvis.config | 1 + 1 file changed, 1 insertion(+) diff --git a/elvis.config b/elvis.config index 8bbc3e0..b14991c 100644 --- a/elvis.config +++ b/elvis.config @@ -13,6 +13,7 @@ filter => "*.erl", ruleset => erl_files, rules => [ {elvis_style, no_block_expressions, disable} + , {elvis_style, no_debug_call, disable} ] } ] From f1d62d719df2987b4f77681d05c4d305781a1c07 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:44:47 +0100 Subject: [PATCH 09/10] Make all Jw[] JW[] for readability --- src/id_token_jwks.erl | 4 ++-- src/id_token_jws.erl | 4 ++-- src/id_token_sign.erl | 4 ++-- test/id_token_SUITE.erl | 28 ++++++++++++++-------------- test/prop_id_token_jwt.erl | 22 +++++++++++----------- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/id_token_jwks.erl b/src/id_token_jwks.erl index c12b80c..a7ebc91 100644 --- a/src/id_token_jwks.erl +++ b/src/id_token_jwks.erl @@ -33,8 +33,8 @@ get_pub_keys(Uri) -> get_jwks_uri(Uri) -> case hackney:request(get, Uri, [], <<>>, [with_body]) of {ok, 200, _Headers, Body} -> - #{<<"jwks_uri">> := JwksUri} = jsx:decode(Body, [return_maps]), - {ok, JwksUri}; + #{<<"jwks_uri">> := JWKSUri} = jsx:decode(Body, [return_maps]), + {ok, JWKSUri}; {ok, _, _, _} -> {error, service_unavailable}; {error, Reason} -> diff --git a/src/id_token_jws.erl b/src/id_token_jws.erl index 88b393d..ee4702c 100644 --- a/src/id_token_jws.erl +++ b/src/id_token_jws.erl @@ -72,8 +72,8 @@ generate_key_for(Alg, Options) -> <<"ES", _S/binary>> -> generate_ec_key(Alg, Options); _ -> generate_rsa_key(Alg, Options) end, - Jwk0 = jose_jwk:from_key(Key), - JWK = Jwk0#jose_jwk{fields = #{<<"kid">> => kid(Options), + JWK0 = jose_jwk:from_key(Key), + JWK = JWK0#jose_jwk{fields = #{<<"kid">> => kid(Options), <<"use">> => <<"sig">>, <<"iat">> => iat(Options)}}, {_, PublicKeyMap} = jose_jwk:to_public_map(JWK), diff --git a/src/id_token_sign.erl b/src/id_token_sign.erl index 0c27e14..39916db 100644 --- a/src/id_token_sign.erl +++ b/src/id_token_sign.erl @@ -75,8 +75,8 @@ handle_info(_Request, Timers0) -> %%% Internal functions %%%=================================================================== put_key_for(Alg, Options) -> - {Jwk, PublicKeyMap} = id_token_jws:generate_key_for(Alg, Options), - SignKeyFun = fun() -> Jwk end, + {JWK, PublicKeyMap} = id_token_jws:generate_key_for(Alg, Options), + SignKeyFun = fun() -> JWK end, #{<<"kid">> := Kid, <<"iat">> := Iat} = PublicKeyMap, TTU = maps:get(ttu, Options, ?TTU), Exp = Iat + TTU, diff --git a/test/id_token_SUITE.erl b/test/id_token_SUITE.erl index b59c04d..4daebb0 100644 --- a/test/id_token_SUITE.erl +++ b/test/id_token_SUITE.erl @@ -19,26 +19,26 @@ init_per_suite(Config) -> end_per_suite(_Config) -> ok. init_per_testcase(_TestCase, Config) -> - {Jwk, PublicKeyMap} = + {JWK, PublicKeyMap} = id_token_jws:generate_key_for(<<"RS256">>, #{key_size => 1024}), Claims = #{ <<"exp">> => erlang:system_time(second) + 10}, - Jwt = id_token_jws:sign(Claims, Jwk), + JWT = id_token_jws:sign(Claims, JWK), mock_id_provider(PublicKeyMap, 0), application:ensure_all_started(id_token), - [{jwt, Jwt}, {pubkeys, [PublicKeyMap]} | Config]. + [{jwt, JWT}, {pubkeys, [PublicKeyMap]} | Config]. end_per_testcase(_TestCase, Config) -> application:stop(id_token), meck:unload([id_token_jwks, hackney]), Config. validate_jwt(Config) -> - Jwt = ?config(jwt, Config), - ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)). + JWT = ?config(jwt, Config), + ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, JWT)). keys_are_cached(Config) -> - Jwt = ?config(jwt, Config), - ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)), - ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)), + JWT = ?config(jwt, Config), + ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, JWT)), + ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, JWT)), 1 = meck:num_calls(id_token_jwks, get_pub_keys, 1), ok. @@ -49,9 +49,9 @@ keys_are_only_refreshed_once_per_kid(Config) -> ok = meck:expect(id_token_provider, get_cached_keys, 1, CurrentKeyCache), %% create JWT with kid that's not yet in the pubkey cache - {Jwk, NewPubkey} = id_token_jws:generate_key_for(<<"RS256">>, #{key_size => 1024}), + {JWK, NewPubkey} = id_token_jws:generate_key_for(<<"RS256">>, #{key_size => 1024}), Claims = #{ <<"exp">> => erlang:system_time(second) + 10 }, - Jwt = id_token_jws:sign(Claims, Jwk), + JWT = id_token_jws:sign(Claims, JWK), %% set up provider to return new pubkeys with a 50 ms delay HttpReponseDelay = 50, @@ -59,14 +59,14 @@ keys_are_only_refreshed_once_per_kid(Config) -> ?assertEqual(0, meck:num_calls(id_token_jwks, get_pub_keys, 1)), %% try to validate multiple JWTs based on kid that's not yet in the key cache - spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end), - spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end), - spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end), + spawn(fun() -> id_token:validate(?ID_PROVIDER, JWT) end), + spawn(fun() -> id_token:validate(?ID_PROVIDER, JWT) end), + spawn(fun() -> id_token:validate(?ID_PROVIDER, JWT) end), timer:sleep(10 * HttpReponseDelay), %% ensure that the pubkey cache was only refreshed once ?assertEqual(1, meck:num_calls(id_token_jwks, get_pub_keys, 1)), - ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)), + ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, JWT)), meck:unload([id_token_provider]). diff --git a/test/prop_id_token_jwt.erl b/test/prop_id_token_jwt.erl index 966b51f..ee769b2 100644 --- a/test/prop_id_token_jwt.erl +++ b/test/prop_id_token_jwt.erl @@ -26,36 +26,36 @@ eunit_test_() -> %%% Properties %%% %%%%%%%%%%%%%%%%%% prop_valid_signature() -> - ?FORALL({{Jwk, PublicKeyMap}, Claims}, + ?FORALL({{JWK, PublicKeyMap}, Claims}, {key_pair(), jwt_claims()}, begin #{<<"exp">> := Exp} = Claims, - Jwt = id_token_jws:sign(Claims, Jwk), - Result = id_token_jws:validate(Jwt, [PublicKeyMap]), + JWT = id_token_jws:sign(Claims, JWK), + Result = id_token_jws:validate(JWT, [PublicKeyMap]), Exp =< erlang:system_time(second) andalso {error, expired} =:= Result orelse {ok, Claims} =:= Result end). prop_invalid_signature() -> - ?FORALL({{Jwk, PublicKeyMap}, {OtherJwk, OtherPublicKeyMap}, Claims}, + ?FORALL({{JWK, PublicKeyMap}, {OtherJWK, OtherPublicKeyMap}, Claims}, {key_pair(), key_pair(), jwt_claims()}, begin - #jose_jwk{fields = OtherFields} = OtherJwk, - JwkWithChangedKid = Jwk#jose_jwk{fields = OtherFields}, - Jwt = id_token_jws:sign(Claims, JwkWithChangedKid), + #jose_jwk{fields = OtherFields} = OtherJWK, + JWKWithChangedKid = JWK#jose_jwk{fields = OtherFields}, + JWT = id_token_jws:sign(Claims, JWKWithChangedKid), {error, invalid_signature} - =:= id_token_jws:validate(Jwt, [OtherPublicKeyMap]) + =:= id_token_jws:validate(JWT, [OtherPublicKeyMap]) end). prop_no_matching_key() -> - ?FORALL({[{Jwk, PublicKeyMap} | OtherKeys], Claims}, + ?FORALL({[{JWK, PublicKeyMap} | OtherKeys], Claims}, {non_empty(list(key_pair())), jwt_claims()}, begin - Jwt = id_token_jws:sign(Claims, Jwk), + JWT = id_token_jws:sign(Claims, JWK), PublicKeys = lists:map(fun({_, Key}) -> Key end, OtherKeys), {error, no_public_key_matches} - =:= id_token_jws:validate(Jwt, PublicKeys) + =:= id_token_jws:validate(JWT, PublicKeys) end). %%%%%%%%%%%%%%%%%% From 6ab9d054d0c5b6e401160952a035dba7b7d10111 Mon Sep 17 00:00:00 2001 From: "Paulo F. Oliveira" Date: Thu, 29 Aug 2024 16:45:52 +0100 Subject: [PATCH 10/10] Delete unwarranted push --- .tool-versions | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .tool-versions diff --git a/.tool-versions b/.tool-versions deleted file mode 100644 index 9045e20..0000000 --- a/.tool-versions +++ /dev/null @@ -1 +0,0 @@ -erlang 25.3.2.13