[Major Bug Fix ⚠️] : set_role_color goes into infinite loop when input name is Administrator || Guest || User

[rautniraj]

1. In my previous PR #5879 i have introduced retry logic so that the slightest possibility of assigning same colors to different role is avoided
2. This retry misses the check if admin user try to create a role and accidentally inputs User || Guest || Administrator as new role name.
3. If the above three values are input in lowercase - there is no issue as validation in the model catches duplicate name.
4. In case the input name is same as above three then the set_role_color goes into infinite loop.
5. So added minor check that retry should only work if the Role name in user input don't exist in db.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rautniraj]

Comment on imagemagick Installation Step - Issue in workflow file

https://github.com/bigbluebutton/greenlight/blob/c2cae830767a97cc624d767fb17d355d65e9efca/.github/workflows/ci.yml#L40

• Ensure the package lists are updated before installing imagemagick to address potential 404 errors when fetching packages from outdated mirrors.
• Note: This step has been tested twice to verify correct functionality in the workflow.
• I am not sure about the fix

- name: Install imagemagick
  run: |
  sudo apt-get update --fix-missing
  sudo apt-get install -y imagemagick --fix-missing

[farhatahmad]

Would it not be better to update the validation to catch non lower case reserved role names?

[rautniraj]

Hello Sir,

already we are having validation on name column
validates :name, presence: true, uniqueness: { case_sensitive: false, scope: :provider } Hence duplicate name irrespective of case shouldn't be allowed

But validation comes into picture in later stage. Prior to that there is set_role_color function which runs and populate the color column. Now in this function suppose the value of name is user then it passes the function and sets a color. Now when both name and color gets populated, here come the validation logic which blocks the Role.create Hence we r safe.

Case 2: suppose the value of name is User, again this set_role_color function runs and in its try to populate color column it goes into infinite role and application crashes. We dont have any validation that comes into picture.

I have look into the code :

  validates :name, presence: true, uniqueness: { case_sensitive: false, scope: :provider }
  validates :provider, presence: true
  before_validation :set_role_color, on: :create

Case 1 and 2 is working according to defined line. set_role_color runs before_validation.

Even if i switch to after_validation :set_role_color, on: :create still i face the infinite loop issue (which according to my understanding should not happen - but i dont know why this unexpected behaviour)

[rautniraj]

Other solution which i prefer will be

  def set_role_color
    case name
    when 'Administrator'
      self.color = '#228B22'
    when 'User'
      self.color = '#4169E1'
    when 'Guest'
      self.color = '#FFA500'
    else
      begin
        self.color = "##{SecureRandom.hex(3)}"
      end while Role.exists?(color: color, provider: provider)
    end
  end

This handles all the edge case like User and user

Let me know which solution you want.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud