Skip to content

fix: handle missing expires_in in OAuth token response#3992

Open
ecthelion77 wants to merge 1 commit intoIBM:mainfrom
forterro:fix/oauth-no-expiry-fallback-upstream
Open

fix: handle missing expires_in in OAuth token response#3992
ecthelion77 wants to merge 1 commit intoIBM:mainfrom
forterro:fix/oauth-no-expiry-fallback-upstream

Conversation

@ecthelion77
Copy link
Copy Markdown
Contributor

🐛 Bug-fix PR

📌 Summary

When an OAuth2 provider omits the expires_in field from the token response, the gateway crashes with a TypeError during token storage. The expires_in parameter is RECOMMENDED but not REQUIRED per RFC 6749 Section 5.1, so providers may legitimately omit it (e.g., GitHub OAuth Apps, some Azure AD configurations).

Fixes #3989

🔁 Reproduction Steps

  1. Configure a gateway with OAuth2 pointing to a provider that does not return expires_in in the token response
  2. Trigger the OAuth authorization code flow
  3. Token exchange succeeds, but storing the token fails with TypeError: unsupported operand type(s) for +: 'datetime' and 'NoneType'

🐞 Root Cause

In oauth_manager.py, token_response.get("expires_in", self.settings.oauth_default_timeout) always returns a value, which hides the issue. However, when the provider returns expires_in: null or omits it entirely and the fallback default is also None, the code in token_storage_service.py tries to compute datetime.now() + timedelta(seconds=int(None)) which raises TypeError.

More importantly, the current code papers over the absence of expires_in by substituting oauth_default_timeout, which may not reflect the actual token lifetime — the token could be valid indefinitely.

💡 Fix Description

oauth_manager.py: Extract expires_in with .get() and explicitly convert to int only when present. Pass None when the provider does not specify expiration.

token_storage_service.py:

  • Change expires_in parameter type from int to Optional[int]
  • When expires_in is None, set expires_at = None and log an informational message
  • When expires_in is provided, compute expires_at as before

The existing _is_token_expired() method already handles expires_at=None correctly (returns False, treating the token as non-expired).

🧪 Verification

Check Command Status
Lint suite make lint ⚠️ Not run (no local dev env)
Unit tests make test ⚠️ Not run (no local dev env)
Coverage ≥ 80 % make coverage ⚠️ Not run (no local dev env)
Manual regression no longer fails Tested in production (Kubernetes) for 2+ weeks with Freshservice OAuth (which returns expires_in) and verified graceful handling when expires_in is absent

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted
  • No secrets/credentials committed
  • DCO Signed-off-by included

@jonpspri jonpspri added bug Something isn't working MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe labels Apr 9, 2026
@ecthelion77 ecthelion77 force-pushed the fix/oauth-no-expiry-fallback-upstream branch from e4c8753 to 7f4b8f3 Compare April 13, 2026 10:25
@ecthelion77
Copy link
Copy Markdown
Contributor Author

Suggested labels: python, security

@ecthelion77 ecthelion77 force-pushed the fix/oauth-no-expiry-fallback-upstream branch from 7f4b8f3 to a8afade Compare April 14, 2026 12:46
@ecthelion77 ecthelion77 force-pushed the fix/oauth-no-expiry-fallback-upstream branch 2 times, most recently from 4251229 to 7cecdb3 Compare April 14, 2026 15:35
Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com>
@ecthelion77 ecthelion77 force-pushed the fix/oauth-no-expiry-fallback-upstream branch from 7cecdb3 to ce0704b Compare April 14, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: OAuth token exchange fails when provider omits expires_in from token response

2 participants