Skip to content

Commit b47123e

Browse files
authored
fix(FIR-46131): Atomic authentication mechanism and token expiry (#142)
1 parent 82a3b01 commit b47123e

File tree

5 files changed

+279
-93
lines changed

5 files changed

+279
-93
lines changed

src/auth/index.ts

Lines changed: 112 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,26 @@ import {
66
ServiceAccountAuth,
77
UsernamePasswordAuth
88
} from "../types";
9-
import { TokenKey, inMemoryCache, noneCache } from "../common/tokenCache";
10-
import ReadWriteLock from "rwlock";
9+
import {
10+
TokenKey,
11+
TokenRecord,
12+
inMemoryCache,
13+
noneCache,
14+
rwLock
15+
} from "../common/tokenCache";
1116

1217
type Login = {
1318
access_token: string;
1419
token_type: string;
1520
expires_in: number;
1621
};
1722

23+
type TokenInfo = {
24+
access_token: string;
25+
// seconds until expiration
26+
expires_in: number;
27+
};
28+
1829
const AUTH_AUDIENCE = "https://api.firebolt.io";
1930
const AUTH_GRANT_TYPE = "client_credentials";
2031

@@ -23,7 +34,9 @@ export class Authenticator {
2334
options: ConnectionOptions;
2435

2536
accessToken?: string;
26-
private readonly rwlock = new ReadWriteLock();
37+
// Expiration time is half way to the actual expiration time
38+
// to allow for some buffer time before the token expires
39+
tokenExpiryTimestampMs?: number;
2740

2841
constructor(context: Context, options: ConnectionOptions) {
2942
context.httpClient.authenticator = this;
@@ -62,23 +75,48 @@ export class Authenticator {
6275
}
6376

6477
private setToken(token: string, expiresIn: number) {
78+
// Set expiration to half of the expiresIn value
79+
// to allow for some buffer time before the token expires
80+
const tokenExpiryTimestampMs = Date.now() + (expiresIn * 1000) / 2;
6581
this.accessToken = token;
82+
this.tokenExpiryTimestampMs = tokenExpiryTimestampMs;
83+
// Update cache
6684
const key = this.getCacheKey();
67-
key && this.getCache().set(key, { token, expiration: expiresIn });
85+
key &&
86+
this.getCache().set(key, {
87+
token,
88+
tokenExpiryTimestampMs
89+
});
6890
}
6991

70-
private getCachedToken(): string | undefined {
92+
private getCachedTokenRecord(): TokenRecord | undefined {
7193
const key = this.getCacheKey();
72-
return key ? this.getCache().get(key)?.token : undefined;
73-
}
74-
75-
getHeaders() {
76-
if (this.accessToken) {
94+
if (!key) return undefined;
95+
96+
const cachedTokenRecord = this.getCache().get(key);
97+
// Check if token exists and is not expired
98+
if (
99+
cachedTokenRecord &&
100+
Date.now() < cachedTokenRecord.tokenExpiryTimestampMs
101+
) {
77102
return {
78-
Authorization: `Bearer ${this.accessToken}`
103+
token: cachedTokenRecord.token,
104+
tokenExpiryTimestampMs: cachedTokenRecord.tokenExpiryTimestampMs
79105
};
80106
}
81-
return {};
107+
108+
return undefined;
109+
}
110+
111+
async getToken(): Promise<string | undefined> {
112+
if (
113+
(this.tokenExpiryTimestampMs &&
114+
this.tokenExpiryTimestampMs < Date.now()) ||
115+
!this.accessToken
116+
) {
117+
await this.authenticate();
118+
}
119+
return this.accessToken;
82120
}
83121

84122
private static getAuthEndpoint(apiEndpoint: string) {
@@ -94,7 +132,9 @@ export class Authenticator {
94132
return myURL.toString();
95133
}
96134

97-
private async authenticateUsernamePassword(auth: UsernamePasswordAuth) {
135+
private async authenticateUsernamePassword(
136+
auth: UsernamePasswordAuth
137+
): Promise<TokenInfo> {
98138
const { httpClient, apiEndpoint } = this.context;
99139
const { username, password } = auth;
100140
const url = `${apiEndpoint}/${USERNAME_PASSWORD_LOGIN}`;
@@ -103,19 +143,21 @@ export class Authenticator {
103143
password
104144
});
105145

106-
this.accessToken = undefined;
107-
146+
// Expiration is in seconds
108147
const { access_token, expires_in } = await httpClient
109148
.request<Login>("POST", url, {
110149
body,
111-
retry: false
150+
retry: false,
151+
noAuth: true
112152
})
113153
.ready();
114154

115-
this.setToken(access_token, expires_in);
155+
return { access_token, expires_in };
116156
}
117157

118-
private async authenticateServiceAccount(auth: ServiceAccountAuth) {
158+
private async authenticateServiceAccount(
159+
auth: ServiceAccountAuth
160+
): Promise<TokenInfo> {
119161
const { httpClient, apiEndpoint } = this.context;
120162
const { client_id, client_secret } = auth;
121163

@@ -128,19 +170,19 @@ export class Authenticator {
128170
});
129171
const url = `${authEndpoint}${SERVICE_ACCOUNT_LOGIN}`;
130172

131-
this.accessToken = undefined;
132-
173+
// Expiration is in seconds
133174
const { access_token, expires_in } = await httpClient
134175
.request<Login>("POST", url, {
135176
retry: false,
136177
headers: {
137178
"Content-Type": "application/x-www-form-urlencoded"
138179
},
139-
body: params
180+
body: params,
181+
noAuth: true
140182
})
141183
.ready();
142184

143-
this.setToken(access_token, expires_in);
185+
return { access_token, expires_in };
144186
}
145187

146188
isUsernamePassword() {
@@ -161,22 +203,45 @@ export class Authenticator {
161203

162204
async authenticate(): Promise<void> {
163205
// Try to get token from cache using read lock
164-
const cachedToken = await this.tryGetCachedToken();
206+
const cachedToken = await this.tryGetCachedTokenRecord();
165207
if (cachedToken) {
166-
this.accessToken = cachedToken;
208+
this.accessToken = cachedToken.token;
209+
this.tokenExpiryTimestampMs = cachedToken.tokenExpiryTimestampMs;
167210
return;
168211
}
169-
170212
// No cached token, acquire write lock and authenticate
171213
await this.acquireWriteLockAndAuthenticate();
172214
}
173215

174-
private async tryGetCachedToken(): Promise<string | undefined> {
216+
async reAuthenticate(): Promise<void> {
217+
// Acquire write lock, clear cache and authenticate
175218
return new Promise((resolve, reject) => {
176-
this.rwlock.readLock(releaseReadLock => {
219+
rwLock.writeLock(async releaseWriteLock => {
177220
try {
178-
const cachedToken = this.getCachedToken();
179-
resolve(cachedToken);
221+
// Clear the cache under write lock
222+
const key = this.getCacheKey();
223+
key && this.getCache().clear(key);
224+
225+
// Perform authentication directly rather than calling acquireWriteLockAndAuthenticate
226+
// since we already have the write lock
227+
await this.performAuthentication();
228+
229+
resolve();
230+
} catch (error) {
231+
reject(error instanceof Error ? error : new Error(String(error)));
232+
} finally {
233+
releaseWriteLock();
234+
}
235+
});
236+
});
237+
}
238+
239+
private async tryGetCachedTokenRecord(): Promise<TokenRecord | undefined> {
240+
return new Promise((resolve, reject) => {
241+
rwLock.readLock(releaseReadLock => {
242+
try {
243+
const cachedTokenRecord = this.getCachedTokenRecord();
244+
resolve(cachedTokenRecord);
180245
} catch (error) {
181246
reject(error instanceof Error ? error : new Error(String(error)));
182247
} finally {
@@ -188,15 +253,16 @@ export class Authenticator {
188253

189254
private async acquireWriteLockAndAuthenticate(): Promise<void> {
190255
return new Promise((resolve, reject) => {
191-
this.rwlock.writeLock(async releaseWriteLock => {
256+
rwLock.writeLock(async releaseWriteLock => {
192257
try {
193258
// Double-check cache in case another thread authenticated while waiting
194-
const cachedToken = this.getCachedToken();
195-
if (cachedToken) {
196-
this.accessToken = cachedToken;
259+
const cachedTokenInfo = this.getCachedTokenRecord();
260+
if (cachedTokenInfo) {
261+
this.accessToken = cachedTokenInfo.token;
262+
this.tokenExpiryTimestampMs =
263+
cachedTokenInfo.tokenExpiryTimestampMs;
197264
return resolve();
198265
}
199-
200266
await this.performAuthentication();
201267

202268
resolve();
@@ -212,14 +278,20 @@ export class Authenticator {
212278
private async performAuthentication(): Promise<void> {
213279
const options = this.options.auth || this.options;
214280

215-
if (this.isUsernamePassword()) {
216-
return this.authenticateUsernamePassword(options as UsernamePasswordAuth);
217-
}
281+
let auth: TokenInfo;
218282

219-
if (this.isServiceAccount()) {
220-
return this.authenticateServiceAccount(options as ServiceAccountAuth);
283+
if (this.isUsernamePassword()) {
284+
auth = await this.authenticateUsernamePassword(
285+
options as UsernamePasswordAuth
286+
);
287+
} else if (this.isServiceAccount()) {
288+
auth = await this.authenticateServiceAccount(
289+
options as ServiceAccountAuth
290+
);
291+
} else {
292+
throw new Error("Please provide valid auth credentials");
221293
}
222294

223-
throw new Error("Please provide valid auth credentials");
295+
this.setToken(auth.access_token, auth.expires_in);
224296
}
225297
}

src/common/tokenCache.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import ReadWriteLock from "rwlock";
12
import { AccountInfo } from "../connection/connection_v1";
23

34
export type TokenKey = {
@@ -8,7 +9,7 @@ export type TokenKey = {
89

910
export type TokenRecord = {
1011
token: string;
11-
expiration: number;
12+
tokenExpiryTimestampMs: number;
1213
};
1314

1415
export type AccountKey = {
@@ -62,7 +63,6 @@ export class InMemoryCacheStorage<KeyType, RecordType> {
6263
const lookup = this.makeLookupString(key);
6364
const record = this.storage[lookup];
6465
if (!this.isValidRecord(record)) {
65-
this.clear(key);
6666
return null;
6767
}
6868
return record;
@@ -84,11 +84,12 @@ export class InMemoryTokenCacheStorage extends InMemoryCacheStorage<
8484
TokenRecord
8585
> {
8686
protected isValidRecord(record: TokenRecord | undefined): boolean {
87-
return typeof record != "undefined" && Date.now() < record.expiration;
87+
return (
88+
typeof record != "undefined" && Date.now() < record.tokenExpiryTimestampMs
89+
);
8890
}
8991

9092
protected modifyRecord(record: TokenRecord): TokenRecord {
91-
record.expiration = Date.now() + record.expiration;
9293
return record;
9394
}
9495
}
@@ -116,3 +117,4 @@ export class InMemoryCache implements Cache {
116117

117118
export const inMemoryCache = new InMemoryCache();
118119
export const noneCache = new NoneCache();
120+
export const rwLock = new ReadWriteLock();

0 commit comments

Comments
 (0)