workers-oauth-provider icon indicating copy to clipboard operation
workers-oauth-provider copied to clipboard

handleApiRequest - access token validation never checks to see if the client who obtained the token still exists

Open DevInABoxLLC opened this issue 6 months ago • 0 comments

When deleteClient is called it does not remove the grants issued to the client id. By only deleting the client record, it orphans all active authorizations and tokens that were previously issued to that client.

However, the access token validation process never checks if the client who obtained the token still exists.

This allows use of the access token until it is expired, regardless of it the client has been deleted or not.

It would be better to ensure that the access token cannot be used if the client is deleted - like the implementation for handleRefreshTokenGrant

the relevant code is - I've added a comment '// ISSUE HERE': ` private async handleApiRequest(request: Request, env: any, ctx: ExecutionContext): Promise<Response> { // Get access token from Authorization header const authHeader = request.headers.get('Authorization');

if (!authHeader || !authHeader.startsWith('Bearer ')) {
  return this.createErrorResponse('invalid_token', 'Missing or invalid access token', 401, {
    'WWW-Authenticate':
      'Bearer realm="OAuth", error="invalid_token", error_description="Missing or invalid access token"',
  });
}

const accessToken = authHeader.substring(7);

// Parse the token to extract user ID and grant ID for parallel lookups
const tokenParts = accessToken.split(':');
if (tokenParts.length !== 3) {
  return this.createErrorResponse('invalid_token', 'Invalid token format', 401, {
    'WWW-Authenticate': 'Bearer realm="OAuth", error="invalid_token"',
  });
}

const [userId, grantId, _] = tokenParts;

// Generate token ID from the full token
const accessTokenId = await generateTokenId(accessToken);

// Look up the token record, which now contains the denormalized grant information

// ISSUE HERE - it does not seem like this is handled elsewhere?
const tokenKey = `token:${userId}:${grantId}:${accessTokenId}`;
const tokenData: Token | null = await env.OAUTH_KV.get(tokenKey, { type: 'json' });

// Verify token
if (!tokenData) {
  return this.createErrorResponse('invalid_token', 'Invalid access token', 401, {
    'WWW-Authenticate': 'Bearer realm="OAuth", error="invalid_token"',
  });
}

// Check if token is expired (should be auto-deleted by KV TTL, but double-check)
const now = Math.floor(Date.now() / 1000);
if (tokenData.expiresAt < now) {
  return this.createErrorResponse('invalid_token', 'Access token expired', 401, {
    'WWW-Authenticate': 'Bearer realm="OAuth", error="invalid_token"',
  });
}

// Unwrap the encryption key using the access token
const encryptionKey = await unwrapKeyWithToken(accessToken, tokenData.wrappedEncryptionKey);

// Decrypt the props
const decryptedProps = await decryptProps(encryptionKey, tokenData.grant.encryptedProps);

// Set the decrypted props on the context object
ctx.props = decryptedProps;

// Inject OAuth helpers into env if not already present
if (!env.OAUTH_PROVIDER) {
  env.OAUTH_PROVIDER = this.createOAuthHelpers(env);
}

// Find the appropriate API handler for this URL
const url = new URL(request.url);
const apiHandler = this.findApiHandlerForUrl(url);

if (!apiHandler) {
  // This shouldn't happen since we already checked with isApiRequest,
  // but handle it gracefully just in case
  return this.createErrorResponse('invalid_request', 'No handler found for API route', 404);
}

// Call the API handler based on its type
if (apiHandler.type === HandlerType.EXPORTED_HANDLER) {
  // It's an object with a fetch method
  return apiHandler.handler.fetch(request as Parameters<ExportedHandlerWithFetch['fetch']>[0], env, ctx);
} else {
  // It's a WorkerEntrypoint class - instantiate it with ctx and env in that order
  const handler = new apiHandler.handler(ctx, env);
  return handler.fetch(request);
}

} `

DevInABoxLLC avatar Jun 11 '25 15:06 DevInABoxLLC