Merge pull request Kong#1092 from Mashape/fix/oauth2-exptime

Closes Kong#1089

kong/dao/schemas_validation.lua

@@ -105,7 +105,7 @@ function _M.validate_entity(tbl, schema, options)
                 t[column][arr_k] = stringy.strip(arr_v)
               end
               is_valid_type = validate_type(v.type, t[column])
-            elseif v.type == "number" then
+            elseif v.type == "number" or v.type == "timestamp" then
               t[column] = tonumber(t[column])
               is_valid_type = validate_type(v.type, t[column])
             else -- if string

kong/plugins/oauth2/access.lua

@@ -39,10 +39,16 @@ local TOKEN_URL = "^%s/oauth2/token(/?(\\?[^\\s]*)?)$"
 local function generate_token(conf, credential, authenticated_userid, scope, state, expiration, disable_refresh)
   local token_expiration = expiration or conf.token_expiration
 
+  local refresh_token
+  if not disable_refresh and token_expiration > 0 then
+    refresh_token = utils.random_string()
+  end
+
   local token, err = singletons.dao.oauth2_tokens:insert({
     credential_id = credential.id,
     authenticated_userid = authenticated_userid,
     expires_in = token_expiration,
+    refresh_token = refresh_token,
     scope = scope
   }, {ttl = token_expiration > 0 and 1209600 or nil}) -- Access tokens (and their associated refresh token) are being 
                                                       -- permanently deleted after 14 days (1209600 seconds)
@@ -51,11 +57,6 @@ local function generate_token(conf, credential, authenticated_userid, scope, sta
     return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
   end
 
-  local refresh_token = token_expiration > 0 and token.refresh_token or nil
-  if disable_refresh then
-    refresh_token = nil
-  end
-
   return {
     access_token = token.access_token,
     token_type = "bearer",
@@ -184,7 +185,7 @@ local function authorize(conf)
           }
         else
           -- Implicit grant, override expiration to zero
-          response_params = generate_token(conf, client, parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, 0)
+          response_params = generate_token(conf, client, parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, nil, true)
         end
       end
     end
@@ -300,7 +301,7 @@ local function issue_token(conf)
           if not ok then
             response_params = scopes -- If it's not ok, then this is the error message
           else
-            response_params = generate_token(conf, client, parameters.authenticated_userid, table.concat(scopes, " "), state, conf.token_expiration, true)
+            response_params = generate_token(conf, client, parameters.authenticated_userid, table.concat(scopes, " "), state, nil, true)
           end
         end
       elseif grant_type == GRANT_PASSWORD then

kong/plugins/oauth2/api.lua

@@ -1,4 +1,5 @@
 local crud = require "kong.api.crud_helpers"
+local utils = require "kong.tools.utils"
 
 return {
   ["/oauth2_tokens/"] = {

kong/plugins/oauth2/daos.lua

@@ -8,13 +8,6 @@ local function generate_if_missing(v, t, column)
   return true
 end
 
-local function generate_refresh_token(v, t, column)
-  if t.expires_in and t.expires_in > 0 then
-    return generate_if_missing(v, t, column)
-  end
-  return true
-end
-
 local OAUTH2_CREDENTIALS_SCHEMA = {
   primary_key = {"id"},
   table = "oauth2_credentials",
@@ -54,7 +47,7 @@ local OAUTH2_TOKENS_SCHEMA = {
     token_type = { type = "string", required = true, enum = { BEARER }, default = BEARER },
     expires_in = { type = "number", required = true },
     access_token = { type = "string", required = false, unique = true, func = generate_if_missing },
-    refresh_token = { type = "string", required = false, unique = true, func = generate_refresh_token },
+    refresh_token = { type = "string", required = false, unique = true },
     authenticated_userid = { type = "string", required = false },
     scope = { type = "string" },
     created_at = { type = "timestamp", immutable = true, dao_insert_value = true }

spec/plugins/oauth2/access_spec.lua

@@ -285,7 +285,7 @@ describe("Authentication Plugin", function()
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
-        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&token_type=bearer$"))
+        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&expires_in=[\\d]+&token_type=bearer$"))
 
         -- Checking headers
         assert.are.equal("no-store", headers["cache-control"])
@@ -297,17 +297,36 @@ describe("Authentication Plugin", function()
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
-        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&state=wot&token_type=bearer$"))
+        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&expires_in=[\\d]+&state=wot&token_type=bearer$"))
+      end)
+
+      it("should return success and the token should have the right expiration", function()
+        local response, status, headers = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "token" }, {host = "oauth2.com"})
+        local body = cjson.decode(response)
+        assert.are.equal(200, status)
+        assert.are.equal(1, utils.table_size(body))
+        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&expires_in=[\\d]+&token_type=bearer$"))
+
+        local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&expires_in=[\\d]+&token_type=bearer$")
+        local access_token
+        for line in matches do
+          access_token = line
+        end
+        local data = dao_factory.oauth2_tokens:find_all {access_token = access_token}
+        assert.are.equal(1, #data)
+        assert.are.equal(access_token, data[1].access_token)
+        assert.are.equal(5, data[1].expires_in)
+        assert.falsy(data[1].refresh_token)
       end)
 
       it("should return success and store authenticated user properties", function()
         local response, status = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email  profile", response_type = "token", authenticated_userid = "userid123" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
-        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&token_type=bearer$"))
+        assert.truthy(rex.match(body.redirect_uri, "^http://google\\.com/kong\\?access_token=[\\w]{32,32}&expires_in=[\\d]+&token_type=bearer$"))
 
-        local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&token_type=bearer$")
+        local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&expires_in=[\\d]+&token_type=bearer$")
         local access_token
         for line in matches do
           access_token = line
@@ -320,15 +339,15 @@ describe("Authentication Plugin", function()
         assert.are.equal("email profile", data[1].scope)
 
         -- Checking that there is no refresh token since it's an implicit grant
-        assert.are.equal(0, data[1].expires_in)
+        assert.are.equal(5, data[1].expires_in)
         assert.falsy(data[1].refresh_token)
       end)
 
       it("should return set the right upstream headers", function()
         local response = http_client.post(PROXY_SSL_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email  profile", response_type = "token", authenticated_userid = "userid123" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
 
-        local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&token_type=bearer$")
+        local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?access_token=([\\w]{32,32})&expires_in=[\\d]+&token_type=bearer$")
         local access_token
         for line in matches do
           access_token = line
@@ -776,7 +795,7 @@ describe("Authentication Plugin", function()
       assert.are.equal(5, body.expires_in)
     end)
 
-    it("#only should expire after 5 seconds", function()
+    it("should expire after 5 seconds", function()
       local token = provision_token()
       local _, status = http_client.post(STUB_POST_URL, { }, {host = "oauth2.com", authorization = "bearer "..token.access_token})
       assert.are.equal(200, status)

spec/plugins/oauth2/api_spec.lua

@@ -126,7 +126,7 @@ describe("OAuth 2 Credentials API", function()
         assert.equal(credential.id, token.credential_id)
         assert.equal(10, token.expires_in)
         assert.truthy(token.access_token)
-        assert.truthy(token.refresh_token)
+        assert.falsy(token.refresh_token)
         assert.equal("bearer", token.token_type)
       end)
       it("[FAILURE] should return proper errors", function()
@@ -137,14 +137,14 @@ describe("OAuth 2 Credentials API", function()
     end)
 
     describe("PUT", function()
-      it("[SUCCESS] should create a oauth2 token", function()
+      it("#only [SUCCESS] should create a oauth2 token", function()
         local response, status = http_client.put(BASE_URL, {credential_id = credential.id, expires_in = 10})
         assert.equal(201, status)
         token = json.decode(response)
         assert.equal(credential.id, token.credential_id)
         assert.equal(10, token.expires_in)
         assert.truthy(token.access_token)
-        assert.truthy(token.refresh_token)
+        assert.falsy(token.refresh_token)
         assert.equal("bearer", token.token_type)
       end)
       it("[FAILURE] should return proper errors", function()
@@ -175,12 +175,37 @@ describe("OAuth 2 Credentials API", function()
         assert.equal(200, status)
         local body = json.decode(response)
         assert.equals("helloworld", body.access_token)
+        assert.falsy(body.refresh_token)
 
         -- Check it has really been updated
         response, status = http_client.get(BASE_URL..token.id)
         assert.equal(200, status)
         body = json.decode(response)
         assert.equals("helloworld", body.access_token)
+        assert.falsy(body.refresh_token)
+      end)
+
+      describe("PUT", function()
+        it("should update every field", function()
+          local response, status = http_client.get(BASE_URL..token.id)
+          assert.equal(200, status)
+          local body = json.decode(response)
+          body.refresh_token = nil
+          body.access_token = "helloworld"
+
+          local response, status = http_client.put(BASE_URL..token.id, body)
+          assert.equal(200, status)
+          local body = json.decode(response)
+          assert.equals("helloworld", body.access_token)
+          assert.falsy(body.refresh_token)
+
+          -- Check it has really been updated
+          response, status = http_client.get(BASE_URL..token.id)
+          assert.equal(200, status)
+          body = json.decode(response)
+          assert.equals("helloworld", body.access_token)
+          assert.falsy(body.refresh_token)
+        end)
       end)
     end)