Skip to content
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

Cannot set cookies within custom storage #407

Open
gustavo0197 opened this issue Jul 10, 2024 · 0 comments
Open

Cannot set cookies within custom storage #407

gustavo0197 opened this issue Jul 10, 2024 · 0 comments

Comments

@gustavo0197
Copy link

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:

  1. Run npx create-miro-app@latest command.
  2. Select Next.js framework with Web SDK and API client installed [WEB SDK & REST API].
  3. Choose Typescript.
  4. After project creation, implement a custom storage with a Redis backend following the guide at Miro Node.js - Implement Storage for Data Persistence.
  5. Attempt to set a cookie within the RedisStorage's set method.
  6. Navigate to http://localhost:3000 and click the "Login" button.
  7. Proceed through the steps to add the app to a Miro team.
  8. 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:

  1. Receive a request (/api/redirect endpoint).
  2. Extract the code from query parameters.
  3. Call the exchangeCodeForAccessToken method.
  4. The storage's set method is called. exchangeCodeForAccessToken is calling it.
  5. Redirect the user to "/".
  6. Save session data in Redis.
  7. 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.

Screenshot

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant