Merge branch 'pr-158-take-2'

Author: bodewig

lib/resty/openidc.lua

@@ -830,6 +830,51 @@ local function openidc_load_jwt_and_verify_crypto(opts, jwt_string, asymmetric_s
   return jwt_obj
 end
 
+--
+-- Load and validate id token from the id_token properties of the token endpoint response
+-- Parameters :
+--     - opts the openidc module options
+--     - jwt_id_token the id_token from the id_token properties of the token endpoint response
+--     - session the current session
+-- Return the id_token, nil if valid
+-- Return nil, the error if invalid
+--
+local function openidc_load_and_validate_jwt_id_token(opts, jwt_id_token, session)
+
+  local jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, jwt_id_token, opts.secret, opts.client_secret,
+          opts.discovery.id_token_signing_alg_values_supported)
+  if err then
+    local alg = (jwt_obj and jwt_obj.header and jwt_obj.header.alg) or ''
+    local is_unsupported_signature_error = jwt_obj and not jwt_obj.verified and not is_algorithm_supported(jwt_obj.header)
+    if is_unsupported_signature_error then
+      if opts.accept_unsupported_alg == nil or opts.accept_unsupported_alg then
+        ngx.log(ngx.WARN, "ignored id_token signature as algorithm '" .. alg .. "' is not supported")
+      else
+        err = "token is signed using algorithm \"" .. alg .. "\" which is not supported by lua-resty-jwt"
+        ngx.log(ngx.ERR, err)
+        return nil, err
+      end
+    else
+      ngx.log(ngx.ERR, "id_token '" .. alg .. "' signature verification failed")
+      return nil, err
+    end
+  end
+  local id_token = jwt_obj.payload
+
+  ngx.log(ngx.DEBUG, "id_token header: ", cjson.encode(jwt_obj.header))
+  ngx.log(ngx.DEBUG, "id_token payload: ", cjson.encode(jwt_obj.payload))
+
+  -- validate the id_token contents
+  if openidc_validate_id_token(opts, id_token, session.data.nonce) == false then
+    err = "id_token validation failed"
+    ngx.log(ngx.ERR, err)
+    return nil, err
+  end
+
+  return id_token
+
+end
+
 -- handle a "code" authorization response from the OP
 local function openidc_authorization_response(opts, session)
   local args = ngx.req.get_uri_args()
@@ -880,34 +925,8 @@ local function openidc_authorization_response(opts, session)
     return nil, err, session.data.original_url, session
   end
 
-  local jwt_obj
-  jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, json.id_token, opts.secret, opts.client_secret,
-      opts.discovery.id_token_signing_alg_values_supported)
+  local id_token, err = openidc_load_and_validate_jwt_id_token(opts, json.id_token, session);
   if err then
-    local alg = (jwt_obj and jwt_obj.header and jwt_obj.header.alg) or ''
-    local is_unsupported_signature_error = jwt_obj and not jwt_obj.verified and not is_algorithm_supported(jwt_obj.header)
-    if is_unsupported_signature_error then
-      if opts.accept_unsupported_alg == nil or opts.accept_unsupported_alg then
-        ngx.log(ngx.WARN, "ignored id_token signature as algorithm '" .. alg .. "' is not supported")
-      else
-        err = "token is signed using algorithm \"" .. alg .. "\" which is not supported by lua-resty-jwt"
-        ngx.log(ngx.ERR, err)
-        return nil, err, session.data.original_url, session
-      end
-    else
-      ngx.log(ngx.ERR, "id_token '" .. alg .. "' signature verification failed")
-      return nil, err, session.data.original_url, session
-    end
-  end
-  local id_token = jwt_obj.payload
-
-  ngx.log(ngx.DEBUG, "id_token header: ", cjson.encode(jwt_obj.header))
-  ngx.log(ngx.DEBUG, "id_token payload: ", cjson.encode(jwt_obj.payload))
-
-  -- validate the id_token contents
-  if openidc_validate_id_token(opts, id_token, session.data.nonce) == false then
-    err = "id_token validation failed"
-    ngx.log(ngx.ERR, err)
     return nil, err, session.data.original_url, session
   end
 
@@ -1090,16 +1109,35 @@ local function openidc_access_token(opts, session, try_to_renew)
   if err then
     return nil, err
   end
+  local id_token
+  if json.id_token then
+    id_token, err = openidc_load_and_validate_jwt_id_token(opts, json.id_token, session)
+    if err then
+      ngx.log(ngx.ERR, "invalid id token, discarding tokens returned while refreshing")
+      return nil, err
+    end
+  end
   ngx.log(ngx.DEBUG, "access_token refreshed: ", json.access_token, " updated refresh_token: ", json.refresh_token)
 
   session:start()
   session.data.access_token = json.access_token
   session.data.access_token_expiration = current_time + openidc_access_token_expires_in(opts, json.expires_in)
-  if json.refresh_token ~= nil then
+  if json.refresh_token then
     session.data.refresh_token = json.refresh_token
   end
 
-  -- save the session with the new access_token and optionally the new refresh_token
+  if json.id_token and
+    (store_in_session(opts, 'enc_id_token') or store_in_session(opts, 'id_token')) then
+    ngx.log(ngx.DEBUG, "id_token refreshed: ", json.id_token)
+    if store_in_session(opts, 'enc_id_token') then
+      session.data.enc_id_token = json.id_token
+    end
+    if store_in_session(opts, 'id_token') then
+      session.data.id_token = id_token
+    end
+  end
+
+  -- save the session with the new access_token and optionally the new refresh_token and id_token
   session:save()
 
   return session.data.access_token, err

tests/spec/test_support.lua

@@ -83,6 +83,7 @@ local DEFAULT_FAKE_ACCESS_TOKEN_SIGNATURE = "false"
 local DEFAULT_FAKE_ID_TOKEN_SIGNATURE = "false"
 local DEFAULT_BREAK_ID_TOKEN_SIGNATURE = "false"
 local DEFAULT_NONE_ALG_ID_TOKEN_SIGNATURE = "false"
+local DEFAULT_REFRESH_RESPONSE_CONTAINS_ID_TOKEN = "true"
 
 local DEFAULT_UNAUTH_ACTION = "nil"
 
@@ -192,8 +193,13 @@ JWT_VERIFY_SECRET]=]
                 local auth = ngx.req.get_headers()["Authorization"]
                 ngx.log(ngx.ERR, "token authorization header: " .. (auth and auth or ""))
                 ngx.header.content_type = 'application/json;charset=UTF-8'
-                local id_token = ID_TOKEN
                 local args = ngx.req.get_post_args()
+                local id_token
+                if args.grant_type == "authorization_code" then
+                  id_token = ID_TOKEN
+                else
+                  id_token = REFRESH_ID_TOKEN
+                end
                 local access_token = "a_token"
                 local refresh_token = "r_token"
                 if args.grant_type == "authorization_code" then
@@ -226,8 +232,10 @@ JWT_VERIFY_SECRET]=]
                   access_token = access_token,
                   expires_in = TOKEN_RESPONSE_EXPIRES_IN,
                   refresh_token = TOKEN_RESPONSE_CONTAINS_REFRESH_TOKEN and refresh_token or nil,
-                  id_token = jwt_token
                 }
+                if args.grant_type == "authorization_code" or REFRESH_RESPONSE_CONTAINS_ID_TOKEN then
+                  token_response.id_token = jwt_token
+                end
                 delay(TOKEN_DELAY_RESPONSE)
                 ngx.say(cjson.encode(token_response))
             }
@@ -348,6 +356,7 @@ local function write_config(out, custom_config)
   custom_config = custom_config or {}
   local oidc_config = merge(merge({}, DEFAULT_OIDC_CONFIG), custom_config["oidc_opts"] or {})
   local id_token = merge(merge({}, DEFAULT_ID_TOKEN), custom_config["id_token"] or {})
+  local refresh_id_token = merge({}, id_token)
   local verify_opts = merge(merge({}, DEFAULT_VERIFY_OPTS), custom_config["verify_opts"] or {})
   local access_token = merge(merge({}, DEFAULT_ACCESS_TOKEN), custom_config["access_token"] or {})
   local token_header = merge(merge({}, DEFAULT_TOKEN_HEADER), custom_config["token_header"] or {})
@@ -360,10 +369,14 @@ local function write_config(out, custom_config)
   local token_response_contains_refresh_token = custom_config["token_response_contains_refresh_token"]
     or DEFAULT_TOKEN_RESPONSE_CONTAINS_REFRESH_TOKEN
   local refreshing_token_fails = custom_config["refreshing_token_fails"] or DEFAULT_REFRESHING_TOKEN_FAILS
+  local refresh_response_contains_id_token = custom_config["refresh_response_contains_id_token"] or DEFAULT_REFRESH_RESPONSE_CONTAINS_ID_TOKEN
   local access_token_opts = merge(merge({}, DEFAULT_OIDC_CONFIG), custom_config["access_token_opts"] or {})
   for _, k in ipairs(custom_config["remove_id_token_claims"] or {}) do
     id_token[k] = nil
   end
+  for _, k in ipairs(custom_config["remove_refresh_id_token_claims"] or {}) do
+    refresh_id_token[k] = nil
+  end
   for _, k in ipairs(custom_config["remove_access_token_claims"] or {}) do
     access_token[k] = nil
   end
@@ -383,6 +396,7 @@ local function write_config(out, custom_config)
     :gsub("TOKEN_RESPONSE_EXPIRES_IN", token_response_expires_in)
     :gsub("TOKEN_RESPONSE_CONTAINS_REFRESH_TOKEN", token_response_contains_refresh_token)
     :gsub("REFRESHING_TOKEN_FAILS", refreshing_token_fails)
+    :gsub("REFRESH_RESPONSE_CONTAINS_ID_TOKEN", refresh_response_contains_id_token)
     :gsub("ACCESS_TOKEN_OPTS", serpent.block(access_token_opts, {comment = false }))
     :gsub("JWK_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).jwk or DEFAULT_DELAY_RESPONSE))
     :gsub("TOKEN_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).token or DEFAULT_DELAY_RESPONSE))
@@ -395,6 +409,7 @@ local function write_config(out, custom_config)
     :gsub("FAKE_ID_TOKEN_SIGNATURE", custom_config["fake_id_token_signature"] or DEFAULT_FAKE_ID_TOKEN_SIGNATURE)
     :gsub("BREAK_ID_TOKEN_SIGNATURE", custom_config["break_id_token_signature"] or DEFAULT_BREAK_ID_TOKEN_SIGNATURE)
     :gsub("NONE_ALG_ID_TOKEN_SIGNATURE", custom_config["none_alg_id_token_signature"] or DEFAULT_NONE_ALG_ID_TOKEN_SIGNATURE)
+    :gsub("REFRESH_ID_TOKEN", serpent.block(refresh_id_token, {comment = false }))
     :gsub("ID_TOKEN", serpent.block(id_token, {comment = false }))
     :gsub("ACCESS_TOKEN", serpent.block(access_token, {comment = false }))
     :gsub("UNAUTH_ACTION", custom_config["unauth_action"] and ('"' .. custom_config["unauth_action"] .. '"') or DEFAULT_UNAUTH_ACTION)

tests/spec/token_refresh_spec.lua

@@ -59,6 +59,10 @@ describe("if there is an active but expired login and refresh is not configured
   it("the token gets refreshed", function()
     assert.error_log_contains("request body for token endpoint call: .*grant_type=refresh_token.*")
   end)
+  -- token endpoint response contains id token by default
+  it ("the id token gets refreshed", function()
+    assert.error_log_contains("id_token refreshed")
+  end)
   it("the access token is returned by authenticate", function()
     assert.is_not.error_log_contains("authenticate didn't return any access token")
   end)
@@ -153,3 +157,50 @@ describe("if there is an active but expired login and refreshing it fails", func
   end)
 end)
 
+describe("if token refresh doesn't add a new id_token", function()
+  test_support.start_server({
+    token_response_expires_in = 0,
+    refresh_response_contains_id_token = "false",
+  })
+  teardown(test_support.stop_server)
+  local _, _, cookies = test_support.login()
+  os.execute("sleep 1.5")
+  local _, status = http.request({
+    url = "http://localhost/default/t",
+    redirect = false,
+    headers = { cookie = cookies },
+  })
+  it("no redirect occurs on the next call", function()
+    assert.are.equals(200, status)
+  end)
+  it ("the id token doesn't get refreshed", function()
+    assert.is_not.error_log_contains("id_token refreshed")
+  end)
+  it("the access token is returned by authenticate", function()
+    assert.is_not.error_log_contains("authenticate didn't return any access token")
+  end)
+end)
+
+describe("if refresh contains an invalid id_token", function()
+  test_support.start_server({
+    token_response_expires_in = 0,
+    remove_refresh_id_token_claims = { "iss" }
+  })
+  teardown(test_support.stop_server)
+  local _, _, cookies = test_support.login()
+  os.execute("sleep 1.5")
+  local _, status = http.request({
+    url = "http://localhost/default/t",
+    redirect = false,
+    headers = { cookie = cookies },
+  })
+  it ("the id token doesn't get refreshed", function()
+    assert.is_not.error_log_contains("id_token refreshed")
+  end)
+  it("the tokens are rejected", function()
+    assert.error_log_contains("invalid id token, discarding tokens returned while refreshing")
+  end)
+  it("the access token is discarded", function()
+    assert.error_log_contains("lost access token")
+  end)
+end)