-
Notifications
You must be signed in to change notification settings - Fork 259
feat(plugins): Implement plugin API key management and authentication middleware #746
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
feat(plugins): Implement plugin API key management and authentication middleware #746
Conversation
… middleware The purpose of this is to allow plugins to access certain internal APIs via - Added PluginAPIKey and APIKeyManager for managing API keys associated with plugins. - Introduced PluginAuthMiddleware to handle API key validation for plugin requests. - Updated RouterDef to support plugin accessible endpoints with authentication. - Modified various API registration functions to include plugin accessibility checks. - Enhanced plugin lifecycle management to generate and revoke API keys as needed. - Updated plugin specifications to include permitted API endpoints for access control.
|
It's a pretty big PR, sorry about that, but about 2/3rds of the additions (1100 lines) are from the example plugin so it looks worse than it is |
|
regarding
That should be possible, currently a given API key lives only as long as the plugin is running, no attempt is made at persisting the token DB since tokens are regenerated at runtime. Note: the distinction between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "review" is just a code tour, I do this with big PRs to make them easier to digest.
Feel free to resolve things as you go to reduce the clutter
Actually, expanding on this further, I wonder how viable it would be to have the example plugin read a list of endpoints to register and endpoints to hit from a config file and then dynamically create that "report" in the UI based on that list Thinking about it more, yeah, I could definitely do that. The real question then is whether to include that in this PR or wait for this to be merged and do it in a new PR The format could be something like: permitted-endpoints:
- endpoint: /api/access/list
method: GET
reason: used as an example of a permitted and pluginAccessible endpoint
- endpoint: /api/cert/tls
method: GET
reason: used as an example of a "permitted" but not pluginAccessible
endpoints-to-hit:
- endpoint: /api/access/list
expected-result: success
- endpoint: /api/access/list
api-key-override: invalid-key
expected-result: redirect
- endpoint: /api/cert/tls
expected-result: redirect
- endpoint: /api/access/list
method-override: HEAD
expected-result: unauthorized
- endpoint: /api/acme/listExpiredDomains
expected-result: unauthorized
- endpoint: /api/proxy/list
expected-result: unauthorizedwhich could allow the plugin to be used as a kind of test-suite for the rest API The webUI of the plugin would generate those blocks automatically based on the response (green for success, red for redirect, yellow for unauthorized, purple for other) and report whether the response matched the expectation |
tobychui
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work and high quality code!
I have left some comments in how the handleFunc and plugin API should be two separated handlers instead of using the same handler as auth.handleFunc. Maybe you can restore the api.go and create a new plugin_api.go with a new handler instead?
That way, we can keep the behavior of old auth.handleFunc and make it more easy for future expansion regarding the token store which automatic testing script can utilize.
src/mod/auth/router.go
Outdated
| } | ||
|
|
||
| func (router *RouterDef) HandleFunc(endpoint string, handler func(http.ResponseWriter, *http.Request)) error { | ||
| func (router *RouterDef) HandleFunc(endpoint string, handler func(http.ResponseWriter, *http.Request), pluginAccessible bool) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this will break the compatibility with the native net.http HandleFunc function. I guess it would be better to implement it as alternative API instead (maybe adding a new handle function instead?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I didn't even think about that, great point
|
Sure, those comments make a lot of sense. Also, on that point, which endpoints should be included there? Should I just start with the ones I have marked as pluginAccessible and leave the rest as future work? I'll probably get started on this tomorrow |
nvm, it was easier than I thought, commits incoming |
|
@tobychui |
|
@AnthonyMichaelTDM Cool! Let me release v3.2.5 first and I will fork a v3.2.6 branch and merge this into the new branch. This should give us more time testing it before it got released. Will wait for your example update, let me know when you are ready :) |
|
|
tobychui
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I few minor things that I might wanna change but I will merge this first.
Sounds good, do you want to open an issue for those minor things? |
No, I think I will directly push it into the v3.2.6 branch. |


Motivation
Currently, Zoraxy renders plugins in a webframe and allows various ways of deciding which kinds of requests should be handled by a plugin.
A big limitation of the plugin system is that while plugins can make requests to their own backend, they cannot interract with the zoraxy api in any meaningful capacity.
I'm implementing a crowdsec bouncer for zoraxy, ideally I'd be able to adjust the zoraxy blacklist directly but due to the previously mentioned limitations I had to implement the bouncer as a dynamic capture plugin which is less then ideal for a variety of reasons.
Purpose
The purpose of this PR is to give plugins scoped access to internal APIs over https.
Overview
Plugins can declare a list of
PermittedAPIEndpoint's in theirIntroSpect, each must include the endpoint, the http method (e.g. GET, PUSH), and the reason.Plugins that declare such a list will receive an API key as part of their
ConfigureSpecThese API keys are generated for each applicable plugin when they start, and are invalidated when the plugin stops.
When making requests to the Zoraxy API, the api key should be included as
Bearer <API_KEY>in theAuthorizationheader.The
authRouterhas been modified so that handlers forpluginAccessibleendpoints are wrapped in an additional layer of middleware that will look for the Auth Bearer header, and if its present will validate that the provided API key has access to the endpoint. If the key has access, the normal auth mechanism can be bypassed. If the key is invalid or does not have access, the request fails with a 403 Unauthorized message.Requests to non-
pluginAccessibleendpoints redirect to the login page as they did before.Requests to
pluginAccessibleendpoints that are missing the Auth Bearer header are treated as they were before.Status
Pictures
Where PermittedAPIEndpoints are displayed in the web UI
UI of the example plugin
This UI demonstrates that:
Future work