Repository: tipsy/javalin (kotlin)

Issue:
CORS plugin supersedes OPTIONS endpoint mappings
When enabling CORS, adding an endpoint handler for OPTIONS no longer works. Example:

```kotlin
val app = Javalin.create { config ->
  config.enableCorsForAllOrigins()
}.options("/test") { ctx -> ctx.result("test") }

app.start()
```
```bash
curl -X OPTIONS http://localhost:7000/test
# no value for response
```

Looking at the code, `io.javalin.core.util.CorsPlugin` adds both a before handler and an endpoint handler (for `*`). After the endpoint handler runs, the servlet ignores every other possible match for the route.

It is possible to work around this by adding an after handler to the desired path and testing for the OPTIONS method:

```kotlin
val app = Javalin.create { config ->
  config.enableCorsForAllOrigins()
}.after("/test") {
  if (ctx.method() == "OPTIONS") {
    ctx -> ctx.result("test")
  }
}

app.start()
```
```bash
curl -X OPTIONS http://localhost:7000/test
# test
```

But that feels contrived. I feel at least a warning in the docs about not being able to use OPTIONS endpoint handlers together with the CORS plugin would be nice, but ideally enabling CORS wouldn't be so intrusive.

With regards to use-cases, RFC 7231 says that _"the OPTIONS method requests information about the communication
   options available for the target resource"_ . In our case, we wanted to add some metadata to some tricky endpoint payloads, to make the life of API clients easier.

Pull request:
Fixes #1051

Patch:
diff --git a/javalin/src/main/java/io/javalin/core/security/SecurityUtil.kt b/javalin/src/main/java/io/javalin/core/security/SecurityUtil.kt
index 38297bc94..831201c64 100644
--- a/javalin/src/main/java/io/javalin/core/security/SecurityUtil.kt
+++ b/javalin/src/main/java/io/javalin/core/security/SecurityUtil.kt
@@ -34,8 +34,6 @@ object SecurityUtil {
 
 }
 
-internal enum class CoreRoles : Role { NO_WRAP } // used to avoid wrapping CORS options
-
 /**
  * Auth credentials for basic HTTP authorization.
  * Contains the Base64 decoded [username] and [password] from the Authorization header.
diff --git a/javalin/src/main/java/io/javalin/core/util/CorsPlugin.kt b/javalin/src/main/java/io/javalin/core/util/CorsPlugin.kt
index 06714aa1e..542b657c1 100644
--- a/javalin/src/main/java/io/javalin/core/util/CorsPlugin.kt
+++ b/javalin/src/main/java/io/javalin/core/util/CorsPlugin.kt
@@ -2,10 +2,7 @@ package io.javalin.core.util
 
 import io.javalin.Javalin
 import io.javalin.core.plugin.Plugin
-import io.javalin.core.security.CoreRoles
-import io.javalin.core.security.SecurityUtil.roles
 import io.javalin.http.util.CorsBeforeHandler
-import io.javalin.http.util.CorsOptionsHandler
 
 class CorsPlugin(private val origins: List<String>) : Plugin {
 
@@ -25,7 +22,6 @@ class CorsPlugin(private val origins: List<String>) : Plugin {
 
     override fun apply(app: Javalin) {
         app.before(CorsBeforeHandler(origins))
-        app.options("*", CorsOptionsHandler(), roles(CoreRoles.NO_WRAP))
     }
 
 }
diff --git a/javalin/src/main/java/io/javalin/http/JavalinServlet.kt b/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
index 5053a06ad..53ee428cc 100644
--- a/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
+++ b/javalin/src/main/java/io/javalin/http/JavalinServlet.kt
@@ -8,8 +8,8 @@ package io.javalin.http
 
 import io.javalin.Javalin
 import io.javalin.core.JavalinConfig
-import io.javalin.core.security.CoreRoles
 import io.javalin.core.security.Role
+import io.javalin.core.util.CorsPlugin
 import io.javalin.core.util.Header
 import io.javalin.core.util.LogUtil
 import io.javalin.core.util.Util
@@ -52,6 +52,9 @@ class JavalinServlet(val config: JavalinConfig) : HttpServlet() {
                     if (config.inner.resourceHandler?.handle(wrappedReq, JavalinResponseWrapper(rawRes, rwc)) == true) return@tryWithExceptionMapper
                     if (config.inner.singlePageHandler.handle(ctx)) return@tryWithExceptionMapper
                 }
+                if (type == HandlerType.OPTIONS && isCorsEnabled(config)) { // CORS is enabled, so we return 200 for OPTIONS
+                    return@tryWithExceptionMapper
+                }
                 if (ctx.handlerType == HandlerType.BEFORE) { // no match, status will be 404 or 405 after this point
                     ctx.endpointHandlerPath = "No handler matched request path/method (404/405)"
                 }
@@ -115,9 +118,9 @@ class JavalinServlet(val config: JavalinConfig) : HttpServlet() {
     private fun hasGetHandlerMapped(requestUri: String) = matcher.findEntries(HandlerType.GET, requestUri).isNotEmpty()
 
     fun addHandler(handlerType: HandlerType, path: String, handler: Handler, roles: Set<Role>) {
-        val shouldWrap = handlerType.isHttpMethod() && !roles.contains(CoreRoles.NO_WRAP)
-        val protectedHandler = if (shouldWrap) Handler { ctx -> config.inner.accessManager.manage(handler, ctx, roles) } else handler
+        val protectedHandler = if (handlerType.isHttpMethod()) Handler { ctx -> config.inner.accessManager.manage(handler, ctx, roles) } else handler
         matcher.add(HandlerEntry(handlerType, path, protectedHandler, handler))
     }
 
+    private fun isCorsEnabled(config: JavalinConfig) = config.inner.plugins[CorsPlugin::class.java] != null
 }
diff --git a/javalin/src/main/java/io/javalin/http/util/CorsUtil.kt b/javalin/src/main/java/io/javalin/http/util/CorsUtil.kt
index 20248f32d..a8040f031 100644
--- a/javalin/src/main/java/io/javalin/http/util/CorsUtil.kt
+++ b/javalin/src/main/java/io/javalin/http/util/CorsUtil.kt
@@ -19,17 +19,14 @@ class CorsBeforeHandler(private val origins: List<String>) : Handler {
                 ctx.header(Header.ACCESS_CONTROL_ALLOW_CREDENTIALS, "true")
             }
         }
-    }
-}
 
-class CorsOptionsHandler : Handler {
-    @OpenApi(ignore = true)
-    override fun handle(ctx: Context) {
-        ctx.header(Header.ACCESS_CONTROL_REQUEST_HEADERS)?.let {
-            ctx.header(Header.ACCESS_CONTROL_ALLOW_HEADERS, it)
-        }
-        ctx.header(Header.ACCESS_CONTROL_REQUEST_METHOD)?.let {
-            ctx.header(Header.ACCESS_CONTROL_ALLOW_METHODS, it)
+        if (ctx.method() == "OPTIONS") {
+            ctx.header(Header.ACCESS_CONTROL_REQUEST_HEADERS)?.let {
+                ctx.header(Header.ACCESS_CONTROL_ALLOW_HEADERS, it)
+            }
+            ctx.header(Header.ACCESS_CONTROL_REQUEST_METHOD)?.let {
+                ctx.header(Header.ACCESS_CONTROL_ALLOW_METHODS, it)
+            }
         }
     }
 }


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestCors.kt b/javalin/src/test/java/io/javalin/TestCors.kt
index 7f359a5f8..f9814eaa6 100644
--- a/javalin/src/test/java/io/javalin/TestCors.kt
+++ b/javalin/src/test/java/io/javalin/TestCors.kt
@@ -84,4 +84,20 @@ class TestCors {
         }
     }
 
+
+    @Test
+    fun `enableCorsForAllOrigins allows options endpoint mapping`() {
+        val javalin = Javalin.create { it.enableCorsForAllOrigins() }
+        TestUtil.test(javalin) { app, http ->
+            app.options("/") { ctx -> ctx.result("Hello") }
+            val response = Unirest.options(http.origin)
+                    .header(ACCESS_CONTROL_REQUEST_HEADERS, "123")
+                    .header(ACCESS_CONTROL_REQUEST_METHOD, "TEST")
+                    .asString()
+            assertThat(response.headers[ACCESS_CONTROL_ALLOW_HEADERS]!![0]).isEqualTo("123")
+            assertThat(response.headers[ACCESS_CONTROL_ALLOW_METHODS]!![0]).isEqualTo("TEST")
+            assertThat(response.body).isEqualTo("Hello")
+        }
+    }
+
 }