Conversation
The @saleor/sdk package (v0.6.0) is deprecated and no longer maintained, blocking independent dependency upgrades. This commit copies the SDK source into src/legacy-sdk/ and updates all imports from @saleor/sdk to @dashboard/legacy-sdk. Changes: - Copy SDK source (auth, apollo, react modules) into src/legacy-sdk/ - Update 17 files to use @dashboard/legacy-sdk imports - Remove @saleor/sdk from package.json and pnpm-workspace.yaml - Remove cross-fetch import (use native browser fetch) - Fix type errors: PropsWithChildren for React 18, union type narrowing - Fix lint errors: empty catch blocks - Exclude legacy-sdk from codegen document scanning Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover all core business logic that was previously untested after inlining @saleor/sdk into src/legacy-sdk: - core/storage: token persistence, localStorage sync, autologin init - core/auth: all 12 AuthSDK methods (login, logout, token refresh, external auth flows, password management) - core/user: all 10 UserSDK methods (account CRUD, address management, email change, account deletion) - core/helpers: permission validation - helpers: internal token detection - apollo/client: createFetch auth header injection, auto token refresh, expired signature retry logic - react/SaleorProvider: context propagation, client updates - react/hookFactory: hook creation, error on missing provider Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| const setRefreshToken = (token: string | null): void => { | ||
| if (token) { | ||
| localStorage.setItem(SALEOR_REFRESH_TOKEN, token); |
Check failure
Code scanning / CodeQL
Clear text storage of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the problem is that setRefreshToken writes the refresh token directly to localStorage. To fix it without changing the external behavior of the SDK, we will transparently encrypt the refresh token when writing to localStorage and decrypt it when reading it back at initialization. The storage API (setRefreshToken, getRefreshToken, setTokens, clear) will remain unchanged.
Concretely, in src/legacy-sdk/core/storage.ts:
-
Introduce small helper functions
encryptTokenanddecryptTokenthat:- Are
asyncand use the standard browsercrypto.subtleWeb Crypto API (available in modern browsers, which is the typical environment forlocalStorageusage). - Derive an encryption key from a static passphrase using PBKDF2 (or similar) and encrypt/decrypt using AES‑GCM.
- Fall back gracefully (returning the original value) if
window.crypto/crypto.subtleis not available, so behavior does not completely break in legacy environments.
- Are
-
Update
createStorage:- When initializing
refreshTokenfromlocalStorage, call an asyncdecryptStoredRefreshTokenhelper that reads the ciphertext fromSALEOR_REFRESH_TOKENand decrypts it into the in‑memoryrefreshTokenvariable. - Keep
accessTokenpurely in memory as today.
- When initializing
-
Modify
setRefreshToken:- If
tokenis non‑null:- Encrypt it with
encryptTokenand store the ciphertext (e.g., base64‑encoded) inlocalStorage.setItem(SALEOR_REFRESH_TOKEN, encryptedToken).
- Encrypt it with
- Otherwise, remove the key from
localStorageas before. - Always update the in‑memory
refreshTokenvariable with the plain token (so the rest of the code usingstorage.getRefreshToken()behaves exactly as before).
- If
-
Because
createStoragecurrently returnsvoid, but we need to perform async decryption, we will:- Add internal helpers and kick off decryption without changing the signature, by:
- Initializing
refreshTokentonull. - After defining
setRefreshToken, call a self‑invoking async function that, ifautologinEnabledandLOCAL_STORAGE_EXISTS, reads fromlocalStorageand decrypts intorefreshToken. This preserves the synchronous signature ofcreateStoragewhile eventually populatingrefreshToken.
- Initializing
- Add internal helpers and kick off decryption without changing the signature, by:
No new third‑party libraries are required; we only use the standard Web Crypto API.
Differences Found✅ No packages or licenses were added. SummaryExpand
|
There was a problem hiding this comment.
Pull request overview
This PR replaces remaining usages of @saleor/sdk with the in-repo @dashboard/legacy-sdk implementation and adds extensive unit test coverage for the inlined legacy SDK modules (core auth/user/storage, Apollo fetch/client behavior, and basic React integration).
Changes:
- Switched multiple imports from
@saleor/sdk/@saleor/sdk/dist/apollo/typesto@dashboard/legacy-sdk. - Added the in-repo legacy SDK implementation (
src/legacy-sdk/**) including core, Apollo, and React wrapper APIs. - Added Jest test suites for legacy SDK core logic and Apollo fetch/client behavior; removed
@saleor/sdkfrom dependencies and workspace config.
Reviewed changes
Copilot reviewed 55 out of 58 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/handlers/metadataUpdateHelpers.ts | Switch MetadataInput import to legacy SDK types. |
| src/utils/errors/account.ts | Switch SDK types import to legacy SDK types. |
| src/utils/credentialsManagement.ts | Switch UserDetailsFragment import to legacy SDK types. |
| src/shipping/views/RateUpdate.tsx | Switch ShippingMethodPostalCodeRule type import to legacy SDK. |
| src/products/views/ProductList/export.test.ts | Switch ExportInfoInput type import to legacy SDK types. |
| src/products/fixtures.ts | Switch ProductType type import to legacy SDK types. |
| src/legacy-sdk/react/index.ts | Add legacy SDK React barrel exports. |
| src/legacy-sdk/react/hooks/user.ts | Add useUser hook via hookFactory. |
| src/legacy-sdk/react/hooks/saleorConfig.ts | Add useSaleorConfig hook via hookFactory. |
| src/legacy-sdk/react/hooks/index.ts | Add hooks barrel exports. |
| src/legacy-sdk/react/hooks/auth.ts | Add useAuth and useAuthState hooks. |
| src/legacy-sdk/react/helpers/hookStateFactory.ts | Add Apollo cache-only state hook factory. |
| src/legacy-sdk/react/helpers/hookFactory.ts | Add context-based hook factory for SDK methods. |
| src/legacy-sdk/react/helpers/hookFactory.test.tsx | Add tests for hookFactory behavior and provider requirement. |
| src/legacy-sdk/react/components/index.ts | Add components barrel exports. |
| src/legacy-sdk/react/components/SaleorProvider.tsx | Add SaleorProvider + SaleorContext implementation. |
| src/legacy-sdk/react/components/SaleorProvider.test.tsx | Add tests for SaleorProvider context behavior. |
| src/legacy-sdk/index.ts | Add top-level legacy SDK barrel exports. |
| src/legacy-sdk/helpers.ts | Add helper for internal token detection. |
| src/legacy-sdk/helpers.test.ts | Add tests for internal token helper. |
| src/legacy-sdk/core/user.ts | Add legacy user SDK methods implementation. |
| src/legacy-sdk/core/user.test.ts | Add tests for user SDK mutations wiring + logout side-effect. |
| src/legacy-sdk/core/types.ts | Add legacy SDK public/core types and method result types. |
| src/legacy-sdk/core/storage.ts | Add token/auth plugin storage implementation. |
| src/legacy-sdk/core/storage.test.ts | Add tests for storage persistence and lifecycle behaviors. |
| src/legacy-sdk/core/state.ts | Add helper to read auth state from Apollo cache. |
| src/legacy-sdk/core/index.ts | Add core barrel exports (createFetch/createSaleorClient/types). |
| src/legacy-sdk/core/helpers.ts | Add permission helper used by auth flows. |
| src/legacy-sdk/core/helpers.test.ts | Add tests for permission helper behavior. |
| src/legacy-sdk/core/createSaleorClient.ts | Add Saleor client factory (Apollo + core SDK wiring + autologin). |
| src/legacy-sdk/core/constants.ts | Add legacy SDK storage key constants. |
| src/legacy-sdk/core/auth.ts | Add legacy auth SDK methods implementation. |
| src/legacy-sdk/core/auth.test.ts | Add tests for auth SDK behaviors and cache/storage side effects. |
| src/legacy-sdk/constants.ts | Add environment capability constants (window/localStorage/dev mode). |
| src/legacy-sdk/config.ts | Add legacy SDK test/config constants. |
| src/legacy-sdk/apollo/queries.ts | Add legacy SDK USER queries. |
| src/legacy-sdk/apollo/mutations.ts | Add legacy SDK auth/user mutations. |
| src/legacy-sdk/apollo/index.ts | Add apollo barrel export. |
| src/legacy-sdk/apollo/fragments.ts | Add legacy SDK GraphQL fragments. |
| src/legacy-sdk/apollo/client.ts | Add Apollo client + fetch wrapper with auth/refresh logic. |
| src/legacy-sdk/apollo/client.test.ts | Add tests for createFetch behaviors (auth header, refresh, retries). |
| src/index.tsx | Switch app’s SaleorProvider import from @saleor/sdk to legacy SDK. |
| src/graphql/client.ts | Switch createFetch/createSaleorClient import to legacy SDK. |
| src/components/DevModePanel/utils.ts | Switch createFetch import to legacy SDK. |
| src/components/DevModePanel/utils.test.ts | Update mocks/imports to legacy SDK createFetch. |
| src/components/AppLayout/AppChannelContext.tsx | Switch useSaleorConfig import to legacy SDK hook. |
| src/components/AddressEdit/useAddressValidation.ts | Switch ChoiceValue type import to legacy SDK types. |
| src/auth/views/NewPassword.tsx | Switch useAuth hook import to legacy SDK. |
| src/auth/types.ts | Switch auth-related type exports import to legacy SDK. |
| src/auth/hooks/useAuthProvider.ts | Switch auth hooks/types import to legacy SDK. |
| src/auth/hooks/useAuthProvider.test.ts | Update mocks/imports to legacy SDK auth hooks. |
| src/auth/AuthProvider.test.tsx | Update mocks/imports to legacy SDK auth hooks. |
| pnpm-workspace.yaml | Remove @saleor/sdk from minimumReleaseAgeExclude list. |
| pnpm-lock.yaml | Remove @saleor/sdk dependency lock entries. |
| package.json | Remove @saleor/sdk dependency. |
| codegen-main.ts | Update CodegenConfig import to type-only; exclude legacy SDK GraphQL operations from codegen inputs. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| try { | ||
| if (refreshPromise) { | ||
| await refreshPromise; | ||
| } else if (Date.now() >= expirationTime) { | ||
| if (isInternalToken(owner)) { |
There was a problem hiding this comment.
In the auto-refresh path, refreshPromise is never set when starting a refresh. That means concurrent requests with an expired token can trigger multiple refresh mutations in parallel, defeating the deduplication logic used later for refreshOnUnauthorized. Consider assigning refreshPromise = authClient.refreshToken()/refreshExternalToken() before awaiting, and then awaiting the shared promise.
| const setRefreshToken = (token: string | null): void => { | ||
| if (token) { | ||
| localStorage.setItem(SALEOR_REFRESH_TOKEN, token); | ||
| } else { | ||
| localStorage.removeItem(SALEOR_REFRESH_TOKEN); | ||
| } |
There was a problem hiding this comment.
setRefreshToken writes to localStorage unconditionally. If createStorage() is used in a non-browser environment (where LOCAL_STORAGE_EXISTS is false), calling storage.setRefreshToken(...) / storage.clear() will throw. Guard the localStorage reads/writes in setRefreshToken the same way as setAuthPluginId.
| is_staff: boolean; | ||
| }; | ||
|
|
||
| // Meethods opts |
There was a problem hiding this comment.
Typo in comment: “Meethods opts” → “Methods opts”.
| export type UpdateAccountAddressOpts = MutationAccountAddressUpdateArgs; | ||
| export type ConfirmAccountOpts = AccountConfirmMutationVariables; | ||
|
|
||
| // Meethods results |
There was a problem hiding this comment.
Typo in comment: “Meethods results” → “Methods results”.
| /** | ||
| * Refresh JWT token. Mutation will try to take refreshToken from the function's arguments. | ||
| * If it fails, it will try to use refreshToken from the http-only cookie called refreshToken. | ||
| * |
There was a problem hiding this comment.
The JSDoc for refreshToken mentions taking refreshToken from function arguments / an http-only cookie, but the method signature only accepts includeUser and always reads the token from storage. Please update the doc comment to match the actual behavior to avoid misleading SDK consumers.
| * Change the password of the logged in user. | ||
| * | ||
| * @param opts - Object with password and new password. | ||
| * @returns Errors if the passoword change has failed. |
There was a problem hiding this comment.
Typo in JSDoc: “passoword” → “password”.
| export const createSaleorClient = ({ | ||
| apiUrl, | ||
| channel, | ||
| opts = {}, | ||
| }: SaleorClientOpts): SaleorClient => { | ||
| let _channel = channel; | ||
| const { autologin = true, fetchOpts } = opts; | ||
|
|
||
| const setChannel = (channel: string): string => { | ||
| _channel = channel; | ||
|
|
||
| return _channel; | ||
| }; | ||
|
|
||
| createStorage(autologin); | ||
|
|
||
| const apolloClient = createApolloClient(apiUrl, autologin, fetchOpts); | ||
| const coreInternals = { apolloClient, channel: _channel }; | ||
| const authSDK = auth(coreInternals); | ||
| const userSDK = user(coreInternals); | ||
|
|
||
| const refreshToken = storage.getRefreshToken(); | ||
|
|
||
| if (autologin && refreshToken) { | ||
| const owner = jwtDecode<JWTToken>(refreshToken).owner; | ||
|
|
||
| if (isInternalToken(owner)) { | ||
| authSDK.refreshToken(true); | ||
| } else { | ||
| authSDK.refreshExternalToken(true); | ||
| } | ||
| } |
There was a problem hiding this comment.
createSaleorClient contains important behavioral logic (channel switching + autologin refresh) but currently has no dedicated tests. Adding tests that verify config.setChannel(...) affects subsequent auth/user calls (and that autologin triggers the correct refresh path) would prevent regressions here.
| const apolloClient = createApolloClient(apiUrl, autologin, fetchOpts); | ||
| const coreInternals = { apolloClient, channel: _channel }; | ||
| const authSDK = auth(coreInternals); | ||
| const userSDK = user(coreInternals); |
There was a problem hiding this comment.
setChannel only mutates the local _channel variable, but coreInternals.channel is created from the initial value and never updates. As a result, auth()/user() closures (and config.channel) will keep using the initial channel, so later setChannel(...) calls won’t affect subsequent mutations. Consider storing channel in a mutable object/ref shared with the SDK methods, or making channel a getter that reads the latest value, and ensure config.channel stays in sync.
lkostrowski
changed the title
@saleor/sdk into dashboard
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6405 +/- ##
==========================================
+ Coverage 43.13% 43.46% +0.32%
==========================================
Files 2524 2548 +24
Lines 44009 44386 +377
Branches 10011 10479 +468
==========================================
+ Hits 18983 19291 +308
- Misses 24985 25053 +68
- Partials 41 42 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Scope of the change
Adds comprehensive test coverage for the legacy-sdk modules that were inlined from @saleor/sdk in the previous commit. These tests cover all core business logic: token storage and persistence, authentication flows (internal and external), user account management, and React integration.
Test coverage includes:
Storage: localStorage persistence, autologin initialization, token lifecycle
Auth: All 12 methods covering internal/external login, logout, token refresh, password management
User: All 10 methods for account CRUD, address management, email changes, account deletion
Apollo: Auth header injection, auto token refresh on expiration, expired signature retry logic
React: Context provider, hook factories, error handling when provider is missing
I confirm I added ripples for changes (see src/ripples) or my feature doesn't contain any user-facing changes
I used analytics "trackEvent" for important events