Skip to content

Commit 0490e32

Browse files
Merge pull request #339 from Cartmanishere/authorization
Add authorization middleware based on roles
2 parents 35466aa + 8d94875 commit 0490e32

File tree

12 files changed

+276
-59
lines changed

12 files changed

+276
-59
lines changed

controllers/users.js

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11

22
const userQuery = require('../models/users')
3-
const accountOwners = require('../mockdata/appOwners')
43
const imageService = require('../services/imageService')
54
/**
65
* Fetches the data about our users
@@ -23,22 +22,6 @@ const getUsers = async (req, res) => {
2322
}
2423
}
2524

26-
/**
27-
* Fetches the data about our account Owners
28-
*
29-
* @param req {Object} - Express request object
30-
* @param res {Object} - Express response object
31-
*/
32-
33-
const getAccountOwners = async (req, res) => {
34-
try {
35-
return accountOwners
36-
} catch (error) {
37-
logger.error(`Error while fetching application owners: ${error}`)
38-
return res.boom.badImplementation('Something went wrong please contact admin')
39-
}
40-
}
41-
4225
/**
4326
* Fetches the data about user with given id
4427
*
@@ -166,6 +149,5 @@ module.exports = {
166149
getSelfDetails,
167150
getUser,
168151
getUsernameAvailabilty,
169-
getAccountOwners,
170152
postUserPicture
171153
}

firebase.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"emulators": {
33
"firestore": {
4-
"port": 8080
4+
"port": 8081
55
},
66
"ui": {
77
"enabled": true,

middlewares/authorization.js

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,54 @@
1-
const { isSuperUser } = require('../models/members')
2-
module.exports = (req, res, next) => {
3-
try {
4-
const { username } = req.userData
5-
if (!isSuperUser(username)) {
6-
return res.boom.forbidden('You are not allowed to perform this action.')
1+
/**
2+
* Describe the priority order of roles. Any route requiring `x` role is allowed
3+
* for user having authority of role `x` or a higher authority.
4+
* For e.g
5+
* - Route requiring `superUser` role is only allowed for `super_user`.
6+
* - Route requiring `appOwner` role is allowed for `superUser` and `app_owner`.
7+
*/
8+
const REQUIRED_ROLES_PRIORITY = {
9+
superUser: ['super_user'],
10+
appOwner: ['app_owner', 'super_user'],
11+
default: ['default', 'super_user', 'app_owner']
12+
}
13+
14+
/**
15+
* Check if the user has enough authorization based on their role.
16+
* User would have following roles schema -
17+
* userRoles = {app_owner: true, super_user: true}
18+
* @param {String} requiredRole - Required role level to authorize request.
19+
* @param {Object} userRoles - Roles information of the current user.
20+
* @returns {Boolean} - Whether the current user is authorized for required role level.
21+
*/
22+
const userHasPermission = (requiredRole, userRoles) => {
23+
const allowedRoles = REQUIRED_ROLES_PRIORITY[`${requiredRole}`] || ['default']
24+
return allowedRoles.some((role) => {
25+
return Boolean(userRoles[`${role}`])
26+
})
27+
}
28+
29+
/**
30+
* Create an authorization middleware for a route based on the required role needed
31+
* for that route. Currently following roles are supported:
32+
* - `authorizeUser('superUser')`
33+
* - `authorizeUser('appOwner')`
34+
* Note: This must be added on routes after the `authenticate` middleware.
35+
* @param {String} requiredRole - The least role authority required for a route.
36+
* @returns {Function} - A middleware function that authorizes given role.
37+
*/
38+
const authorizeUser = (requiredRole) => {
39+
return (req, res, next) => {
40+
const { roles = {} } = req.userData
41+
// All users should have `default` role
42+
roles.default = true
43+
44+
if (!userHasPermission(requiredRole, roles)) {
45+
return res.boom.unauthorized('You are not authorized for this action.')
746
}
847
return next()
9-
} catch (err) {
10-
logger.error(err)
11-
return res.boom.forbidden('You are not allowed to perform this action.')
1248
}
1349
}
50+
51+
module.exports = {
52+
authorizeUser,
53+
userHasPermission
54+
}

models/members.js

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,6 @@ const fetchMembers = async () => {
3737
}
3838
}
3939

40-
/**
41-
* Checks whether the user is a superuser
42-
* @return {Boolean}
43-
*/
44-
45-
const isSuperUser = (username) => {
46-
return username === 'ankush'
47-
}
48-
4940
module.exports = {
50-
fetchMembers,
51-
isSuperUser
41+
fetchMembers
5242
}

routes/stocks.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
const express = require('express')
22
const router = express.Router()
33
const authenticate = require('../middlewares/authenticate')
4-
const authorization = require('../middlewares/authorization')
5-
const { addNewStock, fetchStocks, getSelfStocks } = require('../controllers/stocks')
4+
const { authorizeUser } = require('../middlewares/authorization')
5+
const { addNewStock, fetchStocks } = require('../controllers/stocks')
66
const { createStock } = require('../middlewares/validators/stocks')
77

88
/**
@@ -62,7 +62,7 @@ router.get('/', fetchStocks)
6262
* schema:
6363
* $ref: '#/components/schemas/errors/badImplementation'
6464
*/
65-
router.post('/', authenticate, authorization, createStock, addNewStock)
65+
router.post('/', authenticate, authorizeUser('superUser'), createStock, addNewStock)
6666

6767
/**
6868
* @swagger

routes/tasks.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const router = express.Router()
33
const authenticate = require('../middlewares/authenticate')
44
const tasks = require('../controllers/tasks')
55
const { createTask, updateTask } = require('../middlewares/validators/tasks')
6-
const authorizeOwner = require('../middlewares/authorizeOwner')
6+
const { authorizeUser } = require('../middlewares/authorization')
77

88
/**
99
* @swagger
@@ -93,7 +93,7 @@ router.get('/self', authenticate, tasks.getSelfTasks)
9393
* schema:
9494
* $ref: '#/components/schemas/errors/badImplementation'
9595
*/
96-
router.post('/', authenticate, authorizeOwner, createTask, tasks.addNewTask)
96+
router.post('/', authenticate, authorizeUser('appOwner'), createTask, tasks.addNewTask)
9797

9898
/**
9999
* @swagger
@@ -124,7 +124,7 @@ router.post('/', authenticate, authorizeOwner, createTask, tasks.addNewTask)
124124
* schema:
125125
* $ref: '#/components/schemas/errors/badImplementation'
126126
*/
127-
router.patch('/:id', authenticate, authorizeOwner, updateTask, tasks.updateTask)
127+
router.patch('/:id', authenticate, authorizeUser('appOwner'), updateTask, tasks.updateTask)
128128

129129
/**
130130
* @swagger

routes/wallets.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const express = require('express')
22
const router = express.Router()
33
const wallet = require('../controllers/wallets')
44
const authenticate = require('../middlewares/authenticate')
5-
const authorization = require('../middlewares/authorization')
5+
const { authorizeUser } = require('../middlewares/authorization')
66

77
/**
88
* @swagger
@@ -66,6 +66,6 @@ router.get('/', authenticate, wallet.getOwnWallet)
6666
* schema:
6767
* $ref: '#/components/schemas/errors/badImplementation'
6868
*/
69-
router.get('/:username', authenticate, authorization, wallet.getUserWallet)
69+
router.get('/:username', authenticate, authorizeUser('superUser'), wallet.getUserWallet)
7070

7171
module.exports = router

test/fixtures/user/user.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,43 @@ module.exports = () => {
5252
roles: {
5353
restricted: true
5454
}
55+
},
56+
{
57+
username: 'sagar',
58+
first_name: 'Sagar',
59+
last_name: 'Bajpai',
60+
yoe: 3,
61+
img: './img.png',
62+
linkedin_id: 'sagarbajpai',
63+
github_id: 'sagarbajpai',
64+
github_display_name: 'Sagar Bajpai',
65+
phone: '1234567890',
66+
67+
tokens: {
68+
githubAccessToken: 'githubAccessToken'
69+
},
70+
roles: {
71+
restricted: false,
72+
app_owner: true
73+
}
74+
},
75+
{
76+
username: 'ankush',
77+
first_name: 'Ankush',
78+
last_name: 'Dharkar',
79+
yoe: 10,
80+
img: './img.png',
81+
linkedin_id: 'ankushdharkar',
82+
github_id: 'ankushdharkar',
83+
github_display_name: 'Ankush Dharkar',
84+
phone: '1234567890',
85+
86+
tokens: {
87+
githubAccessToken: 'githubAccessToken'
88+
},
89+
roles: {
90+
super_user: true
91+
}
5592
}
5693
]
5794
}
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
const chai = require('chai')
2+
const { expect } = chai
3+
4+
const { authorizeUser } = require('../../middlewares/authorization')
5+
const authenticate = require('../../middlewares/authenticate')
6+
const authService = require('../../services/authService')
7+
const addUser = require('../utils/addUser')
8+
const cleanDb = require('../utils/cleanDb')
9+
const config = require('config')
10+
const cookieName = config.get('userToken.cookieName')
11+
const userData = require('../fixtures/user/user')()
12+
13+
const defaultUser = userData[0] // user with no `roles` key
14+
const appOwner = userData[3]
15+
const superUser = userData[4]
16+
17+
// Setup some routes with various permissions for testing
18+
const express = require('express')
19+
const router = express.Router()
20+
const AppMiddlewares = require('../../middlewares')
21+
22+
const pongHandler = (_, res) => {
23+
return res.json({ message: 'pong' })
24+
}
25+
26+
router.get('/for-everyone', authenticate, pongHandler)
27+
router.get('/for-app-owner', authenticate, authorizeUser('appOwner'), pongHandler)
28+
router.get('/for-super-user', authenticate, authorizeUser('superUser'), pongHandler)
29+
30+
const app = express()
31+
AppMiddlewares(app)
32+
app.use('/', router)
33+
34+
describe('authorizeUser', function () {
35+
let defaultJwt, appOwnerJwt, superUserJwt
36+
37+
before(async function () {
38+
const defaultUserId = await addUser(defaultUser)
39+
const appOwnerId = await addUser(appOwner)
40+
const superUserId = await addUser(superUser)
41+
defaultJwt = authService.generateAuthToken({ userId: defaultUserId })
42+
appOwnerJwt = authService.generateAuthToken({ userId: appOwnerId })
43+
superUserJwt = authService.generateAuthToken({ userId: superUserId })
44+
})
45+
46+
after(async function () {
47+
await cleanDb()
48+
})
49+
50+
it('should authorize default user for route with no required role', function (done) {
51+
chai
52+
.request(app)
53+
.get('/for-everyone')
54+
.set('cookie', `${cookieName}=${defaultJwt}`)
55+
.end((err, res) => {
56+
if (err) { return done(err) }
57+
expect(res).to.have.status(200)
58+
return done()
59+
})
60+
})
61+
62+
it('should authorize user with role for route with no required role', function (done) {
63+
chai
64+
.request(app)
65+
.get('/for-everyone')
66+
.set('cookie', `${cookieName}=${appOwnerJwt}`)
67+
.end((err, res) => {
68+
if (err) { return done(err) }
69+
expect(res).to.have.status(200)
70+
return done()
71+
})
72+
})
73+
74+
it('should authorize appOwner for route with appOwner required role', function (done) {
75+
chai
76+
.request(app)
77+
.get('/for-app-owner')
78+
.set('cookie', `${cookieName}=${appOwnerJwt}`)
79+
.end((err, res) => {
80+
if (err) { return done(err) }
81+
expect(res).to.have.status(200)
82+
return done()
83+
})
84+
})
85+
86+
it('should not allow user not having role for route with appOwner required role', function (done) {
87+
chai
88+
.request(app)
89+
.get('/for-app-owner')
90+
.set('cookie', `${cookieName}=${defaultJwt}`)
91+
.end((err, res) => {
92+
if (err) { return done(err) }
93+
expect(res).to.have.status(401)
94+
return done()
95+
})
96+
})
97+
98+
it('should authorize superUser for route with appOwner required role', function (done) {
99+
chai
100+
.request(app)
101+
.get('/for-app-owner')
102+
.set('cookie', `${cookieName}=${superUserJwt}`)
103+
.end((err, res) => {
104+
if (err) { return done(err) }
105+
expect(res).to.have.status(200)
106+
return done()
107+
})
108+
})
109+
110+
it('should authorize superUser for route with superUser required role', function (done) {
111+
chai
112+
.request(app)
113+
.get('/for-super-user')
114+
.set('cookie', `${cookieName}=${superUserJwt}`)
115+
.end((err, res) => {
116+
if (err) { return done(err) }
117+
expect(res).to.have.status(200)
118+
return done()
119+
})
120+
})
121+
122+
it('should not allow appOwner for route with superUser required role', function (done) {
123+
chai
124+
.request(app)
125+
.get('/for-super-user')
126+
.set('cookie', `${cookieName}=${appOwnerJwt}`)
127+
.end((err, res) => {
128+
if (err) { return done(err) }
129+
expect(res).to.have.status(401)
130+
return done()
131+
})
132+
})
133+
})

0 commit comments

Comments
 (0)