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

Incorrect API Key Lock/Unlock and Save/Delete Button Behavior in Settings Window #29

Closed
drankush opened this issue Feb 11, 2025 · 0 comments
Assignees
Labels
bug Something isn't working encryption-keys

Comments

@drankush
Copy link
Owner

The settings window (settings_window.py) in the application was exhibiting incorrect behavior related to the saving, deleting, locking, and unlocking of API keys for the Transcription, Text, and Multimodal models (First introduced in #10). The core problems were:

  1. Incorrect File Path: The application was looking for the encrypted key files in the wrong directory. It was appending the key file names to the path of the settings.ini file, instead of just the configuration directory. This caused the application to always think that the key files did not exist.

  2. Inconsistent Button States: The Lock/Unlock and Save/Delete buttons were not correctly reflecting the state of the API keys (whether they were loaded, saved, or locked). The initial state was often incorrect, and the buttons did not always update properly after user actions. This led to a confusing user experience where keys could appear to be unlocked when they were not, and vice-versa. The Lock/Unlock button was often disabled when it should have been enabled.

  3. Missing config Updates: When deleting keys, the corresponding variables in the config module (e.g., config.TRANSCRIPTION_API_KEY) were not being set to None. This meant the application still considered the key to be "loaded" even after the encrypted file was deleted.

  4. Multimodal Model Checkbox: The Multimodal Model tab's checkbox did not correctly enable/disable the associated API key entry field when there was no saved API key.

Root Cause:

  • The primary root cause was the incorrect file path used to locate the encrypted key files, resulting from using os.path.join(get_default_config_path(), "key_file.encrypted"), which appended the key file name to the settings file path.
  • Secondary issues stemmed from inconsistent updates to the UI elements (buttons and entry fields) and the config variables.

Solution:

The solution involved several key changes:

  1. get_config_dir() Function: A new helper function, get_config_dir(), was introduced to return only the configuration directory path, without the settings.ini filename. This function is now used to construct the correct paths to the encrypted key files. This was done because we were instructed not to modify the existing get_default_config_path() function.

    def get_config_dir():
        """Helper function to get ONLY the config directory, without settings.ini"""
        if os.name == "nt":  # Windows
            config_dir = os.path.join(os.environ["APPDATA"], "VOXRAD")
        else:  # Assuming macOS or Linux
            config_dir = os.path.join(os.path.expanduser("~"), ".voxrad")
        if not os.path.exists(config_dir):
            os.makedirs(config_dir)
        return config_dir
  2. Corrected File Paths: All instances of os.path.join(get_default_config_path(), ...) used for accessing the key files were replaced with os.path.join(get_config_dir(), ...):

    # Example (Transcription Model):
    transcription_key_path = os.path.join(get_config_dir(), "transcription_key.encrypted")

    This was also updated on the utils/encryption.py file to use get_config_dir() instead.

  3. update_..._ui() Functions: For each model tab (Transcription, Text, Multimodal), a dedicated update_..._ui() function was created. These functions encapsulate all the logic for updating the UI elements (entry field state, save/delete button state, lock/unlock button state) based on the existence of the encrypted file and the value of the corresponding config variable (e.g., config.TRANSCRIPTION_API_KEY). These functions are called:

    • On initialization.
    • After saving a key.
    • After deleting a key.
    • After (un)locking.

    This ensures consistency and makes the code much easier to maintain.

  4. Clearing config Variables: The delete_..._key_ui() functions were modified to set the corresponding config variable (e.g., config.TRANSCRIPTION_API_KEY) to None when a key is deleted.

  5. Checkbox and Combobox Logic (Multimodal): The Multimodal tab's logic was corrected to properly handle the checkbox state and enable/disable the API key entry field and combobox appropriately, including handling cases where the key file doesn't exist.

  6. Added Debug Print Statements. This would help understand the flow.

Code Changes (Illustrative Examples):

The complete code is quite large, so I'm providing illustrative examples of the key changes in Transcription Tab.

  • settings_window.py (Transcription Model - Excerpt):

    # ... (rest of the code) ...
    
    # Transcription API Key Setting
    transcription_key_label = tk.Label(transcription_tab, text="API Key:")
    transcription_key_label.grid(row=2, column=0, padx=5, pady=5, sticky="w")
    transcription_key_var = tk.StringVar(transcription_tab)
    transcription_key_entry = tk.Entry(transcription_tab, textvariable=transcription_key_var, show="*", width=30)
    transcription_key_entry.grid(row=2, column=1, padx=5, pady=5, sticky="w")
    
    # Corrected path:
    transcription_key_path = os.path.join(get_config_dir(), "transcription_key.encrypted")
    print(f"[DEBUG] Transcription key path: {transcription_key_path}")
    transcription_key_file_exists = os.path.exists(transcription_key_path)
    
    
    save_delete_button = tk.Button(transcription_tab, width=12)
    lock_unlock_button = tk.Button(transcription_tab, width=12)
    
    def update_transcription_ui():
        nonlocal transcription_key_file_exists
        transcription_key_file_exists = os.path.exists(transcription_key_path)
        print(f"[DEBUG] Transcription key file exists: {transcription_key_file_exists}")
    
        if transcription_key_file_exists:
            transcription_key_entry.config(state="readonly")
            transcription_key_var.set("**********************************")
            save_delete_button.config(text="Delete Key")
            print(f"[DEBUG] Encrypted file exists - Setting entry to readonly and button to 'Delete Key'")
            if config.TRANSCRIPTION_API_KEY:
                lock_unlock_button.config(text="🔒 Lock Key", state="normal")
                print(f"[DEBUG] API key loaded - Setting Lock/Unlock button to 'Lock Key' and enabled")
            else:
                lock_unlock_button.config(text="🔓 Unlock Key", state="normal")
                print(f"[DEBUG] API key NOT loaded - Setting Lock/Unlock button to 'Unlock Key' and enabled")
        else:
            transcription_key_entry.config(state="normal")
            transcription_key_var.set("")
            save_delete_button.config(text="Save Key")
            lock_unlock_button.config(text="🔓 Unlock Key", state="disabled")
            print(f"[DEBUG] No encrypted file - Setting entry to normal and empty, and Lock/Unlock to disabled")
    
        if config.TRANSCRIPTION_API_KEY is None and transcription_key_file_exists:
            lock_unlock_button.config(state="normal")
            print(f"[DEBUG] API key is None and file exists - Lock/Unlock button state set to normal")
    
    def delete_transcription_key_ui():
        delete_transcription_key()
        config.TRANSCRIPTION_API_KEY = None  # Clear the key on delete
        print(f"[DEBUG] Transcription key deleted and config.TRANSCRIPTION_API_KEY set to None")
        update_transcription_ui()
    
    # ... (rest of the Transcription Model code, using update_transcription_ui) ...

Conclusion:

These changes corrected the file path issue, ensured consistent UI updates, and fixed the logic for managing the API keys. The settings window now behaves as expected, allowing users to save, delete, lock, and unlock API keys reliably.

@drankush drankush added bug Something isn't working encryption-keys labels Feb 11, 2025
@drankush drankush self-assigned this Feb 11, 2025
drankush added a commit that referenced this issue Feb 11, 2025
Fixed multiple issues with #29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working encryption-keys
Projects
None yet
Development

No branches or pull requests

1 participant