[FEATURE] Move premium user check to middleware

[dmitrykulakovfrontend]

Is this a unique feature?

• I have checked "open" AND "closed" issues and this is not a duplicate

Is your feature request related to a problem/unavailable functionality? Please describe.

For /pages/account/statistics we are using "getServerSideProps" redirection instead of using middleware.js. The middleware ends code duplication and reduces possibility of introducing errors.

Proposed Solution

I'm proposing we consolidate authentication checking using only middleware.js.

Screenshots

Do you want to work on this issue?

Yes

If you want to work on this issue...

I would add in middleware.js a code that would look at current user route and if it is /account/statistics and user account type is not premium, it would redirect him

[github-actions]

To reduce notifications, issues are locked until they are 🏁 status: ready for dev You can asked for this issue to be assigned (if not already assigned) and to be assigned. You can learn more in our contributing guide https://github.com/EddieHubCommunity/BioDrop/blob/main/CONTRIBUTING.md

[github-actions]

The issue has been unlocked and is now ready for dev. If you would like to work on this issue, you can comment to have it assigned to you. You can learn more in our contributing guide https://github.com/EddieHubCommunity/BioDrop/blob/main/CONTRIBUTING.md

[eddiejaoude]

This will clean up the code. I am not sure how possible it is to get the session in the middleware to do the check though - so we might have some challenges

ℹ️ eddiejaoude has some opened assigned issues: 🔧View assigned issues

[dmitrykulakovfrontend]

Upon investigation there are 3 different path forward that have been investigated so far. Each of them having there own pros and cons.

Next.js "middleware.js" runs into a separate light weight runtime environment called Edge.

"The Edge Runtime offers lower latency, higher scalability, and better security than Node.js, but it has some limitations, such as not supporting native Node.js APIs."

There is an ongoing discussion for middleware.js to allow Node.js runtime:
vercel/next.js#46722

[kbventures]

Proposal 1

Adding accountType on the JWT Token

pages/api/[...next-auth].js

async jwt({ token, account, profile }) {
      // Persist the OAuth access_token and or the user id to the token right after signin
      if (account) {
        token.accessToken = account.access_token;
        token.id = profile.id;
        token.username = profile.login;
      }
      // we added this code
      const user = await User.findOne({ _id: token.sub });
      if (user) {
        token.accountType = user.type;
      } else {
        token.accountType = "free";
      }
      return token;
    },

middleware.js

We use the accountType property from the session(JWT Token) object to create a logic within the middleware.js to ascertain whether the user is a free or premium user.

  const session = await getToken({
    req: req,
    secret: process.env.NEXTAUTH_SECRET,
  });

console.log(session.accountType) // "Premium" "Free"

[dmitrykulakovfrontend]

The advantage of this solution is that it leverages existing code (next-auth), but a drawback is that we would need to reset the JWT Secret, resulting in the logout of all users.

[kbventures]

Solution 2

As a work around to being unable to make MongoDB calls from the middleware.js file:

middleware.js

Make a request to Next.js API end point and include in the header an authenticated cookie(required by next-auth).:

const res = await fetch("http://localhost:3000/api/getSession", {
method: 'GET',
headers: {
"cookie": req.headers.get("cookie")
}
});
const serverSession = await res.json();

pages/api/getSession.js

import { authOptions } from "./auth/[...nextauth]";


export default async function handler(req, res) {
if (req.method != "GET") {
return res
.status(400)
.json({ error: "Invalid request: GET request required" });
}
const session = await getServerSession(req, res, authOptions);


return res.status(200).json(session);
}

CON: Making an API and database requests could be an additional resource burden.
PROS: No need to change the JWT Secret key.

[eddiejaoude]

Thank you for the different solutions, I will go through in more details tomorrow. Please note: we have the session and the mongodb connection in the middleware already here...

BioDrop/middleware.js

Lines 31 to 34 in b11b991

	const session = await getToken({
	req: req,
	secret: process.env.NEXTAUTH_SECRET,
	});

[dmitrykulakovfrontend]

Thank you for the different solutions, I will go through in more details tomorrow. Please note: we have the session and the mongodb connection in the middleware already here...

BioDrop/middleware.js

Lines 31 to 34 in b11b991

	const session = await getToken({
	req: req,
	secret: process.env.NEXTAUTH_SECRET,
	});

But according to the next auth documentation: https://next-auth.js.org/tutorials/securing-pages-and-api-routes#using-gettoken

getToken function is being used to get jwt token information, which doesn't contains accountType property

[eddiejaoude]

getToken function is being used to get jwt token information, which doesn't contains accountType property

We add this to the session here

BioDrop/pages/api/auth/[...nextauth].js

Line 74 in b257fc2

session.accountType = user.type;

[dmitrykulakovfrontend]

getToken function is being used to get jwt token information, which doesn't contains accountType property

We add this to the session here

BioDrop/pages/api/auth/[...nextauth].js

Line 74 in b257fc2

session.accountType = user.type;

You right, but there is a difference between session and jwt token, session is being stored in the database (to which we don't have access due to edge runtime), and jwt is being stored in user cookie, here an example of what jwt actually looks like:

const session = await getToken({
    req: req,
    secret: process.env.NEXTAUTH_SECRET,
  });
  console.log(session)
  /*
    {
      name: 'Dmitry Kulakov',
      email: 'atomeistee@gmail.com',
      picture: 'https://avatars.githubusercontent.com/u/36533957?v=4',
      sub: '6542306a8e45c14d99cb27ea',
      accessToken: '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',
      id: 36533957,
      username: 'dmitrykulakovfrontend',
      iat: 1699335414,
      exp: 1701927414,
      jti: '74aa1844-6e30-403e-bab7-d7e611884d20'
    }
  */

This is the reason why we were suggesting Solution #1, which would basically do the same thing like session.accountType = user.type; but with jwt token instead. But because that would create a difference between users with new jwt tokens and old we would need to sign out everyone and make sure that everyone has same jwt, by changing the JWT Secret.

I think it is some kind of missunderstanding naming result of getToken as session

[eddiejaoude]

Oh I see, now I think I understand better, thank you for explaining 👍

ℹ️ eddiejaoude has some opened assigned issues: 🔧View assigned issues

[kbventures]

The current naming convention is confusing and it will be remedied in the solution we propose. (session is actually JWT token).

const session = await getToken({
    req: req,
    secret: process.env.NEXTAUTH_SECRET,
  });

  // if no session reject request
  if (!session) {
    if (reqPathName.startsWith("/api")) {
      return NextResponse.json({}, { status: 401 });
    }
    return NextResponse.redirect(new URL("/auth/signin", req.url));
  }

session should be renamed to something resembling JWT Token.

[eddiejaoude]

Pull Request merged, great collaboration