fix: update to return authorization result instead of throwing exception

fonoster · Dec 28, 2024 · b48f4ea · b48f4ea
1 parent 1010376
commit b48f4ea
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 31 deletions.
diff --git a/mods/authz/src/makeCheckMethodAuthorized.ts b/mods/authz/src/makeCheckMethodAuthorized.ts
@@ -56,33 +56,49 @@ function makeCheckMethodAuthorized(authzServer: string, methods: string[]) {
       return call;
     }
 
-    logger.silly("checking if method is authorized", { method });
-
     const accessKeyId = getAccessKeyIdFromCall(call);
 
+    logger.verbose("checking if method is authorized", { method, accessKeyId });
+
     return new ServerInterceptingCall(call, {
       start: async (next) => {
         try {
-          await authz.checkMethodAuthorized({
+          const authorized = await authz.checkMethodAuthorized({
             accessKeyId,
             method
           } as CheckMethodAuthorizedRequest);
 
-          logger.verbose("method authorized by external service", {
+          logger.verbose("the status of the method authorization", {
             method,
-            accessKeyId
+            accessKeyId,
+            authorized
           });
 
+          if (!authorized) {
+            logger.verbose("method unauthorized by external service", {
+              method,
+              accessKeyId
+            });
+            createInterceptingCall({
+              call,
+              code: status.PERMISSION_DENIED,
+              details: `Method '${method}' unauthorized by external service - accessKeyId ${accessKeyId}`
+            });
+            return;
+          }
+
           next();
         } catch (error) {
-          logger.verbose("method unauthorized by external service", {
+          logger.error("error checking if method is authorized", {
             method,
-            accessKeyId
+            accessKeyId,
+            error
           });
+
           createInterceptingCall({
             call,
-            code: status.PERMISSION_DENIED,
-            details: `Method '${method}' unauthorized by external service - accessKeyId ${accessKeyId}`
+            code: status.INTERNAL,
+            details: "Internal server error"
           });
         }
       }

diff --git a/mods/authz/src/server/AuthzServer.ts b/mods/authz/src/server/AuthzServer.ts
@@ -54,19 +54,12 @@ class AuthzServer {
           logger.verbose("checkSessionAuthorized called", call.request);
 
           try {
-            const isAuthorized = await handler.checkSessionAuthorized(
+            const authorized = await handler.checkSessionAuthorized(
               call.request
             );
-            if (isAuthorized) {
-              callback(null, { authorized: true });
-            } else {
-              callback({
-                code: grpc.status.PERMISSION_DENIED,
-                message: "Session is not authorized."
-              });
-            }
+            callback(null, { authorized });
           } catch (error) {
-            logger.error("Error in checkSessionAuthorized:", error);
+            logger.error("error in checkSessionAuthorized:", error);
             callback({
               code: grpc.status.INTERNAL,
               message: "Internal server error."
@@ -80,19 +73,12 @@ class AuthzServer {
           logger.verbose("checkMethodAuthorized called", call.request);
 
           try {
-            const isAuthorized = await handler.checkMethodAuthorized(
+            const authorized = await handler.checkMethodAuthorized(
               call.request
             );
-            if (isAuthorized) {
-              callback(null, { authorized: true });
-            } else {
-              callback({
-                code: grpc.status.PERMISSION_DENIED,
-                message: "Method is not authorized."
-              });
-            }
+            callback(null, { authorized });
           } catch (error) {
-            logger.error("Error in checkMethodAuthorized:", error);
+            logger.error("error in checkMethodAuthorized:", error);
             callback({
               code: grpc.status.INTERNAL,
               message: "Internal server error."

diff --git a/mods/common/src/errors/handleError.ts b/mods/common/src/errors/handleError.ts
@@ -101,8 +101,8 @@ function handleError(
       );
       break;
     default:
-      logger.error("unknown error:", error);
-      callback({ code: status.UNKNOWN, message: "Unknown error" });
+      logger.error("internal server error:", error);
+      callback({ code: status.INTERNAL, message: "Internal server error" });
   }
 }