beyond-all-reason / teiserver

Middleware server for online gaming
https://www.beyondallreason.info/
MIT License
61 stars 53 forks source link

Make OAuth discovery work locally #523

Closed CollinsSpencer closed 1 week ago

CollinsSpencer commented 2 weeks ago

Resolves #499

Problems

  1. If the environment variable TEI_DOMAIN_NAME isn't set in local dev, then the domain_name local in runtime.exs used the default of "beyondallreason.info".
  2. Even if TEI_DOMAIN_NAME was set, the manual string interpolation "https://#{domain_name}" didn't give the correct urls if TLS/SSL wasn't enabled.
  3. This causes the http://localhost:4000/.well-known/oauth-authorization-server url to return incorrect endpoints.

Solution

L-e-x-o-n commented 1 week ago

Thanks

NortySpock commented 1 week ago

@CollinsSpencer @L-e-x-o-n I noticed that, as a result of this change, test/teiserver_web/controllers/o_auth/code_controller_test.exs:320 is now failing locally because it is now returning localhost rather than beyondallreason.info (Which is, as I understand it, an intended outcome of this change.).

Would it make sense to tag the aformentioned unit test as something like exclude_from_local_development so that developers can easily ignore it using the tag? (Happy to raise a PR to accomplish this; just thought I'd ask before I put time into it.)

test medatata endpoint can query oauth metadata (TeiserverWeb.OAuth.CodeControllerTest)
     test/teiserver_web/controllers/o_auth/code_controller_test.exs:320
     Assertion with == failed
     code:  assert resp == %{
              "issuer" => "https://beyondallreason.info",
              "authorization_endpoint" => "https://beyondallreason.info/oauth/authorize",
              "token_endpoint" => "https://beyondallreason.info/oauth/token",
              "token_endpoint_auth_methods_supported" => ["none", "client_secret_post", "client_secret_basic"],
              "grant_types_supported" => ["authorization_code", "refresh_token", "client_credentials"],
              "code_challenge_methods_supported" => ["S256"],
              "response_types_supported" => ["code", "token"]
            }
     left:  %{
              "authorization_endpoint" => "http://localhost:4002/oauth/authorize",
              "code_challenge_methods_supported" => ["S256"],
              "grant_types_supported" => ["authorization_code", "refresh_token", "client_credentials"],
              "issuer" => "http://localhost:4002",
              "response_types_supported" => ["code", "token"],
              "token_endpoint" => "http://localhost:4002/oauth/token",
              "token_endpoint_auth_methods_supported" => ["none", "client_secret_post", "client_secret_basic"]
            }
     right: %{
              "authorization_endpoint" => "https://beyondallreason.info/oauth/authorize",
              "code_challenge_methods_supported" => ["S256"],
              "grant_types_supported" => ["authorization_code", "refresh_token", "client_credentials"],
              "issuer" => "https://beyondallreason.info",
              "response_types_supported" => ["code", "token"],
              "token_endpoint" => "https://beyondallreason.info/oauth/token",
              "token_endpoint_auth_methods_supported" => ["none", "client_secret_post", "client_secret_basic"]
            }
     stacktrace:
       test/teiserver_web/controllers/o_auth/code_controller_test.exs:323: (test)
CollinsSpencer commented 1 week ago

@NortySpock Good call. Totally missed this. I just put up an MR. https://github.com/beyond-all-reason/teiserver/pull/528

L-e-x-o-n commented 1 week ago

Thanks again