feat: Add OAuth 2.0 Client Credentials support for Shopify integration - #438
feat: Add OAuth 2.0 Client Credentials support for Shopify integration#438RohnRoy wants to merge 2 commits into
Conversation
Confidence Score: 5/5
Reviews (8): Last reviewed commit: "Merge branch 'develop' into feat/shopify..." | Re-trigger Greptile |
| def _validate_oauth_credentials_if_needed(self): | ||
| """Validate OAuth credentials by generating a test token when credentials change.""" | ||
| if not self.is_enabled(): | ||
| return | ||
|
|
||
| if self.authentication_method != "OAuth 2.0 Client Credentials": | ||
| return | ||
|
|
||
| if self.has_value_changed("client_id") or self.has_value_changed("client_secret"): | ||
| # self.client_secret holds plaintext during validate (before encrypted store write) | ||
| client_secret = self.client_secret or self._get_password_safe("client_secret") | ||
| if not client_secret: | ||
| return # Will be caught by _validate_authentication_fields | ||
|
|
||
| try: | ||
| validate_oauth_credentials( | ||
| self.shopify_url, | ||
| self.client_id, | ||
| client_secret, | ||
| ) | ||
| frappe.msgprint( | ||
| _("OAuth credentials validated successfully. Token will be auto-generated on save."), | ||
| indicator="green", | ||
| alert=True, | ||
| ) | ||
| except Exception: | ||
| raise |
There was a problem hiding this comment.
Synchronous HTTP calls inside
validate() and before_save()
_validate_oauth_credentials_if_needed() (called from validate()) and before_save() both make live HTTP requests to Shopify's token endpoint synchronously. This is against Frappe's lifecycle contract — a network timeout or Shopify outage will block or fail every document save, and Frappe does not expect validate to perform external I/O. Per the project's own custom rules, external API calls in validate/before_save should move to after_insert/on_update or be dispatched via frappe.enqueue().
Additionally, when credentials change, both _validate_oauth_credentials_if_needed (which calls generate_oauth_token and discards the result) and _handle_webhooks (which calls _get_or_generate_oauth_token and stores the result) both fire in the same validate() call — generating and discarding a token unnecessarily before storing a second one.
Context Used: This is a Frappe Framework application (Python bac... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: ecommerce_integrations/shopify/doctype/shopify_setting/shopify_setting.py
Line: 102-128
Comment:
**Synchronous HTTP calls inside `validate()` and `before_save()`**
`_validate_oauth_credentials_if_needed()` (called from `validate()`) and `before_save()` both make live HTTP requests to Shopify's token endpoint synchronously. This is against Frappe's lifecycle contract — a network timeout or Shopify outage will block or fail every document save, and Frappe does not expect `validate` to perform external I/O. Per the project's own custom rules, external API calls in `validate`/`before_save` should move to `after_insert`/`on_update` or be dispatched via `frappe.enqueue()`.
Additionally, when credentials change, both `_validate_oauth_credentials_if_needed` (which calls `generate_oauth_token` and discards the result) and `_handle_webhooks` (which calls `_get_or_generate_oauth_token` and stores the result) both fire in the same `validate()` call — generating and discarding a token unnecessarily before storing a second one.
**Context Used:** This is a Frappe Framework application (Python bac... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.6b1c5f5 to
6fef948
Compare
9b934dd to
df12e1b
Compare
b01f860 to
65f2af9
Compare
Shopify dev-dashboard apps (post Jan 2026) issue short-lived tokens via Client Credentials Grant instead of static access tokens. This adds dual-mode auth: existing installs keep working (Static Token), new apps use OAuth 2.0 with auto-refresh every 24h. - oauth.py: token generation, encrypted storage, auto-refresh - connection.py: session decorator + HMAC webhook validation for both modes - shopify_setting.py: validate/before_save handle in-memory password field - shopify_setting.json: new fields (auth method, client_id, client_secret, token store, expiry); password/shared_secret conditional on Static Token - patches/set_default_shopify_auth_method.py: backward-compat migration
65f2af9 to
9b52fad
Compare
|
@ankush , could you please review the changes related to this issue: frappe/frappe#38111 If everything looks good, please merge this PR after your review. Thank you! |
feat(shopify): Add OAuth 2.0 Client Credentials auth for Dev Dashboard apps
Summary
Since January 1, 2026, Shopify no longer allows creating Custom Apps
directly in the Shopify Admin. All new apps must be created through the
Shopify Dev Dashboard (
shopify.dev), which issues aClient ID+Client Secretinstead of a permanent access token.This PR adds dual authentication to
ecommerce_integrations, so both oldand new Shopify apps continue to work without any breaking changes.
Supersedes the unmerged PR #399 by @Z4nzu which had the right approach but
was never reviewed.
Related forum thread: https://discuss.frappe.io/t/issues-with-integrating-shopify-post-jan-2026-updates/161201
What changed in Shopify
The new token exchange is a standard Client Credentials Grant:
Reference: https://shopify.dev/docs/apps/build/authentication-authorization/access-tokens/client-credentials-grant
Changes
New file:
shopify/oauth.pyCore of the PR. Public functions:
get_valid_access_token(setting)— main entry point called byconnection.py;returns a valid token for the current session, auto-refreshing when the token
is missing or within 5 minutes of its 24-hour expiry.
refresh_oauth_token(setting, client_secret=None)— exchanges Client ID +Client Secret for a new token via HTTP POST, stores the token encrypted via
set_encrypted_password, and persiststoken_expires_at. The optionalclient_secretparameter allows callers to supply the plaintext secret duringvalidate/before_save(before Frappe writes the encrypted value to theauth store).
validate_oauth_credentials(shopify_url, client_id, client_secret)— calledduring
validate()to give immediate feedback if credentials are wrong.generate_oauth_token(shopify_url, client_id, client_secret)— raw HTTP call;raises
frappe.ValidationErrorwith a clean error message on failure, neverlogs the client secret.
is_token_valid(token_expires_at, buffer_minutes=5)— returnsFalsewhenthe token is missing or within the 5-minute refresh buffer.
Modified:
shopify/connection.pytemp_shopify_sessiondecorator now calls the new_get_access_token(setting)helper, which branches on
authentication_method: OAuth mode callsget_valid_access_token; Static Token mode readssetting.get_password("password")as before.
_validate_request(webhook HMAC check) usessettings.get_password("client_secret")as the signing key in OAuth mode and
settings.shared_secretin Static Token mode._handle_webhooksinshopify_setting.pypasses the correct token toregister_webhooks/unregister_webhooksbased on auth mode, including asafe fallback when the token is empty (prevents a crash on disable).
The Static Token code path is 100% unchanged. Existing installations are
not affected.
Modified:
shopify/doctype/shopify_setting/shopify_setting.pyNew methods:
_get_password_safe(fieldname)— wrapsget_passwordin a try/except;returns
""instead of raising when the encrypted value doesn't exist yet._validate_authentication_fields()— checks that the fields required by theselected auth mode are present; raises
frappe.throwwith a clear message._validate_oauth_credentials_if_needed()— callsvalidate_oauth_credentialswhen
client_idorclient_secretchanges, giving users immediate feedback.before_save()— pre-generates and caches an OAuth token on save so thefirst sync doesn't incur an extra round-trip.
_get_or_generate_oauth_token()— returns the cached token if still valid,otherwise calls
refresh_oauth_token; passes the in-memoryself.client_secretduring the save flow to avoid a Frappe password-store race condition.
validate()calls_set_default_authentication_method()to ensure the fieldis always populated before other validation runs.
_handle_webhooksguards against callingunregister_webhookswith an emptytoken (previously crashed when disabling with no password set).
Modified:
shopify/doctype/shopify_setting/shopify_setting.jsonFive new fields:
authentication_method"Static Token"or"OAuth 2.0 Client Credentials"; default"Static Token"client_idmandatory_depends_onenforcedclient_secretmandatory_depends_onenforcedoauth_access_tokenhidden=1,read_only=1token_expires_athidden=1,read_only=1Existing
passwordandshared_secretfields gaindepends_onso they onlyshow in Static Token mode, keeping the UI uncluttered for new installs.
New file:
patches/set_default_shopify_auth_method.pyMigration patch — sets
authentication_method = "Static Token"on anyexisting Shopify Setting record so there is zero behaviour change for existing
installs. Registered in
patches.txt.Backward compatibility
Existing
Static Tokensetups: no change required, no action neededThe new
authentication_methodfield defaults to"Static Token". Themigration patch ensures existing records get this value on
bench migrate.Every code path for Static Token delegates to the original implementation.
Testing
bench run-tests --app ecommerce_integrations \ --module ecommerce_integrations.shopify.tests.test_shopify_oauthTests cover (
shopify/tests/test_shopify_oauth.py):is_token_valid— missing / valid / within-buffer / expiredget_oauth_token_endpoint— plain domain,https://strip,http://strip, trailing slash stripgenerate_oauth_token— success, HTTP 401 raisesValidationError, client_secret never loggedget_valid_access_token— cached token (no HTTP), expired triggers refresh, missing triggers fetch_validate_requestHMAC — OAuth usesclient_secret, Static Token usesshared_secret, wrong secret failsManual test checklist
Static Token (existing setup)
passwordandshared_secretfields visibleclient_id,client_secret, OAuth token fields hiddenOAuth 2.0 (new Dev Dashboard app)
client_id,client_secretfields appear;password,shared_secrethiddenoauth_access_tokenandtoken_expires_atpopulate automaticallytoken_expires_at, next sync auto-refreshes the tokenclient_secret)Checklist
bench migratetested locallydevelopbranchshopify/tests/test_shopify_oauth.py, 13 tests)