reconfigure clears stored credentials before validating, so a failed switch can destroy a working client secret

[azchohfi]

Summary

CLIConfigurator.ConfigureAsync mutates the OS credential store before it validates the new configuration, and only writes settings.json on success. A failed reconfigure can therefore delete a working client secret while leaving settings.json still configured to use one.

Since Entra only reveals a client secret at creation time, recovery means minting a new secret.

Sequence

In MSStore.CLI/Services/CLIConfigurator.cs:

Line	What happens
282-293	Write or clear the stored credential — mutates the OS credential store
327	_storeAPIFactory.CreateAsync(config, ct) — first actual validation
357	_configurationManager.SaveAsync(config, ct) — settings.json written only on success

The else branch at 290-293 calls ClearCredentials whenever neither clientSecret nor certificatePassword was supplied.

Repro

Starting from a working client-secret configuration:

1. msstore reconfigure --clientAssertion (same clientId, MSSTORE_CLIENT_ASSERTION not yet set)
2. Neither clientSecret nor certificatePassword is passed → else at line 290 → ClearCredentials deletes the working secret
3. Validation at line 327 fails (no assertion available)
4. SaveAsync at line 357 never runs, so settings.json still says client-secret mode

Result: config expects a client secret; the secret is gone.

--certificateThumbprint reaches the same branch, so this is not specific to client assertion.

Notes on fixing

This is not a simple reordering:

• WriteCredential must stay ahead of validation, because StoreAPIFactory calls ReadCredential to construct the client.
• Deferring only the else branch leaves a stale secret in place during validation. On the certificate-file path that value is passed as the PKCS#12 password (StoreAPIFactory.cs:129), so behaviour changes for a mode unrelated to the trigger.

A likely shape is to validate the candidate configuration without mutating stored credentials, then commit settings.json and clear obsolete credentials only after success. Worth covering all three modes (client secret, certificate, client assertion) with tests.

Context

Pre-existing; predates #145. Noticed while reviewing that PR, where --clientAssertion made it considerably easier to trigger — "forgot to set the environment variable" is a natural first-time mistake, whereas the certificate path fails far more rarely.

Worth landing before the next release so the client-assertion flow doesn't ship with the sharper edge exposed.