Skip to content

Authentication Semi-integration #10

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.

Sign up for GitHub

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

Closed
itssimmons wants to merge 2 commits into from
Closed

Authentication Semi-integration #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
818984d
feat(modules/auth): semi-intergation of auth module
itssimmons Apr 19, 2026
adea5ff
f
itssimmons Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/preview.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ jobs:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
- name: Comment on PR
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
24 changes: 24 additions & 0 deletions modules/auth/controllers/index.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import jwt from "jsonwebtoken";
import { ZodError } from "zod";

import jwtConfig from "@/config/jwt";
import prisma from "@/database/prisma/client";
import redisClient from "@/database/redis/client";

import NotFoundError from "../exceptions/notfound.exception";
import UnauthorizedError from "../exceptions/unauthorized.exception";
import CredentialSchema from "../schemas/credential.schema";
import RefreshTokenSchema from "../schemas/refresh-token.schema";
import SignInSchema from "../schemas/signin.schema";
import type { JwtSubject } from "../types/jwt";

export default {
Expand Down Expand Up @@ -87,6 +89,28 @@ export default {
}
},

async signin(req: FastifyRequest, reply: FastifyReply) {
try {
const { password, ...credentials } = SignInSchema.parse(req.body);

const userCounter = await prisma.user.count({
where: {
email: credentials.email,
Comment on lines +94 to +98
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

signin destructures password and requires username in the schema, but neither password nor username is used in the handler (only credentials.email). This is likely a logic error (or premature schema) and also triggers unused-variable linting; either use these fields as part of the flow or remove/relax them in the schema/destructuring to match what the endpoint actually needs.

Suggested change
const { password, ...credentials } = SignInSchema.parse(req.body);
const userCounter = await prisma.user.count({
where: {
email: credentials.email,
const { email } = SignInSchema.pick({ email: true }).parse(req.body);
const userCounter = await prisma.user.count({
where: {
email,

Copilot uses AI. Check for mistakes.
},
});

if (userCounter > 0) {
Comment on lines +96 to +102
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

Since email is @unique in the Prisma schema, doing a count() is more work than necessary for an existence check. Prefer findUnique({ where: { email }, select: { id: true } }) (or similar) to reduce DB work and make intent explicit.

Suggested change
const userCounter = await prisma.user.count({
where: {
email: credentials.email,
},
});
if (userCounter > 0) {
const existingUser = await prisma.user.findUnique({
where: {
email: credentials.email,
},
select: {
id: true,
},
});
if (existingUser) {

Copilot uses AI. Check for mistakes.
throw new UnauthorizedError(req.t("Email already in use"));
}
} catch (e) {
if (e instanceof UnauthorizedError) {
reply.status(401).send({ error: e.message });
} else {
reply.status(500).send({ error: req.t("Server error") });
}
Comment on lines +105 to +110
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

signin can complete without ever sending a response (when userCounter === 0), which will leave the client hanging or produce an unintended empty response. Ensure the handler returns/sends a success payload (and likely creates the user / issues tokens) for the non-error path.

Suggested change
} catch (e) {
if (e instanceof UnauthorizedError) {
reply.status(401).send({ error: e.message });
} else {
reply.status(500).send({ error: req.t("Server error") });
}
return reply.status(204).send();
} catch (e) {
if (e instanceof ZodError) {
return reply.status(400).send({
error: req.t("Invalid credentials"),
details: e.issues,
});
}
if (e instanceof UnauthorizedError) {
return reply.status(401).send({ error: e.message });
}
return reply.status(500).send({ error: req.t("Server error") });

Copilot uses AI. Check for mistakes.
Comment on lines +105 to +110
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

This handler parses SignInSchema but doesn't handle ZodError; invalid request bodies will be reported as 500 instead of a 400 with validation details (unlike login, refresh, revoke). Consider mirroring the existing error handling pattern here (including console.debug and returning e.issues).

Copilot uses AI. Check for mistakes.
Comment on lines +103 to +110
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

The "Email already in use" case is returned as 401 Unauthorized. That status code is typically reserved for missing/invalid authentication; for a registration conflict, use a more appropriate status (commonly 409) and error type/message to avoid confusing API consumers.

Suggested change
throw new UnauthorizedError(req.t("Email already in use"));
}
} catch (e) {
if (e instanceof UnauthorizedError) {
reply.status(401).send({ error: e.message });
} else {
reply.status(500).send({ error: req.t("Server error") });
}
return reply.status(409).send({
error: req.t("Email already in use"),
});
}
} catch (e) {
reply.status(500).send({ error: req.t("Server error") });

Copilot uses AI. Check for mistakes.
}
},

logout(req: FastifyRequest, reply: FastifyReply) {
try {
const bearerToken = req.headers.authorization;
Expand Down
9 changes: 9 additions & 0 deletions modules/auth/schemas/signin.schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { z } from "zod";

const SignInSchema = z.object({
email: z.email(),
username: z.string().min(3).max(30),
password: z.string().min(8),
Comment on lines +5 to +6
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

SignInSchema currently requires email, username, and password, but the new signin handler only uses email (and discards password). If the endpoint is meant to authenticate, requiring both email and username is unusually strict; if it's meant to register, the schema name is misleading. Adjust the schema (e.g., make fields optional/union) and/or rename it to match the intended flow.

Suggested change
username: z.string().min(3).max(30),
password: z.string().min(8),
username: z.string().min(3).max(30).optional(),
password: z.string().min(8).optional(),

Copilot uses AI. Check for mistakes.
});

export default SignInSchema;
Loading