Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement an adapter for Plug #1

Closed
madebyherzblut opened this issue Jun 4, 2019 · 3 comments · Fixed by #2
Closed

Implement an adapter for Plug #1

madebyherzblut opened this issue Jun 4, 2019 · 3 comments · Fixed by #2
Labels
enhancement New feature or request

Comments

@madebyherzblut
Copy link
Contributor

DoorFrame should be able to support different adapters in the future, but for the first version I suggest to focus on Plug. However, to make implementing other adapters easier in the future I think we should keep the adapter model in mind–even for Plug only.

Describe the solution you'd like
By adapter model I mean that we should follow oauth-server:

  1. A request comes in the form of a Plug.Conn
  2. DoorFrame converts it to a DoorFrame.Request. The conversion extracts the required headers and the body payload. It also parses e. g. the Bearer token from the Authorization header
  3. Authentication is done using DoorFrame.Request and the response is stored in a DoorFrame.Response struct
  4. The DoorFrame.Response is converted back to a Plug.Conn

Describe alternatives you've considered
We could go ahead and skip the adapter model and add Plug as a hard dependency. From my perspective this would us save some time now, but would also make testing harder.

@madebyherzblut madebyherzblut added the enhancement New feature or request label Jun 4, 2019
@madebyherzblut
Copy link
Contributor Author

@joelwallis your opinion is greatly appreciated. Do you think we should implement an adapter abstraction right now? Do you think an adapter approach is even necessar at all considering Plug is by far the most important web server abstraction?

@madebyherzblut
Copy link
Contributor Author

madebyherzblut commented Jun 5, 2019

I thought about it a bit more and think we should implement the adapter approach right away. The reason is that we can implement a GRPC adapter for service-to-service authentication in our product.

This is the API of DoorFrame I have in mind right now:

# controllers/auth_controller.ex
defmodule AppWeb.StatusController do
  use AppWeb, :controller

  def auth(conn, _) do
    case DoorFrame.Adapter.Plug.authenticate(conn, App.Auth.Handler) do
      {:ok, conn, _response} ->
        conn

      {:error, conn, _error} ->
        conn
    end
  end
end

# door_frame/adapter/plug.ex
defmodule DoorFrame.Adapter.Plug do
  def authenticate(%Plug.Conn{} = conn, handler) do
    # Parse headers
    # Parse body
    request = parse_conn(conn)

    case handler.authenticate(request) do
      {:ok, response} ->
        # Redirect if necessary
        conn =
          conn
          |> put_status(response.status)
          |> json(response)

        {:ok, conn, response}

      {:error, error} ->
        conn
        |> put_status(error.status)
        |> json(error.payload)

        {:ok, conn, error}
    end
  end
end
# …

@joeljuca
Copy link
Contributor

joeljuca commented Jun 6, 2019

Hey @madebyherzblut,

To put down here what we've discussed before, shaw we proceed with a simpler approach, targeting the Plug architecture? I like the idea of having adapters for different types of HTTP interfaces, but I'm afraid that premature optimization could distract us from the main problem we're trying to solve (build a non-opinionated OAuth library). Since the Raxx userbase is still small, we could support it in future versions .

So for the first version we would use DoorFrame module for common logic and DoorFrame.Plug for Plug-specific logic and implementation. Future ones could have the architecture matter revisited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants