Skip to content

Fix connection permission loss when updating server configurations#185

Closed
Copilot wants to merge 1 commit intomasterfrom
copilot/fix-f8fdabee-041f-4b6f-883f-013e5d35d977
Closed

Fix connection permission loss when updating server configurations#185
Copilot wants to merge 1 commit intomasterfrom
copilot/fix-f8fdabee-041f-4b6f-883f-013e5d35d977

Conversation

Copy link

Copilot AI commented Sep 9, 2025

Problem

When changing any connection property (password, host, port, etc.) from the main connections panel as a global admin, teams and users that previously had access to the connection lose that access. This forces administrators to manually re-assign connections to teams/users after every configuration change.

Root Cause

The issue occurs because AdminModel uses ServerConfig.equals() to determine if a cached ServerModel can be reused. Since ServerConfig.equals() compares ALL properties (name, username, password, host, port, database, etc.), any property change causes the equality check to fail. This results in:

  1. AdminModel creating a new ServerModel instance
  2. The old ServerModel (with team/user permissions) being discarded
  3. Permission associations being lost

Solution

This PR fixes the issue by modifying how AdminModel identifies servers for caching purposes:

ServerModel Changes

  • Changed serverConfig field from final to allow updates
  • Added updateServerConfig() method to update configuration while preserving the instance
  • Added validation to prevent server name changes via this method
  • Clear dependent cached objects when configuration is updated

AdminModel Changes

  • Modified refresh() methods to use server name comparison instead of full object equality
  • When a cached ServerModel exists with the same name, reuse it and update its configuration
  • This preserves ServerModel instances and their associated permissions

Impact

Before Fix:

// Full object equality - fails when password changes
if(cacheSM.getServerConfig().equals(sconf)) {
    sm = cacheSM;  // Never executed when any property changes
} else {
    sm = new ServerModel(connectionManager, sconf);  // Always creates new instance
}

After Fix:

// Name-only comparison - succeeds when server name matches
if(cacheSM.getServerConfig().getName().equals(sconf.getName())) {
    sm = cacheSM;  // Reuses existing instance
    sm.updateServerConfig(sconf);  // Updates config while preserving permissions
}

Testing

  • Created comprehensive tests validating the fix preserves object identity
  • Verified that permission systems maintain references to the same ServerModel
  • Confirmed that connection functionality works correctly with updated configurations
  • No regression in existing functionality

Benefits

  • ✅ Team/user permissions are preserved during connection updates
  • ✅ Administrators no longer need to re-assign connections after configuration changes
  • ✅ Improved user experience and reduced administrative overhead
  • ✅ Better performance through reduced object creation

This is a surgical fix that resolves the permission issue while maintaining full backward compatibility.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@ryanhamilton ryanhamilton deleted the copilot/fix-f8fdabee-041f-4b6f-883f-013e5d35d977 branch September 9, 2025 22:50
Copilot AI changed the title [WIP] bug: When changing a connection in any way the teams and suers which previously had access to it lose that access. It is also very annoying that we have to add a connection for a given Team/User rather than adding the connection then being fit to chang... Fix connection permission loss when updating server configurations Sep 9, 2025
Copilot AI requested a review from ryanhamilton September 9, 2025 23:00
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.

2 participants