-
Notifications
You must be signed in to change notification settings - Fork 726
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
Add user script setting #2836
Add user script setting #2836
Conversation
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.
There are few linter issues detected and there is conflict in the locale files.
internal/database/migrations.go
Outdated
@@ -942,4 +942,10 @@ var migrations = []func(tx *sql.Tx) error{ | |||
_, err = tx.Exec(sql) | |||
return err | |||
}, | |||
func(tx *sql.Tx) (err error) { | |||
sql := `ALTER TABLE users ADD COLUMN script text not null default '';` |
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.
I'm wondering if it would not be better to rename this field to custom_js
instead of script
.
I know that there is an existing stylesheet
field, but it would have been better to use custom_css
.
<style nonce="{{ $stylesheetNonce }}">{{ .user.Stylesheet | safeCSS }}</style> | ||
{{ end }} | ||
{{ if .user.Script }} | ||
<script type="module" nonce="{{ $stylesheetNonce }}">{{ .user.Script | safeJS }}</script> |
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.
Instead of reusing $stylesheetNonce
, it would be preferable to rename this variable to something generic, for example: $cspNonce
.
What is the benefit of using type="module"
here?
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.
With type="module"
you can use import
and top-level await. In short, makes it easier to use modern JS features without a bundler, which is quite nice for something like this IMHO.
e488b78
to
e5abfae
Compare
Rebased, linted, and fixed the variable names! |
internal/model/user.go
Outdated
@@ -21,6 +21,7 @@ type User struct { | |||
EntryDirection string `json:"entry_sorting_direction"` | |||
EntryOrder string `json:"entry_sorting_order"` | |||
Stylesheet string `json:"stylesheet"` | |||
Script string `json:"script"` |
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.
Can you please rename Script
to CustomJS
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.
Done!
<style nonce="{{ $stylesheetNonce }}">{{ .user.Stylesheet | safeCSS }}</style> | ||
{{ if .user }} | ||
{{ $cspNonce := nonce }} | ||
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src * data:; media-src *; frame-src *; style-src 'self'{{ if .user.Stylesheet }} 'nonce-{{ $cspNonce }}'{{ end }}{{ if .user.Script }}; script-src 'nonce-{{ $cspNonce }}'{{ end }}; require-trusted-types-for 'script'; trusted-types ttpolicy;"> |
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.
The CSP is incorrect. The script-src
directive should always includes self
otherwise it won't load the application JS files.
it should be something like this: script-src 'self'{{ if .user.Script }} 'nonce-{{ $cspNonce }}'{{ end }}
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, that explains a regression I started noticing after the rebase. Fixed.
e5abfae
to
59bbb81
Compare
59bbb81
to
6fd059e
Compare
<style nonce="{{ $cspNonce }}">{{ .user.Stylesheet | safeCSS }}</style> | ||
{{ end }} | ||
{{ if .user.Script }} | ||
<script type="module" nonce="{{ $cspNonce }}">{{ .user.Script | safeJS }}</script> |
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.
@milhnl You forgot to rename .user.Script
to .user.CustomJS
in the template.
My bad. Had a very busy week. Thanks for merging! Learned a lesson in trusting gopls' rename feature 😇 |
This PR adds the Custom JS option.
I tried to follow the "Custom JS" translations, and have been running this for myself for about a week now.
I know I am not good at testing software. I don't really know what could break, I tried adding a script, and removing it, but this isn't a feature with a lot of varying code paths. Feedback is very welcome! I don't really have a lot of experience with Go.
Fixes #1742
Do you follow the guidelines?