Skip to content

Commit

Permalink
chore: modernise the repository a bit (#18)
Browse files Browse the repository at this point in the history
* Pin specific versions and help Renovate

This way it's easier for us to reason on the updates
semantically.

* Add a few "good practice" compiler warnings

As per https://github.com/kivra/erlang_template

* "Temporarily" prevent jose's deprecation warning from compiling self

* Add `dialyzer` options

* Act on dialyzer's `unmatched_returns`

* Prevent Dialyzer's "Unknown function jsx:decode"

... while applying the good practice to list
all deps under .app.src

* xref: stop caring about "unused exports"

It's either this or flagging all "interface API" functions
as -ignore_xref, which is potentially more difficult to
maintain. Choices (?)

* Enable test coverage

* Don't fight Elvis on this (work around it)

If we fix for Elvis we get a compiler warning

* Fix compiler warnings

* Act on CI results: fix for `rebar3_lint`

* Own it
  • Loading branch information
kivra-pauoli authored Sep 3, 2024
1 parent 8450b8f commit 396d05b
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 15 deletions.
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# the project is owned by the platform team
* @kivra/platform-team
31 changes: 29 additions & 2 deletions rebar.config
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
{erl_opts, [debug_info]}.
{erl_opts, [
debug_info,
warnings_as_errors,
warn_export_vars,
warn_unused_import,
warn_keywords
]}.

{dialyzer, [
{plt_apps, all_deps},
incremental,
{warnings, [unmatched_returns]}
]}.

{overrides, [
{override, jose, [{erl_opts, [debug_info, no_warnings_as_errors]}]}
]}.

{deps, [ {jsx, {git, "https://github.com/talentdeficit/jsx.git" , {tag, "v3.1.0"}}}
, {jose, {git, "https://github.com/potatosalad/erlang-jose.git" , {tag, "1.11.5"}}}
, {hackney, {git, "https://github.com/benoitc/hackney.git" , {tag, "1.18.0"}}}
Expand All @@ -15,9 +32,19 @@

{profiles,
[{test, [
{cover_enabled, true},
{cover_opts, [verbose]},
{erl_opts, [nowarn_export_all]},
{deps, [ proper
{deps, [ {proper, "1.4.0"}
, {meck, "0.8.13"}
]}
]}
]}.

{xref_checks, [
undefined_function_calls,
undefined_functions,
locals_not_used,
deprecated_function_calls,
deprecated_functions
]}.
2 changes: 1 addition & 1 deletion src/ets_pubkeys_storage.erl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ put(#{<<"kid">> := Kid} = Key) ->

%% gen_server callbacks
init(A) ->
ets:new(?MODULE, ?ETS_OPTIONS),
?MODULE = ets:new(?MODULE, ?ETS_OPTIONS),
{ok, A}.
handle_call(_, _, S) -> {noreply, S}.
handle_cast(_, S) -> {noreply, S}.
Expand Down
3 changes: 2 additions & 1 deletion src/id_token.app.src
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
[kernel,
stdlib,
hackney,
jose
jose,
jsx
]},
{env,[]},
{modules, []},
Expand Down
4 changes: 2 additions & 2 deletions src/id_token_provider.erl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ add_provider(Name, Uri) ->
%%% gen_server callbacks
%%%===================================================================
init([]) ->
ets:new(?ID_TOKEN_CACHE, ?ETS_OPTIONS),
?ID_TOKEN_CACHE = ets:new(?ID_TOKEN_CACHE, ?ETS_OPTIONS),
Providers = id_token_jwks:get_providers(),
lists:foreach(fun add_provider/1, Providers),
case application:get_env(id_token, async_revalidate, false) of
Expand Down Expand Up @@ -79,7 +79,7 @@ handle_info({refresh, Provider}, State) ->
%% the price and re-initiate async_revalidate-loop after 10 seconds
10_000
end,
timer:send_after(Delay, self(), {refresh, Provider}),
{ok, _} = timer:send_after(Delay, self(), {refresh, Provider}),
{noreply, State}.

maybe_refresh(Provider, #{force_refresh := true}) ->
Expand Down
2 changes: 1 addition & 1 deletion src/id_token_pubkeys_storage.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
-define(call_callback(Args),
begin
Mod = ?BACKEND,
code:ensure_loaded(Mod),
_ = code:ensure_loaded(Mod),
case erlang:function_exported(Mod, ?FUNCTION_NAME, ?FUNCTION_ARITY) of
true -> erlang:apply(Mod, ?FUNCTION_NAME, Args);
false -> {error, not_exported}
Expand Down
2 changes: 1 addition & 1 deletion src/id_token_sign.erl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ add_key_for(Alg, Options) ->
%%%===================================================================
init([]) ->
id_token_pubkeys_storage:start(),
ets:new(?MODULE, ?ETS_OPTIONS),
?MODULE = ets:new(?MODULE, ?ETS_OPTIONS),
SignKeys = application:get_env(id_token, sign_keys, []),
Timers = lists:sort([put_key_for(Alg, Opts) || {Alg, Opts} <- SignKeys]),
{ok, Timers, timeout(Timers)}.
Expand Down
6 changes: 4 additions & 2 deletions test/prop_id_token_jwt.erl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<<"RS256">>, <<"RS384">>, <<"RS512">>,
<<"ES256">>, <<"ES384">>, <<"ES512">>]).

-elvis([{elvis_style, used_ignored_variable, disable}]).

%%%%%%%%%%%%%%%%%%%%
%%% Eunit runner %%%
%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -38,7 +40,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,
Expand All @@ -49,7 +51,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),
Expand Down
4 changes: 2 additions & 2 deletions test/prop_id_token_sign.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
%%%%%%%%%%%%%%%%%%%%
eunit_test_() ->
Opts = [{numtests, 30}],
?_assert(proper:quickcheck(prop_test(), Opts)).
?_assert(proper:quickcheck(prop_test2(), Opts)).

%%%%%%%%%%%%%%%%%%
%%% PROPERTIES %%%
%%%%%%%%%%%%%%%%%%
prop_test() ->
prop_test2() ->
?FORALL(Cmds, commands(?MODULE),
begin
id_token_sign:start_link(),
Expand Down
6 changes: 3 additions & 3 deletions test/prop_pubkeys_storage.erl
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
%%%%%%%%%%%%%%%%%%%%
eunit_test_() ->
Opts = [{numtests, 30}],
?_assert(proper:quickcheck(prop_test(), Opts)).
?_assert(proper:quickcheck(prop_test1(), Opts)).

%%%%%%%%%%%%%%%%%%
%%% PROPERTIES %%%
%%%%%%%%%%%%%%%%%%
prop_test() ->
prop_test1() ->
?FORALL(Cmds, commands(?MODULE),
begin
id_token_pubkeys_storage:start(),
Expand Down Expand Up @@ -54,7 +54,7 @@ postcondition(State, {call, _Mod, get_all, _Args}, {ok, Res}) ->
lists:sort(maps:values(State)) =:= lists:sort(Res);
postcondition(State, {call, _Mod, get, [Kid]}, Res) ->
case {maps:find(Kid, State), Res} of
{{ok, _V}, {ok, _V}} -> true;
{{ok, V}, {ok, V}} -> true;
{error, {error, not_found}} -> true;
_ -> false
end;
Expand Down

0 comments on commit 396d05b

Please sign in to comment.