Skip to content

Conversation

@Jax0312
Copy link
Contributor

@Jax0312 Jax0312 commented Dec 25, 2025

  • Changed ts-node-dev to tsx for improved ESM support.
  • Organised endpoints so that those created under the private directory automatically have JWT checking enabled.
  • Completed the authentication flow.

The authentication process is as follows: the frontend sends “initdata” to the auth/ endpoints to verify the request originates from our TMA client. Once verified, a JWT (valid for two hours) is generated and sent back to the frontend. Subsequent requests to protected endpoints must include the JWT in their header.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements Telegram Mini App authentication with JWT tokens for the backend API. It migrates from CommonJS to ESM modules, switches from ts-node-dev to tsx for better ESM support, and introduces an organized routing structure where endpoints under src/routes/private automatically require JWT authentication while src/routes/public endpoints remain open.

Changes:

  • Implemented Telegram Web App authentication flow with JWT token generation (2-hour expiry)
  • Reorganized API structure with automatic JWT checking for private routes using Fastify's plugin system
  • Migrated build system from CommonJS to ESM with updated TypeScript configuration

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
backend/tsconfig.json Updated TypeScript configuration for ESM support (NodeNext module resolution)
backend/package.json Changed from ts-node-dev to tsx, updated to ESM module type, added authentication dependencies
backend/package-lock.json Updated dependencies for tsx and removed ts-node-dev related packages
backend/src/types.ts Added TypeScript definitions for Telegram Web App data structures and extended Fastify request interface
backend/src/server.ts Implemented JWT authentication hook, CORS configuration, and autoload-based routing with separate public/private directories
backend/src/routes/public/auth/index.ts Created Telegram authentication endpoint with signature validation and JWT token generation
backend/src/routes/private/queue/hello.ts Added example protected endpoint demonstrating authenticated route usage
backend/README.md Added setup documentation explaining environment configuration and routing structure
backend/.gitignore Added Vercel and local environment file patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

request.userId = payload.userId as number;

} catch (error) {
return reply.code(401).send({"error": "Unauthorized"})
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handler catches all errors but returns a generic "Unauthorized" message, potentially masking different error types (expired token, malformed token, etc.). Consider differentiating between error types to provide more helpful feedback while avoiding information leakage.

Suggested change
return reply.code(401).send({"error": "Unauthorized"})
if (error instanceof jose.errors.JWTExpired) {
return reply.code(401).send({ error: "Token expired" });
}
if (error instanceof jose.errors.JWTClaimValidationFailed || error instanceof jose.errors.JWSInvalid) {
return reply.code(401).send({ error: "Invalid token" });
}
return reply.code(401).send({ error: "Unauthorized" });

Copilot uses AI. Check for mistakes.
auth_date: params.get('auth_date') ?? '',
hash: params.get('hash') ?? '',
signature: params.get('signature') ?? '',
chat_instance: params.get('chat_instance')!,
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parseAuthData function uses non-null assertions (!) on optional parameters without proper validation. If 'chat_instance' is missing from the URLSearchParams, it will be set to 'undefined' as a string instead of the actual undefined value. Consider using conditional checks or removing the non-null assertion for optional fields.

Suggested change
chat_instance: params.get('chat_instance')!,
chat_instance: params.get('chat_instance') ?? undefined,

Copilot uses AI. Check for mistakes.
"resolveJsonModule": true,
"moduleDetection": "force",
"moduleResolution": "NodeNext",
"types": [],
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'types: []' configuration removes all default type definitions including @types/node. However, the code uses Node.js built-in modules like 'node:crypto'. Consider either adding "@types/node" explicitly or removing this line to include default types, as suggested by the commented lines 15-18.

Suggested change
"types": [],
"types": ["node"],

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +13
`BOT_TOKEN=your_development_bot_token`
`JWT_SECRET=your_jwt_secret_here`
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variable examples in the README are missing quotes around the values. This could lead to parsing issues especially if values contain special characters. Consider formatting them as proper shell variable assignments with quotes, e.g., BOT_TOKEN="your_development_bot_token"

Suggested change
`BOT_TOKEN=your_development_bot_token`
`JWT_SECRET=your_jwt_secret_here`
`BOT_TOKEN="your_development_bot_token"`
`JWT_SECRET="your_jwt_secret_here"`

Copilot uses AI. Check for mistakes.
Comment on lines +41 to 49
fastify.register((fastify) => {

fastify.addHook("preHandler", authHook);

fastify.register(autoLoad, {
dir: join(process.cwd(), 'src/routes/private'),
});

});
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fastify.register call on line 41 wraps the autoload plugin but doesn't provide an optional prefix parameter. This means that the authHook will be applied to all routes loaded under 'src/routes/private', which is correct. However, consider adding documentation or a comment explaining that this anonymous plugin scope is used to apply the authentication hook only to private routes.

Copilot uses AI. Check for mistakes.
Comment on lines +7 to 8
const JWT_SECRET = process.env.JWT_SECRET!;

Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JWT_SECRET environment variable is accessed without validation. If JWT_SECRET is not set, the application will fail at runtime with unclear errors. Consider adding validation at startup to fail fast with a clear error message if required environment variables are missing.

Suggested change
const JWT_SECRET = process.env.JWT_SECRET!;
const jwtSecret = process.env.JWT_SECRET;
if (!jwtSecret) {
throw new Error('JWT_SECRET environment variable is not set. Please set JWT_SECRET before starting the server.');
}
const JWT_SECRET: string = jwtSecret;

Copilot uses AI. Check for mistakes.
Comment on lines +13 to 18
await fastify.register(cors, {
origin: true,
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
allowedHeaders: ['Content-Type', 'Authorization', 'User-Id'],
credentials: true,
});
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CORS configuration sets 'origin: true' which allows all origins with credentials. This is a security risk in production as it permits any domain to make authenticated requests to your API. Consider restricting this to specific allowed origins or at least documenting that this should be changed for production deployment.

Copilot uses AI. Check for mistakes.

const start = async () => {
try {
await fastify.listen({ port: 3000 })
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fastify.listen call doesn't specify a host, which defaults to listening on localhost only. This will prevent the server from being accessible from outside the container in production. Consider adding 'host: "0.0.0.0"' to make it accessible externally.

Suggested change
await fastify.listen({ port: 3000 })
await fastify.listen({ port: 3000, host: '0.0.0.0' })

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +66
function validate(raw: URLSearchParams, appData: TelegramWebAppData, token: string | undefined) {

if (!appData.hash) throw new Error("Missing hash");

if (!token) throw new Error("Missing token");

const dataToCheck: string[] = [];

for (const [key, value] of raw.entries()) {
// hash field should not be included
if (key !== 'hash') {
dataToCheck.push(`${key}=${value}`);
}
}

dataToCheck.sort(); // Telegram requires alphabetical order

const dataCheckString = dataToCheck.join('\n');

const secretKey = createHmac('sha256', 'WebAppData')
.update(token)
.digest();

const calculatedHash = createHmac('sha256', secretKey)
.update(dataCheckString)
.digest('hex');

if (calculatedHash !== appData.hash) {
throw new Error("Invalid signature");
}

}
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validate function doesn't check if the auth_date is within an acceptable time window. According to Telegram's documentation, you should validate that the auth_date is recent (e.g., within the last 24 hours) to prevent replay attacks using old valid authentication data.

Copilot uses AI. Check for mistakes.
"version": "1.0.0",
"description": "",
"main": "index.js",
"main": "src/server.ts",
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "main" field points to "src/server.ts" (TypeScript source) but should point to the compiled JavaScript output like "dist/server.js" for production use. The current value may work for development but could cause issues when the package is used as a dependency or deployed.

Suggested change
"main": "src/server.ts",
"main": "dist/server.js",

Copilot uses AI. Check for mistakes.
Copy link
Member

@andrewsoonqn andrewsoonqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Maybe consider the following:

  • look at GH Copilot's suggestion and decide if u want to make improvements.
  • add tests.
  • better explanation of what the code does for future devs. Maybe a readme in src/public/auth/

Copy link

@yihao03 yihao03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address copilot reviews first. FYI for lms, we are adopting a microservices approach where we obtain the auth token from the main nusc.club website and all user permissions should be handled there. For the next sprint, we shall evaluate adopting that approach for this app too

Comment on lines +4 to +12
const route: FastifyPluginAsync = async (fastify, _) => {

fastify.get('/hello', async (request, reply) => {

return reply.code(200).send(request.userId);

});

};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comments to specify that the endpoint is for development/testing only, consider disabling it in production

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. Thanks

@Jax0312 Jax0312 self-assigned this Jan 15, 2026
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

Successfully merging this pull request may close these issues.

4 participants