fix(oauth2): forbid an authorization code can be exchanged to access_…

…token in different plugin instance

CHANGELOG.md

@@ -94,6 +94,8 @@
 
 - **Zipkin**: Fix an issue where the global plugin's sample ratio overrides route-specific.
   [#9877](https://github.com/Kong/kong/pull/9877)
+- **Oauth2**: forbid an authorization code can be exchanged to access_token in different plugin instance.
+  [#10011](https://github.com/Kong/kong/pull/10011)
 
 #### Core
 

kong-3.2.0-0.rockspec

@@ -304,6 +304,7 @@ build = {
     ["kong.plugins.oauth2.migrations.003_130_to_140"] = "kong/plugins/oauth2/migrations/003_130_to_140.lua",
     ["kong.plugins.oauth2.migrations.004_200_to_210"] = "kong/plugins/oauth2/migrations/004_200_to_210.lua",
     ["kong.plugins.oauth2.migrations.005_210_to_211"] = "kong/plugins/oauth2/migrations/005_210_to_211.lua",
+    ["kong.plugins.oauth2.migrations.006_220_to_221"] = "kong/plugins/oauth2/migrations/006_220_to_221.lua",
     ["kong.plugins.oauth2.handler"] = "kong/plugins/oauth2/handler.lua",
     ["kong.plugins.oauth2.secret"] = "kong/plugins/oauth2/secret.lua",
     ["kong.plugins.oauth2.access"] = "kong/plugins/oauth2/access.lua",

kong/plugins/oauth2/access.lua

@@ -88,6 +88,10 @@ do
   end
 end
 
+local function get_plugin_identifer(conf)
+  return type(conf) == "table" and conf.__key__ or nil
+end
+
 
 local function generate_token(conf, service, credential, authenticated_userid,
                               scope, state, disable_refresh, existing_token)
@@ -365,7 +369,8 @@ local function authorize(conf)
             authenticated_userid = parameters[AUTHENTICATED_USERID],
             scope = scopes,
             challenge = challenge,
-            challenge_method = challenge_method
+            challenge_method = challenge_method,
+            plugin_identifer = get_plugin_identifer(conf)
           }, {
             ttl = 300
           })
@@ -643,6 +648,16 @@ local function issue_token(conf)
           end
         end
 
+        if not response_params[ERROR] and conf.global_credentials then
+          local expected_identifer = get_plugin_identifer(conf)
+          if expected_identifer ~= auth_code.plugin_identifer then
+            response_params = {
+              [ERROR] = "invalid_request",
+              error_description = "Invalid " .. CODE
+            }
+          end
+        end
+
         if not response_params[ERROR] then
           if not auth_code or (service_id and service_id ~= auth_code.service.id)
           then

kong/plugins/oauth2/daos.lua

@@ -79,6 +79,7 @@ local oauth2_authorization_codes = {
     { scope = { type = "string" }, },
     { challenge = { type = "string", required = false }},
     { challenge_method = { type = "string", required = false, one_of = { "S256" } }},
+    { plugin_identifer = { type = "string", required = false } },
   },
 }
 

kong/plugins/oauth2/migrations/006_220_to_221.lua

@@ -0,0 +1,18 @@
+return {
+  postgres = {
+    up = [[
+      DO $$
+      BEGIN
+        ALTER TABLE IF EXISTS ONLY "oauth2_authorization_codes" ADD "plugin_identifer" TEXT;
+      EXCEPTION WHEN DUPLICATE_COLUMN THEN
+        -- Do nothing, accept existing state
+      END$$;
+    ]],
+  },
+
+  cassandra = {
+    up = [[
+      ALTER TABLE oauth2_authorization_codes ADD plugin_identifer text;
+    ]],
+  },
+}

kong/plugins/oauth2/migrations/init.lua

@@ -3,4 +3,5 @@ return {
   "003_130_to_140",
   "004_200_to_210",
   "005_210_to_211",
+  "006_220_to_221",
 }

spec/03-plugins/25-oauth2/03-access_spec.lua

@@ -261,6 +261,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
       local service14   = admin_api.services:insert()
       local service15   = admin_api.services:insert()
       local service16   = admin_api.services:insert()
+      local service17   = admin_api.services:insert()
 
       local route1 = assert(admin_api.routes:insert({
         hosts     = { "oauth2.com" },
@@ -370,6 +371,11 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         service     = service16,
       }))
 
+      local route17 = assert(admin_api.routes:insert({
+        hosts       = { "oauth2_17.com" },
+        protocols   = { "http", "https" },
+        service     = service17,
+      }))
 
       local service_grpc = assert(admin_api.services:insert {
           name = "grpc",
@@ -542,6 +548,14 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
           pkce = "lax",
         }
       })
+
+      admin_api.oauth2_plugins:insert({
+        route = { id = route17.id },
+        config   = {
+          scopes                   = { "email", "profile", "user.email" },
+          global_credentials       = true,
+        }
+      })
     end)
 
     before_each(function ()
@@ -2360,7 +2374,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         assert.same({ error_description = "code_verifier is required for PKCE authorization requests", error = "invalid_request" }, json)
       end)
       it("success when no code_verifier provided for public app without pkce when conf.pkce is none", function()
-        local code = provision_code()
+        local code = provision_code("oauth2_14.com")
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2587,7 +2601,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
       end)
       it("fails when code verifier does not match challenge for confidential app when conf.pkce is strict", function()
         local challenge, _ = get_pkce_tokens()
-        local code = provision_code(nil, nil, nil, challenge)
+        local code = provision_code("oauth2_15.com", nil, nil, challenge)
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2666,7 +2680,8 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         assert.same({ error_description = "Invalid client authentication", error = "invalid_client" }, json)
       end)
       it("fails when no code verifier provided for confidential app when conf.pkce is strict", function()
-        local code = provision_code()
+        local challenge, _ = get_pkce_tokens()
+        local code = provision_code("oauth2_15.com", nil, nil, challenge)
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2687,7 +2702,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
       end)
       it("fails when no code verifier provided for confidential app with pkce when conf.pkce is lax", function()
         local challenge, _ = get_pkce_tokens()
-        local code = provision_code(nil, nil, nil, challenge)
+        local code = provision_code("oauth2_16.com", nil, nil, challenge)
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2708,7 +2723,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
       end)
       it("fails when no code verifier provided for confidential app with pkce when conf.pkce is none", function()
         local challenge, _ = get_pkce_tokens()
-        local code = provision_code(nil, nil, nil, challenge)
+        local code = provision_code("oauth2_14.com", nil, nil, challenge)
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2728,7 +2743,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         assert.same({ error_description = "code_verifier is required for PKCE authorization requests", error = "invalid_request" }, json)
       end)
       it("suceeds when no code verifier provided for confidential app without pkce when conf.pkce is none", function()
-        local code = provision_code()
+        local code = provision_code("oauth2_14.com")
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2753,7 +2768,7 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         assert.matches("%w+", json.refresh_token)
       end)
       it("suceeds when no code verifier provided for confidential app without pkce when conf.pkce is lax", function()
-        local code = provision_code()
+        local code = provision_code("oauth2_16.com")
         local res = assert(proxy_ssl_client:send {
           method  = "POST",
           path    = "/oauth2/token",
@@ -2777,6 +2792,30 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         assert.equal(32, #json.refresh_token)
         assert.matches("%w+", json.refresh_token)
       end)
+
+      it("fails when exchanging a code created by a different plugin instance when both plugin instances set global_credentials to true", function()
+        local code = provision_code("oauth2_16.com") -- obtain a code from plugin oauth2_16.com
+        local res = assert(proxy_ssl_client:send {
+          method  = "POST",
+          path    = "/oauth2/token",
+          body    = {
+            code             = code,
+            client_id        = "clientid123",
+            client_secret    = "secret123",
+            grant_type       = "authorization_code",
+          },
+          headers = {
+            ["Host"]         = "oauth2_17.com", -- exchange the code from plugin oauth2_17.com
+            ["Content-Type"] = "application/json",
+          }
+        })
+        local body = assert.res_status(400, res)
+        local json = cjson.decode(body)
+        assert.same({
+          error = "invalid_request",
+          error_description = "Invalid code",
+        }, json)
+      end)
     end)
 
     describe("Making a request", function()