Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Handling of Bearer Token with trailing space in openidc_get_bearer_access_token #537

Closed
gustoliv opened this issue Apr 3, 2025 · 0 comments · Fixed by #538
Closed

Comments

@gustoliv
Copy link

gustoliv commented Apr 3, 2025

The function openidc_get_bearer_access_token does not properly handle cases where the Bearer token is empty and it has a trailing space in it (e.g., "Bearer ").
Instead of detecting this as an invalid token, the function currently treats it as a valid string.
This can lead to incorrect behavior when validating authorization headers.

Expected Behavior

The function should correctly identify an empty token and return an appropriate error message.

Proposed Fix

Modify the function to strip whitespaces from headers and explicitly check for an empty access token after extracting it:

++local function trim(s)
++  if s then
++    return s:match("^%s*(.-)%s*$")
++  end
++  return s
++end

local function openidc_get_bearer_access_token(opts)

  local err

  local accept_token_as = opts.auth_accept_token_as or "header"

  if accept_token_as:find("cookie") == 1 then
    return openidc_get_bearer_access_token_from_cookie(opts)
  end

  -- get the access token from the Authorization header
  local headers = ngx.req.get_headers()
  local header_name = opts.auth_accept_token_as_header_name or "Authorization"
--  local header = get_first(headers[header_name])
++  local header = trim(get_first(headers[header_name]))

  if header == nil then
    err = "no Authorization header found"
    log(ERROR, err)
    return nil, err
  end

  local divider = header:find(' ')
  if divider == nil or divider == 0 or string.lower(header:sub(0, divider - 1)) ~= string.lower("Bearer") then
    err = "no Bearer authorization header value found"
    log(ERROR, err)
    return nil, err
  end

  local access_token = header:sub(divider + 1)
--  if access_token == nil then
++  if access_token == "" then
    err = "no Bearer access token value found"
    log(ERROR, err)
    return nil, err
  end

  return access_token, err
end

The check was changed from nil to "" because header:sub(divider + 1) always returns a string.
If the token is missing, it will be an empty string (""), not nil.
This ensures the validation correctly identifies an invalid token.

Impact

This issue can cause improper handling of authentication requests.

I'll submit a PR proposing these changes later.

@gustoliv gustoliv changed the title Incorrect Handling of Empty Bearer Token in openidc_get_bearer_access_token Incorrect Handling of Bearer Token with trailing space in openidc_get_bearer_access_token Apr 4, 2025
gustoliv pushed a commit to gustoliv/lua-resty-openidc that referenced this issue Apr 7, 2025
… space in openidc_get_bearer_access_token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant