Skip to content

Add login functionality for Juice Shop #41

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

Open
eduappsec1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add login functionality for Juice Shop #41

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
83 changes: 83 additions & 0 deletions login-juice-shop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright (c) 2014-2026 Bjoern Kimminich & the OWASP Juice Shop contributors.
* SPDX-License-Identifier: MIT
*/
import { type Request, type Response, type NextFunction } from 'express'
import config from 'config'

import * as challengeUtils from '../lib/challengeUtils'
import { challenges, users } from '../data/datacache'
import { BasketModel } from '../models/basket'
import * as security from '../lib/insecurity'
import { UserModel } from '../models/user'
import * as models from '../models/index'
import { type User } from '../data/types'
import * as utils from '../lib/utils'

// vuln-code-snippet start loginAdminChallenge loginBenderChallenge loginJimChallenge
export function login () {

@aikido-pr-checks aikido-pr-checks Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low - The new Juice Shop login flow is dead code in this application

As committed, this handler will never execute because the Express app still binds /login to routes.loginHandler and nothing imports login-juice-shop.js. Even if a later edit tried to wire it in, this file cannot load in the current codebase because the project is CommonJS and the referenced ../lib, ../data, and ../models trees do not exist here, so the PR does not actually add the login functionality it claims to introduce.

Show fix

Replace this transplanted Juice Shop module with a handler that matches this repository's runtime and data model, then explicitly mount it from app.js (or routes/index.js) and add a route-level test that exercises /login. If the new flow is not ready, remove this file from the PR to avoid a misleading dead-code feature addition.

More info - Reply on this comment to give feedback or ignore the issue.

function afterLogin (user: User, res: Response, next: NextFunction) {
verifyPostLoginChallenges(user) // vuln-code-snippet hide-line
BasketModel.findOrCreate({ where: { UserId: user.id } })
.then(([basket]: [BasketModel, boolean]) => {
const authenticatedUser = { data: user, bid: basket.id } // keep track of original basket
const token = security.authorize(authenticatedUser)
security.authenticatedUsers.put(token, authenticatedUser)
res.json({ authentication: { token, bid: basket.id, umail: user.email } })
}).catch((error: Error) => {
next(error)
})
}

return (req: Request, res: Response, next: NextFunction) => {
verifyPreLoginChallenges(req) // vuln-code-snippet hide-line
models.sequelize.query(`SELECT * FROM Users WHERE email = '${req.body.email || ''}' AND password = '${security.hash(req.body.password || '')}' AND deletedAt IS NULL`, { model: UserModel, plain: true }) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge

@github-actions github-actions Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚫 [Bearer] <javascript_lang_sql_injection> reported by reviewdog 🐶

Unsanitized input in SQL query

Description

Using unsanitized data, such as user input or request data, or externally influenced data passed to a function, in SQL query exposes your application to SQL injection attacks. This vulnerability arises when externally controlled data is directly included in SQL statements without proper sanitation, allowing attackers to manipulate queries and access or modify data.

Remediations

  • Do not use raw SQL queries that concatenate unsanitized input directly. 
    var sqlite = new Sequelize("sqlite::memory:");
sqlite.query("SELECT * FROM users WHERE ID = " + req.params.userId); // unsafe
  • Do validate all query inputs to ensure they meet expected patterns or values before using them in a query. 
    var rawId = req.params.userId
if !(/[0-9]+/.test(rawId)) {
  // input is unexpected; don't make the query
}
  • Do use prepared (or parameterized) statements for querying databases to safely include external input. 
    var sqlite = new Sequelize("sqlite::memory:");
sqlite.query(
  "SELECT * FROM users WHERE ID = ?",
  { replacements: [req.params.userId] },
  type: sequelize.QueryTypes.SELECT
)

References

@aikido-pr-checks aikido-pr-checks Bot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Potential SQL injection via string-based query concatenation - critical severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Suggested change
models.sequelize.query(`SELECT * FROM Users WHERE email = '${req.body.email || ''}' AND password = '${security.hash(req.body.password || '')}' AND deletedAt IS NULL`, { model: UserModel, plain: true }) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge
models.sequelize.query(`SELECT * FROM Users WHERE email = :email AND password = :password AND deletedAt IS NULL`, { replacements: { email: req.body.email || '', password: security.hash(req.body.password || '') }, model: UserModel, plain: true }) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

.then((authenticatedUser) => { // vuln-code-snippet neutral-line loginAdminChallenge loginBenderChallenge loginJimChallenge
const user = utils.queryResultToJson(authenticatedUser)
if (user.data?.id && user.data.totpSecret !== '') {

@github-actions github-actions Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚫 [Bearer] <javascript_lang_observable_timing> reported by reviewdog 🐶

Observable Timing Discrepancy

Description

Observable Timing Discrepancy occurs when the time it takes for certain operations to complete can be measured and observed by attackers. This vulnerability is particularly concerning when operations involve sensitive information, such as password checks or secret comparisons. If attackers can analyze how long these operations take, they might be able to deduce confidential details, putting your data at risk.

Remediations

  • Do implement algorithms that process sensitive information in constant time. This approach helps prevent attackers from guessing secrets based on the duration of operations.
  • Do use built-in security features and cryptographic libraries that offer functions safe from timing attacks for comparing secret values.
  • Do not use direct string comparisons for sensitive information, as this can lead to early termination of the function if a mismatch is found, revealing timing information. 
      if (apiToken === "zDE9ET!TDq2uZx2oM!FD2") { // unsafe
    ...
  }
  • Do not design application logic that changes execution paths in a manner that could introduce timing discrepancies based on user input or secret values.

References

res.status(401).json({
status: 'totp_token_required',
data: {
tmpToken: security.authorize({
userId: user.data.id,
type: 'password_valid_needs_second_factor_token'
})
}
})
} else if (user.data?.id) {
afterLogin(user.data, res, next)
} else {
res.status(401).send(res.__('Invalid email or password.'))
}
}).catch((error: Error) => {
next(error)
})
}
// vuln-code-snippet end loginAdminChallenge loginBenderChallenge loginJimChallenge

function verifyPreLoginChallenges (req: Request) {
challengeUtils.solveIf(challenges.weakPasswordChallenge, () => { return req.body.email === 'admin@' + config.get<string>('application.domain') && req.body.password === 'admin123' })
challengeUtils.solveIf(challenges.loginSupportChallenge, () => { return req.body.email === 'support@' + config.get<string>('application.domain') && req.body.password === 'J6aVjTgOpRs@?5l!Zkq2AYnCE@RF$P' })
challengeUtils.solveIf(challenges.loginRapperChallenge, () => { return req.body.email === 'mc.safesearch@' + config.get<string>('application.domain') && req.body.password === 'Mr. N00dles' })
challengeUtils.solveIf(challenges.loginAmyChallenge, () => { return req.body.email === 'amy@' + config.get<string>('application.domain') && req.body.password === 'K1f.....................' })
challengeUtils.solveIf(challenges.dlpPasswordSprayingChallenge, () => { return req.body.email === 'J12934@' + config.get<string>('application.domain') && req.body.password === '0Y8rMnww$*9VFYE§59-!Fg1L6t&6lB' })
challengeUtils.solveIf(challenges.oauthUserPasswordChallenge, () => { return req.body.email === 'bjoern.kimminich@gmail.com' && req.body.password === 'bW9jLmxpYW1nQGhjaW5pbW1pay5ucmVvamI=' })

@aikido-pr-checks aikido-pr-checks Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Exposed secret in login-juice-shop.js - high severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info

challengeUtils.solveIf(challenges.exposedCredentialsChallenge, () => { return req.body.email === 'testing@' + config.get<string>('application.domain') && req.body.password === 'IamUsedForTesting' })
}

function verifyPostLoginChallenges (user: User) {
challengeUtils.solveIf(challenges.loginAdminChallenge, () => { return user.id === users.admin.id })
challengeUtils.solveIf(challenges.loginJimChallenge, () => { return user.id === users.jim.id })
challengeUtils.solveIf(challenges.loginBenderChallenge, () => { return user.id === users.bender.id })
challengeUtils.solveIf(challenges.ghostLoginChallenge, () => { return user.id === users.chris.id })
if (challengeUtils.notSolved(challenges.ephemeralAccountantChallenge) && user.email === 'acc0unt4nt@' + config.get<string>('application.domain') && user.role === 'accounting') {
UserModel.count({ where: { email: 'acc0unt4nt@' + config.get<string>('application.domain') } }).then((count: number) => {
if (count === 0) {
challengeUtils.solve(challenges.ephemeralAccountantChallenge)
}
}).catch(() => {
throw new Error('Unable to verify challenges! Try again')
})
}
}
}
Loading