coherence icon indicating copy to clipboard operation
coherence copied to clipboard

Configurable confirmation URL + IPs stored in plain format

Open sashman opened this issue 6 years ago • 4 comments

To configure confirmation URL

config :coherence,
  external_hostname: {:system, "EXTERNAL_HOSTNAME"}, # Set external hostname, e.g.: http://localhost:4000
  confimarion_url: ":external_hostname/#/user/confirm/:token", # Can use either hardcoded hostname or :external_hostname, and use :token to embed the generated token

sashman avatar Feb 06 '18 09:02 sashman

Can you please explain the use case for using the "EXTERNAL_HOSTNAME" env variable? The system already creates the url based on the Endpoint's http: [host: "..."] and https: [host: "..."] configuration. So I don't see the need to have a separate env variable. Am I missing something?

smpallen99 avatar Apr 15 '18 10:04 smpallen99

Hey @smpallen99, thanks for replying!

This env variable might not have the best name, but in my use case, this arises from using the coherence in a microservice. So what I end up with is http: [host: "..."] and https: [host: "..."] pointing to the internal hostname of the microservice, which the user cannot navigate from the outside, only the front end service is publicly accessible, This is why I had to add a separate host variable to point to this public endpoint.

Hope this makes sense. I'm more than willing to change this to a more sensible solution in case I have misunderstood something.

sashman avatar Apr 15 '18 11:04 sashman

Hey @smpallen99, any thoughts on this?

sashman avatar May 11 '18 14:05 sashman

Sorry for the delay getting back to you on this @sashman. I've reviewed the PR and have some comments:

  • Why have you have only implemented this for the confirmation_url? If I understand the feature correctly, wouldn't you want all email links to be translated?
  • Adding a extra xxx_url's to the config just seems a little off.
  • There is a fairly elegant way (IMHO) to implement this in a project without any changes to coherence. See my example below.
  • The default should taken from the existing project. See the external_url/1 function from my example below.
  • This is a very specific case, and if there was community demand for such a feature, I would like to see it done a little more generically.

I just tried this in a test project with my master branch. The only file that needs changing is lib/coh_mass_assign_web/emails/coherence/user_email.ex.

Code.ensure_loaded Phoenix.Swoosh

defmodule CohMassAssignWeb.Coherence.UserEmail do
  @moduledoc false
  use Phoenix.Swoosh, view: CohMassAssignWeb.Coherence.EmailView, layout: {CohMassAssignWeb.Coherence.LayoutView, :email}
  alias Swoosh.Email
  require Logger
  alias Coherence.Config
  import CohMassAssignWeb.Gettext

  defp site_name, do: Config.site_name(inspect Config.module)

  def password(user, url) do
    Logger.info "reset url: #{inspect url}"
    url = external_url(url)  # add this line to each of the mail functions
    Logger.info "reset external url: #{inspect url}"
    %Email{}
    |> from(from_email())
    |> to(user_email(user))
    |> add_reply_to()
    |> subject(dgettext("coherence", "%{site_name} - Reset password instructions", site_name: site_name()))
    |> render_body("password.html", %{url: url, name: first_name(user.name)})
  end

  def confirmation(user, url) do
    url = external_url(url) 
   # ...
  end

  # ...

  defp external_url(url) do
    base_url = CohMassAssignWeb.Router.Helpers.url(CohMassAssignWeb.Endpoint)
    String.replace(url, base_url, Application.get_env(:coh_mass_assign, :external_url, base_url))
  end
end

By adding a :external_url config item like "https://my.external.domain", or "https://10.10.10.10:4000" to your projects field, I believe you can get the behaviour you're looking for.

What do you think? Steve

smpallen99 avatar Aug 27 '18 17:08 smpallen99