fix: [BAS][Deploy] Password field in the deploy page disappears when the user edits the username. (#4934)

* fix: * Add dedicated flag which determines whether the login form should be displayed. The previously used flag has other meaning and is misused.
* Chnage the behaviour of getOrCreateServiceProvider(). In case the method is called with credentials we always get a fresh provider instance not a cached one.
* Fix an issue with the getSystemConfig - the backendTarget is always ignoired since the PromptState.abapDeployConfig is always an object. We return the config in the promot state only if it is a valid one otherwise we return the provided backednTarget.

* fix: * When we select xyz_noauth -> xyz_basic -> xyz_noauth then no credentials prompt appear when the xyz is selected without auth for the second time. This is because both xyz_basic and xyz_noauth represent the same system and use the cached provider for the auth system (xyz_auth). The comparator used to determine the equality of two systems is wrong it disregards the destination.
* Fix chageset summary.

* fix: Parametrize multiple tests into a single one.

Author: avasilev-sap

.changeset/short-beers-clap.md

@@ -0,0 +1,5 @@
+---
+'@sap-ux/abap-deploy-config-inquirer': patch
+---
+
+fix: [BAS][Deploy] Password field in the deploy page disappears when the user edits the username.

packages/abap-deploy-config-inquirer/src/prompts/conditions.ts

@@ -115,6 +115,11 @@ export async function showUsernameQuestion(backendTarget?: BackendTarget): Promi
     PromptState.transportAnswers.transportConfig = transportConfig;
     PromptState.transportAnswers.transportConfigNeedsCreds = transportConfigNeedsCreds ?? false;
 
+    // Track that credential fields should remain visible for the duration of the auth flow.
+    // This flag is NOT reset by validateCredentials, unlike transportConfigNeedsCreds which
+    // must become false after successful auth so downstream questions (package, transport) appear.
+    PromptState.transportAnswers.areCredentialFieldsVisible = transportConfigNeedsCreds ?? false;
+
     // Provide context to the CLI when username credentials are required
     if (transportConfigNeedsCreds) {
         LoggerHelper.logger.info(t('errors.atoUnauthorisedSystem'));
@@ -128,7 +133,7 @@ export async function showUsernameQuestion(backendTarget?: BackendTarget): Promi
  * @returns boolean
  */
 export function showPasswordQuestion(): boolean {
-    return Boolean(PromptState.transportAnswers.transportConfigNeedsCreds);
+    return !!PromptState.transportAnswers.areCredentialFieldsVisible;
 }
 
 /**

packages/abap-deploy-config-inquirer/src/service-provider-utils/abap-service-provider.ts

@@ -1,15 +1,15 @@
+import type { AbapServiceProvider, AxiosRequestConfig, ProviderConfiguration } from '@sap-ux/axios-extension';
 import { isAppStudio } from '@sap-ux/btp-utils';
-import { isSameSystem } from '../utils.js';
-import { createAbapServiceProvider } from '@sap-ux/system-access';
+import { setGlobalRejectUnauthorized } from '@sap-ux/nodejs-utils';
 import { AuthenticationType } from '@sap-ux/store';
-import { PromptState } from '../prompts/prompt-state.js';
-import LoggerHelper from '../logger-helper.js';
-import type { AbapServiceProvider, AxiosRequestConfig, ProviderConfiguration } from '@sap-ux/axios-extension';
 import type { DestinationAbapTarget, UrlAbapTarget } from '@sap-ux/system-access';
-import type { BackendTarget, Credentials, SystemConfig } from '../types.js';
+import { createAbapServiceProvider } from '@sap-ux/system-access';
 import type { AbapTarget } from '@sap-ux/ui5-config';
-import { setGlobalRejectUnauthorized } from '@sap-ux/nodejs-utils';
 import { t } from '../i18n.js';
+import LoggerHelper from '../logger-helper.js';
+import { PromptState } from '../prompts/prompt-state.js';
+import { areSystemConfigEquals, isValidSystemConfig } from '../system-utils.js';
+import type { BackendTarget, Credentials, SystemConfig } from '../types.js';
 
 /**
  * Class to manage the ABAP service provider used during prompting.
@@ -38,25 +38,36 @@ export class AbapServiceProviderManager {
             ignoreCertErrors = true;
             setGlobalRejectUnauthorized(false);
         }
-        // 1. Use existing service provider
-        if (this.isExistingServiceProviderValid(backendTarget)) {
+
+        const isExistingProviderValid = this.isExistingServiceProviderValid(backendTarget);
+        if (isExistingProviderValid && !this.areCredentialsProvided(credentials)) {
             return this.abapServiceProvider as AbapServiceProvider;
         }
 
-        // 2. Use connected service provider passed in prompt options with backend target
-        if (this.isBackendTargetServiceProviderValid(backendTarget)) {
-            this.abapServiceProvider = backendTarget?.serviceProvider as AbapServiceProvider;
-            await this.setIsDefaultAbapCloud();
-
-            return this.abapServiceProvider;
+        if (!isExistingProviderValid && this.isBackendTargetServiceProviderValid(backendTarget)) {
+            this.abapServiceProvider = backendTarget!.serviceProvider as AbapServiceProvider;
+        } else {
+            this.abapServiceProvider = await this.createNewServiceProvider(
+                credentials,
+                backendTarget,
+                ignoreCertErrors
+            );
         }
-        // 3. Create a new service provider
-        this.abapServiceProvider = await this.createNewServiceProvider(credentials, backendTarget, ignoreCertErrors);
-        await this.setIsDefaultAbapCloud();
 
+        await this.setIsDefaultAbapCloud();
         return this.abapServiceProvider;
     }
 
+    /**
+     * Checks if valid credentials (both username and password) are provided.
+     *
+     * @param credentials - user credentials
+     * @returns true if both username and password are non-empty
+     */
+    private static areCredentialsProvided(credentials?: Credentials): boolean {
+        return !!credentials?.username && !!credentials?.password;
+    }
+
     /**
      * Checks if the service provider has a valid connection.
      *
@@ -73,12 +84,12 @@ export class AbapServiceProviderManager {
      * @returns - system config
      */
     private static getSystemConfig(backendTarget?: BackendTarget): SystemConfig {
-        const { url, client, destination } = PromptState.abapDeployConfig ?? backendTarget?.abapTarget ?? {};
-        return {
-            url,
-            client,
-            destination
-        };
+        // PromptState.abapDeployConfig is always an object but could lack url and destination both,
+        // in that case we use the backendTarget.
+        const { url, destination, client } = isValidSystemConfig(PromptState.abapDeployConfig)
+            ? PromptState.abapDeployConfig
+            : (backendTarget?.abapTarget ?? {});
+        return { url, destination, client };
     }
 
     /**
@@ -89,10 +100,7 @@ export class AbapServiceProviderManager {
      */
     private static isExistingServiceProviderValid(backendTarget?: BackendTarget): boolean {
         const systemConfig = this.getSystemConfig(backendTarget);
-        if (
-            this.abapServiceProvider &&
-            isSameSystem(systemConfig, this.system?.url, this.system?.client, this.system?.destination)
-        ) {
+        if (this.abapServiceProvider && areSystemConfigEquals(systemConfig, this.system)) {
             this.system = systemConfig;
             return true;
         }
@@ -110,17 +118,8 @@ export class AbapServiceProviderManager {
     private static isBackendTargetServiceProviderValid(backendTarget?: BackendTarget): boolean {
         if (
             backendTarget?.serviceProvider &&
-            (isSameSystem(
-                {
-                    url: PromptState.abapDeployConfig.url,
-                    client: PromptState.abapDeployConfig.client,
-                    destination: PromptState.abapDeployConfig.destination
-                },
-                backendTarget?.abapTarget.url,
-                backendTarget?.abapTarget.client,
-                backendTarget?.abapTarget.destination
-            ) ||
-                (!PromptState.abapDeployConfig.url && !PromptState.abapDeployConfig.destination))
+            (areSystemConfigEquals(PromptState.abapDeployConfig, backendTarget?.abapTarget) ||
+                !isValidSystemConfig(PromptState.abapDeployConfig))
         ) {
             this.system = backendTarget?.abapTarget;
             return true;

packages/abap-deploy-config-inquirer/src/system-utils.ts

@@ -0,0 +1,46 @@
+import { type SystemConfig } from './types.js';
+
+/**
+ * Checks whether a system configuration is valid by verifying that it has either a URL or a destination defined.
+ *
+ * @param config - the system configuration to validate
+ * @returns `true` if the configuration has a URL or destination, `false` otherwise
+ */
+export function isValidSystemConfig(config?: SystemConfig): boolean {
+    if (!config) {
+        return false;
+    }
+    const { url, destination } = config;
+    return !!url || !!destination;
+}
+
+/**
+ * Compares two system configurations for equality.
+ * Two configurations are considered equal if they have the same normalized URL, client, and destination.
+ * Returns `false` if either configuration is invalid.
+ *
+ * @param configA - the first system configuration
+ * @param configB - the second system configuration
+ * @returns `true` if both configurations are valid and equal, `false` otherwise
+ */
+export function areSystemConfigEquals(configA?: SystemConfig, configB?: SystemConfig): boolean {
+    if (!isValidSystemConfig(configA) || !isValidSystemConfig(configB)) {
+        return false;
+    }
+
+    return (
+        normalizeSystemUrl(configA?.url) === normalizeSystemUrl(configB?.url) &&
+        configA?.client === configB?.client &&
+        configA?.destination === configB?.destination
+    );
+}
+
+/**
+ * Normalizes a system URL by trimming whitespace and removing a trailing slash.
+ *
+ * @param url - the URL to normalize
+ * @returns the normalized URL, or `undefined` if no URL was provided
+ */
+function normalizeSystemUrl(url?: string): string | undefined {
+    return url?.trim().replace(/\/$/, '');
+}

packages/abap-deploy-config-inquirer/src/types.ts

@@ -228,6 +228,11 @@ export interface TransportAnswers {
     transportConfig?: TransportConfig;
     transportConfigError?: string;
     transportConfigNeedsCreds?: boolean;
+    /**
+     *  Tracks that credential fields (username/password) should remain visible
+     * throughout the authentication flow.
+     */
+    areCredentialFieldsVisible?: boolean;
     transportList?: TransportListItem[];
     newTransportNumber?: string;
 }

packages/abap-deploy-config-inquirer/src/utils.ts

@@ -72,24 +72,6 @@ export function findBackendSystemByUrl(backendUrl: string): BackendSystem | unde
     return cachedBackendSystems?.find((backend: BackendSystem) => backend.url === backendUrl);
 }
 
-/**
- * Check if the current system is the same as the one in the answers.
- *
- * @param abapSystem - system configuration
- * @param url - url
- * @param client - client
- * @param destination - destination
- * @returns true if the system is the same
- */
-export function isSameSystem(abapSystem?: SystemConfig, url?: string, client?: string, destination?: string): boolean {
-    return Boolean(
-        (abapSystem?.url &&
-            abapSystem.url.trim()?.replace(/\/$/, '') === url?.trim()?.replace(/\/$/, '') &&
-            abapSystem.client === client) ||
-        (!!abapSystem?.destination && destination === abapSystem?.destination)
-    );
-}
-
 /**
  * Get transport configuration from the backend.
  *

packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts

@@ -189,7 +189,7 @@ describe('Test abap deploy config inquirer conditions', () => {
     });
 
     test('should show password questions', () => {
-        PromptState.transportAnswers.transportConfigNeedsCreds = true;
+        PromptState.transportAnswers.areCredentialFieldsVisible = true;
         expect(showPasswordQuestion()).toBe(true);
     });
 

packages/abap-deploy-config-inquirer/test/service-provider-utils/abap-service-provider.test.ts

@@ -32,6 +32,7 @@ describe('getOrCreateServiceProvider', () => {
 
     afterEach(() => {
         AbapServiceProviderManager.deleteExistingServiceProvider();
+        mockCreateAbapServiceProvider.mockReset();
     });
 
     beforeEach(() => {
@@ -70,8 +71,8 @@ describe('getOrCreateServiceProvider', () => {
             LoggerHelper.logger
         );
 
-        // use existing provider when called again
-        const serviceProvider2 = await AbapServiceProviderManager.getOrCreateServiceProvider(undefined, credentials);
+        // use existing provider when called again without credentials.
+        const serviceProvider2 = await AbapServiceProviderManager.getOrCreateServiceProvider(undefined);
         expect(serviceProvider2).toBe(serviceProvider);
     });
 
@@ -148,4 +149,48 @@ describe('getOrCreateServiceProvider', () => {
         );
         expect(buildRequestOptionsSpy).toHaveBeenCalledWith(undefined, true);
     });
+
+    it('should create a new service provider when called again with different credentials on the same system', async () => {
+        const abapServiceProviderA = new AbapServiceProvider();
+        const abapServiceProviderB = new AbapServiceProvider();
+        mockIsAppStudio.mockReturnValue(false);
+        mockCreateAbapServiceProvider.mockResolvedValueOnce(abapServiceProviderA);
+        mockCreateAbapServiceProvider.mockResolvedValueOnce(abapServiceProviderB);
+
+        PromptState.abapDeployConfig = {
+            url: 'http://target.url',
+            client: '100',
+            scp: false
+        };
+
+        const credentials1 = { username: 'userA', password: 'passwordA' };
+        const credentials2 = { username: 'userB', password: 'passwordB' };
+
+        const serviceProvider1 = await AbapServiceProviderManager.getOrCreateServiceProvider(undefined, credentials1);
+        expect(serviceProvider1).toBe(abapServiceProviderA);
+
+        const serviceProvider2 = await AbapServiceProviderManager.getOrCreateServiceProvider(undefined, credentials2);
+        expect(serviceProvider2).toBe(abapServiceProviderB);
+        expect(serviceProvider2).not.toBe(serviceProvider1);
+        expect(mockCreateAbapServiceProvider).toHaveBeenCalledTimes(2);
+    });
+
+    it('should return cached service provider when called without credentials after initial creation', async () => {
+        const abapServiceProvider = new AbapServiceProvider();
+        mockIsAppStudio.mockReturnValue(false);
+        mockCreateAbapServiceProvider.mockResolvedValueOnce(abapServiceProvider);
+
+        PromptState.abapDeployConfig = {
+            url: 'http://target.url',
+            client: '100',
+            scp: false
+        };
+
+        const serviceProviderA = await AbapServiceProviderManager.getOrCreateServiceProvider();
+        expect(serviceProviderA).toBe(abapServiceProvider);
+
+        const serviceProviderB = await AbapServiceProviderManager.getOrCreateServiceProvider();
+        expect(serviceProviderB).toBe(serviceProviderA);
+        expect(mockCreateAbapServiceProvider).toHaveBeenCalledTimes(1);
+    });
 });