Skip to content

Conversation

@zzooeeyy
Copy link
Contributor

@zzooeeyy zzooeeyy commented Nov 18, 2025

What

Adds support for expiring offline access tokens with refresh tokens.

Related change in shopify_app gem:

Why

Enables apps to use time-limited offline access tokens for improved security through automatic token rotation.

Changes

Core Features:

  • New config option: expiring_offline_access_tokens in Context setup
  • Extended Session class with refresh_token, refresh_token_expires, and expiration helpers (expired?, refresh_token_expired?)
  • ShopifyAPI::Auth::RefreshToken.refresh_access_token - Exchange refresh token for new access token
  • ShopifyAPI::Auth::TokenExchange.migrate_to_expiring_token - Migrate existing non-expiring tokens to expiring tokens (one-time, irreversible per shop)

OAuth Flows:

  • Authorization code grant and token exchange automatically request expiring tokens when config is enabled
  • HttpClient now serializes OAuth error responses (error, error_description) for better error messages
Error before Error after
old http-error

@zzooeeyy zzooeeyy force-pushed the zl/add_expiring_token_support branch from ecc5474 to 3f0c967 Compare November 18, 2025 23:16
@zzooeeyy zzooeeyy changed the title Add expiring offline access token support [Can't merge yet] Add expiring offline access token support Nov 19, 2025
@zzooeeyy zzooeeyy marked this pull request as ready for review November 24, 2025 21:46
@zzooeeyy zzooeeyy requested a review from a team as a code owner November 24, 2025 21:46
def serialized_error(response)
body = {}
body["errors"] = response.body["errors"] if response.body["errors"]
body["error"] = response.body["error"] if response.body["error"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Let me know what you think

Because the error and error_description fields are specific to oauth requests I wonder if we could move this logic to a file where it make that more explicit.

For example, in RefreshToken or TokenExchange:

  rescue ShopifyAPI::Errors::HttpResponseError => e
    # Parse OAuth-specific error from response.body
    oauth_error = e.response.body["error"]
    oauth_description = e.response.body["error_description"]
    # Re-raise with better message or handle specifically
  end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I slightly disagree on this since http_client is a generic class we provide in the library, it should be able to handle errors and be able to serialize it regardless of where it came from. Apps could use HTTPClient to implement their own OAuth strategy which they would now have to parse specific errors from the response themselves as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is a fair point. I don't feel too strongly either way so I am good to leave it as is. 👍

@zzooeeyy zzooeeyy force-pushed the zl/add_expiring_token_support branch 2 times, most recently from 7642665 to 2846034 Compare December 3, 2025 17:48
@zzooeeyy zzooeeyy force-pushed the zl/add_expiring_token_support branch from 2846034 to 447b66c Compare December 3, 2025 17:54
@zzooeeyy zzooeeyy requested a review from lizkenyon December 3, 2025 17:54
lizkenyon and others added 2 commits December 5, 2025 15:13
Prevents sessions from losing refresh token data during copy operations, which would break token refresh functionality.

Fixed merge conflict resolution: After merging main's refactored copy_attributes_from (direct assignment instead of JSON.parse), the new refresh_token fields were inadvertently omitted.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@lizkenyon lizkenyon merged commit 0d89f9b into main Dec 5, 2025
7 checks passed
@lizkenyon lizkenyon deleted the zl/add_expiring_token_support branch December 5, 2025 22:10
@zzooeeyy zzooeeyy changed the title [Can't merge yet] Add expiring offline access token support Add expiring offline access token support Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants