diff --git a/backend/src/middleware/permissionsMiddleware.js b/backend/src/middleware/permissionsMiddleware.js index bc9b4c525..077c4a2a4 100644 --- a/backend/src/middleware/permissionsMiddleware.js +++ b/backend/src/middleware/permissionsMiddleware.js @@ -1,4 +1,4 @@ -import { rule, shield, allow, or } from 'graphql-shield' +import { rule, shield, deny, allow, or } from 'graphql-shield' /* * TODO: implement @@ -16,6 +16,12 @@ const isAdmin = rule()(async (parent, args, { user }, info) => { return user && user.role === 'admin' }) +const onlyYourself = rule({ + cache: 'no_cache', +})(async (parent, args, context, info) => { + return context.user.id === args.id +}) + const isMyOwn = rule({ cache: 'no_cache', })(async (parent, args, context, info) => { @@ -48,6 +54,13 @@ const belongsToMe = rule({ return Boolean(notification) }) +/* TODO: decide if we want to remove this check: the check + * `onlyEnabledContent` throws authorization errors only if you have + * arguments for `disabled` or `deleted` assuming these are filter + * parameters. Soft-delete middleware obfuscates data on its way out + * anyways. Furthermore, `neo4j-graphql-js` offers many ways to filter for + * data so I believe, this is not a good check anyways. + */ const onlyEnabledContent = rule({ cache: 'strict', })(async (parent, args, ctx, info) => { @@ -81,46 +94,58 @@ const isAuthor = rule({ }) // Permissions -const permissions = shield({ - Query: { - Notification: isAdmin, - statistics: allow, - currentUser: allow, - Post: or(onlyEnabledContent, isModerator), +const permissions = shield( + { + Query: { + '*': deny, + Notification: isAdmin, + statistics: allow, + currentUser: allow, + Post: or(onlyEnabledContent, isModerator), + Comment: allow, + User: allow, + isLoggedIn: allow, + }, + Mutation: { + '*': deny, + login: allow, + UpdateNotification: belongsToMe, + CreateUser: isAdmin, + UpdateUser: onlyYourself, + CreatePost: isAuthenticated, + UpdatePost: isAuthor, + DeletePost: isAuthor, + report: isAuthenticated, + CreateBadge: isAdmin, + UpdateBadge: isAdmin, + DeleteBadge: isAdmin, + AddUserBadges: isAdmin, + CreateSocialMedia: isAuthenticated, + DeleteSocialMedia: isAuthenticated, + // AddBadgeRewarded: isAdmin, + // RemoveBadgeRewarded: isAdmin, + reward: isAdmin, + unreward: isAdmin, + // addFruitToBasket: isAuthenticated + follow: isAuthenticated, + unfollow: isAuthenticated, + shout: isAuthenticated, + unshout: isAuthenticated, + changePassword: isAuthenticated, + enable: isModerator, + disable: isModerator, + CreateComment: isAuthenticated, + DeleteComment: isAuthor, + }, + User: { + email: isMyOwn, + password: isMyOwn, + privateKey: isMyOwn, + }, }, - Mutation: { - UpdateNotification: belongsToMe, - CreatePost: isAuthenticated, - UpdatePost: isAuthor, - DeletePost: isAuthor, - report: isAuthenticated, - CreateBadge: isAdmin, - UpdateBadge: isAdmin, - DeleteBadge: isAdmin, - AddUserBadges: isAdmin, - CreateSocialMedia: isAuthenticated, - DeleteSocialMedia: isAuthenticated, - // AddBadgeRewarded: isAdmin, - // RemoveBadgeRewarded: isAdmin, - reward: isAdmin, - unreward: isAdmin, - // addFruitToBasket: isAuthenticated - follow: isAuthenticated, - unfollow: isAuthenticated, - shout: isAuthenticated, - unshout: isAuthenticated, - changePassword: isAuthenticated, - enable: isModerator, - disable: isModerator, - CreateComment: isAuthenticated, - DeleteComment: isAuthor, - // CreateUser: allow, + { + fallbackRule: allow, }, - User: { - email: isMyOwn, - password: isMyOwn, - privateKey: isMyOwn, - }, -}) +) export default permissions diff --git a/backend/src/middleware/slugifyMiddleware.spec.js b/backend/src/middleware/slugifyMiddleware.spec.js index 79bba0a5d..4e060dc90 100644 --- a/backend/src/middleware/slugifyMiddleware.spec.js +++ b/backend/src/middleware/slugifyMiddleware.spec.js @@ -7,12 +7,14 @@ let headers const factory = Factory() beforeEach(async () => { - await factory.create('User', { email: 'user@example.org', password: '1234' }) + const adminParams = { role: 'admin', email: 'admin@example.org', password: '1234' } + await factory.create('User', adminParams) await factory.create('User', { email: 'someone@example.org', password: '1234', }) - headers = await login({ email: 'user@example.org', password: '1234' }) + // we need to be an admin, otherwise we're not authorized to create a user + headers = await login(adminParams) authenticatedClient = new GraphQLClient(host, { headers }) }) diff --git a/backend/src/schema/resolvers/user_management.spec.js b/backend/src/schema/resolvers/user_management.spec.js index cf648a6bd..463c5ea6d 100644 --- a/backend/src/schema/resolvers/user_management.spec.js +++ b/backend/src/schema/resolvers/user_management.spec.js @@ -315,6 +315,8 @@ describe('change password', () => { describe('do not expose private RSA key', () => { let headers let client + let authenticatedClient + const queryUserPuplicKey = gql` query($queriedUserSlug: String) { User(slug: $queriedUserSlug) { @@ -332,7 +334,7 @@ describe('do not expose private RSA key', () => { } ` - const actionGenUserWithKeys = async () => { + const generateUserWithKeys = async authenticatedClient => { // Generate user with "privateKey" via 'CreateUser' mutation instead of using the factories "factory.create('User', {...})", see above. const variables = { id: 'bcb2d923-f3af-479e-9f00-61b12e864667', @@ -341,7 +343,7 @@ describe('do not expose private RSA key', () => { name: 'Apfel Strudel', email: 'apfel-strudel@test.org', } - await client.request( + await authenticatedClient.request( gql` mutation($id: ID, $password: String!, $slug: String, $name: String, $email: String!) { CreateUser(id: $id, password: $password, slug: $slug, name: $name, email: $email) { @@ -353,14 +355,23 @@ describe('do not expose private RSA key', () => { ) } - // not authenticate beforeEach(async () => { + const adminParams = { + role: 'admin', + email: 'admin@example.org', + password: '1234', + } + // create an admin user who has enough permissions to create other users + await factory.create('User', adminParams) + const headers = await login(adminParams) + authenticatedClient = new GraphQLClient(host, { headers }) + // but also create an unauthenticated client to issue the `User` query client = new GraphQLClient(host) }) describe('unauthenticated query of "publicKey" (does the RSA key pair get generated at all?)', () => { it('returns publicKey', async () => { - await actionGenUserWithKeys() + await generateUserWithKeys(authenticatedClient) await expect( await client.request(queryUserPuplicKey, { queriedUserSlug: 'apfel-strudel' }), ).toEqual( @@ -378,7 +389,7 @@ describe('do not expose private RSA key', () => { describe('unauthenticated query of "privateKey"', () => { it('throws "Not Authorised!"', async () => { - await actionGenUserWithKeys() + await generateUserWithKeys(authenticatedClient) await expect( client.request(queryUserPrivateKey, { queriedUserSlug: 'apfel-strudel' }), ).rejects.toThrow('Not Authorised') @@ -393,7 +404,7 @@ describe('do not expose private RSA key', () => { describe('authenticated query of "publicKey"', () => { it('returns publicKey', async () => { - await actionGenUserWithKeys() + await generateUserWithKeys(authenticatedClient) await expect( await client.request(queryUserPuplicKey, { queriedUserSlug: 'apfel-strudel' }), ).toEqual( @@ -411,7 +422,7 @@ describe('do not expose private RSA key', () => { describe('authenticated query of "privateKey"', () => { it('throws "Not Authorised!"', async () => { - await actionGenUserWithKeys() + await generateUserWithKeys(authenticatedClient) await expect( client.request(queryUserPrivateKey, { queriedUserSlug: 'apfel-strudel' }), ).rejects.toThrow('Not Authorised') diff --git a/backend/src/schema/resolvers/users.spec.js b/backend/src/schema/resolvers/users.spec.js index a5c50f4f9..6334272dd 100644 --- a/backend/src/schema/resolvers/users.spec.js +++ b/backend/src/schema/resolvers/users.spec.js @@ -1,5 +1,5 @@ import { GraphQLClient } from 'graphql-request' -import { host } from '../../jest/helpers' +import { login, host } from '../../jest/helpers' import Factory from '../../seed/factories' const factory = Factory() @@ -18,27 +18,58 @@ describe('users', () => { } } ` - client = new GraphQLClient(host) - - it('with password and email', async () => { + describe('given valid password and email', () => { const variables = { name: 'John Doe', password: '123', email: '123@123.de', } - const expected = { - CreateUser: { - id: expect.any(String), - }, - } - await expect(client.request(mutation, variables)).resolves.toEqual(expected) + + describe('unauthenticated', () => { + beforeEach(async () => { + client = new GraphQLClient(host) + }) + + it('is not allowed to create users', async () => { + await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + }) + }) + + describe('authenticated admin', () => { + beforeEach(async () => { + const adminParams = { + role: 'admin', + email: 'admin@example.org', + password: '1234', + } + await factory.create('User', adminParams) + const headers = await login(adminParams) + client = new GraphQLClient(host, { headers }) + }) + + it('is allowed to create new users', async () => { + const expected = { + CreateUser: { + id: expect.any(String), + }, + } + await expect(client.request(mutation, variables)).resolves.toEqual(expected) + }) + }) }) }) describe('UpdateUser', () => { - beforeEach(async () => { - await factory.create('User', { id: 'u47', name: 'John Doe' }) - }) + const userParams = { + email: 'user@example.org', + password: '1234', + id: 'u47', + name: 'John Doe', + } + const variables = { + id: 'u47', + name: 'John Doughnut', + } const mutation = ` mutation($id: ID!, $name: String) { @@ -48,38 +79,62 @@ describe('users', () => { } } ` - client = new GraphQLClient(host) - it('name within specifications', async () => { - const variables = { - id: 'u47', - name: 'James Doe', - } - const expected = { - UpdateUser: { - id: 'u47', + beforeEach(async () => { + await factory.create('User', userParams) + }) + + describe('as another user', () => { + beforeEach(async () => { + const someoneElseParams = { + email: 'someoneElse@example.org', + password: '1234', name: 'James Doe', - }, - } - await expect(client.request(mutation, variables)).resolves.toEqual(expected) + } + + await factory.create('User', someoneElseParams) + const headers = await login(someoneElseParams) + client = new GraphQLClient(host, { headers }) + }) + + it('is not allowed to change other user accounts', async () => { + await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + }) }) - it('with no name', async () => { - const variables = { - id: 'u47', - name: null, - } - const expected = 'Username must be at least 3 characters long!' - await expect(client.request(mutation, variables)).rejects.toThrow(expected) - }) + describe('as the same user', () => { + beforeEach(async () => { + const headers = await login(userParams) + client = new GraphQLClient(host, { headers }) + }) - it('with too short name', async () => { - const variables = { - id: 'u47', - name: ' ', - } - const expected = 'Username must be at least 3 characters long!' - await expect(client.request(mutation, variables)).rejects.toThrow(expected) + it('name within specifications', async () => { + const expected = { + UpdateUser: { + id: 'u47', + name: 'John Doughnut', + }, + } + await expect(client.request(mutation, variables)).resolves.toEqual(expected) + }) + + it('with no name', async () => { + const variables = { + id: 'u47', + name: null, + } + const expected = 'Username must be at least 3 characters long!' + await expect(client.request(mutation, variables)).rejects.toThrow(expected) + }) + + it('with too short name', async () => { + const variables = { + id: 'u47', + name: ' ', + } + const expected = 'Username must be at least 3 characters long!' + await expect(client.request(mutation, variables)).rejects.toThrow(expected) + }) }) }) })