Skip to content

fix: resolve open issues and add clase-7#22

Merged
midudev merged 1 commit intomainfrom
midudev/fix-issues-add-clase7
Apr 10, 2026
Merged

fix: resolve open issues and add clase-7#22
midudev merged 1 commit intomainfrom
midudev/fix-issues-add-clase7

Conversation

@midudev
Copy link
Copy Markdown
Owner

@midudev midudev commented Apr 10, 2026

Resuelve todas las issues abiertas:

  • # A6ade **/.env al .gitignore ra para ignorar archivos .env en todos los subdirectorios
  • # Reemplaza la API random-data-api.com (ca21a) en clase-6 por generaci nnn local de username con Math.random
  • Codigo clase No.7 #13, Clase n°7  #14, # A20ade clase-7 con autenticaci nnn JWT (Express + bcrypt + cookie-parser + ejs)
    • Nombre del paquete corregido
      Validation corregido
    • Campo _id en User.create corregido
    • console.log de debug eliminado
    • Template index.ejs limpio con auto-login tras registro

Cierra: #6, #13, #14, #20, #21

- Add **/.env to root .gitignore (closes #6)
- Fix clase-6 username generation: replace broken random-data-api.com
  with local generation using Math.random (closes #21)
- Add clase-7: JWT auth with Express, bcrypt, cookie-parser and ejs
  (closes #13, #14, #20)
  - Fix package name (was '6.-user-aouth-midu')
  - Fix typo Validatiton -> Validation
  - Fix _id field in User.create
  - Remove leftover console.log
  - Use cleaner index.ejs template with auto-login after register

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 10, 2026 17:31
@midudev midudev merged commit adb9da8 into main Apr 10, 2026
Copy link
Copy Markdown

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

Este PR incorpora el contenido de clase-7 (autenticación con JWT en Express usando bcrypt/cookies/EJS) y resuelve issues abiertas relacionadas con .gitignore y la generación de usernames en clase-6.

Changes:

  • Añade **/.env al .gitignore raíz para ignorar .env en subdirectorios.
  • Sustituye el consumo de random-data-api.com en clase-6 por generación local de username con Math.random.
  • Agrega clase-7 con endpoints de login/registro/logout, repositorio de usuarios con db-local, y vistas EJS (incl. auto-login tras registro).

Reviewed changes

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

Show a summary per file
File Description
.gitignore Ignora .env en cualquier subdirectorio (**/.env).
clase-6/client/index.html Elimina dependencia externa para usernames y usa generación local.
clase-7/package.json Define dependencias y scripts para la app de autenticación JWT.
clase-7/config.js Centraliza configuración (PORT, SALT_ROUND, SECRET_JWT_KEY).
clase-7/index.js Implementa servidor Express con JWT en cookie y rutas auth/protected.
clase-7/user-repository.js Persistencia local de usuarios + hashing/validación.
clase-7/views/index.ejs UI de login/registro y lógica de auto-login en frontend.
clase-7/views/protected.ejs Vista simple para contenido protegido.
clase-7/.gitignore Ignora node_modules y carpeta db para clase-7.

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

Comment thread clase-7/config.js
Comment on lines +3 to +5
SALT_ROUND = 10,
SECRET_JWT_KEY = 'your-secret-key-here'
} = process.env
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

SECRET_JWT_KEY has a hard-coded default value. If the env var isn’t set, anyone who knows this repo can forge valid JWTs and impersonate users. Prefer requiring SECRET_JWT_KEY (fail fast on boot), or only allowing a fallback in explicit dev mode (and never in production).

Suggested change
SALT_ROUND = 10,
SECRET_JWT_KEY = 'your-secret-key-here'
} = process.env
SALT_ROUND = 10
} = process.env
const { SECRET_JWT_KEY } = process.env
if (!SECRET_JWT_KEY) {
throw new Error('Missing required environment variable: SECRET_JWT_KEY')
}
export { SECRET_JWT_KEY }

Copilot uses AI. Check for mistakes.
Comment thread clase-7/index.js
Comment on lines +18 to +21
try {
const data = jwt.verify(token, SECRET_JWT_KEY)
req.session.user = data
} catch {}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The auth middleware silently swallows all JWT verification errors (catch {}), which makes misconfiguration/debugging difficult and can hide unexpected failures. Consider early-returning when access_token is missing and handling/logging non-expected verification errors (and/or clearing the cookie) instead of ignoring everything.

Suggested change
try {
const data = jwt.verify(token, SECRET_JWT_KEY)
req.session.user = data
} catch {}
if (!token) return next()
try {
const data = jwt.verify(token, SECRET_JWT_KEY)
req.session.user = data
} catch (error) {
if (
error?.name === 'TokenExpiredError' ||
error?.name === 'JsonWebTokenError' ||
error?.name === 'NotBeforeError'
) {
res.clearCookie('access_token')
} else {
console.error('Unexpected JWT verification error:', error)
}
}

Copilot uses AI. Check for mistakes.
Comment thread clase-7/index.js
Comment on lines +7 to +12
const app = express()

app.set('view engine', 'ejs')

app.use(express.json())
app.use(cookieParser())
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

app.set('view engine', 'ejs') relies on Express’s default views directory (relative to process.cwd()). If the app is started from the repo root (e.g., node clase-7/index.js), templates won’t be found. Set an explicit views path relative to this module to make startup location-independent.

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +7
import { SALT_ROUND } from './config.js'

const { Schema } = new DBLocal({ path: './db' })

Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

DBLocal is configured with path: './db', which is relative to the current working directory. If the server is started from outside clase-7/, this will create/read a db folder in the wrong place. Prefer resolving the DB path relative to this file (so it consistently uses clase-7/db).

Suggested change
import { SALT_ROUND } from './config.js'
const { Schema } = new DBLocal({ path: './db' })
import path from 'node:path'
import { fileURLToPath } from 'node:url'
import { SALT_ROUND } from './config.js'
const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)
const dbPath = path.join(__dirname, 'db')
const { Schema } = new DBLocal({ path: dbPath })

Copilot uses AI. Check for mistakes.
Comment thread clase-7/index.js
Comment on lines +31 to +51
app.post('/login', async (req, res) => {
const { username, password } = req.body
try {
const user = await UserRepository.login({ username, password })
const token = jwt.sign(
{ id: user._id, username: user.username },
SECRET_JWT_KEY,
{ expiresIn: '1h' }
)
res
.cookie('access_token', token, {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',
sameSite: 'strict',
maxAge: 1000 * 60 * 60
})
.status(200)
.json({ user })
} catch (error) {
res.status(401).json({ error: error.message })
}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

/login returns HTTP 401 for any thrown error, including input validation errors (e.g., username too short). To keep API semantics consistent with other parts of the repo that treat validation as 400, consider distinguishing validation errors (400) from authentication failures (401).

Copilot uses AI. Check for mistakes.
@midudev midudev mentioned this pull request Apr 10, 2026
@midudev midudev deleted the midudev/fix-issues-add-clase7 branch April 10, 2026 17:36
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