Skip to content

Commit c3a75fa

Browse files
phirvonetsyrjanen
phirvone
authored and
tsyrjanen
committed
Introspection writes to error log if authorization bearer header is missing (#18)
* openidc.introspect is invoked only if request has Authorization Bearer header to prevent error logging from openidc.introspect. Added first draft of unit tests. * Got rid of luanit_agent. * lib removed.
1 parent bba6e57 commit c3a75fa

File tree

8 files changed

+481
-11
lines changed

8 files changed

+481
-11
lines changed

‎kong/plugins/oidc/handler.lua

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ local utils = require("kong.plugins.oidc.utils")
44
local filter = require("kong.plugins.oidc.filter")
55
local session = require("kong.plugins.oidc.session")
66

7-
local openidc = require("resty.openidc")
87
local cjson = require("cjson")
98

109
OidcHandler.PRIORITY = 1000
@@ -19,18 +18,17 @@ function OidcHandler:access(config)
1918
local oidcConfig = utils.get_options(config, ngx)
2019

2120
if filter.shouldProcessRequest(oidcConfig) then
22-
ngx.log(ngx.DEBUG, "In plugin OidcHandler:access calling authenticate, requested path: " .. ngx.var.request_uri)
2321
session.configure(config)
2422
handle(oidcConfig)
2523
else
26-
ngx.log(ngx.DEBUG, "In plugin OidcHandler:access NOT calling authenticate, requested path: " .. ngx.var.request_uri)
24+
ngx.log(ngx.DEBUG, "OidcHandler ignoring request, path: " .. ngx.var.request_uri)
2725
end
2826

29-
ngx.log(ngx.DEBUG, "In plugin OidcHandler:access Done")
27+
ngx.log(ngx.DEBUG, "OidcHandler done")
3028
end
3129

3230
function handle(oidcConfig)
33-
local response = nil
31+
local response
3432
if oidcConfig.introspection_endpoint then
3533
response = introspect(oidcConfig)
3634
if response then
@@ -48,7 +46,8 @@ function handle(oidcConfig)
4846
end
4947

5048
function make_oidc(oidcConfig)
51-
local res, err = openidc.authenticate(oidcConfig)
49+
ngx.log(ngx.DEBUG, "OidcHandler calling authenticate, requested path: " .. ngx.var.request_uri)
50+
local res, err = require("resty.openidc").authenticate(oidcConfig)
5251
if err then
5352
if oidcConfig.recovery_page_path then
5453
ngx.log(ngx.DEBUG, "Entering recovery page: " .. oidcConfig.recovery_page_path)
@@ -60,11 +59,15 @@ function make_oidc(oidcConfig)
6059
end
6160

6261
function introspect(oidcConfig)
63-
local res, err = openidc.introspect(oidcConfig)
64-
if err then
65-
return nil
62+
if utils.has_bearer_access_token() then
63+
local res, err = require("resty.openidc").introspect(oidcConfig)
64+
if err then
65+
return nil
66+
end
67+
ngx.log(ngx.DEBUG, "OidcHandler introspect succeeded, requested path: " .. ngx.var.request_uri)
68+
return res
6669
end
67-
return res
70+
return nil
6871
end
6972

7073

‎kong/plugins/oidc/utils.lua

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
local M = {}
22

33
local function parseFilters(csvFilters)
4-
filters = {}
4+
local filters = {}
55
if (not (csvFilters == nil)) then
66
for pattern in string.gmatch(csvFilters, "[^,]+") do
77
table.insert(filters, pattern)
@@ -66,4 +66,15 @@ function M.injectUser(user)
6666
ngx.ctx.authenticated_consumer = tmp_user
6767
end
6868

69+
function M.has_bearer_access_token()
70+
local header = ngx.req.get_headers()['Authorization']
71+
if header and header:find(" ") then
72+
local divider = header:find(' ')
73+
if string.lower(header:sub(0, divider-1)) == string.lower("Bearer") then
74+
return true
75+
end
76+
end
77+
return false
78+
end
79+
6980
return M

‎test/unit/test_filter.lua

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,5 @@ function TestFilter:testProcessRequestWhenTheyAreNoFiltersEmpty()
4343
end
4444

4545
lu.run()
46+
47+

‎test/unit/test_filters_advanced.lua

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
local filter = require("kong.plugins.oidc.filter")
2+
local lu = require("luaunit")
3+
4+
TestFilter = {}
5+
ngx = {
6+
var = {
7+
uri = ""
8+
}
9+
}
10+
11+
local config = {
12+
filters = { "^/auth$","^/auth[^%w_%-%.~]","^/arc$","^/arc[^%w_%-%.~]","^/projects/%d+/zeppelin[^%w_%-%.~]","^/projects/%d+/zeppelin$"}
13+
}
14+
function TestFilter:testIgnoreRequestWhenUriIsAuth()
15+
ngx.var.uri = "/auth"
16+
lu.assertFalse(filter.shouldProcessRequest(config))
17+
18+
ngx.var.uri = "/auth/"
19+
lu.assertFalse(filter.shouldProcessRequest(config))
20+
end
21+
22+
23+
function TestFilter:testIgnoreRequestWhenUriIsArc()
24+
ngx.var.uri = "/arc"
25+
lu.assertFalse(filter.shouldProcessRequest(config))
26+
27+
ngx.var.uri = "/arc/"
28+
lu.assertFalse(filter.shouldProcessRequest(config))
29+
end
30+
31+
32+
function TestFilter:testProcessRequestWhichAreAllowed()
33+
ngx.var.uri = "/not_auth"
34+
assert(filter.shouldProcessRequest(config) == true)
35+
end
36+
37+
---------------------------NEW TESTS
38+
function TestFilter:testIgnoreRequestBeingIdenticalToFilter()
39+
ngx.var.uri = "/arc"
40+
lu.assertFalse(filter.shouldProcessRequest(config) )
41+
end
42+
43+
function TestFilter:testIgnoreRequestStartingWithFilterFollowedBySlash()
44+
ngx.var.uri = "/arc/"
45+
lu.assertFalse(filter.shouldProcessRequest(config) )
46+
end
47+
48+
function TestFilter:testIgnoreRequestStartingWithFilterFollowedByPaths()
49+
ngx.var.uri = "/arc/de/triomphe"
50+
lu.assertFalse(filter.shouldProcessRequest(config) )
51+
52+
end
53+
54+
function TestFilter:testIgnoreRequestStartingWithFilterFollowedByQuestionmark()
55+
ngx.var.uri = "/arc?"
56+
lu.assertFalse(filter.shouldProcessRequest(config) )
57+
58+
ngx.var.uri = "/arc?de=triomphe"
59+
lu.assertFalse(filter.shouldProcessRequest(config) )
60+
61+
end
62+
63+
64+
function TestFilter:testIgnoreRequestStartingWithFilterFollowedByQuestionmark()
65+
ngx.var.uri = "/arc?"
66+
lu.assertFalse(filter.shouldProcessRequest(config) )
67+
68+
ngx.var.uri = "/arc?de=triomphe"
69+
lu.assertFalse(filter.shouldProcessRequest(config) )
70+
71+
end
72+
73+
function TestFilter:testPrefixNotAtTheStart()
74+
ngx.var.uri = "/process_this/arc"
75+
lu.assertTrue(filter.shouldProcessRequest(config) )
76+
77+
ngx.var.uri = "/process_this/arc/de/triomphe"
78+
lu.assertTrue(filter.shouldProcessRequest(config) )
79+
80+
ngx.var.uri = "/process_this/architecture"
81+
lu.assertTrue(filter.shouldProcessRequest(config) )
82+
83+
end
84+
85+
86+
function TestFilter:testLowercaseLetterAfterPrefix()
87+
ngx.var.uri = "/architecture"
88+
lu.assertTrue(filter.shouldProcessRequest(config) )
89+
end
90+
91+
function TestFilter:testUppercaseLetterLetterAfterPrefix()
92+
ngx.var.uri = "/archITACTURE"
93+
lu.assertTrue(filter.shouldProcessRequest(config) )
94+
end
95+
96+
function TestFilter:testDigitAfterPrefix()
97+
ngx.var.uri = "/arc123"
98+
lu.assertTrue(filter.shouldProcessRequest(config) )
99+
end
100+
101+
function TestFilter:testHyphenAfterPrefix()
102+
ngx.var.uri = "/arc-123"
103+
lu.assertTrue(filter.shouldProcessRequest(config) )
104+
end
105+
106+
function TestFilter:testPeriodAfterPrefix()
107+
ngx.var.uri = "/arc.123"
108+
lu.assertTrue(filter.shouldProcessRequest(config) )
109+
end
110+
111+
112+
function TestFilter:testUnderscoreAfterPrefix()
113+
ngx.var.uri = "/arc_123"
114+
lu.assertTrue(filter.shouldProcessRequest(config) )
115+
end
116+
117+
function TestFilter:testTildeAfterPrefix()
118+
ngx.var.uri = "/arc~123"
119+
lu.assertTrue(filter.shouldProcessRequest(config) )
120+
end
121+
122+
123+
124+
--zeppelin tests
125+
function TestFilter:testZeppelin()
126+
ngx.var.uri = "/projects/10/zeppelin"
127+
lu.assertFalse(filter.shouldProcessRequest(config))
128+
end
129+
130+
function TestFilter:testSlashAfterZeppelin()
131+
ngx.var.uri = "/projects/10/zeppelin/"
132+
lu.assertFalse(filter.shouldProcessRequest(config))
133+
end
134+
135+
136+
function TestFilter:testQuestionMarkAfterZeppelin()
137+
ngx.var.uri = "/projects/10/zeppelin?"
138+
lu.assertFalse(filter.shouldProcessRequest(config))
139+
end
140+
141+
function TestFilter:testExtraCharactersAfterZeppelin()
142+
ngx.var.uri = "/projects/10/zeppelinextras"
143+
lu.assertTrue(filter.shouldProcessRequest(config))
144+
end
145+
146+
function TestFilter:testZeppelinNotAtStart()
147+
ngx.var.uri = "/this/projects/10/zeppelin"
148+
lu.assertTrue(filter.shouldProcessRequest(config))
149+
end
150+
151+
152+
153+
154+
lu.run()
155+
156+

‎test/unit/test_handler_mocking_openidc.lua

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package.path = package.path .. ";test/lib/?.lua;;" -- kong & co
2+
3+
local lu = require("luaunit")
4+
5+
6+
TestHandler = {}
7+
8+
function TestHandler:setUp()
9+
self.logs = {}
10+
self.ngx_headers = {}
11+
self.mocked_ngx = {
12+
DEBUG = "debug",
13+
ctx = {},
14+
header = {},
15+
var = {request_uri = "/"},
16+
req = {
17+
get_uri_args = function(...) end,
18+
set_header = function(...) end,
19+
get_headers = function(...) return self.ngx_headers end
20+
},
21+
log = function(...)
22+
self.logs[#self.logs+1] = table.concat(arg, " ")
23+
print("ngx.log: ", unpack(arg))
24+
end,
25+
say = function(...) end,
26+
exit = function(...) end,
27+
redirect = function(...) end
28+
}
29+
self.ngx = _G.ngx
30+
_G.ngx = self.mocked_ngx
31+
32+
self.resty = package.loaded.resty
33+
package.loaded["resty.openidc"] = nil
34+
self.module_resty = {openidc = {
35+
authenticate = function(...) return {}, nil end }
36+
}
37+
package.preload["resty.openidc"] = function()
38+
return self.module_resty.openidc
39+
end
40+
41+
self.cjson = package.loaded.cjson
42+
package.loaded.cjson = nil
43+
package.preload["cjson"] = function()
44+
return {encode = function(...) return "encoded" end}
45+
end
46+
47+
self.handler = require("kong.plugins.oidc.handler")()
48+
end
49+
50+
function TestHandler:tearDown()
51+
_G.ngx = self.ngx
52+
package.loaded.resty = self.resty
53+
package.loaded.cjson = self.cjson
54+
end
55+
56+
function TestHandler:log_contains(str)
57+
return table.concat(self.logs, "//"):find(str) and true or false
58+
end
59+
60+
function TestHandler:test_authenticate_ok_no_userinfo()
61+
self.module_resty.openidc.authenticate = function(opts)
62+
return {}, false
63+
end
64+
65+
self.handler:access({})
66+
lu.assertTrue(self:log_contains("calling authenticate"))
67+
end
68+
69+
function TestHandler:test_authenticate_ok_with_userinfo()
70+
self.module_resty.openidc.authenticate = function(opts)
71+
return {user = {sub = "sub"}}, false
72+
end
73+
74+
self.handler:access({})
75+
lu.assertTrue(self:log_contains("calling authenticate"))
76+
end
77+
78+
function TestHandler:test_authenticate_nok_no_recovery()
79+
self.module_resty.openidc.authenticate = function(opts)
80+
return {}, true
81+
end
82+
83+
self.handler:access({})
84+
lu.assertTrue(self:log_contains("calling authenticate"))
85+
end
86+
87+
function TestHandler:test_authenticate_nok_with_recovery()
88+
self.module_resty.openidc.authenticate = function(opts)
89+
return {}, true
90+
end
91+
92+
self.handler:access({recovery_page_path = "x"})
93+
lu.assertTrue(self:log_contains("recovery page"))
94+
end
95+
96+
function TestHandler:test_introspect_ok_no_userinfo()
97+
self.module_resty.openidc.introspect = function(opts)
98+
return false, false
99+
end
100+
self.ngx_headers = {Authorization = "Bearer xxx"}
101+
102+
self.handler:access({introspection_endpoint = "x"})
103+
lu.assertTrue(self:log_contains("introspect succeeded"))
104+
end
105+
106+
function TestHandler:test_introspect_ok_with_userinfo()
107+
self.module_resty.openidc.introspect = function(opts)
108+
return {}, false
109+
end
110+
self.ngx_headers = {Authorization = "Bearer xxx"}
111+
112+
self.handler:access({introspection_endpoint = "x"})
113+
lu.assertTrue(self:log_contains("introspect succeeded"))
114+
end
115+
116+
117+
lu.run()
118+
119+

0 commit comments

Comments
 (0)