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

Fixed setPassword body is compressed #8790

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Conversation

GGGuenni
Copy link
Collaborator

@GGGuenni GGGuenni commented Mar 2, 2023

  • Updated server to also use the correct Basic implementation

Everything should be done client-wise once this is merged if you don't have any more issues @oynqr @touhidurrr

+ Updated server to also use the correct Basic implementation
@touhidurrr
Copy link
Contributor

Also this needs 100% rollout I guess for touhidurrr/UncivServer.xyz#5 to be merged.

@yairm210 yairm210 merged commit 6f7279d into yairm210:master Mar 2, 2023
@GGGuenni GGGuenni deleted the auth-fixes branch March 2, 2023 18:40
@touhidurrr
Copy link
Contributor

touhidurrr commented Mar 2, 2023

@GGGuenni @yairm210 I think we need to use domains or groups as keys. Currently, uncivserver.xyz and replit.uncivserver.xyz will be considered to be two separate entities although they share the same database to serve game files.
There is 2 solutions that I can suggest:

  1. use domain name as key for the hash
  2. ping /isalive and collect group name and use it as a key. something like { authVersion: 2, group: 'uncivserver.xyz' }.

@touhidurrr
Copy link
Contributor

touhidurrr commented Mar 2, 2023

In solution 2, the client will keep track of domains under a group and then group password.
Something like:

const settings = {
  groups: {
    ["uncivserver.xyz"]: {
      passwords: {
        userId: "password",
      },
      hostnames: ["uncivserver.xyz", "replit.uncivserver.xyz"],
    },
  },
};

or,

const settings = {
  hostToGroupMap: {
    ["uncivserver.xyz"]: "uncivserver.xyz",
    ["replit.uncivserver.xyz"]: "uncivserver.xyz",
  },
  groupPasswords: {
    ["uncivserver.xyz"]: {
      userId: "password",
    },
  },
};

for easy access.
maybe /isalive can share group info too for cahcing purposes:

const isAlive = {
  authVersion: 2,
  group: "uncivserver.xyz",
  hosts: ["uncivserver.xyz", "replit.uncivserver.xyz", "bd.uncivserver.xyz"],
};

@GGGuenni
Copy link
Collaborator Author

GGGuenni commented Mar 3, 2023

What are you using the replit subdomain for if they both serve the same files?

The only thing I see that might be a problem is setting the password for one changes the password for the other (if the server is implemented that way). But I don't think that's a huge problem. If you change the password and switch the domain you will be asked for a password once and that's it

@touhidurrr
Copy link
Contributor

What are you using the replit subdomain for if they both serve the same files?

Basically the database is the same, but the server is different. That subdomain runs on a different virtual server.
Since UncivServer.xyz has a Redis cache mechanism per server, and the server never waits for the game to uploaded to the central db, different servers sees different latencies for different users based on region.

@touhidurrr
Copy link
Contributor

touhidurrr commented Mar 3, 2023

@GGGuenni server error message is not being displayed on client on set password failure after touhidurrr/UncivServer.xyz@dd4a244, can you fix this?

@GGGuenni
Copy link
Collaborator Author

GGGuenni commented Mar 3, 2023

Since I can only change future releases (or the current with hotfixes) it does not make any difference if I change something.
4.5.2 will work, everything below is stuck with Failed to set password!

@touhidurrr
Copy link
Contributor

touhidurrr commented Mar 3, 2023

Since I can only change future releases (or the current with hotfixes) it does not make any difference if I change something. 4.5.2 will work, everything below is stuck with Failed to set password!

I am targeting the next release actually.

@GGGuenni
Copy link
Collaborator Author

GGGuenni commented Mar 4, 2023

Still applies that it will not trigger as 4.5.3+ users will not get the message but are the first ones which could receive it

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.

3 participants