Bearer authentication support

[duderman]

Google's CalDav uses Bearer authentication header so I added :bearer support for auth key in client

Closes #11

[tomekzaw]

Hello! Thanks for your contribution. I've left two closely related comments in the code.

[duderman]

@tomekzaw thanks for review! Following your comments I added a new client %BearerClient{} and slightly changed Tesla module

[tomekzaw]

@duderman Thanks for applying the changes so quickly! I've left two suggestions in the code.

I like your idea with adding another type of client for backward-compatibility purposes. However, the naming suggest that BearerClient is a specialization of Client but in fact it's an alternative to Client. Also, these structs share a common field (namely server_url).

Maybe it would be better to have just one %CalDAVClient.Client with two fields: server_url and auth which can be one of: %CalDAVClient.Auth.Basic, %CalDAVClient.Auth.Digest or %CalDAVClient.Auth.Bearer? This API seems to be more intuitive and also supports checking fields in compile-time.

Once we have it, we can use pattern matching to make specific Tesla middleware just like you did.

I know this changes the API a bit but we will have to bump the version anyway. Hopefully the migration is quite easy.

[duderman]

@tomekzaw My idea was to keep backward compatibility. But you are right. Having different structs for different auth mechanisms feels much more clear.

Just pushed some more changes 👍🏻

[tomekzaw]

@duderman Thanks a lot for the changes! I need some time to setup my environment so I can actually test it out. After this I think we are ready for merge and release.

[duderman]

@tomekzaw hey mate 😃 did you manage to test it out by any chance? Thanks!

[tomekzaw]

@duderman I've managed to setup the environment but unfortunately the integration tests failed. Most likely I've messed something up and I need some more time.

BTW Can you please rename tesla_text.exs to tesla_test.exs?

[duderman]

@tomekzaw oops. weird typo 😄 thanks!

I've used a baikal image from dockerhub for tests. worked quite well

docker run --rm -it -p 8800:80 ckulka/baikal:nginx

[tomekzaw]

@duderman I've tried running the full test suite with mix test --include integration and got the following runtime error:

** (UndefinedFunctionError) function CalDAVClient.Auth.Basic.fetch/2 is undefined (CalDAVClient.Auth.Basic does not implement the Access behaviour)

Looks like Map.from_struct/1 call was missing:

-  defp credentials(auth = %{username: _, password: _}), do: auth
+  defp credentials(auth = %{username: _, password: _}), do: Map.from_struct(auth)

Here's a more verbose implementation that also works for me:

  def make_tesla_client(%{server_url: server_url, auth: auth}, middleware \\ []) do
    Tesla.client([
      {Tesla.Middleware.BaseUrl, server_url},
      auth_middleware(auth)
      | middleware
    ])
  end

  defp auth_middleware(%CalDAVClient.Auth.Basic{username: username, password: password}),
    do: {Tesla.Middleware.BasicAuth, %{username: username, password: password}}

  defp auth_middleware(%CalDAVClient.Auth.Digest{username: username, password: password}),
    do: {Tesla.Middleware.DigestAuth, %{username: username, password: password}}

  defp auth_middleware(%CalDAVClient.Auth.Bearer{token: token}),
    do: {Tesla.Middleware.BearerAuth, [token: token]}

Honestly, I like your implementation better due to its simplicity. Can you please submit the fix with Map.from_struct/1?

Also, I'm thinking we can also split auth.ex into auth/{basic,digest,bearer}.ex so the paths are compliant with Elixir module names, what do you think about it?

Finally, thanks for your note about the endpoint URL. I've just tested it out and it still works with cal.php. Can you please to either replace all occurrences with dav.php or revert back to cal.php? We would like to keep the examples and test config consistent.

I've used a baikal image from dockerhub for tests. worked quite well

FYI I plan to submit a PR that adds docker-compose.yml for Baïkal and detailed instructions on how to run integration tests.

[duderman]

@tomekzaw done ;)

[tomekzaw]

Great work! Thanks again for your contribution! 👏