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

Make OAuth discovery work locally #523

Merged

Conversation

CollinsSpencer
Copy link
Contributor

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 L-e-x-o-n merged commit f3450ef into beyond-all-reason:master Nov 16, 2024
2 of 3 checks passed
@L-e-x-o-n
Copy link
Collaborator

Thanks

@NortySpock
Copy link
Contributor

@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
Copy link
Contributor Author

CollinsSpencer commented Nov 19, 2024

@NortySpock Good call. Totally missed this. I just put up an MR. #528

@L-e-x-o-n
Copy link
Collaborator

L-e-x-o-n commented Nov 19, 2024

Thanks again

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

Successfully merging this pull request may close these issues.

Make OAuth discovery work locally
3 participants