Skip to content

Conversation

@dionlow
Copy link
Contributor

@dionlow dionlow commented Jul 24, 2025

Summary

This PR adds the ability for the builder to give a callback getToken to get a JWT. The API service will detect either an API key or a getToken option and use if only one is provided.

  • adds jose library to handle jwt token expiration / decoding
  • adds jwt token provider to ampersand context
  • refactor api service to use jwt token provider
  • refactors api service to use API key and JWT headers.
  • ENABLE_JWT_AUTH_FF hard coded feature flag blocks usage of JWT for time being though the param is available in AmpersandProvider

Supported

  • InstallationProvider (supported)
  • ConnectProvider (supported)
  • InstallIntegration (not supported yet)

Tested

test API key workflow for ConnectProvider

test jwt flow

  1. Turn on feature flag ENABLE_JWT_AUTH_FF=true
  2. In react run yarn build
  3. In mailmonkey add a getToken callback
const getToken = async (consumerRef: string, groupRef: string) => {
  return await Promise.resolve("your-jwt-token");
}

// add getToken to AmpersandProvider 
 <AmpersandProvider options={{
  // apiKey: defaultApiKey, // comment out apiKey; cannot have both
     projectId: defaultProjectId,
    getToken: getToken
}}>
  1. Set up preview environment (source vars.sh & ./run_mailmonkey.sh )
  2. Generate JWT token using go run generateJWT.go in feat/jwt-integration
  3. Copy in JWT token to mailmonkey
  4. test localhost:3000/connect (ConnectProvider)

JWT Logic

  1. getToken gets token from builder's backend
  2. amp-labs/react api service will look in local cache (react) and session storage(browser) for a cached jwt token or call getToken to use and set in cache/session storage.
  3. expired JWT will be removed from storage upon mount and when getting a cached token that has expired
  4. cached jwt will be loaded into JWT provider to be used in the API service
Session Storage vs Local Storage

Session Storage: same session / same tab - auto clear when tab closes
Local Storage: works across tabs, need manual deletion

Session storage is marginally safer as the need to clear cache is a little less important considering that user will likely close tabs. Session storage will still allow the app to work when user refreshes the page.

Not included

To be most secure, we'll want to implement a http cookie, but 10 min jwt tokens will stil minimize access.

README pr

#1220

@dionlow dionlow changed the title feat: add jwt provider session storage part 1 [ENG-2416] feat: add jwt provider session storage part 1 Jul 25, 2025
@dionlow dionlow force-pushed the dl-jwt-provider-session-storage branch from ba9da51 to 03d13cb Compare July 30, 2025 20:56
Dion Low added 2 commits July 30, 2025 13:57
- adds jose library to handle jwt token expiration / decoding
- adds jwt token provider to ampersand context
- refactor api service to use jwt token provider
add installation provider to connect provider

add feature flag for jwt auth
@dionlow dionlow force-pushed the dl-jwt-provider-session-storage branch from 03d13cb to 9e57468 Compare July 30, 2025 20:57
Dion Low added 2 commits July 31, 2025 12:30
- sessionStorage is now used to cache the JWT token
- default token expiration time is 10 minutes
- jwt token is now cached in sessionStorage
- jwt token is now cached in the in-memory cache
- jwt token is now cached in the sessionStorage
- jwt token is now cached in the in-memory cache
@dionlow dionlow marked this pull request as ready for review July 31, 2025 23:26
Copilot AI review requested due to automatic review settings July 31, 2025 23:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds JWT token authentication as an alternative to API key authentication for the Ampersand provider. It implements JWT token caching with session storage, expiration handling, and integrates JWT authentication into the API service layer.

  • Adds JWT token provider with caching and session storage
  • Refactors API service to support both API key and JWT authentication
  • Updates AmpersandProvider to accept either apiKey or getToken callback

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/services/api.ts Refactors API service to support both API key and JWT authentication with helper functions
src/context/JwtTokenContextProvider.tsx Implements JWT token provider with caching, session storage, and expiration handling
src/context/ApiKeyContextProvider.tsx Removes strict error requirement for API key context
src/context/AmpersandContextProvider/AmpersandContextProvider.tsx Updates provider to accept getToken callback and enforce mutual exclusivity with apiKey
src/components/Connect/ConnectProvider.tsx Wraps ConnectionsProvider with InstallationProvider for JWT auth support
package.json Adds jose library dependency for JWT handling

Comment on lines +45 to +46
const decoded = await jwtVerify(token, new Uint8Array(0), {
algorithms: [], // Skip signature verification
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

Using an empty Uint8Array and empty algorithms array completely bypasses JWT signature verification, which is a serious security vulnerability. This allows any malformed or malicious JWT to be accepted as valid.

Suggested change
const decoded = await jwtVerify(token, new Uint8Array(0), {
algorithms: [], // Skip signature verification
const decoded = await jwtVerify(token, "your-verification-key", {
algorithms: ["HS256", "RS256"], // Specify allowed algorithms

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +48
const decoded = await jwtVerify(token, new Uint8Array(0), {
algorithms: [], // Skip signature verification
});
const payload = decoded.payload;
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

Skipping signature verification defeats the purpose of JWT security. Consider using jose's decodeJwt() function instead if you only need to read the payload without verification, or implement proper signature verification.

Suggested change
const decoded = await jwtVerify(token, new Uint8Array(0), {
algorithms: [], // Skip signature verification
});
const payload = decoded.payload;
const payload = decodeJwt(token);

Copilot uses AI. Check for mistakes.
});

// TODO: remove this flag when we have a proper JWT auth flow
const ENABLE_JWT_AUTH_FF = false;
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

Hard-coded feature flag should be configurable through environment variables or build-time configuration rather than being embedded in the source code.

Suggested change
const ENABLE_JWT_AUTH_FF = false;
const ENABLE_JWT_AUTH_FF = process.env.ENABLE_JWT_AUTH_FF === "true";

Copilot uses AI. Check for mistakes.
if (cached && cached.expiresAt > now) {
return cached.token;
} else {
tokenCache.delete(cacheKey);
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

The tokenCache.delete() operation inside the else block modifies the state directly rather than using setTokenCache, which could lead to inconsistent state and React rendering issues.

Suggested change
tokenCache.delete(cacheKey);
setTokenCache((prev) => {
const newCache = new Map(prev);
newCache.delete(cacheKey);
return newCache;
});

Copilot uses AI. Check for mistakes.
});

// TODO: remove this flag when we have a proper JWT auth flow
const ENABLE_JWT_AUTH_FF = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

turn this to true to test; we can also inject a env flag here later.

@dionlow dionlow requested a review from eberle1080 July 31, 2025 23:29
@dionlow dionlow merged commit 86359b0 into main Aug 4, 2025
3 checks passed
@dionlow dionlow deleted the dl-jwt-provider-session-storage branch August 4, 2025 18:15
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.

3 participants