fix(oauth2) make optional redirect_uri field (#4150)

Fix #2030

Author: adriengraux

kong/plugins/oauth2/access.lua

@@ -379,31 +379,33 @@ local function issue_token(conf)
 
     -- Check client_id and redirect_uri
     local allowed_redirect_uris, client = get_redirect_uris(client_id)
-    if allowed_redirect_uris then
-      local redirect_uri = parameters[REDIRECT_URI] and
-                           parameters[REDIRECT_URI] or
-                           allowed_redirect_uris[1]
+    if not (grant_type == GRANT_CLIENT_CREDENTIALS) then
+      if allowed_redirect_uris then
+        local redirect_uri = parameters[REDIRECT_URI] and
+          parameters[REDIRECT_URI] or
+          allowed_redirect_uris[1]
 
-      if not table_contains(allowed_redirect_uris, redirect_uri) then
+        if not table_contains(allowed_redirect_uris, redirect_uri) then
+          response_params = {
+            [ERROR] = "invalid_request",
+            error_description = "Invalid " .. REDIRECT_URI .. " that does " ..
+              "not match with any redirect_uri created "  ..
+              "with the application"
+          }
+        end
+
+      else
         response_params = {
-          [ERROR] = "invalid_request",
-          error_description = "Invalid " .. REDIRECT_URI .. " that does " ..
-                              "not match with any redirect_uri created "  ..
-                              "with the application"
+          [ERROR] = "invalid_client",
+          error_description = "Invalid client authentication"
         }
-      end
 
-    else
-      response_params = {
-        [ERROR] = "invalid_client",
-        error_description = "Invalid client authentication"
-      }
-
-      if from_authorization_header then
-        invalid_client_properties = {
-          status = 401,
-          www_authenticate = "Basic realm=\"OAuth2.0\""
-        }
+        if from_authorization_header then
+          invalid_client_properties = {
+            status = 401,
+            www_authenticate = "Basic realm=\"OAuth2.0\""
+          }
+        end
       end
     end
 

kong/plugins/oauth2/daos.lua

@@ -30,7 +30,7 @@ local oauth2_credentials = {
     { client_secret = { type = "string", required = false, auto = true }, },
     { redirect_uris = {
       type = "array",
-      required = true,
+      required = false,
       elements = {
         type = "string",
         custom_validator = validate_uri,

spec/03-plugins/25-oauth2/02-api_spec.lua

@@ -64,6 +64,21 @@ for _, strategy in helpers.each_strategy() do
           assert.equal(consumer.id, body.consumer.id)
           assert.equal("Test APP", body.name)
           assert.same({ "http://google.com/" }, body.redirect_uris)
+
+          res = assert(admin_client:send {
+            method = "POST",
+            path   = "/consumers/bob/oauth2",
+            body   = {
+              name          = "Test APP",
+            },
+            headers = {
+              ["Content-Type"] = "application/json"
+            }
+          })
+          local body = cjson.decode(assert.res_status(201, res))
+          assert.equal(consumer.id, body.consumer.id)
+          assert.equal("Test APP", body.name)
+          assert.same(ngx.null, body.redirect_uris)
         end)
         it("creates a oauth2 credential with multiple redirect_uris", function()
           local res = assert(admin_client:send {
@@ -122,7 +137,7 @@ for _, strategy in helpers.each_strategy() do
             })
             local body = assert.res_status(400, res)
             local json = cjson.decode(body)
-            assert.same({ redirect_uris = "required field missing", name = "required field missing" }, json.fields)
+            assert.same({ name = "required field missing" }, json.fields)
           end)
           it("returns bad request with invalid redirect_uris", function()
             local res = assert(admin_client:send {
@@ -209,6 +224,22 @@ for _, strategy in helpers.each_strategy() do
           assert.equal("Test APP", body.name)
           assert.equal("client_one", body.client_id)
           assert.same({ "http://google.com/" }, body.redirect_uris)
+
+          local res = assert(admin_client:send {
+            method  = "PUT",
+            path    = "/consumers/bob/oauth2/client_one",
+            body = {
+              name             = "Test APP",
+            },
+            headers = {
+              ["Content-Type"] = "application/json"
+            }
+          })
+          local body = cjson.decode(assert.res_status(200, res))
+          assert.equal(consumer.id, body.consumer.id)
+          assert.equal("Test APP", body.name)
+          assert.equal("client_one", body.client_id)
+          assert.same(ngx.null, body.redirect_uris)
         end)
         describe("errors", function()
           it("returns bad request", function()
@@ -222,7 +253,7 @@ for _, strategy in helpers.each_strategy() do
             })
             local body = assert.res_status(400, res)
             local json = cjson.decode(body)
-            assert.same({ redirect_uris = "required field missing", name = "required field missing" }, json.fields)
+            assert.same({ name = "required field missing" }, json.fields)
           end)
         end)
       end)

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

@@ -148,6 +148,14 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
         consumer      = { id = consumer.id },
       }
 
+      admin_api.oauth2_credentials:insert {
+        client_id     = "clientid10112",
+        client_secret = "secret10112",
+        redirect_uris = ngx.null,
+        name          = "testapp311",
+        consumer      = { id = consumer.id },
+      }
+
       local service1    = admin_api.services:insert()
       local service2    = admin_api.services:insert()
       local service2bis = admin_api.services:insert()
@@ -1077,25 +1085,23 @@ describe("Plugin: oauth2 [#" .. strategy .. "]", function()
           local body = assert.res_status(200, res)
           assert.is_table(ngx.re.match(body, [[^\{"token_type":"bearer","access_token":"[\w]{32,32}","expires_in":5\}$]]))
         end)
-        it("fails with an application that has multiple redirect_uri, and by passing an invalid redirect_uri", function()
+        it("returns success with an application that has not redirect_uri", function()
           local res = assert(proxy_ssl_client:send {
             method  = "POST",
             path    = "/oauth2/token",
             body    = {
-              client_id        = "clientid456",
-              client_secret    = "secret456",
+              client_id        = "clientid10112",
+              client_secret    = "secret10112",
               scope            = "email",
               grant_type       = "client_credentials",
-              redirect_uri     = "http://two.com/two/hello"
             },
             headers = {
               ["Host"]         = "oauth2_4.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 redirect_uri that does not match with any redirect_uri created with the application" }, json)
+          local body = assert.res_status(200, res)
+          assert.is_table(ngx.re.match(body, [[^\{"token_type":"bearer","access_token":"[\w]{32,32}","expires_in":5\}$]]))
         end)
         it("returns success with authenticated_userid and valid provision_key", function()
           local res = assert(proxy_ssl_client:send {