Skip to content

Commit 4f775ba

Browse files
fix: encrypted session race condition (#177)
1 parent a84b9ff commit 4f775ba

File tree

5 files changed

+78
-103
lines changed

5 files changed

+78
-103
lines changed

server/encrypted-session.js

Lines changed: 52 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -49,63 +49,47 @@ async function encryptedSession(fastify) {
4949
},
5050
});
5151

52-
fastify.addHook('onRequest', (request, _reply, next) => {
53-
const userEncryptionKey = getUserEncryptionKeyFromUserCookie(request);
54-
if (!userEncryptionKey) {
55-
request.log.info({ plugin: 'encrypted-session' }, 'user-side encryption key not found, creating new one');
56-
57-
let newEncryptionKey = generateSecureEncryptionKey();
58-
setUserEncryptionKeyIntoUserCookie(request, newEncryptionKey);
59-
request[REQUEST_DECORATOR] = createStore();
60-
newEncryptionKey = undefined;
61-
} else {
62-
request.log.info({ plugin: 'encrypted-session' }, 'user-side encryption key found, using existing one');
63-
64-
const loadedEncryptionKey = Buffer.from(userEncryptionKey, 'base64');
65-
66-
const encryptedStore = request.session.get('encryptedStore');
67-
if (encryptedStore) {
68-
try {
69-
const { cipherText, iv, tag } = encryptedStore;
70-
71-
const decryptedCypherText = decryptSymetric(cipherText, iv, tag, loadedEncryptionKey);
72-
const decryptedStore = JSON.parse(decryptedCypherText);
73-
request[REQUEST_DECORATOR] = createStore(decryptedStore);
74-
} catch (error) {
75-
request.log.error({ plugin: 'encrypted-session' }, 'Failed to parse encrypted session store', error);
76-
request[REQUEST_DECORATOR] = createStore();
77-
}
78-
} else {
79-
// we could not parse the encrypted store, so we create a new one and it would overwrite the previously stored store.
80-
request.log.info({ plugin: 'encrypted-session' }, 'No encrypted store found, creating new empty store');
81-
request[REQUEST_DECORATOR] = createStore();
82-
}
52+
await fastify.decorateRequest(REQUEST_DECORATOR, {
53+
getter() {
54+
return createStore(this);
8355
}
84-
85-
next();
8656
});
57+
}
8758

88-
//TODO maybe move to onResponse after res is send. Lifecycle Doc https://fastify.dev/docs/latest/Reference/Lifecycle/
89-
// onSend is called before the response is send. Here we take encrypt the Session object and store it in the fastify-session.
90-
// Then we also want to make sure the unencrypted object is removed from memory
91-
fastify.addHook('onSend', async (request, _reply, payload) => {
92-
const encryptionKey = Buffer.from(getUserEncryptionKeyFromUserCookie(request), 'base64');
93-
if (!encryptionKey) {
94-
// if no encryption key is found in the secure session, we cannot encrypt the store. This should not happen since an encrption key is generated when the request arrived
95-
request.log.error(
96-
{ plugin: 'encrypted-session' },
97-
'No encryption key found in secure session, cannot encrypt store',
98-
);
99-
throw new Error('No encryption key found in secure session, cannot encrypt store');
100-
}
59+
export default fp(encryptedSession);
10160

102-
//we store everything in one value in the session, that might be problematic for future redis with expiration times per key. we might want to split this
103-
const stringifiedData = request[REQUEST_DECORATOR].stringify();
104-
const { cipherText, iv, tag } = encryptSymetric(stringifiedData, encryptionKey);
61+
function createStore(request) {
62+
let unencryptedStore = {}; // Private variable
63+
64+
//read previous values
65+
let userEncryptionKey = getUserEncryptionKeyFromUserCookie(request);
66+
if (!userEncryptionKey) {
67+
request.log.info({ plugin: 'encrypted-session' }, 'user-side encryption key not found, creating new one');
10568

106-
//remove unencrypted data from memory
107-
delete request[REQUEST_DECORATOR];
108-
request[REQUEST_DECORATOR] = null;
69+
userEncryptionKey = generateSecureEncryptionKey();
70+
setUserEncryptionKeyIntoUserCookie(request, userEncryptionKey);
71+
}
72+
73+
const loadedEncryptionKey = Buffer.from(userEncryptionKey, 'base64');
74+
const encryptedStore = request.session.get('encryptedStore');
75+
if (encryptedStore) {
76+
try {
77+
const { cipherText, iv, tag } = encryptedStore;
78+
79+
const decryptedCypherText = decryptSymetric(cipherText, iv, tag, loadedEncryptionKey);
80+
const decryptedStore = JSON.parse(decryptedCypherText);
81+
unencryptedStore = decryptedStore;
82+
} catch (error) {
83+
request.log.error({ plugin: 'encrypted-session' }, 'Failed to parse encrypted session store', error);
84+
}
85+
} else {
86+
// we could not parse the encrypted store, so we create a new one and it would overwrite the previously stored store.
87+
request.log.info({ plugin: 'encrypted-session' }, 'No encrypted store found, creating new empty store');
88+
}
89+
90+
async function save() {
91+
const stringifiedData = JSON.stringify(unencryptedStore);
92+
const { cipherText, iv, tag } = encryptSymetric(stringifiedData, loadedEncryptionKey);
10993

11094
request.session.set('encryptedStore', {
11195
cipherText,
@@ -114,46 +98,38 @@ async function encryptedSession(fastify) {
11498
});
11599
await request.session.save();
116100
request.log.info('store encrypted and set into request.session.encryptedStore');
117-
118-
return payload;
119-
});
120-
121-
function getUserEncryptionKeyFromUserCookie(request) {
122-
return request[ENCRYPTED_COOKIE_REQUEST_DECORATOR].get(ENCRYPTED_COOKIE_KEY_ENCRYPTION_KEY);
123-
}
124-
125-
function setUserEncryptionKeyIntoUserCookie(request, key) {
126-
request[ENCRYPTED_COOKIE_REQUEST_DECORATOR].set(ENCRYPTED_COOKIE_KEY_ENCRYPTION_KEY, key.toString('base64'));
127101
}
128-
}
129-
130-
export default fp(encryptedSession);
131102

132-
// use a closure to encapsulate the session data so noone can reference it and we are the only ones keeping a reference
133-
function createStore(previousValue) {
134-
let unencryptedStore = {}; // Private variable
135-
if (previousValue) {
136-
unencryptedStore = previousValue;
137-
}
138103
return {
139-
set(key, value) {
104+
async set(key, value) {
140105
unencryptedStore[key] = value;
106+
await save()
141107
},
142108
get(key) {
143109
return unencryptedStore[key];
144110
},
145-
delete(key) {
111+
async delete(key) {
146112
delete unencryptedStore[key];
113+
await save()
147114
},
148-
stringify() {
149-
return JSON.stringify(unencryptedStore);
115+
print() {
116+
console.log("printing", unencryptedStore)
150117
},
151-
clear() {
118+
async clear() {
152119
unencryptedStore = {}; // Clear all data
120+
await save()
153121
},
154122
};
155123
}
156124

125+
function getUserEncryptionKeyFromUserCookie(request) {
126+
return request[ENCRYPTED_COOKIE_REQUEST_DECORATOR].get(ENCRYPTED_COOKIE_KEY_ENCRYPTION_KEY);
127+
}
128+
129+
function setUserEncryptionKeyIntoUserCookie(request, key) {
130+
request[ENCRYPTED_COOKIE_REQUEST_DECORATOR].set(ENCRYPTED_COOKIE_KEY_ENCRYPTION_KEY, key.toString('base64'));
131+
}
132+
157133
// generates a secure encryption key for aes-256-gcm.
158134
// Returns a buffer of 32 bytes (256 bits).
159135
function generateSecureEncryptionKey() {

server/plugins/auth-utils.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ async function authUtilsPlugin(fastify) {
7777
};
7878
});
7979

80-
fastify.decorate('prepareOidcLoginRedirect', (request, oidcConfig, authorizationEndpoint, stateKey) => {
80+
fastify.decorate('prepareOidcLoginRedirect', async (request, oidcConfig, authorizationEndpoint, stateKey) => {
8181
if (stateKey === undefined) {
8282
stateKey = 'oauthState';
8383
}
@@ -88,16 +88,16 @@ async function authUtilsPlugin(fastify) {
8888
request.log.error(`Invalid redirectTo: "${redirectTo}".`);
8989
throw new AuthenticationError('Invalid redirectTo.');
9090
}
91-
request.encryptedSession.set('postLoginRedirectRoute', redirectTo);
91+
await request.encryptedSession.set('postLoginRedirectRoute', redirectTo);
9292

9393
const { clientId, redirectUri, scopes } = oidcConfig;
9494

9595
const state = crypto.randomBytes(16).toString('hex');
9696
const codeVerifier = crypto.randomBytes(32).toString('base64url');
9797
const codeChallenge = crypto.createHash('sha256').update(codeVerifier).digest('base64url');
9898

99-
request.encryptedSession.set(stateKey, state);
100-
request.encryptedSession.set('codeVerifier', codeVerifier);
99+
await request.encryptedSession.set(stateKey, state);
100+
await request.encryptedSession.set('codeVerifier', codeVerifier);
101101
request.log.info(
102102
{
103103
stateSet: Boolean(state),

server/plugins/http-proxy.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ function proxyPlugin(fastify) {
4343
const refreshToken = request.encryptedSession.get(keyRefreshToken);
4444
if (!refreshToken) {
4545
request.log.error('Missing refresh token; deleting encryptedSession.');
46-
request.encryptedSession.clear(); //TODO: also clear user encrpytion key?
46+
await request.encryptedSession.clear(); //TODO: also clear user encrpytion key?
4747
return reply.unauthorized('Session expired without token refresh capability.');
4848
}
4949

@@ -61,23 +61,23 @@ function proxyPlugin(fastify) {
6161
);
6262
if (!refreshedTokenData || !refreshedTokenData.accessToken) {
6363
request.log.error('Token refresh failed (no access token); deleting session.');
64-
request.encryptedSession.clear(); //TODO: also clear user encrpytion key?
64+
await request.encryptedSession.clear(); //TODO: also clear user encrpytion key?
6565
return reply.unauthorized('Session expired and token refresh failed.');
6666
}
6767

6868
request.log.info('Token refresh successful; updating the session.');
6969

70-
request.encryptedSession.set(keyAccessToken, refreshedTokenData.accessToken);
70+
await request.encryptedSession.set(keyAccessToken, refreshedTokenData.accessToken);
7171
if (refreshedTokenData.refreshToken) {
72-
request.encryptedSession.set(keyRefreshToken, refreshedTokenData.refreshToken);
72+
await request.encryptedSession.set(keyRefreshToken, refreshedTokenData.refreshToken);
7373
} else {
74-
request.encryptedSession.delete(keyRefreshToken);
74+
await request.encryptedSession.delete(keyRefreshToken);
7575
}
7676
if (refreshedTokenData.expiresIn) {
7777
const newExpiresAt = Date.now() + refreshedTokenData.expiresIn * 1000;
78-
request.encryptedSession.set(keyTokenExpiresAt, newExpiresAt);
78+
await request.encryptedSession.set(keyTokenExpiresAt, newExpiresAt);
7979
} else {
80-
request.encryptedSession.delete(keyTokenExpiresAt);
80+
await request.encryptedSession.delete(keyTokenExpiresAt);
8181
}
8282

8383
request.log.info('Token refresh successful and session updated; continuing with the HTTP request.');

server/routes/auth-mcp.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ async function authPlugin(fastify) {
1212
fastify.decorate('mcpIssuerConfiguration', mcpIssuerConfiguration);
1313

1414
fastify.get('/auth/mcp/login', async function (req, reply) {
15-
const redirectUri = fastify.prepareOidcLoginRedirect(
15+
const redirectUri = await fastify.prepareOidcLoginRedirect(
1616
req,
1717
{
1818
clientId: OIDC_CLIENT_ID_MCP,
@@ -38,13 +38,13 @@ async function authPlugin(fastify) {
3838
stateSessionKey,
3939
);
4040

41-
req.encryptedSession.set('mcp_accessToken', callbackResult.accessToken);
42-
req.encryptedSession.set('mcp_refreshToken', callbackResult.refreshToken);
41+
await req.encryptedSession.set('mcp_accessToken', callbackResult.accessToken);
42+
await req.encryptedSession.set('mcp_refreshToken', callbackResult.refreshToken);
4343

4444
if (callbackResult.expiresAt) {
45-
req.encryptedSession.set('mcp_tokenExpiresAt', callbackResult.expiresAt);
45+
await req.encryptedSession.set('mcp_tokenExpiresAt', callbackResult.expiresAt);
4646
} else {
47-
req.encryptedSession.delete('mcp_tokenExpiresAt');
47+
await req.encryptedSession.delete('mcp_tokenExpiresAt');
4848
}
4949

5050
return reply.redirect(POST_LOGIN_REDIRECT + callbackResult.postLoginRedirectRoute);
@@ -62,7 +62,7 @@ async function authPlugin(fastify) {
6262
const accessToken = req.encryptedSession.get('mcp_accessToken');
6363

6464
const isAuthenticated = Boolean(accessToken);
65-
reply.send({ isAuthenticated });
65+
return reply.send({ isAuthenticated });
6666
});
6767
}
6868

server/routes/auth-onboarding.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ async function authPlugin(fastify) {
1111
fastify.decorate('issuerConfiguration', issuerConfiguration);
1212

1313
fastify.get('/auth/onboarding/login', async function (req, reply) {
14-
const redirectUri = fastify.prepareOidcLoginRedirect(
14+
const redirectUri = await fastify.prepareOidcLoginRedirect(
1515
req,
1616
{
1717
clientId: OIDC_CLIENT_ID,
@@ -37,14 +37,14 @@ async function authPlugin(fastify) {
3737
stateSessionKey,
3838
);
3939

40-
req.encryptedSession.set('onboarding_accessToken', callbackResult.accessToken);
41-
req.encryptedSession.set('onboarding_refreshToken', callbackResult.refreshToken);
42-
req.encryptedSession.set('onboarding_userInfo', callbackResult.userInfo);
40+
await req.encryptedSession.set('onboarding_accessToken', callbackResult.accessToken);
41+
await req.encryptedSession.set('onboarding_refreshToken', callbackResult.refreshToken);
42+
await req.encryptedSession.set('onboarding_userInfo', callbackResult.userInfo);
4343

4444
if (callbackResult.expiresAt) {
45-
req.encryptedSession.set('onboarding_tokenExpiresAt', callbackResult.expiresAt);
45+
await req.encryptedSession.set('onboarding_tokenExpiresAt', callbackResult.expiresAt);
4646
} else {
47-
req.encryptedSession.delete('onboarding_tokenExpiresAt');
47+
await req.encryptedSession.delete('onboarding_tokenExpiresAt');
4848
}
4949

5050
return reply.redirect(POST_LOGIN_REDIRECT + callbackResult.postLoginRedirectRoute);
@@ -64,14 +64,13 @@ async function authPlugin(fastify) {
6464

6565
const isAuthenticated = Boolean(accessToken);
6666
const user = isAuthenticated ? userInfo : null;
67-
68-
reply.send({ isAuthenticated, user });
67+
return reply.send({ isAuthenticated, user });
6968
});
7069

7170
fastify.post('/auth/logout', async function (req, reply) {
7271
// TODO: Idp sign out flow
73-
req.encryptedSession.clear();
74-
reply.send({ message: 'Logged out' });
72+
await req.encryptedSession.clear();
73+
return reply.send({ message: 'Logged out' });
7574
});
7675
}
7776

0 commit comments

Comments
 (0)