Skip to content

feat: Add optional WORKOS_ISSUER access token issuer validation - #476

Open
m0tzy wants to merge 2 commits into
mainfrom
devin/1788635895-optional-issuer
Open

feat: Add optional WORKOS_ISSUER access token issuer validation#476
m0tzy wants to merge 2 commits into
mainfrom
devin/1788635895-optional-issuer

Conversation

@m0tzy

@m0tzy m0tzy commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in iss claim check to access token verification. When WORKOS_ISSUER is set, it is passed to jose.jwtVerify as the expected issuer; when unset, behavior is unchanged (signature + expiry only, no iss check).

WORKOS_ISSUER may be a single issuer or a comma-separated list; getExpectedIssuer() splits on commas (trimmed, empties dropped) and forwards a plain string for one value or a string[] for several — jose's issuer option accepts both.

function getExpectedIssuer(): string | string[] | undefined {
  if (!WORKOS_ISSUER) return undefined;
  const issuers = WORKOS_ISSUER.split(',').map((s) => s.trim()).filter(Boolean);
  return issuers.length <= 1 ? issuers[0] : issuers;
}
const issuer = getExpectedIssuer();
await jwtVerify(accessToken, JWKS(), issuer ? { issuer } : undefined);
WORKOS_ISSUER=https://api.workos.com/user_management/client_123
WORKOS_ISSUER=https://api.workos.com,https://api.workos.com/user_management/client_123

Opt-in rather than defaulted because the API does not mint a single issuer shape — it varies by environment (https://api.workos.com for legacy environments, https://api.workos.com/user_management/<clientId> for environments created since mid-2025, custom auth domains, and flag-gated /sso/<clientId> and /convex/<clientId> variants). A hardcoded default would reject valid tokens and force a refresh on every request for most environments. The list form covers apps that accept tokens from more than one of those issuers (e.g. during an issuer migration). Same shape as the issuer override in workos/authkit-react-router#85.

Link to Devin session: https://app.devin.ai/sessions/0ee38e859a9849658a7cdb2d215d89a6
Open in Devin Desktop: https://app.devin.ai/desktop/session/0ee38e859a9849658a7cdb2d215d89a6?variant=devin
Requested by: @m0tzy

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@m0tzy
m0tzy requested a review from a team as a code owner September 5, 2026 19:20
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from madison.packer

can we patch this SDK so that the issuer can be either by default (if not passed) or passed a specific issuer?

const issuer = opts.issuer ?? https://${getConfig('apiHostname')}

workos/authkit-react-router#83

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown

Greptile Summary

Adds optional access-token issuer validation configured through WORKOS_ISSUER.

  • Supports one expected issuer or a comma-separated issuer allowlist.
  • Passes configured issuers to jose.jwtVerify.
  • Documents the environment variable and adds verification-option tests.
  • Currently permits an all-empty configured issuer list to disable validation silently.

Confidence Score: 3/5

The PR is not yet safe to merge because malformed nonempty issuer configuration silently disables the new authentication constraint.

An all-empty parsed issuer list becomes undefined, causing jwtVerify to run without issuer validation even though the operator configured WORKOS_ISSUER; this fail-open path should be corrected before merging. The previous refresh-path finding was manually resolved after Greptile accepted the implementation context and withdrew it.

Files Needing Attention: src/session.ts

Security Review

Malformed, nonempty WORKOS_ISSUER values containing only whitespace or delimiters fail open by causing access-token verification to omit issuer validation.

Important Files Changed

Filename Overview
src/session.ts Adds issuer-list parsing and passes the result to JWT verification, but an empty parsed list silently disables the constraint.
src/env-variables.ts Exposes the optional WORKOS_ISSUER process-level configuration value.
src/session.spec.ts Covers unset, single, and comma-separated issuer options but not malformed all-empty input.
README.md Documents the optional single or comma-separated issuer configuration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  E[WORKOS_ISSUER] --> P[Split, trim, and remove empty entries]
  P --> C{Parsed issuers exist?}
  C -->|Yes| V[jwtVerify with expected issuer]
  C -->|No| U[jwtVerify without issuer validation]
  V --> R[Accept only matching issuer]
  U --> F[Fail-open configuration path]
Loading
Prompt To Fix All With AI
### Issue 1
src/session.ts:611
**Malformed issuer disables validation**

If `WORKOS_ISSUER` contains only whitespace or delimiters, such as `","`, parsing produces an empty list and returns `undefined`. `verifyAccessToken` then omits the issuer option, so otherwise-valid tokens are accepted without the configured issuer constraint. Malformed security configuration should reject the token rather than silently disabling validation.

**How this was verified:** The raw environment value reaches this parser without validation, and an empty parsed list causes `jwtVerify` to receive no expected issuer.

```suggestion
  if (issuers.length === 0) {
    throw new Error('WORKOS_ISSUER must contain at least one issuer');
  }
  return issuers.length === 1 ? issuers[0] : issuers;
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (2): Last reviewed commit: "Accept a comma-separated list of issuers..." | Re-trigger Greptile

Comment thread src/session.ts Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment thread src/session.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant