Skip to content

Commit f1508b2

Browse files
authored
Merge pull request #486 from OpenBeta/fix/auth-401-response
fix: return HTTP 401 for unauthorized mutations
2 parents 14713ac + 634426e commit f1508b2

File tree

4 files changed

+16
-7
lines changed

4 files changed

+16
-7
lines changed

src/__tests__/bulkImport.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,26 +64,26 @@ describe('bulkImportAreas', () => {
6464
await inMemoryDB.close()
6565
})
6666

67-
it('should return 403 if no user', async () => {
67+
it('should return 401 if no user', async () => {
6868
const res = await queryAPI({
6969
app,
7070
query,
7171
operationName: 'bulkImportAreas',
7272
variables: {input: exampleImportData}
7373
})
74-
expect(res.statusCode).toBe(200)
74+
expect(res.statusCode).toBe(401)
7575
expect(res.body.errors[0].message).toBe('Not Authorised!')
7676
})
7777

78-
it('should return 403 if user is not an editor', async () => {
78+
it('should return 401 if user is not an editor', async () => {
7979
const res = await queryAPI({
8080
app,
8181
userUuid,
8282
query,
8383
operationName: 'bulkImportAreas',
8484
variables: {input: exampleImportData}
8585
})
86-
expect(res.statusCode).toBe(200)
86+
expect(res.statusCode).toBe(401)
8787
expect(res.body.errors[0].message).toBe('Not Authorised!')
8888
})
8989

src/__tests__/organizations.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ describe('organizations API', () => {
170170
roles: ['editor'],
171171
app
172172
})
173-
expect(response.statusCode).toBe(200)
173+
expect(response.statusCode).toBe(401)
174174
expect(response.body.data.organization).toBeNull()
175175
expect(response.body.errors[0].message).toBe('Not Authorised!')
176176
})

src/auth/middleware.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ async function validateTokenAndExtractUser (req: Request): Promise<CustomContext
4545
}
4646
} catch (e) {
4747
logger.error(`Can't verify JWT token ${e.toString() as string}`)
48-
throw new Error("Unauthorized. Can't verify JWT token")
48+
// Return empty user instead of throwing - allows public queries to work
49+
// Mutations will be blocked by graphql-shield permissions
50+
return { user: EMTPY_USER }
4951
}
5052
}
5153

src/auth/permissions.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { GraphQLError } from 'graphql'
12
import { allow, and, or, shield } from 'graphql-shield'
23
import { isEditor, isMediaOwner, isOwner, isUserAdmin, isValidEmail } from './rules.js'
34

@@ -24,7 +25,13 @@ const permissions = shield({
2425
},
2526
{
2627
allowExternalErrors: true,
27-
fallbackRule: allow
28+
fallbackRule: allow,
29+
fallbackError: new GraphQLError('Not Authorised!', {
30+
extensions: {
31+
code: 'FORBIDDEN',
32+
http: { status: 401 }
33+
}
34+
})
2835
})
2936

3037
export default permissions

0 commit comments

Comments
 (0)