[Shopify] Migrate Shopify Connector to Expiring Offline Access Tokens

[onbuyuka]

Background

As of December 2025, Shopify supports expiring offline access tokens with refresh-token rotation. Public apps created before 2026-04-01 must migrate to expiring tokens by 2027-01-01; after that date, REST/GraphQL Admin API requests made with non-expiring tokens are rejected. The Shopify Connector currently uses non-expiring offline tokens and must be migrated.

Token characteristics (Shopify):

• Access token lifetime: 1 hour (expires_in: 3600)
• Refresh token lifetime: 90 days (refresh_token_expires_in: 7776000)
• Migration is a one-time, irreversible token exchange per shop (the original non-expiring token is revoked on success)
• Only one refreshable expiring token per app + store

Refs: About offline access tokens - Migrating from non-expiring to expiring tokens

Approach

Token validity is centralized in a single orchestrator, EnsureValidAccessToken(Store), invoked from Shpfy Communication Mgt.GetAccessToken - so every API call (interactive and background) transparently migrates a legacy token or refreshes an expiring one before use. A scheduled backstop job additionally keeps idle shops access/refresh tokens alive and completes migrations for shops that never make an on-demand call.

Changes

Token storage - Shpfy Registered Store New (table 30138)

• New fields Token Expires At / Refresh Token Expires At; refresh token kept in IsolatedStorage.

Acquisition, refresh & migration - Shpfy Authentication Mgt. (codeunit 30199)

• New installs request expiring tokens (expiring=1).
• EnsureValidAccessToken orchestrates migrate-if-legacy / refresh-if-near-expiry, serialized per shop.
• RefreshAccessToken (grant_type=refresh_token) with transient retry and a terminal reconnect error.
• MigrateToExpiringToken (one-time token exchange, best-effort - keeps the working token on failure).
• ForceTokenRefresh, IsRefreshTokenExpired, and a Shop Card reconnect notification.

Consumption - Shpfy Communication Mgt.

• GetAccessToken invokes the orchestrator; reactive 401 -> refresh -> retry in ExecuteWebRequest.

Scheduled backstop - Shpfy Token Refresh (30431) + Shpfy Token Refresh Shop (30432)

• Dispatcher (TableNo = Job Queue Entry) iterates enabled shops and runs the per-shop worker (TableNo = Shpfy Shop) via Codeunit.Run for isolation; recurring Job Queue entry registered via installer + upgrade.

UX - Shpfy Shop Card

• Reconnect notification when the refresh token has expired.

Docs & tests

• Shpfy Token Refresh Test unit tests; living docs refreshed (business-logic, data-model, patterns, CLAUDE.md).

Telemetry

Emitted for migration success/failure, refresh success/transient/terminal and backstop per-shop failures, under event IDs 0000UIV-0000UJ1.

Test plan

• Shopify App project compiles clean.
• Shpfy Token Refresh Test unit tests for the expiry decisions.
• Install / refresh / migration / backstop / notification scenarios validated locally.

Fixes AB#637954

[github-actions]

Copilot PR Review

Iteration 11 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Error Handling	1	1	0	1	0
Performance	1	1	0	0	0

Totals: 2 knowledge-backed · 0 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

• layer-disabled (knowledge) : 2 file(s)

_{Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.}

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

SaveTokenResponse (new in this PR) stores the Shopify offline access token via RegisteredStoreNew.SetAccessToken(), which persists it with IsolatedStorage.Set (unencrypted at rest), while the refresh token introduced by this same PR is correctly stored via SetRefreshToken() using IsolatedStorage.SetEncrypted.

Both are OAuth credentials for the same store and belong in the same encrypted class. This inconsistency becomes load-bearing now: every store going forward will have an encrypted refresh token sitting next to an unencrypted, equally sensitive access token in the same record. Change SetAccessToken (Shpfy Registered Store New table) to use SetEncrypted for parity with the refresh token.

Knowledge:

• microsoft/knowledge/security/isolatedstorage-setencrypted-for-sensitive-values.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

The new SetRefreshToken procedure correctly uses IsolatedStorage.SetEncrypted for the 90-day refresh token, but the pre-existing SetAccessToken (line 68 of the same table) still uses the unencrypted IsolatedStorage.Set for the access token, sitting right next to the new encrypted call.

Both values are Shopify credentials of comparable sensitivity per the referenced guidance ('anything that would harm the tenant if leaked — API keys, tokens, connection strings, OAuth client secrets — uses SetEncrypted'). Since this PR is already touching this table's secret-handling code, it's a good opportunity to align SetAccessToken with SetEncrypted for consistency.

Knowledge:

• microsoft/knowledge/security/isolatedstorage-setencrypted-for-sensitive-values.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 7}}$

In ShpfyCommunicationMgt, after HandleUnauthorizedResponse forces a token refresh on a 401, the code Clears HttpResponseMessage and re-sends the request but discards the boolean return value of the retry's HttpClient.Send(HttpRequestMessage, HttpResponseMessage) call - unlike the original Send() a few lines above, whose return value is checked.

If the retry Send fails to obtain a response (e.g. a transient network failure right after the 401), HttpResponseMessage is left as the just-Cleared, empty default instance. The subsequent while-loop condition (IsBlockedByEnvironment / EvaluateResponse, both driven by HttpStatusCode()) then evaluates a status code of 0, which matches none of EvaluateResponse's retry branches (429, 500-599), so the loop silently exits and the caller receives a blank/default response instead of a clear signal that the retried request never completed.

Recommendation:

• check the retry Send()'s return value the same way the first Send() is checked, and surface/handle the false case explicitly rather than falling through to evaluate an empty response.

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[onbuyuka]

Addressed the latest review findings:

• Encrypt access token at rest (Security, iters 2 & 4): SetAccessToken on Shpfy Registered Store New now uses IsolatedStorage.SetEncrypted, matching the refresh token. Existing unencrypted values remain readable via IsolatedStorage.Get; future writes are encrypted. (14e8487... / access-token commit)
• Retry Send() result after 401 (Agent, iter 7): ExecuteWebRequest now tracks the send result and gates the retry loop on it, so a transient failure on the post-refresh re-send exits cleanly instead of evaluating an empty (status 0) response. Also hardens the pre-existing retry-loop resend.

The cross-company-same-store serialization finding remains intentionally open for reviewer input (a tenant-wide token store is a larger change than this migration slice).

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling} \quad \color{gray}{\texttt{\small Iteration\ 11}}$

RefreshAccessToken raises a plain Error(RefreshTokenExpiredErr, Store) whose message explicitly names the fix ('Open the Shopify Shop card and reconnect the store to continue') but offers no way to get there.

The PR already built the pieces for a Show-it/Fix-it action for this exact scenario (Shpfy Authentication Mgt.ReconnectFromNotification and the Shop Card notification action), so when this error surfaces in an interactive context it dead-ends the user instead of using ErrorInfo with AddNavigationAction (Page::"Shpfy Shop Card") or AddAction pointing at the existing reconnect handler. Wrapping the three Error(RefreshTokenExpiredErr, Store) call sites in an ErrorInfo with a navigation/fix-it action would make the error self-service instead of a dead end.

Knowledge:

• microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}