(fix) OAuth2 security fixes

kong/plugins/oauth2/access.lua

@@ -168,6 +168,7 @@ local function authorize(conf)
       if not response_params[ERROR] then
         if response_type == CODE then
           local authorization_code, err = singletons.dao.oauth2_authorization_codes:insert({
+            credential_id = client.id,
             authenticated_userid = parameters[AUTHENTICATED_USERID],
             scope = table.concat(scopes, " ")
           }, {ttl = 300})
@@ -285,8 +286,11 @@ local function issue_token(conf)
         local authorization_code = code and singletons.dao.oauth2_authorization_codes:find_all({code = code})[1]
         if not authorization_code then
           response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..CODE}
+        elseif authorization_code.credential_id ~= client.id then
+          response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..CODE}
         else
           response_params = generate_token(conf, client, authorization_code.authenticated_userid, authorization_code.scope, state)
+          singletons.dao.oauth2_authorization_codes:delete({id=authorization_code.id}) -- Delete authorization code so it cannot be reused
         end
       elseif grant_type == GRANT_CLIENT_CREDENTIALS then
         -- Only check the provision_key if the authenticated_userid is being set

kong/plugins/oauth2/daos.lua

@@ -49,6 +49,7 @@ local OAUTH2_AUTHORIZATION_CODES_SCHEMA = {
   table = "oauth2_authorization_codes",
   fields = {
     id = { type = "id", dao_insert_value = true },
+    credential_id = { type = "id", required = true, foreign = "oauth2_credentials:id" },
     code = { type = "string", required = false, unique = true, immutable = true, func = generate_if_missing },
     authenticated_userid = { type = "string", required = false },
     scope = { type = "string" },

kong/plugins/oauth2/migrations/cassandra.lua

@@ -103,5 +103,15 @@ return {
         end
       end
     end
+  },
+  {
+    name = "2016-07-15-oauth2_code_credential_id",
+    up = [[
+      TRUNCATE oauth2_authorization_codes;
+      ALTER TABLE oauth2_authorization_codes ADD credential_id uuid;
+    ]],
+    down = [[
+      ALTER TABLE oauth2_authorization_codes DROP credential_id;
+    ]]
   }
 }

kong/plugins/oauth2/migrations/postgres.lua

@@ -80,5 +80,15 @@ return {
       DROP TABLE oauth2_authorization_codes;
       DROP TABLE oauth2_tokens;
     ]]
+  },
+  {
+    name = "2016-07-15-oauth2_code_credential_id",
+    up = [[
+      DELETE FROM oauth2_authorization_codes;
+      ALTER TABLE oauth2_authorization_codes ADD COLUMN credential_id uuid REFERENCES oauth2_credentials (id) ON DELETE CASCADE;
+    ]],
+    down = [[
+      ALTER TABLE oauth2_authorization_codes DROP COLUMN credential_id;
+    ]]
   }
 }

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

@@ -3,14 +3,15 @@ local helpers = require "spec.helpers"
 
 describe("Plugin: oauth2 (access)", function()
   local proxy_ssl_client, proxy_client
+  local client1
   setup(function()
     helpers.kill_all()
     helpers.prepare_prefix()
 
     local consumer = assert(helpers.dao.consumers:insert {
       username = "bob"
     })
-    assert(helpers.dao.oauth2_credentials:insert {
+    client1 = assert(helpers.dao.oauth2_credentials:insert {
       client_id = "clientid123",
       client_secret = "secret123",
       redirect_uri = "http://google.com/kong",
@@ -142,6 +143,7 @@ describe("Plugin: oauth2 (access)", function()
   end)
 
   local function provision_code()
+    local proxy_ssl_client = helpers.proxy_ssl_client()
     local res = assert(proxy_ssl_client:send {
       method = "POST",
       path = "/oauth2/authorize",
@@ -158,8 +160,8 @@ describe("Plugin: oauth2 (access)", function()
         ["Content-Type"] = "application/json"
       }
     })
-
     local body = cjson.decode(assert.res_status(200, res))
+    proxy_ssl_client:close()
     if body.redirect_uri then
       local iterator, err = ngx.re.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?code=([\\w]{32,32})&state=hello$")
       assert.is_nil(err)
@@ -538,6 +540,7 @@ describe("Plugin: oauth2 (access)", function()
         assert.are.equal(m[1], data[1].code)
         assert.are.equal("userid123", data[1].authenticated_userid)
         assert.are.equal("email", data[1].scope)
+        assert.are.equal(client1.id, data[1].credential_id)
       end)
       it("returns success with a dotted scope and store authenticated user properties", function()
         local res = assert(proxy_ssl_client:send {
@@ -1311,6 +1314,63 @@ describe("Plugin: oauth2 (access)", function()
       assert.are.equal("email", body.headers["x-authenticated-scope"])
       assert.are.equal("userid123", body.headers["x-authenticated-userid"])
     end)
+    it("fails when an authorization code is used more than once", function()
+      local code = provision_code()
+
+      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.com",
+            ["Content-Type"] = "application/json"
+          }
+        })
+        local body = assert.res_status(200, res)
+        assert.is_table(ngx.re.match(body, [[^\{"refresh_token":"[\w]{32,32}","token_type":"bearer","access_token":"[\w]{32,32}","expires_in":5\}$]]))
+
+        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.com",
+            ["Content-Type"] = "application/json"
+          }
+        })
+        local body = assert.res_status(400, res)
+        assert.equal([[{"error":"invalid_request","error_description":"Invalid code"}]], body)
+    end)
+    it("fails when an authorization code is used by another application", function()
+      local code = provision_code()
+
+      local res = assert(proxy_ssl_client:send {
+          method = "POST",
+          path = "/oauth2/token",
+          body = {
+            code = code,
+            client_id = "clientid789",
+            client_secret = "secret789",
+            grant_type = "authorization_code"
+          },
+          headers = {
+            ["Host"] = "oauth2.com",
+            ["Content-Type"] = "application/json"
+          }
+        })
+        local body = assert.res_status(400, res)
+        assert.equal([[{"error":"invalid_request","error_description":"Invalid code"}]], body)
+    end)
   end)
 
   describe("Making a request", function()