BioDrop
BioDrop copied to clipboard
[FEATURE] Move premium user check to middleware
Is this a unique feature?
- [X] 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
To reduce notifications, issues are locked until they are https://github.com/EddieHubCommunity/BioDrop/labels/%F0%9F%8F%81%20status%3A%20ready%20for%20dev and to be assigned. You can learn more in our contributing guide https://github.com/EddieHubCommunity/BioDrop/blob/main/CONTRIBUTING.md
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
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
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: https://github.com/vercel/next.js/discussions/46722
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"
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.
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.
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...
https://github.com/EddieHubCommunity/BioDrop/blob/b11b991cbc56f20066c694763f0e4e92597fcfe9/middleware.js#L31-L34
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...
https://github.com/EddieHubCommunity/BioDrop/blob/b11b991cbc56f20066c694763f0e4e92597fcfe9/middleware.js#L31-L34
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
getToken function is being used to get jwt token information, which doesn't contains accountType property
We add this to the session here
https://github.com/EddieHubCommunity/BioDrop/blob/b257fc2a05b20edcadec124c09e8ba27f826a898/pages/api/auth/%5B...nextauth%5D.js#L74
getToken function is being used to get jwt token information, which doesn't contains accountType property
We add this to the session here
https://github.com/EddieHubCommunity/BioDrop/blob/b257fc2a05b20edcadec124c09e8ba27f826a898/pages/api/auth/%5B...nextauth%5D.js#L74
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: '[email protected]',
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
Oh I see, now I think I understand better, thank you for explaining 👍
ℹ️ eddiejaoude has some opened assigned issues: 🔧View assigned issues
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.