-
Notifications
You must be signed in to change notification settings - Fork 16
Description
Issue Details:
I encountered an issue where I cannot set a cookie when using a custom storage with Redis.
When attempting to set the cookie within the set method, it only works if placed at the beginning of the method. However, it does not work if I place it at the end of the method (I don't get any error in console).
Steps to Reproduce:
- Run
npx create-miro-app@latestcommand. - Select
Next.js framework with Web SDK and API client installed [WEB SDK & REST API]. - Choose Typescript.
- After project creation, implement a custom storage with a Redis backend following the guide at Miro Node.js - Implement Storage for Data Persistence.
- Attempt to set a cookie within the
RedisStorage'ssetmethod. - Navigate to
http://localhost:3000and click the "Login" button. - Proceed through the steps to add the app to a Miro team.
- Upon redirection to
http://localhost:3000, notice the "Login" button is still present, indicating the "miro_tokens" cookie was not set.
Code Example:
Below is the custom storage class:
const tokensCookie = "miro_tokens";
export class RedisStorage implements Storage {
private redisClient: any;
// Initiate a connection to the Redis instance.
// On subsequent calls, it returns the same Redis connection
async _getClient() {
if (!this.redisClient) {
const client = createClient();
await client.connect();
this.redisClient = client;
}
return this.redisClient;
}
// Return the state from Redis, if this data exists
async get(userId: ExternalUserId) {
const client = await this._getClient();
const value = await client.get(userId.toString());
if (!value) return undefined;
return JSON.parse(value);
}
// Store the state in Redis.
// If the state is undefined, the corresponding Redis key is deleted
async set(userId: ExternalUserId, state: State | undefined) {
// If I set the cookie here, it works as expected. But I'm not sure if I should set it before or after storing the state in Redis.
const client = await this._getClient();
// Delete the state, if it's undefined
if (!state) {
return await client.del(userId.toString());
}
// Store the state in Redis
await client.set(userId.toString(), JSON.stringify(state));
// BUG: If I set the cookie here, a response is already sent to the client (src/app/api/redirect/route.ts). I don't get any error, but the cookie is not set.
cookies().set(tokensCookie, JSON.stringify(state), {
path: "/",
httpOnly: true,
sameSite: "none",
secure: true,
});
}
}Repository Example:
For a specific example to reproduce the bug, refer to this repository (bug/miro-api branch).
Problem in the /api/redirect endpoint:
When a user clicks the 'Login' button, adds the app to a team and is redirected to the app, the following sequence occurs:
- Receive a request (
/api/redirectendpoint). - Extract the code from query parameters.
- Call the
exchangeCodeForAccessTokenmethod. - The storage's
setmethod is called.exchangeCodeForAccessTokenis calling it. - Redirect the user to "/".
- Save session data in Redis.
- Attempt to set a cookie (noted that the user already received a response without the cookie).
NOTE: the step 5 (Redirect the user to "/") must be the last step but user is being redirected before saving the session data in redis and setting the cookie
Root Cause Analysis:
After modifying the Miro class of the @mirohq/miro-api package found in the node_modules folder it is working as expected. It appears there is a missing await when calling the storage's set method in the getToken method.
Suggested Fix:
Consider adding an await in the getToken method of the Miro class, see the screenshot below. This change should be made in this file.
There is an await in the revokeToken method when calling storage's set method (Line 120), I think there should be an await when calling storage's set method in the getToken method (Line 134)