Skip to content
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.

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

RBAC tweaks #250

Merged
merged 7 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions services/api/.env
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ API_URL=http://localhost:2300
UPLOADS_STORE=local
UPLOADS_GCS_BUCKET=bedrock-staging-uploads

# Multi-Tenancy
DEFAULT_ORGANIZATION_NAME=

# Sentry Error Tracking
SENTRY_DSN=

Expand Down
2 changes: 1 addition & 1 deletion services/api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ const { requirePermissions } = require('../utils/middleware/permissions');
router
.use(authenticate())
// Only allow access to users that have write permissions for this organization
.use(requirePermissions({ endpoint: 'shops', level: 'write', context: 'organization' }))
.use(requirePermissions('shops.write', 'organization'))
.post(
'/',
validate({
Expand Down
2 changes: 2 additions & 0 deletions services/api/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const corsMiddleware = require('./utils/middleware/cors');
const bodyMiddleware = require('./utils/middleware/body');
const recordMiddleware = require('./utils/middleware/record');
const serializeMiddleware = require('./utils/middleware/serialize');
const organizationMiddleware = require('./utils/middleware/organization');
const { applicationMiddleware } = require('./utils/middleware/application');
const { loadDefinition } = require('./utils/openapi');
const Sentry = require('@sentry/node');
Expand Down Expand Up @@ -38,6 +39,7 @@ if (['staging', 'development'].includes(ENV_NAME)) {
}

app.use(serializeMiddleware);
app.use(organizationMiddleware);

// Record middleware must occur before serialization to
// derive model names but after errorHandler to capture
Expand Down
6 changes: 5 additions & 1 deletion services/api/src/models/definitions/user.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@
},
"scope": {
"type": "String",
"required": true
"required": true,
"enum": [
"global",
"organization"
]
},
"scopeRef": {
"type": "ObjectId",
Expand Down
4 changes: 1 addition & 3 deletions services/api/src/models/user.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
const mongoose = require('mongoose');
const { createSchema } = require('@bedrockio/model');

const { validScopes } = require('../utils/permissions');
const { setPassword } = require('../utils/auth/password');

const definition = require('./definitions/user.json');

definition.attributes.roles[0].scope.enum = validScopes;
const schema = createSchema(definition);

schema.virtual('name').get(function () {
Expand Down
9 changes: 0 additions & 9 deletions services/api/src/permissions.json

This file was deleted.

36 changes: 19 additions & 17 deletions services/api/src/roles.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
"global"
],
"permissions": {
"auditEntries": "read",
"users": "read-write",
"shops": "read-write",
"products": "read-write",
"organizations": "read-write",
"applications": "read-write",
"uploads": "read-write"
"applications": "all",
"auditEntries": "all",
"organizations": "all",
"products": "all",
"roles": "all",
"shops": "all",
"users": "all",
"uploads": "all"
},
"allowAuthenticationOnRoles": [
"admin",
Expand All @@ -21,28 +22,29 @@
"admin": {
"name": "Admin",
"allowScopes": [
"organization"
"global"
],
"permissions": {
"auditEntries": "read",
"users": "read-write",
"shops": "read-write",
"products": "read-write",
"applications": "read-write",
"uploads": "read-write"
"applications": "all",
"products": "all",
"roles": "all",
"shops": "all",
"users": "all",
"uploads": "all"
}
},
"viewer": {
"name": "Viewer",
"allowScopes": [
"organization"
"global"
],
"permissions": {
"applications": "read",
"auditEntries": "read",
"users": "read",
"shops": "read",
"products": "read",
"applications": "read",
"shops": "read",
"users": "read",
"uploads": "read"
}
}
Expand Down
5 changes: 3 additions & 2 deletions services/api/src/routes/__tests__/uploads.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('/1/uploads', () => {
expect(response.body.data.filename).toBe('test.png');
});

it('should not be able to access private upload as viewer', async () => {
it('should be able to access private upload as viewer', async () => {
const manager = await importFixtures('users/jack');
const upload = await createUpload({
private: true,
Expand All @@ -54,7 +54,8 @@ describe('/1/uploads', () => {
user: manager,
}
);
expect(response.status).toBe(401);
expect(response.status).toBe(200);
expect(response.body.data.filename).toBe('test.png');
});

it('should not allow access private upload when unauthenticated', async () => {
Expand Down
47 changes: 44 additions & 3 deletions services/api/src/routes/__tests__/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,28 @@ describe('/1/users', () => {
);
expect(response.status).toBe(403);
});

it('should error on invalid roles', async () => {
const admin = await createAdmin();
const response = await request(
'POST',
'/1/users',
{
email: '[email protected]',
password: 'verysecurepassword',
firstName: 'Mellow',
lastName: 'Yellow',
roles: [
{
role: 'superAdmin',
scope: 'organization',
},
],
},
{ user: admin }
);
expect(response.status).toBe(400);
});
});

describe('GET /:user', () => {
Expand Down Expand Up @@ -300,7 +322,7 @@ describe('/1/users', () => {
{
roles: [
{
role: 'limitedAdmin',
role: 'admin',
scope: 'global',
},
],
Expand All @@ -309,14 +331,33 @@ describe('/1/users', () => {
);
expect(response.status).toBe(200);
expect(response.body.data.roles.length).toBe(1);
expect(response.body.data.roles[0].role).toBe('limitedAdmin');
expect(response.body.data.roles[0].role).toBe('admin');
expect(response.body.data.roles[0].scope).toBe('global');
const dbUser = await User.findById(user1.id);
expect(dbUser.roles.length).toBe(1);
expect(dbUser.roles[0].role).toBe('limitedAdmin');
expect(dbUser.roles[0].role).toBe('admin');
expect(dbUser.roles[0].scope).toBe('global');
});

it('should error on invalid roles', async () => {
const admin = await createAdmin();
const user = await createUser({ firstName: 'New', lastName: 'Name' });
const response = await request(
'PATCH',
`/1/users/${user.id}`,
{
roles: [
{
role: 'superAdmin',
scope: 'organization',
},
],
},
{ user: admin }
);
expect(response.status).toBe(400);
});

it('should strip out reserved fields', async () => {
const admin = await createAdmin();
const user1 = await createUser({ firstName: 'New', lastName: 'Name' });
Expand Down
4 changes: 2 additions & 2 deletions services/api/src/routes/applications.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const router = new Router();

router
.use(authenticate())
.use(requirePermissions({ endpoint: 'applications', permission: 'read', scope: 'global' }))
.use(requirePermissions('applications.read'))
.param('id', fetchByParam(Application))
.post('/mine/search', validateBody(Application.getSearchValidation()), async (ctx) => {
const { body } = ctx.request;
Expand Down Expand Up @@ -54,7 +54,7 @@ router
data: ctx.state.application,
};
})
.use(requirePermissions({ endpoint: 'applications', permission: 'write', scope: 'global' }))
.use(requirePermissions('applications.write'))
.post('/', validateBody(Application.getCreateValidation()), async (ctx) => {
const { body } = ctx.request;
const apiKey = kebabCase(body.name);
Expand Down
2 changes: 1 addition & 1 deletion services/api/src/routes/audit-entries.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const router = new Router();

router
.use(authenticate())
.use(requirePermissions({ endpoint: 'auditEntries', permission: 'read', scope: 'global' }))
.use(requirePermissions('auditEntries.read'))
.post(
'/search',
validateBody(
Expand Down
4 changes: 2 additions & 2 deletions services/api/src/routes/invites.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ router
}
)
.use(authenticate())
.use(requirePermissions({ endpoint: 'users', permission: 'read', scope: 'global' }))
.use(requirePermissions('users.read'))
.param('id', fetchByParam(Invite))
.post('/search', validateBody(Invite.getSearchValidation()), async (ctx) => {
const { data, meta } = await Invite.search(ctx.request.body);
Expand All @@ -94,7 +94,7 @@ router
meta,
};
})
.use(requirePermissions({ endpoint: 'users', permission: 'write', scope: 'global' }))
.use(requirePermissions('users.invite'))
.post(
'/',
validateBody({
Expand Down
4 changes: 2 additions & 2 deletions services/api/src/routes/organizations.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ router
data: organization,
};
})
.use(requirePermissions({ endpoint: 'organizations', permission: 'read', scope: 'global' }))
.use(requirePermissions('organizations.read'))
.post('/search', validateBody(Organization.getSearchValidation()), async (ctx) => {
const { data, meta } = await Organization.search(ctx.request.body);
ctx.body = {
data,
meta,
};
})
.use(requirePermissions({ endpoint: 'organizations', permission: 'write', scope: 'global' }))
.use(requirePermissions('organizations.write'))
.post('/', validateBody(Organization.getCreateValidation()), async (ctx) => {
const organization = await Organization.create(ctx.request.body);
ctx.body = {
Expand Down
3 changes: 2 additions & 1 deletion services/api/src/routes/uploads.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { authenticate } = require('../utils/middleware/authenticate');
const { validateFiles } = require('../utils/middleware/validate');
const { createUploads, getUploadUrl, createUrlStream } = require('../utils/uploads');
const { userHasAccess } = require('./../utils/permissions');
const { isEqual } = require('@bedrockio/model');
const { Upload } = require('../models');

const router = new Router();
Expand Down Expand Up @@ -41,7 +42,7 @@ router
if (ctx.method === 'GET') {
return true;
} else {
return upload.owner?.equals(ctx.state.authUser.id);
return isEqual(upload.owner, ctx.state.authUser);
}
},
})
Expand Down
Loading
Loading