allow redirect_uri to be specified without a hostname

Author: bodewig

ChangeLog

@@ -6,8 +6,8 @@
 
 09/23/2018
 - added redirect_uri option that specifies an absolute URI for the
-  redirect URI. The existing redirect_uri_path and redirect_uri_scheme
-  options have been deprecated in favor of the new option.
+  redirect URI. The existing redirect_uri_path option has been
+  deprecated in favor of the new option.
 
 09/17/2018
 - added an optional "request decorator" option that can be used to augment

README.md

@@ -109,12 +109,13 @@ http {
 
           local opts = {
              -- the full redirect URI must be protected by this script
+             -- if the URI starts with a / the full redirect URI becomes
+             -- ngx.var.scheme.."://"..ngx.var.http_host..opts.redirect_uri
+             -- unless the scheme was overridden using opts.redirect_uri_scheme or an X-Forwarded-Proto header in the incoming request
              redirect_uri = "https://MY_HOST_NAME/redirect_uri"
              -- up until version 1.6.1 you'd specify
              -- redirect_uri_path = "/redirect_uri",
-             -- and the redirect URI became
-             -- ngx.var.scheme.."://"..ngx.var.http_host..opts.redirect_uri_path
-             -- unless the scheme was overridden using opts.redirect_uri_scheme or an X-Forwarded-Proto header in the incoming request
+             -- and could not set the hostname
 
              discovery = "https://accounts.google.com/.well-known/openid-configuration",
              client_id = "<client_id>",

lib/resty/openidc.lua

@@ -248,16 +248,21 @@ end
 
 -- assemble the redirect_uri
 local function openidc_get_redirect_uri(opts)
+  local path = opts.redirect_uri_path
   if opts.redirect_uri then
-    return opts.redirect_uri
+    if opts.redirect_uri:sub(1, 1) == '/' then
+      path = opts.redirect_uri
+    else
+      return opts.redirect_uri
+    end
   end
   local scheme = opts.redirect_uri_scheme or get_scheme()
   local host = get_host_name()
   if not host then
     -- possibly HTTP 1.0 and no Host header
     ngx.exit(ngx.HTTP_BAD_REQUEST)
   end
-  return scheme .. "://" .. host .. opts.redirect_uri_path
+  return scheme .. "://" .. host .. path
 end
 
 -- perform base64url decoding
@@ -1215,8 +1220,8 @@ end
 -- main routine for OpenID Connect user authentication
 function openidc.authenticate(opts, target_url, unauth_action, session_opts)
 
-  if opts.redirect_uri_path or opts.redirect_uri_scheme then
-    log(WARN, "using deprecated option `opts.redirect_uri_path` or `opts.redirect_uri_scheme` for redirect_uri; switch to using an absolute URI and `opts.redirect_uri` instead")
+  if opts.redirect_uri_path then
+    log(WARN, "using deprecated option `opts.redirect_uri_path`; switch to using an absolute URI and `opts.redirect_uri` instead")
   end
 
   local err

tests/spec/redirect_from_op_spec.lua

@@ -93,6 +93,36 @@ describe("when the full login has been performed and the initial link is called"
 end)
 
 describe("when the redirect_uri is specified as relative URI", function()
+  test_support.start_server({
+    oidc_opts = {
+      redirect_uri = '/default/redirect_uri',
+    },
+  })
+  teardown(test_support.stop_server)
+  local _, _, headers = http.request({
+    url = "http://localhost/default/t",
+    redirect = false
+  })
+  local state = test_support.grab(headers, 'state')
+  test_support.register_nonce(headers)
+  local cookie_header = test_support.extract_cookies(headers)
+  describe("accessing the redirect_uri path with good parameters", function()
+    local _, redirStatus, h = http.request({
+          url = "http://localhost/default/redirect_uri?code=foo&state=" .. state,
+          headers = { cookie = cookie_header },
+          redirect = false
+    })
+    it("redirects to the original URI", function()
+       assert.are.equals(302, redirStatus)
+       assert.are.equals("/default/t", h.location)
+    end)
+  end)
+  it("no deprecation warning is logged", function()
+    assert.is_not.error_log_contains("using deprecated option `opts.redirect_uri_path`")
+  end)
+end)
+
+describe("when the redirect_uri is specified via redirect_uri_path", function()
   test_support.start_server({
     oidc_opts = {
       redirect_uri_path = '/default/redirect_uri',
@@ -119,7 +149,7 @@ describe("when the redirect_uri is specified as relative URI", function()
     end)
   end)
   it("a deprecation warning is logged", function()
-    assert.error_log_contains("using deprecated option `opts.redirect_uri_path` or `opts.redirect_uri_scheme`")
+    assert.error_log_contains("using deprecated option `opts.redirect_uri_path`")
   end)
 end)
 

tests/spec/redirect_to_op_spec.lua

@@ -248,6 +248,29 @@ describe("when discovery endpoint doesn't return proper JSON", function()
   end)
 end)
 
+describe("when accessing the protected resource without token and redirect_uri doesn't contain a hostname", function()
+  test_support.start_server({
+    oidc_opts = {
+      redirect_uri = '/default/redirect_uri',
+    },
+  })
+  teardown(test_support.stop_server)
+  local _, status, headers = http.request({
+    url = "http://127.0.0.1/default/t",
+    headers = {
+      ["x-forwarded-proto"] = "https",
+      ["x-forwarded-host"] = "example.org",
+    },
+    redirect = false
+  })
+  it("the configured forwarded information is used in redirect uri", function()
+    assert.are.equals(302, status)
+    local redir_escaped = test_support.urlescape_for_regex("https://example.org/default/redirect_uri")
+    assert.truthy(string.match(string.lower(headers["location"]),
+                               ".*redirect_uri=" .. string.lower(redir_escaped) .. ".*"))
+  end)
+end)
+
 describe("when accessing the protected resource without token, redirect_uri_path is used and x-forwarded headers exist", function()
   test_support.start_server({
     oidc_opts = {