Define redirect endpoint

[SimonLab]

The callback endpoint is currently hardcoded with:

 def generate_redirect_uri(conn) do
    get_baseurl_from_conn(conn) <> "/auth/google/callback"
  end

I propose that we have an option to let the user define this callback url:

 def generate_redirect_uri(conn, callback_endpoint) when is_binary callback_endpoint do
    get_baseurl_from_conn(conn) <> callback_endpoint
  end

[nelsonic]

@SimonLab the point of having a hard-coded callback endpoint is to have a "sensible default".

Having a callback of /callback as in http://localhost:4000/callback might feel like less to type in the short run, but it's a recipe for confusion the moment there is more than one callback function in any app that is using this package. Having a namespaced URL /auth/google/callback makes it very clear what the endpoint is for.

I can see the appeal to make this configurable but I don't understand why the Phoenix App cannot simply use the existing callback URL? What is the use-case for making the callback URL something different?
Our apps are always going to use the namespaced route for consistency/clarity.

[SimonLab]

While adding the package to the Phoenix app, I've created the callback endpoint under the `api`` scope as I only have api endpoints at the moment on my router.ex:

  scope "/api", AuthApiWeb do
    pipe_through :api
    get "/auth/google/callback", GoogleAuthController, :index
    get "/auth/github/callback", GithubAuthController, :index
    get "/auth/urls", AuthUrlController, :index
  end

I can remove the endpoint from the scope but it felt strange to seperate the endoints as I consider them to be apis:

  scope "/", AppApiWeb do
    pipe_through :api
    get "/auth/google/callback", GoogleAuthController, :index
    get "/auth/github/callback", GithubAuthController, :index
  end

  scope "/api", AppApiWeb do
    pipe_through :api
    get "/auth/urls", AuthUrlController, :index
  end

I understand it adds a bit more complexity to the package, however we still have the default redirect with generate_oauth_url/1:

  def generate_oauth_url(conn) do
    client_id = System.get_env("GOOGLE_CLIENT_ID")
    scope = System.get_env("GOOGLE_SCOPE") || "profile email"
    redirect_uri = generate_redirect_uri(conn)

    "#{@google_auth_url}&client_id=#{client_id}&scope=#{scope}&redirect_uri=#{redirect_uri}"
  end

and if user wants to specify another endpoint they can call generate_oauth_url/2 with:

  def generate_oauth_url(conn, callback_endpoint) do
    client_id = System.get_env("GOOGLE_CLIENT_ID")
    scope = System.get_env("GOOGLE_SCOPE") || "profile email"
    redirect_uri = generate_redirect_uri(conn, callback_endpoint)

    "#{@google_auth_url}&client_id=#{client_id}&scope=#{scope}&redirect_uri=#{redirect_uri}"
  end

However this feature is not a blocker for me.

[nelsonic]

@SimonLab do we need to have separate endpoint for API?

[SimonLab]

do we need to have separate endpoint for API?

I'm not sure what you mean.
At the moment I only have defined api endpoints and I'm using the pipe_through :api.
I guess it's more a naming issue that I try to fix via the package instead of the application

[nelsonic]

Can we just use content negotiation to switch between HTML and JSON handlers based on the content type of the request? i.e. use the same route for both API and Browser (HTML) requests.
See: phoenixframework/phoenix#1054

[SimonLab]

Yes we can use plug and pipe_through :api or pipe_through :browser to check the request type. Although I don't think google is restricting the request to a certain acceptance type for the redirect url.

My controller link to the redirect url doesn't returns any html (only json). I just wanted to have this endpoint matching the same naming as the other api endpoint (starting with /api/...)

I can see that it add more code than needed for now, so I'm closing this issue/PR