search

beyond-all-reason / teiserver

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

Webui: Ticked role checkboxes are not saved when editing user #295

Closed jere0500 closed 2 months ago

jere0500 commented 2 months ago 
          Tldr: I still have the problem, because booleans from the checkboxes are not transferred into the role field.

Unfortunately this issue still persisted for me, maybe it is due to the elixir version (I used 1.14.5) or some dependency behaving differently. (I logged in as root@localhost so that shouldn't be an issue) Thats why I decided to dig into it more.

In a nutshell: 3 files are relevant here:

  1. The html form page for editing the user (where the checkboxes get loaded): https://github.com/beyond-all-reason/teiserver/blob/master/lib/teiserver_web/templates/admin/user/form.html.heex
  2. The change request processing from the form: https://github.com/beyond-all-reason/teiserver/blob/3e13abb6cb79f17c684db2aa8b38fb581724c7e2/lib/teiserver/account/schemas/user.ex#L52
  3. The part where the update gets written into the database: https://github.com/beyond-all-reason/teiserver/blob/3e13abb6cb79f17c684db2aa8b38fb581724c7e2/lib/teiserver/account/libs/user_lib.ex#L169

The Problem

I add IO.inspect() calls to the functions and found out that the attrs that are passed to (2.) from the form (1.) are in another format:

%{
  "Contributor" => "false",
  "Mapping" => "false",
  "permissions" => [],
  "data" => %{
    "bot" => false,
    "chobby_hash" => nil,
    "country" => "??",
    "discord_dm_channel" => nil,
    "discord_dm_channel_id" => nil,
    "discord_id" => nil,
    "email_change_code" => nil,
    "hw_hash" => nil,
    "last_login" => 28609015,
    "last_login_mins" => 28609015,
    "last_login_timex" => "2024-05-24T08:55:12.000625Z",
    "lobby_client" => "SPADS v",
    "lobby_hash" => "57015357 5636bd6e65323f54",
    "moderator" => false,
    "password_hash" => "$argon2id$v=19$m=65536,t=8,p=2$I57FcJMdwpGTcuOL4Xs4HA$SusoCqS/9adD3+/oKsxPF/6CWNuUKrzOYckmAUZmWNk",
    "print_client_messages" => false,
    "print_server_messages" => false,
    "rank" => 0,
    "restricted_until" => nil,
    "restrictions" => [],
    "roles" => [],
    "shadowbanned" => false,
    "spring_password" => true,
    "steam_id" => nil,
    "verified" => false
  },
  "icon" => " fa-solid fa-user",
  "Caster" => "false",
  "Core" => "false",
  "colour" => "#666666",
  "BAR+" => "false",
  "Engine" => "false",
  "Community team" => "false",
  "Verified" => "false",
  "Streamer" => "false",
  "Mentor" => "false",
  "Moderator" => "false",
  "behaviour_score" => "10000",
  "Server" => "false",
  "Tournament" => "false",
  "Reviewer" => "false",
  "Gameplay" => "false",
  "VIP" => "false",
  "Infrastructure" => "false",
  "email" => "spadsbot@spadsbot.com",
  "trust_score" => "10000",
  "roles" => [],
  "Promo team" => "false",
  "Overwatch" => "false",
  "Data export" => "false",
  "Donor" => "false",
  "Trusted" => "false",
  "Bot" => "true",
  "Admin" => "false",
  "Tester" => "false",
  "social_score" => "0",
  "Academy manager" => "false"
}

We can see that each checkbox is passed as their own attribute. However, the form in (1.) loads loads all the checkbox values from the roles field.

"roles" => [],

e.g. 

roles: ["Bot", "Caster", "Mapping", "Mentor", "VIP"]

The fields e.g. ("Bot", "Admin") are not part of the user:

%Teiserver.Account.User{
  __meta__: #Ecto.Schema.Metadata<:loaded, "account_users">,
  id: 28,
  name: "spadsbot[000]",
  email: "spadsbot@spadsbot.com",
  password: "$argon2id$v=19$m=65536,t=8,p=2$I57FcJMdwpGTcuOL4Xs4HA$SusoCqS/9adD3+/oKsxPF/6CWNuUKrzOYckmAUZmWNk",
  icon: " fa-solid fa-user",
  colour: "#666666",
  data: %{
    "bot" => false,
    "chobby_hash" => nil,
    "country" => "??",
    "discord_dm_channel" => nil,
    "discord_dm_channel_id" => nil,
    "discord_id" => nil,
    "email_change_code" => nil,
    "hw_hash" => nil,
    "last_login" => 28609015,
    "last_login_mins" => 28609015,
    "last_login_timex" => "2024-05-24T08:55:12.000625Z",
    "lobby_client" => "SPADS v",
    "lobby_hash" => "57015357 5636bd6e65323f54",
    "moderator" => false,
    "password_hash" => "$argon2id$v=19$m=65536,t=8,p=2$I57FcJMdwpGTcuOL4Xs4HA$SusoCqS/9adD3+/oKsxPF/6CWNuUKrzOYckmAUZmWNk",
    "print_client_messages" => false,
    "print_server_messages" => false,
    "rank" => 0,
    "restricted_until" => nil,
    "restrictions" => [],
    "roles" => [],
    "shadowbanned" => false,
    "spring_password" => true,
    "steam_id" => nil,
    "verified" => false
  },
  roles: [],
  permissions: [],
  restrictions: [],
  restricted_until: nil,
  shadowbanned: false,
  last_login: nil,
  last_login_timex: ~U[2024-05-24 08:55:12Z],
  last_played: nil,
  last_logout: nil,
  discord_id: nil,
  discord_dm_channel_id: nil,
  steam_id: nil,
  user_configs: #Ecto.Association.NotLoaded<association :user_configs is not loaded>,
  clan_id: nil,
  clan: #Ecto.Association.NotLoaded<association :clan is not loaded>,
  smurf_of_id: nil,
  smurf_of: #Ecto.Association.NotLoaded<association :smurf_of is not loaded>,
  user_stat: #Ecto.Association.NotLoaded<association :user_stat is not loaded>,
  behaviour_score: 10000,
  trust_score: 10000,
  social_score: 0,
  inserted_at: ~N[2024-05-24 08:44:35],
  updated_at: ~N[2024-05-24 16:33:05]
}

Possible solution

I made a draft that transforms the attributes and fits them into the roles field, which works and fixes the issue for my setup. (I will link it soon).

However, I am kind of confused why this is not a problem for @jauggy . Maybe there are differences in the versions used.

I used Elixir 1.14.5 and Erlang/OTP 26 [erts-14.2.3] all dependency versions should be the standard ones from the mix.exs file.

Originally posted by @jere0500 in https://github.com/beyond-all-reason/teiserver/issues/272#issuecomment-2131232021

jauggy commented 2 months ago

@geekingfrog @StanczakDominik @L-e-x-o-n Are any of you guys getting this issue? It works fine for me. Jere is unable to save any roles.

geekingfrog commented 2 months ago

Using the root account everything works. If I then give admin role to another account (A) and use that account to modify another account, I don't see the update right away. I need to click on "recache user" for the effect to be persisted. If you then go into show > details, you see the correct roles:

2024-05-26-860x1099-scrot

I think, if anything, we should fix the stale cache issue, but things seems to work fine here.

jere0500 commented 2 months ago

Thanks for the feedback!

Solved Checkbox issue

I recreated the setup and think that I had missed the mix.ecto create command. It works now:

The roles ticked by checkboxes are directly passed in the roles field and applied.
### The passed attributes to [user.ex](https://github.com/beyond-all-reason/teiserver/blob/3e13abb6cb79f17c684db2aa8b38fb581724c7e2/lib/teiserver/account/schemas/user.ex#L52) ``` %{ "Contributor" => "false", "Mapping" => "false", "permissions" => ["Admin", "Moderator", "Reviewer", "Overwatch", "BAR+", "Core", "Contributor", "Trusted", "Bot", "Mentor", "Community team", "Server"], "data" => %{ "bot" => true, "friend_requests" => [], "friends" => [], "ignored" => [], "lobby_client" => "webui", "moderator" => true, "password_hash" => "X03MO1qnZdYdgyfeuILPmQ==", "rank" => 1, "roles" => ["Admin", "Bot", "Core", "Mentor", "Moderator", "Server", "Trusted"], "verified" => false }, "icon" => " fa-solid star", "Caster" => "false", "Core" => "true", "colour" => "#9999aa", "BAR+" => "false", "Engine" => "false", "Community team" => "false", "Verified" => "false", "Streamer" => "false", "Mentor" => "true", "Moderator" => "true", "behaviour_score" => "10000", "Server" => "true", "Tournament" => "false", "Reviewer" => "false", "Gameplay" => "false", "VIP" => "false", "Infrastructure" => "false", "email" => "8094565c-1cd0-11ef-ac02-525400d08557", "trust_score" => "10000", "roles" => ["Admin", "Bot", "Core", "Mentor", "Moderator", "Server", "Trusted"], "Promo team" => "false", "Overwatch" => "false", "Data export" => "false", "Donor" => "false", "Trusted" => "true", "Bot" => "true", "Admin" => "true", "Tester" => "false", "social_score" => "0", "Academy manager" => "false" } ``` ### The [user changeset](https://github.com/beyond-all-reason/teiserver/blob/3e13abb6cb79f17c684db2aa8b38fb581724c7e2/lib/teiserver/account/schemas/user.ex#L59): ``` #Ecto.Changeset< action: nil, changes: %{ data: %{ "bot" => true, "friend_requests" => [], "friends" => [], "ignored" => [], "lobby_client" => "webui", "moderator" => true, "password_hash" => "X03MO1qnZdYdgyfeuILPmQ==", "rank" => 1, "roles" => ["Admin", "Bot", "Core", "Mentor", "Moderator", "Server", "Trusted"], "verified" => false }, permissions: ["Admin", "Moderator", "Reviewer", "Overwatch", "BAR+", "Core", "Contributor", "Trusted", "Bot", "Mentor", "Community team", "Server"], roles: ["Admin", "Bot", "Core", "Mentor", "Moderator", "Server", "Trusted"] }, errors: [], data: #Teiserver.Account.User<>, valid?: true ```

I used for recreation:

Caching issue

I could not confirm the same stale cache issue of @geekingfrog. However, I noticed a different one: When I give another user admin privileges, and login in into that account I can not view or edit other profiles, only if the root account did edit>save changes on that user before. Just recaching does not work. The changes I make with the new admin account are applied directly (no need to recache).

This problem does not exist when the user gets server privileges instead of admin ones.

I tested that with users created by mix teiserver.fakedata.