bug fixing config source and retries

Author: clavery

packages/b2c-tooling-sdk/src/auth/oauth-implicit.ts

@@ -100,6 +100,7 @@ async function openBrowser(url: string): Promise<void> {
 export class ImplicitOAuthStrategy implements AuthStrategy {
   private accountManagerHost: string;
   private localPort: number;
+  private _hasHadSuccess = false;
 
   constructor(private config: ImplicitOAuthConfig) {
     this.accountManagerHost = config.accountManagerHost || DEFAULT_ACCOUNT_MANAGER_HOST;
@@ -132,9 +133,14 @@ export class ImplicitOAuthStrategy implements AuthStrategy {
 
     logger.debug({method, url, status: res.status, duration}, '[Auth] Response');
 
-    // RESILIENCE: If the server says 401, the token might have expired or been revoked.
-    // We retry exactly once after invalidating the cached token.
-    if (res.status === 401) {
+    if (res.status !== 401) {
+      this._hasHadSuccess = true;
+    }
+
+    // RESILIENCE: If we previously had a successful response and now get a 401,
+    // the token likely expired. Retry once after invalidating the cached token.
+    // Skip retry on initial 401 to avoid retrying with bad credentials.
+    if (res.status === 401 && this._hasHadSuccess) {
       logger.debug('[Auth] Received 401, invalidating token and retrying');
       this.invalidateToken();
       const newToken = await this.getAccessToken();

packages/b2c-tooling-sdk/src/auth/oauth.ts

@@ -33,6 +33,7 @@ function decodeJWT(jwt: string): DecodedJWT {
 
 export class OAuthStrategy implements AuthStrategy {
   private accountManagerHost: string;
+  private _hasHadSuccess = false;
 
   constructor(private config: OAuthConfig) {
     this.accountManagerHost = config.accountManagerHost || DEFAULT_ACCOUNT_MANAGER_HOST;
@@ -49,9 +50,14 @@ export class OAuthStrategy implements AuthStrategy {
     // Node.js fetch accepts dispatcher as an undocumented option
     let res = await fetch(url, {...init, headers} as RequestInit);
 
-    // RESILIENCE: If the server says 401, the token might have expired or been revoked.
-    // We retry exactly once after invalidating the cached token.
-    if (res.status === 401) {
+    if (res.status !== 401) {
+      this._hasHadSuccess = true;
+    }
+
+    // RESILIENCE: If we previously had a successful response and now get a 401,
+    // the token likely expired. Retry once after invalidating the cached token.
+    // Skip retry on initial 401 to avoid retrying with bad credentials.
+    if (res.status === 401 && this._hasHadSuccess) {
       this.invalidateToken();
       const newToken = await this.getAccessToken();
       headers.set('Authorization', `Bearer ${newToken}`);

packages/b2c-tooling-sdk/src/cli/ods-command.ts

@@ -36,9 +36,8 @@ export abstract class OdsCommand<T extends typeof Command> extends OAuthCommand<
   static baseFlags = {
     ...OAuthCommand.baseFlags,
     'sandbox-api-host': Flags.string({
-      description: 'ODS API hostname',
+      description: `ODS API hostname (default: ${DEFAULT_ODS_HOST})`,
       env: 'SFCC_SANDBOX_API_HOST',
-      default: DEFAULT_ODS_HOST,
       // helpGroup: 'ODS',
     }),
   };

packages/b2c-tooling-sdk/src/clients/middleware.ts

@@ -56,6 +56,7 @@ const requestBodies = new WeakMap<Request, ArrayBuffer | null>();
  */
 export function createAuthMiddleware(auth: AuthStrategy): Middleware {
   const logger = getLogger();
+  let hasHadSuccess = false;
 
   return {
     async onRequest({request}) {
@@ -75,10 +76,16 @@ export function createAuthMiddleware(auth: AuthStrategy): Middleware {
     },
 
     async onResponse({request, response}) {
-      // Only retry on 401 if we haven't already retried this request
-      // and the strategy supports token invalidation
+      if (response.status !== 401) {
+        hasHadSuccess = true;
+      }
+
+      // Only retry on 401 if we have had a prior successful response (indicating
+      // token expiry rather than bad credentials), haven't already retried this
+      // request, and the strategy supports token invalidation
       if (
         response.status === 401 &&
+        hasHadSuccess &&
         !retriedRequests.has(request) &&
         auth.invalidateToken &&
         auth.getAuthorizationHeader

packages/b2c-tooling-sdk/test/auth/oauth-implicit.test.ts

@@ -74,20 +74,64 @@ describe('auth/oauth-implicit', () => {
       const headers = new Headers(init?.headers);
       seenAuth.push(headers.get('Authorization') ?? '');
 
+      // First call succeeds (establishes prior success)
       if (fetchCalls === 1) {
+        return new Response('ok', {status: 200});
+      }
+
+      // Second call returns 401 (simulates expired token)
+      if (fetchCalls === 2) {
         return new Response('unauthorized', {status: 401});
       }
 
+      // Third call succeeds (retry with fresh token)
       return new Response('ok', {status: 200});
     };
 
+    // First call succeeds - establishes prior success
+    const firstRes = await strategy.fetch('https://example.com/test');
+    expect(firstRes.status).to.equal(200);
+
+    // Second call gets 401 then retries with fresh token
     const res = await strategy.fetch('https://example.com/test');
     expect(res.status).to.equal(200);
 
-    expect(fetchCalls).to.equal(2);
+    expect(fetchCalls).to.equal(3);
     expect(tokenCalls).to.equal(2);
     expect(seenAuth[0]).to.equal('Bearer tok-1');
-    expect(seenAuth[1]).to.equal('Bearer tok-2');
+    expect(seenAuth[1]).to.equal('Bearer tok-1');
+    expect(seenAuth[2]).to.equal('Bearer tok-2');
+
+    strategy.invalidateToken();
+  });
+
+  it('does not retry on initial 401 when no prior success', async () => {
+    const clientId = 'implicit-client-no-retry';
+    const strategy = new ImplicitOAuthStrategy({clientId, scopes: ['a']});
+
+    let tokenCalls = 0;
+    (strategy as unknown as {implicitFlowLogin: () => Promise<TokenResponse>}).implicitFlowLogin = async () => {
+      tokenCalls++;
+      return {
+        accessToken: 'tok-bad',
+        expires: futureDate(30),
+        scopes: ['a'],
+      };
+    };
+
+    let fetchCalls = 0;
+
+    globalThis.fetch = async (_input: string | URL | Request, _init?: RequestInit) => {
+      fetchCalls++;
+      return new Response('unauthorized', {status: 401});
+    };
+
+    // First call gets 401 - should NOT retry since no prior success
+    const res = await strategy.fetch('https://example.com/test');
+    expect(res.status).to.equal(401);
+
+    expect(fetchCalls).to.equal(1); // No retry
+    expect(tokenCalls).to.equal(1); // Only fetched once
 
     strategy.invalidateToken();
   });

packages/b2c-tooling-sdk/test/auth/oauth.test.ts

@@ -269,13 +269,18 @@ describe('auth/oauth', () => {
             apiRequestCount++;
             const authHeader = request.headers.get('Authorization');
 
-            // First request with old token fails
+            // First request succeeds (establishes prior success)
             if (apiRequestCount === 1 && authHeader === `Bearer ${mockToken1}`) {
+              return HttpResponse.json({success: true});
+            }
+
+            // Second request with old token fails (simulates expired token)
+            if (apiRequestCount === 2 && authHeader === `Bearer ${mockToken1}`) {
               return new HttpResponse(null, {status: 401});
             }
 
-            // Second request with new token succeeds
-            if (apiRequestCount === 2 && authHeader === `Bearer ${mockToken2}`) {
+            // Third request with new token succeeds
+            if (apiRequestCount === 3 && authHeader === `Bearer ${mockToken2}`) {
               return HttpResponse.json({success: true});
             }
 
@@ -288,12 +293,50 @@ describe('auth/oauth', () => {
           clientSecret: 'test-secret',
         });
 
+        // First call succeeds - establishes prior success
+        const firstResponse = await strategy.fetch(TEST_API_URL);
+        expect(firstResponse.status).to.equal(200);
+
+        // Second call gets 401 then retries with fresh token
         const response = await strategy.fetch(TEST_API_URL);
 
         expect(tokenRequestCount).to.equal(2); // Fetched twice
-        expect(apiRequestCount).to.equal(2); // API called twice
+        expect(apiRequestCount).to.equal(3); // Initial success + 401 + retry
         expect(response.status).to.equal(200);
       });
+
+      it('should not retry on initial 401 with bad credentials', async () => {
+        const clientId = 'test-client-bad-creds';
+        const mockToken = createMockJWT({sub: clientId});
+        let tokenRequestCount = 0;
+        let apiRequestCount = 0;
+
+        server.use(
+          http.post(AM_URL, () => {
+            tokenRequestCount++;
+            return HttpResponse.json({
+              access_token: mockToken,
+              expires_in: 1800,
+            });
+          }),
+          http.get(TEST_API_URL, () => {
+            apiRequestCount++;
+            return new HttpResponse(null, {status: 401});
+          }),
+        );
+
+        const strategy = new OAuthStrategy({
+          clientId,
+          clientSecret: 'test-secret',
+        });
+
+        // First call gets 401 - should NOT retry since no prior success
+        const response = await strategy.fetch(TEST_API_URL);
+
+        expect(tokenRequestCount).to.equal(1); // Only fetched once
+        expect(apiRequestCount).to.equal(1); // API called once, no retry
+        expect(response.status).to.equal(401);
+      });
     });
 
     describe('getAuthorizationHeader', () => {

packages/b2c-tooling-sdk/test/clients/middleware.test.ts

@@ -91,6 +91,12 @@ describe('clients/middleware', () => {
         throw new Error('Expected middleware to return a Request');
       }
 
+      // Simulate a prior successful response so that hasHadSuccess is true
+      await middleware.onResponse!({
+        request: modifiedRequest,
+        response: new Response('OK', {status: 200}),
+      } as unknown as OnResponseParams);
+
       // Stub global fetch to return success on retry
       const originalFetch = globalThis.fetch;
       globalThis.fetch = sinon.stub().resolves(new Response('OK', {status: 200}));
@@ -146,6 +152,12 @@ describe('clients/middleware', () => {
         throw new Error('Expected middleware to return a Request');
       }
 
+      // Simulate a prior successful response so that hasHadSuccess is true
+      await middleware.onResponse!({
+        request: modifiedRequest,
+        response: new Response('OK', {status: 200}),
+      } as unknown as OnResponseParams);
+
       // Stub global fetch to also return 401
       const originalFetch = globalThis.fetch;
       globalThis.fetch = sinon.stub().resolves(new Response('Unauthorized', {status: 401}));
@@ -245,6 +257,12 @@ describe('clients/middleware', () => {
         throw new Error('Expected middleware to return a Request');
       }
 
+      // Simulate a prior successful response so that hasHadSuccess is true
+      await middleware.onResponse!({
+        request: modifiedRequest,
+        response: new Response('OK', {status: 200}),
+      } as unknown as OnResponseParams);
+
       // Stub global fetch to return success
       const originalFetch = globalThis.fetch;
       globalThis.fetch = sinon.stub().resolves(new Response('Created', {status: 201}));
@@ -269,6 +287,53 @@ describe('clients/middleware', () => {
       }
     });
 
+    it('does not retry on initial 401 when no prior successful response', async () => {
+      let invalidateCalled = false;
+
+      const auth: AuthStrategy = {
+        async fetch() {
+          throw new Error('not used in this unit test');
+        },
+        async getAuthorizationHeader() {
+          return 'Bearer test-token';
+        },
+        invalidateToken() {
+          invalidateCalled = true;
+        },
+      };
+
+      const middleware = createAuthMiddleware(auth);
+      type OnRequestParams = Parameters<NonNullable<typeof middleware.onRequest>>[0];
+      type OnResponseParams = Parameters<NonNullable<typeof middleware.onResponse>>[0];
+
+      const request = new Request('https://example.com/ping', {method: 'GET'});
+      const modifiedRequest = await middleware.onRequest!({request} as unknown as OnRequestParams);
+      if (!modifiedRequest) {
+        throw new Error('Expected middleware to return a Request');
+      }
+
+      // Stub global fetch (should not be called)
+      const originalFetch = globalThis.fetch;
+      const fetchStub = sinon.stub();
+      globalThis.fetch = fetchStub;
+
+      try {
+        // Send a 401 without any prior successful response
+        const unauthorizedResponse = new Response('Unauthorized', {status: 401});
+        const result = await middleware.onResponse!({
+          request: modifiedRequest,
+          response: unauthorizedResponse,
+        } as unknown as OnResponseParams);
+
+        // Should return original 401 without retrying
+        expect(result).to.equal(unauthorizedResponse);
+        expect(fetchStub.called).to.equal(false);
+        expect(invalidateCalled).to.equal(false);
+      } finally {
+        globalThis.fetch = originalFetch;
+      }
+    });
+
     it('passes through non-401 responses unchanged', async () => {
       const auth: AuthStrategy = {
         async fetch() {