spads_controller get_rating not testable when running on Windows WSL

[jauggy]

spads_controller get_rating has this

    {rating_value, uncertainty} =
      if host_ip != conn_ip do
        BalanceLib.get_user_rating_value_uncertainty_pair(-1, "Duel")
      else
        BalanceLib.get_user_rating_value_uncertainty_pair(target_id, actual_type)
      end

When on Windows WSL, the host_ip and conn_ip will not match and therefore you will always get the Duel rating. I have no idea what that check is for but I think it should be removed. Otherwise it is impossible to locally test anything that interacts with the rating endpoint

[L-e-x-o-n]

/get_rating/:target_id/:type and /balance_battle are public endpoints used by SPADS to get ratings and balance players. This check makes it only useable by SPADS and not by everyone, this is intentional, but can probably be done in a better way.
For discussion about possible future Teiserver API read the thread on Discord.

[L-e-x-o-n]

You can remove it for local tests.

[jauggy]

Can I just add a config to dev.exs e.g. devmode: true and then check for it?

if host_ip != conn_ip and !devmode do

[L-e-x-o-n]

Which IP do local SPADS have?

[geekingfrog]

I'm wary of adding checks in the code for dev mode, it tends to lead to more convoluted code. If you want to test something you can modify the code locally. If that becomes a bigger problem, we'll need a better solution.
I had a look at the git history but it's useless to understand why there's this conditional :/

[jauggy]

It's there so that in production, outside sources cannot call that endpoint. In production SPADS and teiserver have the same IP.

Modifying that code when developing requires you to know that code even exists and that you're supposed to change it when developing. If I modify it every time I develop, it could easily be committed by mistake too.

[geekingfrog]

No, host_ip is not teiserver ip it is the ip given by the host of of the lobby. This must match the x-real-ip header given when doing this request. There should be a way to force spads to set this header to the "correct" value.

If this is meant as an access control measure, we should return an error like 401/403 not some bogus stuff.

[jauggy]

Yeah it's really weird it returns a bogus rating instead of an error.

[bigbluejay9]

What is the desired outcome?

I recently worked on this in #512 (see, specifically https://github.com/beyond-all-reason/teiserver/pull/512/files#diff-f81da668c2dd87337f72b9a3a7b16599378313c1380d5641ae2d05eca1fc0dabR232) and can reuse some of that code to close this issue.

There are two issues I can see

• WSL tests fail since host_ip != conn_ip. We can try to fix this so that tests passes.
• Using 'Duel rating' to mean 'Access Denied'.

Perhaps I can try to fix it on WSL and close this issue?

[jauggy]

If you can make it work on WSL that would be preferred. Somehow get this host_ip != conn_ip to allow WSL devs.

[L-e-x-o-n]

II think @geekingfrog has the best solution.
Instead of returning dual rating by default, return a proper HTTP error.
For testing locally you can either edit the code and disable the check or configure SPADS/WSL somehow to use the correct IP.

[L-e-x-o-n]

Alternatively, maybe a site config could be used which would disable the IP check and allow anyone to get ratings, it could then be changed locally or even on integration server temporarily if needed, however even with this solution a proper error should still be returned if this option is disabled.

[jauggy]

Alternatively, maybe a site config could be used which would disable the IP check and allow anyone to get ratings, it could then be changed locally or even on integration server temporarily if needed, however even with this solution a proper error should still be returned if this option is disabled.

This would cover all bases.