Merge pull request #377 from Mashape/fix/oauth

removing authenticated_username

database/migrations/cassandra/2015-06-09-170921_0.4.0.lua

@@ -21,14 +21,14 @@ local Migration = {
       CREATE TABLE IF NOT EXISTS oauth2_authorization_codes(
         id uuid,
         code text,
-        authenticated_username text,
         authenticated_userid text,
         scope text,
         created_at timestamp,
         PRIMARY KEY (id)
       ) WITH default_time_to_live = 300;
 
       CREATE INDEX IF NOT EXISTS ON oauth2_authorization_codes(code);
+      CREATE INDEX IF NOT EXISTS ON oauth2_authorization_codes(authenticated_userid);
 
       CREATE TABLE IF NOT EXISTS oauth2_tokens(
         id uuid,
@@ -37,7 +37,6 @@ local Migration = {
         token_type text,
         refresh_token text,
         expires_in int,
-        authenticated_username text,
         authenticated_userid text,
         scope text,
         created_at timestamp,
@@ -46,6 +45,7 @@ local Migration = {
 
       CREATE INDEX IF NOT EXISTS ON oauth2_tokens(access_token);
       CREATE INDEX IF NOT EXISTS ON oauth2_tokens(refresh_token);
+      CREATE INDEX IF NOT EXISTS ON oauth2_tokens(authenticated_userid);
 
     ]]
   end,

kong/plugins/oauth2/access.lua

@@ -22,19 +22,17 @@ local GRANT_TYPE = "grant_type"
 local GRANT_AUTHORIZATION_CODE = "authorization_code"
 local GRANT_REFRESH_TOKEN = "refresh_token"
 local ERROR = "error"
-local AUTHENTICATED_USERNAME = "authenticated_username"
 local AUTHENTICATED_USERID = "authenticated_userid"
 
 local AUTHORIZE_URL = "^%s/oauth2/authorize/?$"
 local TOKEN_URL = "^%s/oauth2/token/?$"
 
 -- TODO: Expire token (using TTL ?)
-local function generate_token(conf, credential, authenticated_username, authenticated_userid, scope, state, expiration)
+local function generate_token(conf, credential, authenticated_userid, scope, state, expiration)
   local token_expiration = expiration or conf.token_expiration
 
   local token, err = dao.oauth2_tokens:insert({
     credential_id = credential.id,
-    authenticated_username = authenticated_username,
     authenticated_userid = authenticated_userid,
     expires_in = token_expiration,
     scope = scope
@@ -86,6 +84,8 @@ local function authorize(conf)
 
   if conf.provision_key ~= parameters.provision_key then
     response_params = {[ERROR] = "invalid_provision_key", error_description = "Invalid Kong provision_key"}
+  elseif not parameters.authenticated_userid or stringy.strip(parameters.authenticated_userid) == "" then
+    response_params = {[ERROR] = "invalid_authenticated_userid", error_description = "Missing authenticated_userid parameter"}
   else
     local response_type = parameters[RESPONSE_TYPE]
     -- Check response_type
@@ -121,7 +121,6 @@ local function authorize(conf)
     if not response_params[ERROR] then
       if response_type == CODE then
         local authorization_code, err = dao.oauth2_authorization_codes:insert({
-          authenticated_username = parameters[AUTHENTICATED_USERNAME],
           authenticated_userid = parameters[AUTHENTICATED_USERID],
           scope = table.concat(scopes, " ")
         })
@@ -135,7 +134,7 @@ local function authorize(conf)
         }
       else
         -- Implicit grant, override expiration to zero
-        response_params = generate_token(conf, client, parameters[AUTHENTICATED_USERNAME], parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, 0)
+        response_params = generate_token(conf, client, parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, 0)
       end
     end
   end
@@ -186,15 +185,15 @@ local function issue_token(conf)
       if not authorization_code then
         response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..CODE}
       else
-        response_params = generate_token(conf, client, authorization_code.authenticated_username, authorization_code.authenticated_userid, authorization_code.scope, state)
+        response_params = generate_token(conf, client, authorization_code.authenticated_userid, authorization_code.scope, state)
       end
     elseif grant_type == GRANT_REFRESH_TOKEN then
       local refresh_token = parameters[REFRESH_TOKEN]
       local token = refresh_token and dao.oauth2_tokens:find_by_keys({refresh_token = refresh_token})[1] or nil
       if not token then
         response_params = {[ERROR] = "invalid_request", error_description = "Invalid "..REFRESH_TOKEN}
       else
-        response_params = generate_token(conf, client, token.authenticated_username, token.authenticated_userid, token.scope, state)
+        response_params = generate_token(conf, client, token.authenticated_userid, token.scope, state)
         dao.oauth2_tokens:delete({id=token.id}) -- Delete old token
       end
     end
@@ -321,7 +320,6 @@ function _M.execute(conf)
   ngx.req.set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id)
   ngx.req.set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username)
   ngx.req.set_header("x-authenticated-scope", token.scope)
-  ngx.req.set_header("x-authenticated-username", token.authenticated_username)
   ngx.req.set_header("x-authenticated-userid", token.authenticated_userid)
   ngx.ctx.authenticated_entity = credential
 end

kong/plugins/oauth2/api.lua

@@ -1,6 +1,12 @@
 local crud = require "kong.api.crud_helpers"
 
 return {
+  ["/oauth2/"] = {
+    GET = function(self, dao_factory, helpers)
+      crud.paginated_set(self, dao_factory.oauth2_credentials)
+    end
+  },
+
   ["/consumers/:username_or_id/oauth2/"] = {
     before = function(self, dao_factory, helpers)
       crud.find_consumer_by_username_or_id(self, dao_factory, helpers)

kong/plugins/oauth2/daos.lua

@@ -34,7 +34,6 @@ local OAUTH2_AUTHORIZATION_CODES_SCHEMA = {
   fields = {
     id = { type = "id", dao_insert_value = true },
     code = { type = "string", required = false, unique = true, queryable = true, immutable = true, func = generate_if_missing },
-    authenticated_username = { type = "string", required = false },
     authenticated_userid = { type = "string", required = false },
     scope = { type = "string" },
     created_at = { type = "timestamp", dao_insert_value = true }
@@ -51,7 +50,6 @@ local OAUTH2_TOKENS_SCHEMA = {
     access_token = { type = "string", required = false, unique = true, queryable = true, immutable = true, func = generate_if_missing },
     refresh_token = { type = "string", required = false, unique = true, queryable = true, immutable = true, func = generate_refresh_token },
     expires_in = { type = "number", required = true },
-    authenticated_username = { type = "string", required = false },
     authenticated_userid = { type = "string", required = false },
     scope = { type = "string" },
     created_at = { type = "timestamp", dao_insert_value = true }

spec/plugins/oauth2/access_spec.lua

@@ -15,7 +15,7 @@ local STUB_GET_URL = spec_helper.STUB_GET_URL
 local STUB_POST_URL = spec_helper.STUB_POST_URL
 
 local function provision_code()
-  local response = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello", authenticated_username = "user123", authenticated_userid = "userid123" }, {host = "oauth2.com"})
+  local response = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello", authenticated_userid = "userid123" }, {host = "oauth2.com"})
   local body = cjson.decode(response)
   local matches = rex.gmatch(body.redirect_uri, "^http://google\\.com/kong\\?code=([\\w]{32,32})&state=hello$")
   local code
@@ -92,76 +92,89 @@ describe("Authentication Plugin", function()
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(2, utils.table_size(body))
+        assert.are.equal("invalid_authenticated_userid", body.error)
+        assert.are.equal("Missing authenticated_userid parameter", body.error_description)
+      end)
+
+      it("should return an error when only provision_key and authenticated_userid are sent", function()
+        local response, status, headers = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123" }, {host = "oauth2.com"})
+        local body = cjson.decode(response)
+        assert.are.equal(400, status)
+        assert.are.equal(2, utils.table_size(body))
         assert.are.equal("invalid_request", body.error)
         assert.are.equal("Invalid client_id", body.error_description)
+
+        -- Checking headers
+        assert.are.equal("no-store", headers["cache-control"])
+        assert.are.equal("no-cache", headers["pragma"])
       end)
 
       it("should return an error when only the client_is being sent", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(1, utils.table_size(body))
         assert.are.equal("http://google.com/kong?error=invalid_scope&error_description=You%20must%20specify%20a%20scope", body.redirect_uri)
       end)
 
       it("should return an error when an invalid scope is being sent", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "wot" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "wot" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(1, utils.table_size(body))
         assert.are.equal("http://google.com/kong?error=invalid_scope&error_description=%22wot%22%20is%20an%20invalid%20scope", body.redirect_uri)
       end)
 
       it("should return an error when no response_type is being sent", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(1, utils.table_size(body))
         assert.are.equal("http://google.com/kong?error=unsupported_response_type&error_description=Invalid%20response_type", body.redirect_uri)
       end)
 
       it("should return an error with a state when no response_type is being sent", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", state = "somestate" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", state = "somestate" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(1, utils.table_size(body))
         assert.are.equal("http://google.com/kong?error=unsupported_response_type&state=somestate&error_description=Invalid%20response_type", body.redirect_uri)
       end)
 
       it("should return error when the redirect_uri does not match", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code", redirect_uri = "http://hello.com/" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code", redirect_uri = "http://hello.com/" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(400, status)
         assert.are.equal(1, utils.table_size(body))
         assert.are.equal("http://google.com/kong?error=invalid_request&error_description=Invalid%20redirect_uri%20that%20does%20not%20match%20with%20the%20one%20created%20with%20the%20application", body.redirect_uri)
       end)
 
       it("should return success", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {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\\?code=[\\w]{32,32}$"))
       end)
 
       it("should return success with a path", function()
-        local response, status = http_client.post(PROXY_URL.."/somepath/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "mockbin-path.com"})
+        local response, status = http_client.post(PROXY_URL.."/somepath/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "mockbin-path.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\\?code=[\\w]{32,32}$"))
       end)
 
       it("should return success when requesting the url with final slash", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize/", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize/", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code" }, {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\\?code=[\\w]{32,32}$"))
       end)
 
       it("should return success with a state", function()
-        local response, status, headers = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello" }, {host = "oauth2.com"})
+        local response, status, headers = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
@@ -173,7 +186,7 @@ describe("Authentication Plugin", function()
       end)
 
       it("should return success and store authenticated user properties", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello", authenticated_username = "user123", authenticated_userid = "userid123" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "code", state = "hello", authenticated_userid = "userid123" }, {host = "oauth2.com"})
         local body = cjson.decode(response)
         assert.are.equal(200, status)
         assert.are.equal(1, utils.table_size(body))
@@ -188,7 +201,6 @@ describe("Authentication Plugin", function()
         assert.are.equal(1, #data)
         assert.are.equal(code, data[1].code)
 
-        assert.are.equal("user123", data[1].authenticated_username)
         assert.are.equal("userid123", data[1].authenticated_userid)
         assert.are.equal("email", data[1].scope)
       end)
@@ -197,7 +209,7 @@ describe("Authentication Plugin", function()
 
     describe("Implicit Grant", function()
       it("should return success", function()
-        local response, status, headers = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "token" }, {host = "oauth2.com"})
+        local response, status, headers = http_client.post(PROXY_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))
@@ -208,15 +220,15 @@ describe("Authentication Plugin", function()
         assert.are.equal("no-cache", headers["pragma"])
       end)
       it("should return success and the state", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email", response_type = "token", state = "wot" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", client_id = "clientid123", scope = "email", response_type = "token", state = "wot" }, {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\\?token_type=bearer&state=wot&access_token=[\\w]{32,32}$"))
       end)
 
       it("should return success and store authenticated user properties", function()
-        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", client_id = "clientid123", scope = "email  profile", response_type = "token", authenticated_username = "user123", authenticated_userid = "userid123" }, {host = "oauth2.com"})
+        local response, status = http_client.post(PROXY_URL.."/oauth2/authorize", { provision_key = "provision123", authenticated_userid = "id123", 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))
@@ -231,7 +243,6 @@ describe("Authentication Plugin", function()
         assert.are.equal(1, #data)
         assert.are.equal(access_token, data[1].access_token)
 
-        assert.are.equal("user123", data[1].authenticated_username)
         assert.are.equal("userid123", data[1].authenticated_userid)
         assert.are.equal("email profile", data[1].scope)
 
@@ -382,7 +393,6 @@ describe("Authentication Plugin", function()
 
       assert.are.equal(consumer.id, body.headers["x-consumer-id"])
       assert.are.equal(consumer.username, body.headers["x-consumer-username"])
-      assert.are.equal("user123", body.headers["x-authenticated-username"])
       assert.are.equal("userid123", body.headers["x-authenticated-userid"])
       assert.are.equal("email", body.headers["x-authenticated-scope"])
     end)