refactor: Fix version, update elvis config and variable renaming (#17)

* no-release: Fix id_token version but skip the release process

* Simplify maintenance of Elvis linter rules

* rebar3_lint: fix for consistent_variable_casing

* rebar3_lint: fix for no_trailing_whitespace

* 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

* rebar3_lint: fix for used_ignored_variable

* rebar3_lint: fix for operator_spaces

* 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

* Make all Jw[<lowercase>] JW[<uppercase>] for readability

* Delete unwarranted push

---------

Co-authored-by: Paulo F. Oliveira <[email protected]>

Author: zigge-zagge

.gitignore

@@ -18,4 +18,3 @@ _build
 rebar3.crashdump
 *~
 .edts
-.elvis

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

elvis.config

@@ -1,73 +1,19 @@
 %% -*- 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,
+          rules => [ {elvis_style, no_block_expressions, disable}
                    ]
          },
         #{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,
+          rules => [ {elvis_style, no_block_expressions, disable}
+                   , {elvis_style, no_debug_call, disable}
                    ]
          }
       ]

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, [

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,

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} ->

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).
 
@@ -72,12 +72,12 @@ 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),
-  {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}

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,

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,24 +49,24 @@ 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,
   mock_id_provider(NewPubkey, HttpReponseDelay),
   ?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]).
 

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).
 
 %%%%%%%%%%%%%%%%%%

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).
 

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: