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

Bug: get_baseurl_from_conn/1 fails on Fly.io #94

Closed
4 tasks done
nelsonic opened this issue May 16, 2023 · 16 comments
Closed
4 tasks done

Bug: get_baseurl_from_conn/1 fails on Fly.io #94

nelsonic opened this issue May 16, 2023 · 16 comments
Assignees

Comments

@nelsonic
Copy link
Member

nelsonic commented May 16, 2023

Don't know why, but we're having trouble with ElixirAuthGoogle.generate_oauth_url(conn) on our Calendar app: dwyl/calendar#38
We've identified that the conn on Fly.io is actually reporting port: 80 and scheme: :http
get_baseurl_from_conn/1 is setting the scheme to http instead of https which means that Google Auth Fails with the Error 400: redirect_uri_mismatch error. 😢

I think we can check for the presence of the req_headers -> x-forwarded-proto which is set to "https"
Just need to write a test for this ...

Todo

  • Create a test for {"x-forwarded-proto", "https"}
  • Add code to check for it at the top of get_baseurl_from_conn/1
  • Make sure nothing else breaks!
  • Publish new version of the package to test in calendar App.
@nelsonic
Copy link
Member Author

Test:

  test "get_baseurl_from_conn(conn) x-forwarded-proto header for https #94" do
    conn = %{
      host: "gcal.fly.dev",
      port: 80,
      scheme: :http,
      req_headers: %{
        "x-forwarded-proto" => "https"
      }
    }

    assert ElixirAuthGoogle.get_baseurl_from_conn(conn) == "https://gcal.fly.dev"
  end

@nelsonic
Copy link
Member Author

Getting on this now cause I really want to fix dwyl/calendar#38

@nelsonic nelsonic moved this to 🏗 In progress in dwyl app kanban May 16, 2023
@jtormey
Copy link
Contributor

jtormey commented May 16, 2023

Good timing, I'm seeing this as well and was looking to see if it had been reported 🙂

To add to this, when running locally and tunneling through ngrok for testing sign-in with Google, the URL also incorrectly uses HTTP.

Is there a reason the URL is built from conn, rather than the app Endpoint, which should already know how to build a public URL?

@nelsonic
Copy link
Member Author

@jtormey thanks for commenting and sharing your experience.
are you deployed to Fly.io?
No reason for not using Endpoint. We just didn't have a functional way of doing that.
Can we read the Endpoint from the context of a Controller?

Looks like it: https://stackoverflow.com/questions/40243229/how-to-get-the-current-base-domain-of-a-phoenix-app

MyAppWeb.Endpoint.url()
# => "http://localhost:4000"

Ok, I don't really see a situation where this package will be used outside of a Phoenix App ... 💭
So maybe we can shift to using this method to get the base_url ...

@jtormey
Copy link
Contributor

jtormey commented May 16, 2023

Yes! I'm deployed to Fly.

That's a good point, relying on Endpoint would limit use of this library to Phoenix. Maybe would be best to derive from conn by default, with a configuration option where Endpoint.url() can be set as the base URL to use.

@nelsonic
Copy link
Member Author

PR: #95

@SimonLab
Copy link
Member

@nelsonic I'm trying to understand this issue, are you sure it's an issue linked to this package and not a configuration problem on the calendar application (something similar to dwyl/phoenix-liveview-chat-example#111) or a Fly.io issue?

I'm wondering why the google auth is working on the mvp application and not on the calendar.
Could it be linked to dwyl/calendar#37 ?

I think I'm missing something so I'll try to reproduce this error on my side to understand the cause of this bug.

@nelsonic
Copy link
Member Author

@SimonLab excellent question. I had the same thought yesterday. 💭
If we could fix this with a config update I'd be delighted. 😍
However my reading of the dbg(conn) in dwyl/calendar#38 (comment)
is that the details aren't being passed correctly into the ElixirAuthGoogle.generate_oauth_url/2 function.
specifically the scheme: :http part which means that there is a scheme mismatch.

If you take a look at the runtime.exs config: /calendar/config/runtime.exs#L54-L55
You'll see that it's:

  config :cal, CalWeb.Endpoint,
    url: [host: host, port: 443, scheme: "https"] 
...

By contrast in auth we see: /auth/config/runtime.exs#L47-L48

  config :auth, AuthWeb.Endpoint,
    url: [host: host, port: 443],
...

i.e. no scheme specified.
We could just try removing the scheme from the /calendar/config/runtime.exs
and then the following version of the function will be called:

def get_baseurl_from_conn(%{host: h} = conn) do
scheme =
case h do
"localhost" -> :http
_ -> :https
end
get_baseurl_from_conn(Map.put(conn, :scheme, scheme))
end

This would result in the correct https scheme being used. 💭
But then we're requiring people to manually update their runtime.exs file ... 🔧

@nelsonic
Copy link
Member Author

You should have access to https://fly.io/dashboard/dwyl-calendar if you want to debug. 🔍
But ultimately this is what I think is the problem. 💭

@SimonLab
Copy link
Member

Thanks for the explanation #94 (comment)
It helps me understand a bit better what's going on.
I'll have to look at it in more detail to understand it fully

@nelsonic
Copy link
Member Author

Just tried to remove the scheme: "https" from config/runtime.exs#L55

Deployed to Fly.io and still get the same error:

image

So I'm thinking the only way to get around this is to explicitly read the "x-forwarded-proto" on Fly.io
OR follow @jtormey's suggestion and allow people to pass in the App.Endpoint.url()
Which looks like a far more robust way of doing this ... https://hexdocs.pm/phoenix/Phoenix.Endpoint.html#c:url/0

@nelsonic
Copy link
Member Author

Ok. this one is "fixed" by passing in the Endpoint.url() to ElixirAuthGoogle.generate_oauth_url ...
But now we have a different (related) issue:

image

https://fly.io/apps/gcal/monitoring
image

In the CalWeb.GoogleAuthController /lib/cal_web/controllers/google_auth_controller.ex#L8 we call:

...
  {:ok, token} = ElixirAuthGoogle.get_token(code, conn)
  
  conn
  |> put_flash(:token, token)
  |> redirect(to: ~p"/app")
end

i.e. the get_baseurl_from_conn/1 is still being invoked with conn by:

@spec get_token(String.t(), conn) :: {:ok, map} | {:error, any}
def get_token(code, conn) do
body =
Jason.encode!(%{
client_id: google_client_id(),
client_secret: google_client_secret(),
redirect_uri: generate_redirect_uri(conn),
grant_type: "authorization_code",
code: code
})
inject_poison().post(@google_token_url, body)
|> parse_body_response()
end

L101 being the issue ... 🙄

We could create a new version of the get_token/2 function ... 💭

@nelsonic
Copy link
Member Author

Was quick to add so I just added it to the PR: #95

@nelsonic
Copy link
Member Author

Published the new version of the elixir_auth_google package to Hex.pm:

And used it in Calendar: https://gcal.fly.dev/

gcal-working-on-fly io

This is all working as expected now. ✅

SimonLab added a commit that referenced this issue May 18, 2023
PR: Add `generate_redirect_uri/1` receives `Endpoint.url()` as argument #94
@nelsonic
Copy link
Member Author

Fixed. Closing. 👌

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in dwyl app kanban May 19, 2023
@odavidbrito
Copy link

Thanks for this update! Priceless!

I struggled a bit with the changes needed to make it work from the version in the readme.md (newbie here), so for posterity if someone bumps with the same issue.
Things to do so the redirect_url is https (and not http) in Fly.io and allows the Google app to be published (cant have http in there to be published, adding http redirect url to Google app was my workaround before finding this fix)

  1. Increase the "elixir_auth_google" version in mix.exs:
    image

  2. Replace "conn" with the Endpoint.url in the GoogleAuthController:
    image

  3. And the same in the PageController:
    image

My app was named Google so adapt [Google]Web as needed so it gets the endpoint correctly.

Thanks for the work on these great demos (they are a perfect way to learn isolated things that I need for my app, and from scratch to deploy) and fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants