ueberauth / guardian

Elixir Authentication
MIT License
3.4k stars 381 forks source link

Fix AtomEncoding and TextEncoding dialyzer error #666

Closed dolfinus closed 3 years ago

dolfinus commented 3 years ago

Hello.

Here is an example of source code used in one of my projects:


  use Guardian,
    otp_app: :arkenston,
    permissions: %{
      author: [
        :create_author
        :delete_admin_author
        :delete_moderator_author
        :delete_self
        :delete_unassigned_author
        :delete_user_author
        :update_admin_author
        :update_moderator_author
        :update_self
        :update_unassigned_author
        :update_user_author,
        ...
      ]

  use Guardian.Permissions, encoding: Guardian.Permissions.AtomEncoding

The permission encoding and decoding works perfectly, but dialyxir raises errors like that:

lib/arkenston/guardian.ex:10:invalid_contract
The @spec for the function does not match the success typing of the function.

Function:
Arkenston.Guardian.available_permissions/0

Success typing:
@spec available_permissions() :: %{
  :author => [
    :create_author
    | :delete_admin_author
    | :delete_moderator_author
    | :delete_self
    | :delete_unassigned_author
    | :delete_user_author
    | :update_admin_author
    | :update_moderator_author
    | :update_self
    | :update_unassigned_author
    | :update_user_author,
    ...
  ],
  :user => [atom(), ...]
}

________________________________________________________________________________
lib/arkenston/guardian.ex:10:call
The function call will not succeed.

Guardian.Permissions.AtomEncoding.decode(_ :: integer(), _ :: any(), %{
  <<97, 117, 116, 104, 111, 114>> => %{
    <<99, 114, 101, 97, 116, 101, 95, 97, 117, 116, 104, 111, 114>> => 1,
    <<100, 101, 108, 101, 116, 101, 95, 97, 100, 109, 105, 110, 95, 97, 117, 116, 104, 111,
      114>> => 512,
    <<100, 101, 108, 101, 116, 101, 95, 109, 111, 100, 101, 114, 97, 116, 111, 114, 95, 97,
      117, 116, 104, 111, 114>> => 256,
    <<100, 101, 108, 101, 116, 101, 95, 115, 101, 108, 102>> => 1024,
    <<100, 101, 108, 101, 116, 101, 95, 117, 110, 97, 115, 115, 105, 103, 110, 101, 100, 95,
      97, 117, 116, 104, 111, 114>> => 64,
    <<100, 101, 108, 101, 116, 101, 95, 117, 115, 101, 114, 95, 97, 117, 116, 104, 111,
      114>> => 128,
    <<117, 112, 100, 97, 116, 101, 95, 97, 100, 109, 105, 110, 95, 97, 117, 116, 104, 111,
      114>> => 16,
    <<117, 112, 100, 97, 116, 101, 95, 109, 111, 100, 101, 114, 97, 116, 111, 114, 95, 97,
      117, 116, 104, 111, 114>> => 8,
    <<117, 112, 100, 97, 116, 101, 95, 115, 101, 108, 102>> => 32,
    <<117, 112, 100, 97, 116, 101, 95, 117, 110, 97, 115, 115, 105, 103, 110, 101, 100, 95,
      97, 117, 116, 104, 111, 114>> => 2,
    <<117, 112, 100, 97, 116, 101, 95, 117, 115, 101, 114, 95, 97, 117, 116, 104, 111, 114>> =>
      4
  },
  ...
  }
})

will never return since it differs in arguments with
positions 1st from the success typing arguments:

(maybe_improper_list(), any(), any())

First error is connected to this type spec: https://github.com/ueberauth/guardian/blob/75b7d8fdb7eb61f3048ce6b9394d377a5b004b52/lib/guardian/permissions.ex#L110

Bitwise encoder stores permissions as %{entity_atom => %{permission_string => 2**bit}}. But Atom encoder stores permissions as %{entity_atom => [permission_atom, ...]}, which does no satisfy type spec. As well as Text encoder which stores permissions as %{entity_atom => [permission_string, ...]}.

In this PR I've improved this spec to avoid such an error.

Second error is connected to these lines: https://github.com/ueberauth/guardian/blob/75b7d8fdb7eb61f3048ce6b9394d377a5b004b52/lib/guardian/permissions.ex#L284-L289 https://github.com/ueberauth/guardian/blob/75b7d8fdb7/lib/guardian/permissions/atom_encoding.ex#L28-L30

when is_integer(value) clause of Guardian.Permissions.do_decode_permissions matches the same clause of BitwiseEncoding.decode. But AtomEncoding.decode and TextEncoding.decode does not have is_integer clause, which leads to this error.

Here I rearranged guard clauses and make them less strict.

All tests were passes successfully:

Finished in 18.2 seconds
200 tests, 0 failures
dolfinus commented 3 years ago

Hi.

Any updates here?