spacewander commented on a change in pull request #2903:
URL: https://github.com/apache/apisix/pull/2903#discussion_r549056709
##########
File path: t/plugin/openid-connect.t
##########
@@ -263,20 +1053,105 @@ passed
-=== TEST 7: access
---- timeout: 10s
+
+=== TEST 14: Update route URI to '/uri' where upstream endpoint returns
request headers in response body.
Review comment:
The `TEST 14` is repeated?
##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -95,56 +120,107 @@ function _M.check_schema(conf)
end
-local function has_bearer_access_token(ctx)
+local function get_bearer_access_token(ctx)
+ -- Get Authorization header, maybe.
local auth_header = core.request.header(ctx, "Authorization")
if not auth_header then
- return false
+ -- No Authorization header, get X-Access-Token header, maybe.
+ local access_token_header = core.request.header(ctx, "X-Access-Token")
+ if not access_token_header then
+ -- No X-Access-Token header neither.
+ return false, nil, nil
+ end
+
+ -- Return extracted header value.
+ return true, access_token_header, nil
end
+ -- Check format of Authorization header.
local res, err = ngx_re.split(auth_header, " ", nil, nil, 2)
if not res then
- return false, err
+ return false, nil, err
end
- if res[1] == "bearer" then
- return true
+ if string.lower(res[1]) == "bearer" then
+ -- Return extracted token.
+ return true, res[2], nil
end
return false
end
+
local function introspect(ctx, conf)
- if has_bearer_access_token(ctx) or conf.bearer_only then
- local res, err
+ -- Extract token, maybe.
+ local has_token, token, _ = get_bearer_access_token(ctx)
Review comment:
Better to check the err: `_`.
##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -95,56 +120,107 @@ function _M.check_schema(conf)
end
-local function has_bearer_access_token(ctx)
+local function get_bearer_access_token(ctx)
+ -- Get Authorization header, maybe.
local auth_header = core.request.header(ctx, "Authorization")
if not auth_header then
- return false
+ -- No Authorization header, get X-Access-Token header, maybe.
+ local access_token_header = core.request.header(ctx, "X-Access-Token")
+ if not access_token_header then
+ -- No X-Access-Token header neither.
+ return false, nil, nil
+ end
+
+ -- Return extracted header value.
+ return true, access_token_header, nil
end
+ -- Check format of Authorization header.
local res, err = ngx_re.split(auth_header, " ", nil, nil, 2)
if not res then
- return false, err
+ return false, nil, err
end
- if res[1] == "bearer" then
- return true
+ if string.lower(res[1]) == "bearer" then
+ -- Return extracted token.
+ return true, res[2], nil
end
return false
end
+
local function introspect(ctx, conf)
- if has_bearer_access_token(ctx) or conf.bearer_only then
- local res, err
+ -- Extract token, maybe.
+ local has_token, token, _ = get_bearer_access_token(ctx)
+
+ if not has_token then
+ -- Could not find token.
- if conf.public_key then
- res, err = openidc.bearer_jwt_verify(conf)
- if res then
- return res
- end
- else
- res, err = openidc.introspect(conf)
- if err then
- return ngx.HTTP_UNAUTHORIZED, err
- else
- return res
- end
- end
if conf.bearer_only then
- ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm
- .. '",error="' .. err .. '"'
- return ngx.HTTP_UNAUTHORIZED, err
+ -- Token strictly required in request.
+ ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm ..
'"'
+ return ngx.HTTP_UNAUTHORIZED, "No bearer token found in request.",
nil, nil
+ else
+ -- Return empty result.
+ return nil, nil, nil, nil
end
end
- return nil
+ -- If we get here, token was found in request.
+
+ if conf.public_key then
+ -- Validate token against public key.
+ -- TODO: In the called method, the openidc module will try to extract
+ -- the token by itself again -- from a request header or session
cookie.
+ -- It is inefficient that we also need to extract it (just from
headers)
+ -- so we can add it in the configured header. Find a way to use
openidc
+ -- module's internal methods to extract the token.
+ local res, err = openidc.bearer_jwt_verify(conf)
+
+ if err then
+ -- Error while validating or token invalid.
+ ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm ..
+ '", error="invalid_token", error_description="' .. err .. '"'
+ return ngx.HTTP_UNAUTHORIZED, err, nil, nil
+ end
+
+ -- Token successfully validated.
+ return res, err, token, nil
+ else
+ -- Validate token against introspection endpoint.
+ -- TODO: Same as above for public key validation.
+ local res, err = openidc.introspect(conf)
+
+ if err then
+ ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm ..
+ '", error="invalid_token", error_description="' .. err .. '"'
+ return ngx.HTTP_UNAUTHORIZED, err, nil, nil
+ end
+
+ -- Token successfully validated and response from the introspection
+ -- endpoint contains the userinfo.
+ return res, err, token, res
+ end
end
-local function add_user_header(user)
- local userinfo = core.json.encode(user)
- ngx.req.set_header("X-Userinfo", ngx_encode_base64(userinfo))
+local function add_access_token_header(ctx, conf, token)
Review comment:
Better to check token in this function.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
