Skip to content

Commit

Permalink
Backend: show 403 Forbidden rather than 500 if access was not allowed:
Browse files Browse the repository at this point in the history
- I created a custom error class, AccessError, in order to distinguish access errors from the rest in our handleStandardResponse().
- The Service will now send an AccessError if the reason is that the authenticated user lacks autorizaton to a certain resource.
  • Loading branch information
jacobwod committed Dec 21, 2023
1 parent 467b3d2 commit 90b1725
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
10 changes: 6 additions & 4 deletions new-backend/server/apis/v2/services/config.service.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import fs from "fs";
import path from "path";
import log4js from "log4js";
import { XMLParser } from "fast-xml-parser";

import ad from "./activedirectory.service.js";
import asyncFilter from "../utils/asyncFilter.js";
import log4js from "log4js";
import getAnalyticsOptionsFromDotEnv from "../utils/getAnalyticsOptionsFromDotEnv.js";
import { XMLParser } from "fast-xml-parser";
import { AccessError } from "../utils/AccessError.js";

const logger = log4js.getLogger("service.config.v2");

Expand Down Expand Up @@ -81,7 +83,7 @@ class ConfigServiceV2 {

// First, ensure that we have a valid user name. This is necessary for AD lookups.
if ((await ad.isUserValid(user)) !== true) {
const e = new Error(
const e = new AccessError(
"[getMapConfig] AD authentication is active, but no valid user name was supplied. Access restricted."
);
logger.error(e.message);
Expand Down Expand Up @@ -119,7 +121,7 @@ class ConfigServiceV2 {

// If we got this far, it looks as the current user isn't member in any
// of the required groups - hence no access can be given to the map.
const e = new Error(
const e = new AccessError(
`[getMapConfig] Access to map "${map}" not allowed for user "${user}"`
);

Expand Down
18 changes: 18 additions & 0 deletions new-backend/server/apis/v2/utils/AccessError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @summary Basic Error class with unique 'name' property to make it easy
* to distinguish AccessError.
*
* @export
* @class AccessError
* @extends {Error}
*/

export class AccessError extends Error {
constructor(message, options) {
super(message, options);
}

get name() {
return "AccessError";
}
}
14 changes: 12 additions & 2 deletions new-backend/server/apis/v2/utils/handleStandardResponse.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@
* @param {*} data The data Promise that our various services return
*/
export default function handleStandardResponse(res, data, successStatus = 200) {
if (data.error) res.status(500).send(data.error.toString());
else res.status(successStatus).json(data);
// If we encountered a error…
if (data.error) {
// Check if it's AccessError. If so, send a 403 Forbidden.
// Otherwise, send a generic status 500.
res
.status(data.error.name === "AccessError" ? 403 : 500)
.send(data.error.toString());
}
// If there's no error, send the response
else {
res.status(successStatus).json(data);
}
}

0 comments on commit 90b1725

Please sign in to comment.