-
Notifications
You must be signed in to change notification settings - Fork 66
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(core): unified router context #429
Conversation
This now works by: - fetching rules in context from useRuntimeConfig() - ensuring event context is read at the right level by hooking in render:response rather than beforeResponse
- only HTML renderer writes security headers - runtime hook is able to fetch dynamic values from API
make resolveSecurityRules synchronous
minimal swr test case
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@Baroshem @huang-julien @harlan-zw happy to get your feedback on this |
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.
Few comments but overall this is a great piece of work you did there @vejja
I have tested it and it seems that it is working correctly. After you answer or fix the comments, I think we could merge it to main and test once again there and release it as a new version :)
@Baroshem : do you want me to squash commits for clarity ? |
No need @vejja We can keep them as history :) I will merge this PR in a few hours do that we can tesg once again on main and release a new version 🚀 |
Types of changes
Description
This PR is a significant rewrite of the core engine of Nuxt Security, motivated primarily by the introduction of runtime hooks in PR #298 by @huang-julien and comments thereon by @harlan-zw.
Background
The background for this PR is further detailed in discussion #346 (Refactoring Headers) and in RFC #300 (Move out everything from readonly runtimeConfig).
Currently, Nuxt-Security accepts route-level security definitions via the
routeRules
option. Also, Nuxt-Security accepts runtime modification of security definitions via runtime hooks.This has resulted in two parallel code paths, with sometimes different behaviours (e.g. in whether definitions are merged or overwritten). The historic code path used the native
getRouteRules
function, while the hooks code path used a customevent.context
object.This PR unifies these two code paths by moving all security definitions to the new
event.context
object introduced by the runtime hooks, making it the unique source of truth. Instead of using thegetRouteRules
function, we now use aresolveSecurityRules
function everywhere.Benefits
With this PR, we introduce several benefits to Nuxt-Security
All security options can now be modified via runtime hooks
It is now possible to modify any of the Nuxt Security options, and not solely the headers : any other option such as
hidePoweredBy
,rateLimiter
, is now taken into consideration and applied at route level.Route rules are now consistently merged
The router merging strategy is now the same irrespective of the way the security options are set (inline, global,
routeRules
, and runtime hooks). Previously, it was a mix ofdefu
,defuReplaceArray
, and plain overwriting - leading to confusion on how nested rules would apply (see CSP not working for specific routes? (Google Maps) #430 for instance). We now apply thedefuReplaceArray
strategy across the board.Clear scoping of security headers to HTML pages, SWR support
We now make a clearer distinction between the scope of Nitro plugins (modifying HTML pages and their headers) and the scope of Server middlewares (functions that apply to all routes). This avoids to overwrite headers of non-HTML assets with irrelevant options, and as a result we are able to support SWR natively.
Route-level support of RateLimiter
Thanks to the ability to
resolveSecurityRoutes
at runtime, we are now able to support route-based definitions for the Rate Limiter. This solves the issue of getting 429 denials for routes where we want to have a higher rate limit. We also take this opportunity to solve the issue of getting 429s when pre-rendering.New runtime hook
This PR introduces a new runtime hook :
nuxt-security:routeRules
, that allows to modify any security rule on any route. With this hook, the user is now able to apply any strategy for the rule (merge, overwrite, append, etc.).The syntax for this new hook is simpler than the
nuxt-security:headers
syntax.The former
nuxt-security:ready
&nuxt-security:headers
hooks are still supported but we are soft-depecrating them by removing them from the documentation.Issues closed by this PR
defineRouteRules
#385 : All security options are now resolved with the same merging strategyfalse
#432 : The functioninsertNonceInCsp
now correctly handles boolean value for CSP directivesOther notes
This PR also soft-deprecates the substitution merging via string syntax feature. This is now rendered unnecessary because the
defuReplaceArray
strategy is applied consistently everywhere.We are removing corresponding mentions in the documentation, which were confusing (it only applied to headers, and it only applied in the router merging step but not in the definition step). The feature still exists to maintain backwards compatibility.
Please note that some security options can only be applied globally (
removeLoggers
,csrf
andbasicAuth
) because they depend on third-party modules. The TypeScript definitions have been updated to remove these 3 options from the properties that can be set at route-level.Checklist:
Added tests for