Fix auth desugar panic path

ballerina-platform · Sep 1, 2022 · f68cb63 · f68cb63
1 parent 2cd4b90
commit f68cb63
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 40 deletions.
diff --git a/ballerina-tests/Dependencies.toml b/ballerina-tests/Dependencies.toml
@@ -19,6 +19,9 @@ dependencies = [
 	{org = "ballerina", name = "log"},
 	{org = "ballerina", name = "regex"}
 ]
+modules = [
+	{org = "ballerina", packageName = "auth", moduleName = "auth"}
+]
 
 [[package]]
 org = "ballerina"
@@ -39,6 +42,9 @@ scope = "testOnly"
 dependencies = [
 	{org = "ballerina", name = "jballerina.java"}
 ]
+modules = [
+	{org = "ballerina", packageName = "constraint", moduleName = "constraint"}
+]
 
 [[package]]
 org = "ballerina"
@@ -49,6 +55,9 @@ dependencies = [
 	{org = "ballerina", name = "jballerina.java"},
 	{org = "ballerina", name = "time"}
 ]
+modules = [
+	{org = "ballerina", packageName = "crypto", moduleName = "crypto"}
+]
 
 [[package]]
 org = "ballerina"
@@ -63,6 +72,9 @@ dependencies = [
 	{org = "ballerina", name = "regex"},
 	{org = "ballerina", name = "time"}
 ]
+modules = [
+	{org = "ballerina", packageName = "file", moduleName = "file"}
+]
 
 [[package]]
 org = "ballerina"
@@ -101,9 +113,22 @@ org = "ballerina"
 name = "http_tests"
 version = "2.4.0"
 dependencies = [
+	{org = "ballerina", name = "auth"},
+	{org = "ballerina", name = "constraint"},
+	{org = "ballerina", name = "crypto"},
+	{org = "ballerina", name = "file"},
 	{org = "ballerina", name = "http"},
+	{org = "ballerina", name = "io"},
+	{org = "ballerina", name = "jballerina.java"},
+	{org = "ballerina", name = "jwt"},
+	{org = "ballerina", name = "lang.boolean"},
+	{org = "ballerina", name = "lang.float"},
+	{org = "ballerina", name = "lang.int"},
+	{org = "ballerina", name = "lang.runtime"},
 	{org = "ballerina", name = "lang.string"},
+	{org = "ballerina", name = "lang.xml"},
 	{org = "ballerina", name = "mime"},
+	{org = "ballerina", name = "oauth2"},
 	{org = "ballerina", name = "regex"},
 	{org = "ballerina", name = "test"},
 	{org = "ballerina", name = "url"}
@@ -121,12 +146,18 @@ dependencies = [
 	{org = "ballerina", name = "jballerina.java"},
 	{org = "ballerina", name = "lang.value"}
 ]
+modules = [
+	{org = "ballerina", packageName = "io", moduleName = "io"}
+]
 
 [[package]]
 org = "ballerina"
 name = "jballerina.java"
 version = "0.0.0"
 scope = "testOnly"
+modules = [
+	{org = "ballerina", packageName = "jballerina.java", moduleName = "jballerina.java"}
+]
 
 [[package]]
 org = "ballerina"
@@ -143,6 +174,9 @@ dependencies = [
 	{org = "ballerina", name = "regex"},
 	{org = "ballerina", name = "time"}
 ]
+modules = [
+	{org = "ballerina", packageName = "jwt", moduleName = "jwt"}
+]
 
 [[package]]
 org = "ballerina"
@@ -164,6 +198,18 @@ dependencies = [
 	{org = "ballerina", name = "lang.__internal"}
 ]
 
+[[package]]
+org = "ballerina"
+name = "lang.boolean"
+version = "0.0.0"
+scope = "testOnly"
+dependencies = [
+	{org = "ballerina", name = "jballerina.java"}
+]
+modules = [
+	{org = "ballerina", packageName = "lang.boolean", moduleName = "lang.boolean"}
+]
+
 [[package]]
 org = "ballerina"
 name = "lang.decimal"
@@ -173,6 +219,18 @@ dependencies = [
 	{org = "ballerina", name = "jballerina.java"}
 ]
 
+[[package]]
+org = "ballerina"
+name = "lang.float"
+version = "0.0.0"
+scope = "testOnly"
+dependencies = [
+	{org = "ballerina", name = "jballerina.java"}
+]
+modules = [
+	{org = "ballerina", packageName = "lang.float", moduleName = "lang.float"}
+]
+
 [[package]]
 org = "ballerina"
 name = "lang.int"
@@ -181,6 +239,9 @@ scope = "testOnly"
 dependencies = [
 	{org = "ballerina", name = "jballerina.java"}
 ]
+modules = [
+	{org = "ballerina", packageName = "lang.int", moduleName = "lang.int"}
+]
 
 [[package]]
 org = "ballerina"
@@ -196,6 +257,9 @@ scope = "testOnly"
 dependencies = [
 	{org = "ballerina", name = "jballerina.java"}
 ]
+modules = [
+	{org = "ballerina", packageName = "lang.runtime", moduleName = "lang.runtime"}
+]
 
 [[package]]
 org = "ballerina"
@@ -218,6 +282,18 @@ dependencies = [
 	{org = "ballerina", name = "jballerina.java"}
 ]
 
+[[package]]
+org = "ballerina"
+name = "lang.xml"
+version = "0.0.0"
+scope = "testOnly"
+dependencies = [
+	{org = "ballerina", name = "jballerina.java"}
+]
+modules = [
+	{org = "ballerina", packageName = "lang.xml", moduleName = "lang.xml"}
+]
+
 [[package]]
 org = "ballerina"
 name = "log"
@@ -256,6 +332,9 @@ dependencies = [
 	{org = "ballerina", name = "log"},
 	{org = "ballerina", name = "time"}
 ]
+modules = [
+	{org = "ballerina", packageName = "oauth2", moduleName = "oauth2"}
+]
 
 [[package]]
 org = "ballerina"

diff --git a/ballerina-tests/tests/interceptors_error_handling_tests.bal b/ballerina-tests/tests/interceptors_error_handling_tests.bal
@@ -316,7 +316,7 @@ function testAuthzError() returns error? {
         }
     );
     http:Response res = check clientEP->get("/auth");
-    test:assertEquals(res.statusCode, 403);
+    test:assertEquals(res.statusCode, 401);
     assertHeaderValue(check res.getHeader("last-interceptor"), "default-response-error-interceptor");
     assertHeaderValue(check res.getHeader("default-response-error-interceptor"), "true");
     assertHeaderValue(check res.getHeader("last-response-interceptor"), "true");

diff --git a/ballerina/auth_desugar.bal b/ballerina/auth_desugar.bal
@@ -42,12 +42,12 @@ public isolated function authenticateResource(Service serviceRef, string methodN
     if header is string {
         Unauthorized|Forbidden? result = tryAuthenticate(<ListenerAuthConfig[]>authConfig, header);
         if result is Unauthorized {
-            panic error ListenerAuthnError("");
+            panic error InternalListenerAuthnError("");
         } else if result is Forbidden {
-            panic error ListenerAuthzError("");
+            panic error InternalListenerAuthzError("");
         }
     } else {
-        panic error ListenerAuthnError("");
+        panic error InternalListenerAuthnError("");
     }
 }
 

diff --git a/ballerina/http_errors.bal b/ballerina/http_errors.bal
@@ -103,6 +103,12 @@ public type ListenerAuthnError distinct ListenerAuthError;
 # Defines the authorization error types that returned from listener.
 public type ListenerAuthzError distinct ListenerAuthError;
 
+# Defined for internal use when panicing from the auth_desugar
+type InternalListenerAuthnError distinct ListenerAuthError;
+
+# Defined for internal use when panicing from the auth_desugar
+type InternalListenerAuthzError distinct ListenerAuthError;
+
 # Defines the client error types that returned while sending outbound request.
 public type OutboundRequestError distinct ClientError;
 

diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/HttpCallableUnitCallback.java b/native/src/main/java/io/ballerina/stdlib/http/api/HttpCallableUnitCallback.java
@@ -117,7 +117,7 @@ private void returnErrorResponse(BError error) {
         invokeBalMethod(paramFeed, "returnErrorResponse");
     }
 
-    private void invokeBalMethod(Object[] paramFeed, String methodName) {
+    public void invokeBalMethod(Object[] paramFeed, String methodName) {
         Callback returnCallback = new Callback() {
             @Override
             public void notifySuccess(Object result) {
@@ -153,15 +153,16 @@ private void stopObserverContext() {
     public void notifyFailure(BError error) { // handles panic and check_panic
         cleanupRequestMessage();
         // This check is added to update the status code with respect to the auth errors.
-        if (error.getType().getName().equals(HttpErrorType.LISTENER_AUTHN_ERROR.getErrorName())) {
+        if (error.getType().getName().equals(HttpErrorType.INTERNAL_LISTENER_AUTHN_ERROR.getErrorName())) {
             requestMessage.setHttpStatusCode(401);
-        } else if (error.getType().getName().equals(HttpErrorType.LISTENER_AUTHZ_ERROR.getErrorName())) {
+        } else if (error.getType().getName().equals(HttpErrorType.INTERNAL_LISTENER_AUTHZ_ERROR.getErrorName())) {
             requestMessage.setHttpStatusCode(403);
+        } else {
+            requestMessage.setProperty(HttpConstants.INTERCEPTOR_SERVICE_PANIC_ERROR, true);
         }
         if (alreadyResponded(error)) {
             return;
         }
-        requestMessage.setProperty(HttpConstants.INTERCEPTOR_SERVICE_PANIC_ERROR, true);
         invokeErrorInterceptors(error, true);
     }
 

diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/HttpErrorType.java b/native/src/main/java/io/ballerina/stdlib/http/api/HttpErrorType.java
@@ -72,8 +72,8 @@ public enum HttpErrorType {
     REQ_DISPATCHING_ERROR("RequestDispatchingError"),
     SERVICE_DISPATCHING_ERROR("ServiceDispatchingError"),
     RESOURCE_DISPATCHING_ERROR("ResourceDispatchingError"),
-    LISTENER_AUTHZ_ERROR("ListenerAuthzError"),
-    LISTENER_AUTHN_ERROR("ListenerAuthnError");
+    INTERNAL_LISTENER_AUTHZ_ERROR("InternalListenerAuthzError"),
+    INTERNAL_LISTENER_AUTHN_ERROR("InternalListenerAuthnError");
 
     private final String errorName;
 

diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/HttpResponseInterceptorUnitCallback.java b/native/src/main/java/io/ballerina/stdlib/http/api/HttpResponseInterceptorUnitCallback.java
@@ -19,24 +19,19 @@
 package io.ballerina.stdlib.http.api;
 
 import io.ballerina.runtime.api.Environment;
-import io.ballerina.runtime.api.PredefinedTypes;
 import io.ballerina.runtime.api.Runtime;
-import io.ballerina.runtime.api.async.Callback;
 import io.ballerina.runtime.api.types.ServiceType;
 import io.ballerina.runtime.api.values.BArray;
 import io.ballerina.runtime.api.values.BError;
 import io.ballerina.runtime.api.values.BObject;
-import io.ballerina.stdlib.http.api.nativeimpl.ModuleUtils;
 import io.ballerina.stdlib.http.api.nativeimpl.connection.Respond;
 import io.ballerina.stdlib.http.transport.message.HttpCarbonMessage;
 
-import java.util.Objects;
-
 /**
  * {@code HttpResponseInterceptorUnitCallback} is the responsible for acting on notifications received from Ballerina
  * side when a response interceptor service is invoked.
  */
-public class HttpResponseInterceptorUnitCallback implements Callback {
+public class HttpResponseInterceptorUnitCallback extends HttpCallableUnitCallback {
     private static final String ILLEGAL_FUNCTION_INVOKED = "illegal return: response has already been sent";
 
     private final HttpCarbonMessage requestMessage;
@@ -45,18 +40,17 @@ public class HttpResponseInterceptorUnitCallback implements Callback {
     private final Environment environment;
     private final BObject requestCtx;
     private final DataContext dataContext;
-    private final Runtime runtime;
 
 
     public HttpResponseInterceptorUnitCallback(HttpCarbonMessage requestMessage, BObject caller, BObject response,
                                                Environment env, DataContext dataContext, Runtime runtime) {
+        super(requestMessage, runtime);
         this.requestMessage = requestMessage;
         this.requestCtx = (BObject) requestMessage.getProperty(HttpConstants.REQUEST_CONTEXT);
         this.caller = caller;
         this.response = response;
         this.environment = env;
         this.dataContext = dataContext;
-        this.runtime = runtime;
     }
 
     @Override
@@ -74,7 +68,7 @@ public void notifyFailure(BError error) { // handles panic and check_panic
         invokeErrorInterceptors(error, false);
     }
 
-    private void invokeErrorInterceptors(BError error, boolean printError) {
+    public void invokeErrorInterceptors(BError error, boolean printError) {
         requestMessage.setProperty(HttpConstants.INTERCEPTOR_SERVICE_ERROR, error);
         if (printError) {
             error.printStackTrace();
@@ -177,25 +171,4 @@ public void returnErrorResponse(BError error) {
 
         invokeBalMethod(paramFeed, "returnErrorResponse");
     }
-
-    private void invokeBalMethod(Object[] paramFeed, String methodName) {
-        Callback returnCallback = new Callback() {
-            @Override
-            public void notifySuccess(Object result) {
-                printStacktraceIfError(result);
-                Object isPanic = requestMessage.getProperty(HttpConstants.INTERCEPTOR_SERVICE_PANIC_ERROR);
-                if (Objects.nonNull(isPanic) && (boolean) isPanic) {
-                    System.exit(0);
-                }
-            }
-
-            @Override
-            public void notifyFailure(BError result) {
-                sendFailureResponse(result);
-            }
-        };
-        runtime.invokeMethodAsyncSequentially(
-                caller, methodName, null, ModuleUtils.getNotifySuccessMetaData(),
-                returnCallback, null, PredefinedTypes.TYPE_NULL, paramFeed);
-    }
 }