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

api/src/controllers/group.ts

@@ -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()
+    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)

api/src/controllers/user.ts

@@ -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)

api/src/controllers/web.ts

@@ -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()

api/src/middlewares/desktop.ts

@@ -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',

api/src/routes/api/group.ts

@@ -1,17 +1,12 @@
 import express from 'express'
 import { GroupController } from '../../controllers/'
-import {
+import { authenticateAccessToken, verifyAdmin } from '../../middlewares'
-  ldapRestrict,
-  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
+      res.status(err.code).send(err.message)
-
-      delete err.code
-
-      res.status(statusCode).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
+    res.status(err.code).send(err.message)
-
-    delete err.code
-
-    res.status(statusCode).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
+    res.status(err.code).send(err.message)
-
-    delete err.code
-
-    res.status(statusCode).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
+      res.status(err.code).send(err.message)
-
-      delete err.code
-
-      res.status(statusCode).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
+      res.status(err.code).send(err.message)
-
-      delete err.code
-
-      res.status(statusCode).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
+      res.status(err.code).send(err.message)
-
-      delete err.code
-
-      res.status(statusCode).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
+      res.status(err.code).send(err.message)
-
-      delete err.code
-
-      res.status(statusCode).send(err.message)
     }
   }
 )

api/src/routes/api/spec/user.spec.ts

@@ -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({})
       })
     })

api/src/routes/api/user.ts

@@ -3,8 +3,7 @@ import { UserController } from '../../controllers/'
 import {
   authenticateAccessToken,
   verifyAdmin,
-  verifyAdminIfNeeded,
+  verifyAdminIfNeeded
-  ldapRestrict
 } from '../../middlewares'
 import {
   deleteUserValidation,
@@ -15,12 +14,7 @@ import {
 
 const userRouter = express.Router()
 
-userRouter.post(
+userRouter.post('/', authenticateAccessToken, verifyAdmin, async (req, res) => {
-  '/',
-  ldapRestrict,
-  authenticateAccessToken,
-  verifyAdmin,
-  async (req, res) => {
   const { error, value: body } = registerUserValidation(req.body)
   if (error) return res.status(400).send(error.details[0].message)
 
@@ -29,10 +23,9 @@ userRouter.post(
     const response = await controller.createUser(body)
     res.send(response)
   } catch (err: any) {
-      res.status(403).send(err.toString())
+    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)
     }
   }
 )