1
0
mirror of https://github.com/sasjs/server.git synced 2025-12-10 11:24:35 +00:00

fix: no need to restrict api endpoints when ldap auth is applied

This commit is contained in:
2022-09-30 14:41:09 +05:00
parent f915c51b07
commit a14266077d
7 changed files with 126 additions and 120 deletions

View File

@@ -12,6 +12,7 @@ import {
import Group, { GroupPayload, PUBLIC_GROUP_NAME } from '../model/Group'
import User from '../model/User'
import { AuthProviderType } from '../utils'
import { UserResponse } from './user'
export interface GroupResponse {
@@ -147,12 +148,22 @@ export class GroupController {
@Delete('{groupId}')
public async deleteGroup(@Path() groupId: number) {
const group = await Group.findOne({ groupId })
if (group) return await group.remove()
throw {
code: 404,
status: 'Not Found',
message: 'Group not found.'
if (!group)
throw {
code: 404,
status: 'Not Found',
message: 'Group not found.'
}
if (group.authProvider !== AuthProviderType.Internal) {
throw {
code: 405,
status: 'Method Not Allowed',
message: 'Can not delete group created by an external auth provider.'
}
}
return await group.remove()
}
}
@@ -248,6 +259,13 @@ const updateUsersListInGroup = async (
message: `Can't add/remove user to '${PUBLIC_GROUP_NAME}' group.`
}
if (group.authProvider !== AuthProviderType.Internal)
throw {
code: 405,
status: 'Method Not Allowed',
message: `Can't add/remove user to group created by external auth provider.`
}
const user = await User.findOne({ id: userId })
if (!user)
throw {
@@ -256,6 +274,13 @@ const updateUsersListInGroup = async (
message: 'User not found.'
}
if (user.authProvider !== AuthProviderType.Internal)
throw {
code: 405,
status: 'Method Not Allowed',
message: `Can't add/remove user to group created by external auth provider.`
}
const updatedGroup =
action === 'addUser'
? await group.addUser(user)

View File

@@ -17,7 +17,12 @@ import {
import { desktopUser } from '../middlewares'
import User, { UserPayload } from '../model/User'
import { getUserAutoExec, updateUserAutoExec, ModeType } from '../utils'
import {
getUserAutoExec,
updateUserAutoExec,
ModeType,
AuthProviderType
} from '../utils'
import { GroupResponse } from './group'
export interface UserResponse {
@@ -211,7 +216,11 @@ const createUser = async (data: UserPayload): Promise<UserDetailsResponse> => {
// Checking if user is already in the database
const usernameExist = await User.findOne({ username })
if (usernameExist) throw new Error('Username already exists.')
if (usernameExist)
throw {
code: 409,
message: 'Username already exists.'
}
// Hash passwords
const hashPassword = User.hashPassword(password)
@@ -255,7 +264,11 @@ const getUser = async (
'groupId name description -_id'
)) as unknown as UserDetailsResponse
if (!user) throw new Error('User is not found.')
if (!user)
throw {
code: 404,
message: 'User is not found.'
}
return {
id: user.id,
@@ -284,6 +297,19 @@ const updateUser = async (
const params: any = { displayName, isAdmin, isActive, autoExec }
const user = await User.findOne(findBy)
if (
user?.authProvider !== AuthProviderType.Internal &&
(username !== user?.username || displayName !== user?.displayName)
) {
throw {
code: 405,
message:
'Can not update username and display name of user that is created by an external auth provider.'
}
}
if (username) {
// Checking if user is already in the database
const usernameExist = await User.findOne({ username })
@@ -292,7 +318,10 @@ const updateUser = async (
(findBy.id && usernameExist.id != findBy.id) ||
(findBy.username && usernameExist.username != findBy.username)
)
throw new Error('Username already exists.')
throw {
code: 409,
message: 'Username already exists.'
}
}
params.username = username
}
@@ -305,7 +334,10 @@ const updateUser = async (
const updatedUser = await User.findOneAndUpdate(findBy, params, { new: true })
if (!updatedUser)
throw new Error(`Unable to find user with ${findBy.id || findBy.username}`)
throw {
code: 404,
message: `Unable to find user with ${findBy.id || findBy.username}`
}
return {
id: updatedUser.id,
@@ -332,11 +364,19 @@ const deleteUser = async (
{ password }: { password?: string }
) => {
const user = await User.findOne(findBy)
if (!user) throw new Error('User is not found.')
if (!user)
throw {
code: 404,
message: 'User is not found.'
}
if (!isAdmin) {
const validPass = user.comparePassword(password!)
if (!validPass) throw new Error('Invalid password.')
if (!validPass)
throw {
code: 401,
message: 'Invalid password.'
}
}
await User.deleteOne(findBy)

View File

@@ -86,7 +86,7 @@ const login = async (
if (!user) throw new Error('Username is not found.')
if (
process.env.AUTH_MECHANISM === AuthProviderType.LDAP &&
process.env.AUTH_PROVIDERS === AuthProviderType.LDAP &&
user.authProvider === AuthProviderType.LDAP
) {
const ldapClient = await LDAPClient.init()

View File

@@ -1,7 +1,7 @@
import { RequestHandler, Request } from 'express'
import { userInfo } from 'os'
import { RequestUser } from '../types'
import { ModeType, AuthProviderType } from '../utils'
import { ModeType } from '../utils'
const regexUser = /^\/SASjsApi\/user\/[0-9]*$/ // /SASjsApi/user/1
@@ -27,18 +27,6 @@ export const desktopRestrict: RequestHandler = (req, res, next) => {
next()
}
export const ldapRestrict: RequestHandler = (req, res, next) => {
const { AUTH_MECHANISM } = process.env
if (AUTH_MECHANISM === AuthProviderType.LDAP) {
return res
.status(403)
.send(`Not Allowed while AUTH_MECHANISM is '${AuthProviderType.LDAP}'.`)
}
next()
}
export const desktopUser: RequestUser = {
userId: 12345,
clientId: 'desktop_app',

View File

@@ -1,17 +1,12 @@
import express from 'express'
import { GroupController } from '../../controllers/'
import {
ldapRestrict,
authenticateAccessToken,
verifyAdmin
} from '../../middlewares'
import { authenticateAccessToken, verifyAdmin } from '../../middlewares'
import { getGroupValidation, registerGroupValidation } from '../../utils'
const groupRouter = express.Router()
groupRouter.post(
'/',
ldapRestrict,
authenticateAccessToken,
verifyAdmin,
async (req, res) => {
@@ -23,11 +18,7 @@ groupRouter.post(
const response = await controller.createGroup(body)
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
}
)
@@ -38,11 +29,7 @@ groupRouter.get('/', authenticateAccessToken, async (req, res) => {
const response = await controller.getAllGroups()
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
})
@@ -54,11 +41,7 @@ groupRouter.get('/:groupId', authenticateAccessToken, async (req, res) => {
const response = await controller.getGroup(parseInt(groupId))
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
})
@@ -76,18 +59,13 @@ groupRouter.get(
const response = await controller.getGroupByGroupName(name)
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
}
)
groupRouter.post(
'/:groupId/:userId',
ldapRestrict,
authenticateAccessToken,
verifyAdmin,
async (req, res) => {
@@ -101,18 +79,13 @@ groupRouter.post(
)
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
}
)
groupRouter.delete(
'/:groupId/:userId',
ldapRestrict,
authenticateAccessToken,
verifyAdmin,
async (req, res) => {
@@ -126,18 +99,13 @@ groupRouter.delete(
)
res.send(response)
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
}
)
groupRouter.delete(
'/:groupId',
ldapRestrict,
authenticateAccessToken,
verifyAdmin,
async (req, res) => {
@@ -148,11 +116,7 @@ groupRouter.delete(
await controller.deleteGroup(parseInt(groupId))
res.status(200).send('Group Deleted!')
} catch (err: any) {
const statusCode = err.code
delete err.code
res.status(statusCode).send(err.message)
res.status(err.code).send(err.message)
}
}
)

View File

@@ -110,16 +110,16 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden if username is already present', async () => {
it('should respond with Conflict if username is already present', async () => {
await controller.createUser(user)
const res = await request(app)
.post('/SASjsApi/user')
.auth(adminAccessToken, { type: 'bearer' })
.send(user)
.expect(403)
.expect(409)
expect(res.text).toEqual('Error: Username already exists.')
expect(res.text).toEqual('Username already exists.')
expect(res.body).toEqual({})
})
@@ -254,7 +254,7 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden if username is already present', async () => {
it('should respond with Conflict if username is already present', async () => {
const dbUser1 = await controller.createUser(user)
const dbUser2 = await controller.createUser({
...user,
@@ -265,9 +265,9 @@ describe('user', () => {
.patch(`/SASjsApi/user/${dbUser1.id}`)
.auth(adminAccessToken, { type: 'bearer' })
.send({ username: dbUser2.username })
.expect(403)
.expect(409)
expect(res.text).toEqual('Error: Username already exists.')
expect(res.text).toEqual('Username already exists.')
expect(res.body).toEqual({})
})
@@ -349,7 +349,7 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden if username is already present', async () => {
it('should respond with Conflict if username is already present', async () => {
const dbUser1 = await controller.createUser(user)
const dbUser2 = await controller.createUser({
...user,
@@ -360,9 +360,9 @@ describe('user', () => {
.patch(`/SASjsApi/user/by/username/${dbUser1.username}`)
.auth(adminAccessToken, { type: 'bearer' })
.send({ username: dbUser2.username })
.expect(403)
.expect(409)
expect(res.text).toEqual('Error: Username already exists.')
expect(res.text).toEqual('Username already exists.')
expect(res.body).toEqual({})
})
})
@@ -446,7 +446,7 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden when user himself requests and password is incorrect', async () => {
it('should respond with Unauthorized when user himself requests and password is incorrect', async () => {
const dbUser = await controller.createUser(user)
const accessToken = await generateAndSaveToken(dbUser.id)
@@ -454,9 +454,9 @@ describe('user', () => {
.delete(`/SASjsApi/user/${dbUser.id}`)
.auth(accessToken, { type: 'bearer' })
.send({ password: 'incorrectpassword' })
.expect(403)
.expect(401)
expect(res.text).toEqual('Error: Invalid password.')
expect(res.text).toEqual('Invalid password.')
expect(res.body).toEqual({})
})
@@ -528,7 +528,7 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden when user himself requests and password is incorrect', async () => {
it('should respond with Unauthorized when user himself requests and password is incorrect', async () => {
const dbUser = await controller.createUser(user)
const accessToken = await generateAndSaveToken(dbUser.id)
@@ -536,9 +536,9 @@ describe('user', () => {
.delete(`/SASjsApi/user/by/username/${dbUser.username}`)
.auth(accessToken, { type: 'bearer' })
.send({ password: 'incorrectpassword' })
.expect(403)
.expect(401)
expect(res.text).toEqual('Error: Invalid password.')
expect(res.text).toEqual('Invalid password.')
expect(res.body).toEqual({})
})
})
@@ -652,16 +652,16 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden if userId is incorrect', async () => {
it('should respond with Not Found if userId is incorrect', async () => {
await controller.createUser(user)
const res = await request(app)
.get('/SASjsApi/user/1234')
.auth(adminAccessToken, { type: 'bearer' })
.send()
.expect(403)
.expect(404)
expect(res.text).toEqual('Error: User is not found.')
expect(res.text).toEqual('User is not found.')
expect(res.body).toEqual({})
})
@@ -731,16 +731,16 @@ describe('user', () => {
expect(res.body).toEqual({})
})
it('should respond with Forbidden if username is incorrect', async () => {
it('should respond with Not Found if username is incorrect', async () => {
await controller.createUser(user)
const res = await request(app)
.get('/SASjsApi/user/by/username/randomUsername')
.auth(adminAccessToken, { type: 'bearer' })
.send()
.expect(403)
.expect(404)
expect(res.text).toEqual('Error: User is not found.')
expect(res.text).toEqual('User is not found.')
expect(res.body).toEqual({})
})
})

View File

@@ -3,8 +3,7 @@ import { UserController } from '../../controllers/'
import {
authenticateAccessToken,
verifyAdmin,
verifyAdminIfNeeded,
ldapRestrict
verifyAdminIfNeeded
} from '../../middlewares'
import {
deleteUserValidation,
@@ -15,24 +14,18 @@ import {
const userRouter = express.Router()
userRouter.post(
'/',
ldapRestrict,
authenticateAccessToken,
verifyAdmin,
async (req, res) => {
const { error, value: body } = registerUserValidation(req.body)
if (error) return res.status(400).send(error.details[0].message)
userRouter.post('/', authenticateAccessToken, verifyAdmin, async (req, res) => {
const { error, value: body } = registerUserValidation(req.body)
if (error) return res.status(400).send(error.details[0].message)
const controller = new UserController()
try {
const response = await controller.createUser(body)
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
}
const controller = new UserController()
try {
const response = await controller.createUser(body)
res.send(response)
} catch (err: any) {
res.status(err.code).send(err.message)
}
)
})
userRouter.get('/', authenticateAccessToken, async (req, res) => {
const controller = new UserController()
@@ -40,7 +33,7 @@ userRouter.get('/', authenticateAccessToken, async (req, res) => {
const response = await controller.getAllUsers()
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
})
@@ -58,7 +51,7 @@ userRouter.get(
const response = await controller.getUserByUsername(req, username)
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
}
)
@@ -71,13 +64,12 @@ userRouter.get('/:userId', authenticateAccessToken, async (req, res) => {
const response = await controller.getUser(req, parseInt(userId))
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
})
userRouter.patch(
'/by/username/:username',
ldapRestrict,
authenticateAccessToken,
verifyAdminIfNeeded,
async (req, res) => {
@@ -99,14 +91,13 @@ userRouter.patch(
const response = await controller.updateUserByUsername(username, body)
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
}
)
userRouter.patch(
'/:userId',
ldapRestrict,
authenticateAccessToken,
verifyAdminIfNeeded,
async (req, res) => {
@@ -122,14 +113,13 @@ userRouter.patch(
const response = await controller.updateUser(parseInt(userId), body)
res.send(response)
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
}
)
userRouter.delete(
'/by/username/:username',
ldapRestrict,
authenticateAccessToken,
verifyAdminIfNeeded,
async (req, res) => {
@@ -151,14 +141,13 @@ userRouter.delete(
await controller.deleteUserByUsername(username, data, user!.isAdmin)
res.status(200).send('Account Deleted!')
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
}
)
userRouter.delete(
'/:userId',
ldapRestrict,
authenticateAccessToken,
verifyAdminIfNeeded,
async (req, res) => {
@@ -174,7 +163,7 @@ userRouter.delete(
await controller.deleteUser(parseInt(userId), data, user!.isAdmin)
res.status(200).send('Account Deleted!')
} catch (err: any) {
res.status(403).send(err.toString())
res.status(err.code).send(err.message)
}
}
)