-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add support for token provider #1587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,12 +191,20 @@ import { | |
} from './internal/utils/log'; | ||
import { isEmptyObj } from './internal/utils/values'; | ||
|
||
export interface AccessToken { | ||
token: string; | ||
} | ||
export type TokenProvider = () => Promise<AccessToken>; | ||
|
||
export interface ClientOptions { | ||
/** | ||
* Defaults to process.env['OPENAI_API_KEY']. | ||
*/ | ||
apiKey?: string | undefined; | ||
|
||
/** | ||
* A function that returns a token to use for authentication. | ||
*/ | ||
tokenProvider?: TokenProvider | undefined; | ||
/** | ||
* Defaults to process.env['OPENAI_ORG_ID']. | ||
*/ | ||
|
@@ -307,6 +315,7 @@ export class OpenAI { | |
#encoder: Opts.RequestEncoder; | ||
protected idempotencyHeader?: string; | ||
private _options: ClientOptions; | ||
private _tokenProvider: TokenProvider | undefined; | ||
|
||
/** | ||
* API Client for interfacing with the OpenAI API. | ||
|
@@ -330,11 +339,18 @@ export class OpenAI { | |
organization = readEnv('OPENAI_ORG_ID') ?? null, | ||
project = readEnv('OPENAI_PROJECT_ID') ?? null, | ||
webhookSecret = readEnv('OPENAI_WEBHOOK_SECRET') ?? null, | ||
tokenProvider, | ||
...opts | ||
}: ClientOptions = {}) { | ||
if (apiKey === undefined) { | ||
if (apiKey === undefined && !tokenProvider) { | ||
throw new Errors.OpenAIError( | ||
'Missing credentials. Please pass one of `apiKey` and `tokenProvider`, or set the `OPENAI_API_KEY` environment variable.', | ||
); | ||
} | ||
|
||
if (tokenProvider && apiKey) { | ||
throw new Errors.OpenAIError( | ||
"The OPENAI_API_KEY environment variable is missing or empty; either provide it, or instantiate the OpenAI client with an apiKey option, like new OpenAI({ apiKey: 'My API Key' }).", | ||
'The `apiKey` and `tokenProvider` arguments are mutually exclusive; only one can be passed at a time.', | ||
); | ||
} | ||
|
||
|
@@ -343,6 +359,7 @@ export class OpenAI { | |
organization, | ||
project, | ||
webhookSecret, | ||
tokenProvider, | ||
...opts, | ||
baseURL: baseURL || `https://api.openai.com/v1`, | ||
}; | ||
|
@@ -370,7 +387,8 @@ export class OpenAI { | |
|
||
this._options = options; | ||
|
||
this.apiKey = apiKey; | ||
this.apiKey = apiKey ?? 'Missing Key'; | ||
this._tokenProvider = tokenProvider; | ||
this.organization = organization; | ||
this.project = project; | ||
this.webhookSecret = webhookSecret; | ||
|
@@ -390,6 +408,7 @@ export class OpenAI { | |
fetch: this.fetch, | ||
fetchOptions: this.fetchOptions, | ||
apiKey: this.apiKey, | ||
tokenProvider: this._tokenProvider, | ||
organization: this.organization, | ||
project: this.project, | ||
webhookSecret: this.webhookSecret, | ||
|
@@ -438,6 +457,31 @@ export class OpenAI { | |
return Errors.APIError.generate(status, error, message, headers); | ||
} | ||
|
||
async _setToken(): Promise<boolean> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think "refresh" is a better verb than "set" here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The decision to refresh is taken by the input function and not by this method, i.e. it is not guaranteed that the token will be refreshed if this method is called. However, let me know if you feel strongly about it and I can update the name. |
||
if (typeof this._tokenProvider === 'function') { | ||
try { | ||
const token = await this._tokenProvider(); | ||
if (!token || typeof token.token !== 'string') { | ||
throw new Errors.OpenAIError( | ||
`Expected 'tokenProvider' argument to return a string but it returned ${token}`, | ||
); | ||
} | ||
this.apiKey = token.token; | ||
return true; | ||
} catch (err: any) { | ||
if (err instanceof Errors.OpenAIError) { | ||
throw err; | ||
} | ||
throw new Errors.OpenAIError( | ||
`Failed to get token from 'tokenProvider' function: ${err.message}`, | ||
// @ts-ignore | ||
{ cause: err }, | ||
); | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
buildURL( | ||
path: string, | ||
query: Record<string, unknown> | null | undefined, | ||
|
@@ -464,7 +508,9 @@ export class OpenAI { | |
/** | ||
* Used as a callback for mutating the given `FinalRequestOptions` object. | ||
*/ | ||
protected async prepareOptions(options: FinalRequestOptions): Promise<void> {} | ||
protected async prepareOptions(options: FinalRequestOptions): Promise<void> { | ||
await this._setToken(); | ||
} | ||
|
||
/** | ||
* Used as a callback for mutating the given `RequestInit` object. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why call it
tokenProvider
and notapiKeyProvider
? I think the former matches the existing terminology betterThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API key auth is already supported. This feature enables short-lived token-based auth and we want to advertise it as such. Implementation-wise, it is piggybacking off the apiKey field which is just a hack to simplify the implementation.