Skip to content

feat: Task 2 - Authentication & Authorization (Aflaha A) - #3

Open
aflaha01 wants to merge 17 commits into
shipyard-lab:mainfrom
aflaha01:task-2-authentication
Open

feat: Task 2 - Authentication & Authorization (Aflaha A)#3
aflaha01 wants to merge 17 commits into
shipyard-lab:mainfrom
aflaha01:task-2-authentication

Conversation

@aflaha01

Copy link
Copy Markdown

Task 2 — Authentication & Authorization

What's implemented

  • JWT-based auth with access token (15m) + refresh token (7d)
  • Role-based access control: dev, lead, admin
  • Protected project routes — only owners or admins can update/delete
  • Login, register, refresh, logout endpoints
  • Sample user accounts via seed script
  • 12 passing tests (auth + project endpoints)
  • Next.js frontend with login, register, and project dashboard

Stack

  • Backend: Express.js + SQLite
  • Frontend: Next.js

How to run

See README.md

if (!validRoles.includes(role))
return res
.status(400)
.json({ error: "Role must be one of: dev, lead, admin" });

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From a security perspective, try not to expose the internals like this. Generalise the error to something like "Invalid Token Error"/"Unauthorized" etc.

@Carbonite13

Copy link
Copy Markdown
Collaborator

Hey, we have gone through your submission and would like to emphasise a few improvements.

Considering this is a securtiy module, from a security perspective, try not to expose specific errors unless its strictly for testing and debugging(which also should be highly maintained), looking from what we see, a much more loose couple architecture would help with the debugging.

Also, try to reduce the use of plaintext SQL queries embedded in the code, try migrating your code from plaintext to use ORM models from sequelize or similar frameworks.

The final decision will be notified, Stay tuned!

@aflaha01

Copy link
Copy Markdown
Author

Hey, we have gone through your submission and would like to emphasise a few improvements.

Considering this is a securtiy module, from a security perspective, try not to expose specific errors unless its strictly for testing and debugging(which also should be highly maintained), looking from what we see, a much more loose couple architecture would help with the debugging.

Also, try to reduce the use of plaintext SQL queries embedded in the code, try migrating your code from plaintext to use ORM models from sequelize or similar frameworks.

The final decision will be notified, Stay tuned!

oki

@Carbonite13

Copy link
Copy Markdown
Collaborator

Ok, looks good.

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.

2 participants