Enhance API Key encryption and remove masking logic

[tanushahande2003]

API Key is now encrypted on field exit and before saving, unless it is a value expression or already encrypted. Removed the previous masking/tag mechanism for the API Key textbox, simplifying the logic and improving security. Added necessary using directives for new functionality.

Description

Type of Change

• Bug fix - [ ] New feature - [ ] Breaking change - [ ] Plugin update

Checklist

• PR description clearly describes the changes
• Target branch is correct (master for features, Releases/* for fixes)
• Latest code from target branch merged
• No commented/junk code included
• No new build warnings or errors
• All existing unit tests pass
• New unit tests added for new functionality
• Cross-platform compatibility verified (Windows/Linux/macOS)
• CI/CD pipeline passes
• Code follows project conventions (Act{Platform}{Type}, {Platform}Driver)
• Repository objects use [IsSerializedForLocalRepository] where needed
• Error handling uses Reporter.ToLog() pattern
• Documentation updated for user-facing changes
• Self-review completed and code review comments addressed

Summary by CodeRabbit

• Bug Fixes
  • API keys and tokens entered in external configuration screens are now encrypted when the field loses focus and are verified/encrypted again before saving.
• New Features
  • Sealights configuration UI now fully re-initializes controls and validation when logging is enabled, and initializes the test-recommendations option.

[coderabbitai]

Walkthrough

Adds LostKeyboardFocus handlers and pre-save checks to encrypt API keys/tokens in multiple external configuration pages when values are non-empty, not value-expressions, and not already encrypted; includes required using directives and a small UI re-initialization step in the Sealights flow.

Changes

Cohort / File(s)	Summary
VRT External Config Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs	Register LostKeyboardFocus on API key textbox; encrypt value on blur if non-empty, not a value-expression, and not already encrypted; ensure same encryption check before save; add using GingerCore and using System.Windows.Input.
Applitools External Config Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs	Subscribe LostKeyboardFocus for ApiKey textbox and encrypt on blur when appropriate; apply pre-save encryption check for _ApplitoolsConfiguration.ApiKey; add required using directives.
Sealights External Config Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs	Attach LostKeyboardFocus handler for SealightsAgentToken textbox; re-attach after control re-init when enabling logging; perform pre-save encryption check for agent token; add using directives and minor UI re-init/validation wiring.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant UI as UI (TextBox)
    participant Encrypt as EncryptionService
    participant Model as Config Model
    participant Store as Config Store

    User->>UI: Edit API key/token
    UI->>UI: LostKeyboardFocus triggered
    UI->>Encrypt: Check: isValueExpression?
    alt not a value-expression
        UI->>Encrypt: Check: isEncrypted?
        alt not encrypted
            UI->>Encrypt: Encrypt(value)
            Encrypt-->>UI: EncryptedValue
            UI->>Model: Update field with EncryptedValue
        else already encrypted
            UI-->>Model: Keep value
        end
    else value-expression
        UI-->>Model: Keep value as-is
    end
    User->>Store: Save configuration
    Store->>Model: Pre-save: ensure non-empty, not expression and encrypted
    Model-->>Store: Persist

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Mask API key in UI and update only on user edit #4436: Modifies VRTExternalConfigurationsPage.xaml.cs with prior ApiKey masking/handling changes that overlap with this PR's edits.

Suggested reviewers

• ravirk91

Poem

🐇 I nibbled keys by lamp and moon,
On blur I hopped to hum a tune,
I wrapped each secret snug and tight,
Now configs sleep through every night,
Encrypting carrots — safe delight! 🥕🔐

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description clearly outlines what changed (encryption on field exit and save, removal of masking), but all checklist items are unchecked despite being part of the template, making assessment of compliance incomplete.	Complete the checklist by checking applicable items or explicitly documenting why items were skipped (e.g., no new tests added, no user-facing changes).

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: API key encryption enhancement and removal of masking logic, directly reflecting the core modifications across three configuration pages.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch Defect/58711_VRT_key_encryption

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 67-85: The xAPIKeyTextBox_LostKeyboardFocus handler currently only
skips value-expressions and checks EncryptionHandler.IsStringEncrypted before
encrypting; add a guard that treats whitespace-only input as empty by checking
string.IsNullOrWhiteSpace(current) and return (do not call
EncryptionHandler.EncryptwithKey) when true, so whitespace won't be encrypted
and bypass validation; keep existing checks
(ValueExpression.IsThisAValueExpression, EncryptionHandler.IsStringEncrypted)
and only call EncryptionHandler.EncryptwithKey when the string is neither a
value-expression nor whitespace and not already encrypted.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 995cd64 and 08fbba4.

📒 Files selected for processing (1)

• Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs

[MeniKadosh1]

it will handle only new setting of passwords? what about passwords already exist? will stay eaw till next edit?

[tanushahande2003]

Old keys stay as-is until the next edit or save; after that, they become encrypted automatically.

[MeniKadosh1]

Did you handled also Decryption of the password before being used in the backend logic to connect to VRT?

[tanushahande2003]

No, decryption must still be done in the backend before connecting to VRT. You just need to call EncryptionHandler.DecryptwithKey(...) wherever the API key is used. The UI-side encryption does not automatically decrypt it for backend use.

[MeniKadosh1]

Need to handle the same for all API keys / passwords placed on External Configurations and not only VRT if also there the keys are not encrypted

[tanushahande2003]

yes I will do same changes for Applitools & sealights. for other external integration there is already encryption is handled

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs`:
- Around line 61-75: In xAPIKeyTextBox_LostKeyboardFocus, add a guard that
returns early when the current text is null or empty (e.g., string.IsNullOrEmpty
or IsNullOrWhiteSpace on the local variable current) before the
ValueExpression/Encryption checks so you don't attempt to encrypt empty strings;
keep the existing checks (ValueExpression.IsThisAValueExpression and
EncryptionHandler.IsStringEncrypted) and only call
EncryptionHandler.EncryptwithKey when current contains a non-empty,
non-expression, non-encrypted value.

In
`@Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs`:
- Around line 69-72: The commented-out block that sets a default on
xSealighsSessionTimeoutTextBox.ValueTextBox.Text should be either removed or
documented: either delete the three-line dead code entirely, or replace it with
a concise comment explaining why the defaulting logic is disabled (e.g., handled
elsewhere or deprecated) and reference the UI control name
xSealighsSessionTimeoutTextBox and the containing class/method in
SealightsExternalConfigurationsPage.xaml.cs so future maintainers know the
intent.
- Around line 20-21: Remove the duplicate using directive for
Amdocs.Ginger.Common in SealightsExternalConfigurationsPage.xaml.cs: locate the
two identical lines "using Amdocs.Ginger.Common;" and delete one of them so the
namespace is only imported once (no other changes to the
SealightsExternalConfigurationsPage class or its members are required).
- Around line 78-91: The LostKeyboardFocus handler
xSealighsAgentTokenTextBox_LostKeyboardFocus currently attempts to encrypt the
textbox text without checking for null/empty; add the same guard used in the
save handler by skipping encryption when string.IsNullOrWhiteSpace(current) (in
addition to the existing ValueExpression and IsStringEncrypted checks) so empty
or whitespace inputs are not encrypted by EncryptionHandler.EncryptwithKey.
- Around line 120-128: The if-block surrounding
_SealightsConfiguration.SealightsAgentToken has inconsistent indentation;
reformat the lines so the if condition, its continuations, opening brace, body
assignment, and closing brace are aligned consistently (e.g., align the "if("
line and indent the condition continuations one level, place the "{" on the same
line or next line consistently, and indent the assignment). Update the block
that uses ValueExpression.IsThisAValueExpression(...),
EncryptionHandler.IsStringEncrypted(...), and
EncryptionHandler.EncryptwithKey(...) so all parentheses/conditions and the
assignment to _SealightsConfiguration.SealightsAgentToken follow the project's
standard indentation style.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08fbba4 and 28d5424.

📒 Files selected for processing (2)

• Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs
• Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove duplicate using directive.

using Amdocs.Ginger.Common; appears twice (lines 20 and 21). This will cause a compiler warning.

Proposed fix

 using amdocs.ginger.GingerCoreNET;
 using Amdocs.Ginger.Common;
-using Amdocs.Ginger.Common;
 using Ginger.UserControlsLib;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	using Amdocs.Ginger.Common;
	using Amdocs.Ginger.Common;
	using amdocs.ginger.GingerCoreNET;
	using Amdocs.Ginger.Common;
	using Ginger.UserControlsLib;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs`
around lines 20 - 21, Remove the duplicate using directive for
Amdocs.Ginger.Common in SealightsExternalConfigurationsPage.xaml.cs: locate the
two identical lines "using Amdocs.Ginger.Common;" and delete one of them so the
namespace is only imported once (no other changes to the
SealightsExternalConfigurationsPage class or its members are required).

[coderabbitai]

♻️ Duplicate comments (4)

Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs (2)

20-21: ⚠️ Potential issue | 🟡 Minor

Remove the duplicate using Amdocs.Ginger.Common; import.

The same namespace is imported twice, which adds unnecessary compiler noise.

Proposed fix

 using amdocs.ginger.GingerCoreNET;
 using Amdocs.Ginger.Common;
-using Amdocs.Ginger.Common;
 using Ginger.UserControlsLib;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs`
around lines 20 - 21, Remove the duplicate using directive by deleting one of
the repeated "using Amdocs.Ginger.Common;" lines in the
SealightsExternalConfigurationsPage.xaml.cs file (look for the two identical
using statements at the top of the file); ensure only a single "using
Amdocs.Ginger.Common;" remains and check for any other accidental duplicate
using directives in the same file.

---

78-90: ⚠️ Potential issue | 🟡 Minor

Add a whitespace guard before blur-time encryption.

Whitespace-only input is currently encrypted, which can turn an effectively empty token into non-empty ciphertext and bypass empty-value validation.

Proposed fix

 private void xSealighsAgentTokenTextBox_LostKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e)
 {
     var textbox = xSealighsAgentTokenTextBox.ValueTextBox;
     string current = textbox.Text;
+
+    if (string.IsNullOrWhiteSpace(current))
+        return;
 
     // Skip value expressions (AskLisa behavior)
     if (ValueExpression.IsThisAValueExpression(current))
         return;  // same rule as AskLisa [1]

Based on learnings: "When validating API keys or similar sensitive configuration values, use case-insensitive comparison (StringComparison.OrdinalIgnoreCase) with known placeholder values, and check for null, empty, or whitespace with string.IsNullOrWhiteSpace() rather than just string.IsNullOrEmpty()."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs`
around lines 78 - 90, In xSealighsAgentTokenTextBox_LostKeyboardFocus, avoid
encrypting whitespace-only input by checking string.IsNullOrWhiteSpace(current)
before calling EncryptionHandler.EncryptwithKey; if whitespace (or empty) return
early (do not encrypt) so validation still sees an empty token. Also keep the
existing ValueExpression.IsThisAValueExpression(current) check, and when
skipping placeholder values add comparisons using
StringComparison.OrdinalIgnoreCase against any known placeholder strings; use
EncryptionHandler.IsStringEncrypted(current) only after the
whitespace/placeholder guards.

Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs (1)

61-74: ⚠️ Potential issue | 🟡 Minor

Add IsNullOrWhiteSpace guard in blur handler before encryption.

Whitespace-only API key input is currently encrypted and can bypass required-key validation semantics.

Proposed fix

 private void xAPIKeyTextBox_LostKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e)
 {
     var textbox = xAPIKeyTextBox.ValueTextBox;
     string current = textbox.Text;
+
+    if (string.IsNullOrWhiteSpace(current))
+        return;
 
     // Do not encrypt value expressions (AskLisa rule)
     if (ValueExpression.IsThisAValueExpression(current))
         return;

Based on learnings: "When validating API keys or similar sensitive configuration values, use case-insensitive comparison (StringComparison.OrdinalIgnoreCase) with known placeholder values, and check for null, empty, or whitespace with string.IsNullOrWhiteSpace() rather than just string.IsNullOrEmpty()."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs`
around lines 61 - 74, In xAPIKeyTextBox_LostKeyboardFocus, add a guard using
string.IsNullOrWhiteSpace(current) before any encryption so whitespace-only
values are not encrypted; retain the existing
ValueExpression.IsThisAValueExpression check, then if not a value expression and
not whitespace/null, proceed to call EncryptionHandler.IsStringEncrypted and
EncryptionHandler.EncryptwithKey as before; if you also compare against any
placeholder string, use string.Equals(placeholder, current,
StringComparison.OrdinalIgnoreCase) to avoid case-sensitive bypasses.

Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs (1)

71-83: ⚠️ Potential issue | 🟡 Minor

Add whitespace guard before encrypting on blur.

The handler still encrypts whitespace-only input, which can convert an empty/invalid key into a non-empty encrypted value.

Proposed fix

 private void xAPIKeyTextBox_LostKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e)
 {
     var textbox = xAPIKeyTextBox.ValueTextBox;
     string current = textbox.Text;
+
+    if (string.IsNullOrWhiteSpace(current))
+        return;
 
     // Don't touch value expressions
     if (ValueExpression.IsThisAValueExpression(current))
         return;  // 

Based on learnings: "When validating API keys or similar sensitive configuration values, use case-insensitive comparison (StringComparison.OrdinalIgnoreCase) with known placeholder values, and check for null, empty, or whitespace with string.IsNullOrWhiteSpace() rather than just string.IsNullOrEmpty()."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`
around lines 71 - 83, In xAPIKeyTextBox_LostKeyboardFocus, add a whitespace/null
guard before treating the textbox value: first check
string.IsNullOrWhiteSpace(current) and return if true so whitespace-only input
is not encrypted; additionally, if you compare against placeholder values, use
String.Equals(placeholder, current, StringComparison.OrdinalIgnoreCase) to skip
encryption for known placeholders; leave the existing
ValueExpression.IsThisAValueExpression, EncryptionHandler.IsStringEncrypted and
EncryptionHandler.EncryptwithKey calls intact but only run them after the
null/whitespace and placeholder checks.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs`:
- Around line 61-74: In xAPIKeyTextBox_LostKeyboardFocus, add a guard using
string.IsNullOrWhiteSpace(current) before any encryption so whitespace-only
values are not encrypted; retain the existing
ValueExpression.IsThisAValueExpression check, then if not a value expression and
not whitespace/null, proceed to call EncryptionHandler.IsStringEncrypted and
EncryptionHandler.EncryptwithKey as before; if you also compare against any
placeholder string, use string.Equals(placeholder, current,
StringComparison.OrdinalIgnoreCase) to avoid case-sensitive bypasses.

In
`@Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs`:
- Around line 20-21: Remove the duplicate using directive by deleting one of the
repeated "using Amdocs.Ginger.Common;" lines in the
SealightsExternalConfigurationsPage.xaml.cs file (look for the two identical
using statements at the top of the file); ensure only a single "using
Amdocs.Ginger.Common;" remains and check for any other accidental duplicate
using directives in the same file.
- Around line 78-90: In xSealighsAgentTokenTextBox_LostKeyboardFocus, avoid
encrypting whitespace-only input by checking string.IsNullOrWhiteSpace(current)
before calling EncryptionHandler.EncryptwithKey; if whitespace (or empty) return
early (do not encrypt) so validation still sees an empty token. Also keep the
existing ValueExpression.IsThisAValueExpression(current) check, and when
skipping placeholder values add comparisons using
StringComparison.OrdinalIgnoreCase against any known placeholder strings; use
EncryptionHandler.IsStringEncrypted(current) only after the
whitespace/placeholder guards.

In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 71-83: In xAPIKeyTextBox_LostKeyboardFocus, add a whitespace/null
guard before treating the textbox value: first check
string.IsNullOrWhiteSpace(current) and return if true so whitespace-only input
is not encrypted; additionally, if you compare against placeholder values, use
String.Equals(placeholder, current, StringComparison.OrdinalIgnoreCase) to skip
encryption for known placeholders; leave the existing
ValueExpression.IsThisAValueExpression, EncryptionHandler.IsStringEncrypted and
EncryptionHandler.EncryptwithKey calls intact but only run them after the
null/whitespace and placeholder checks.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28d5424 and 884967c.

📒 Files selected for processing (3)

• Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs
• Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs
• Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs