Add cross-request forgery detection

I use the ten first digits of the cookie’s hash, that i believe it is
not a problem, has the advantage of not expiring until the user logs
out, and using a per user session token is explicitly allowed by
OWASP[0].

[0]: https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#synchronizer-token-pattern
This commit is contained in:
jordi fita mas 2023-02-02 11:39:34 +01:00
parent 7a439a40cc
commit 917db31227
12 changed files with 73 additions and 13 deletions

View File

@ -16,6 +16,7 @@ select user_id
, name , name
, role , role
, lang_tag , lang_tag
, left(cookie, 10) as csrf_token
from auth."user" from auth."user"
where email = current_user_email() where email = current_user_email()
and cookie = current_user_cookie() and cookie = current_user_cookie()
@ -27,6 +28,7 @@ select 0
, '' , ''
, 'guest'::name , 'guest'::name
, 'und' , 'und'
, ''
where not exists ( where not exists (
select 1 select 1
from auth."user" from auth."user"

View File

@ -142,6 +142,10 @@ func CompanyTaxDetailsHandler() http.Handler {
http.Error(w, err.Error(), http.StatusBadRequest) http.Error(w, err.Error(), http.StatusBadRequest)
return return
} }
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
if ok := page.DetailsForm.Validate(r.Context(), conn); ok { if ok := page.DetailsForm.Validate(r.Context(), conn); ok {
form := page.DetailsForm form := page.DetailsForm
conn.MustExec(r.Context(), "update company set business_name = $1, vatin = ($11 || $2)::vatin, trade_name = $3, phone = parse_packed_phone_number($4, $11), email = $5, web = $6, address = $7, city = $8, province = $9, postal_code = $10, country_code = $11, currency_code = $12 where company_id = $13", form.BusinessName, form.VATIN, form.TradeName, form.Phone, form.Email, form.Web, form.Address, form.City, form.Province, form.PostalCode, form.Country, form.Currency, company.Id) conn.MustExec(r.Context(), "update company set business_name = $1, vatin = ($11 || $2)::vatin, trade_name = $3, phone = parse_packed_phone_number($4, $11), email = $5, web = $6, address = $7, city = $8, province = $9, postal_code = $10, country_code = $11, currency_code = $12 where company_id = $13", form.BusinessName, form.VATIN, form.TradeName, form.Phone, form.Email, form.Web, form.Address, form.City, form.Province, form.PostalCode, form.Country, form.Currency, company.Id)
@ -243,6 +247,10 @@ func CompanyTaxHandler() http.Handler {
conn := getConn(r) conn := getConn(r)
company := mustGetCompany(r) company := mustGetCompany(r)
if taxId, err := strconv.Atoi(param); err == nil { if taxId, err := strconv.Atoi(param); err == nil {
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
conn.MustExec(r.Context(), "delete from tax where tax_id = $1", taxId) conn.MustExec(r.Context(), "delete from tax where tax_id = $1", taxId)
http.Redirect(w, r, "/company/"+company.Slug+"/tax-details", http.StatusSeeOther) http.Redirect(w, r, "/company/"+company.Slug+"/tax-details", http.StatusSeeOther)
} else { } else {
@ -252,6 +260,10 @@ func CompanyTaxHandler() http.Handler {
http.Error(w, err.Error(), http.StatusBadRequest) http.Error(w, err.Error(), http.StatusBadRequest)
return return
} }
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
if form.Validate() { if form.Validate() {
conn.MustExec(r.Context(), "insert into tax (company_id, name, rate) values ($1, $2, $3 / 100::decimal)", company.Id, form.Name, form.Rate.Integer()) conn.MustExec(r.Context(), "insert into tax (company_id, name, rate) values ($1, $2, $3 / 100::decimal)", company.Id, form.Name, form.Rate.Integer())
http.Redirect(w, r, "/company/"+company.Slug+"/tax-details", http.StatusSeeOther) http.Redirect(w, r, "/company/"+company.Slug+"/tax-details", http.StatusSeeOther)

View File

@ -27,6 +27,10 @@ func ContactsHandler() http.Handler {
http.Error(w, err.Error(), http.StatusBadRequest) http.Error(w, err.Error(), http.StatusBadRequest)
return return
} }
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
if form.Validate(r.Context(), conn) { if form.Validate(r.Context(), conn) {
conn.MustExec(r.Context(), "insert into contact (company_id, business_name, vatin, trade_name, phone, email, web, address, province, city, postal_code, country_code) values ($1, $2, ($12 || $3)::vatin, $4, parse_packed_phone_number($5, $12), $6, $7, $8, $9, $10, $11, $12)", company.Id, form.BusinessName, form.VATIN, form.TradeName, form.Phone, form.Email, form.Web, form.Address, form.City, form.Province, form.PostalCode, form.Country) conn.MustExec(r.Context(), "insert into contact (company_id, business_name, vatin, trade_name, phone, email, web, address, province, city, postal_code, country_code) values ($1, $2, ($12 || $3)::vatin, $4, parse_packed_phone_number($5, $12), $6, $7, $8, $9, $10, $11, $12)", company.Id, form.BusinessName, form.VATIN, form.TradeName, form.Phone, form.Email, form.Web, form.Address, form.City, form.Province, form.PostalCode, form.Country)
http.Redirect(w, r, "/company/"+company.Slug+"/contacts", http.StatusSeeOther) http.Redirect(w, r, "/company/"+company.Slug+"/contacts", http.StatusSeeOther)

View File

@ -17,6 +17,7 @@ const (
ContextConnKey = "numerus-database" ContextConnKey = "numerus-database"
sessionCookie = "numerus-session" sessionCookie = "numerus-session"
defaultRole = "guest" defaultRole = "guest"
csrfTokenField = "csfrToken"
) )
type loginForm struct { type loginForm struct {
@ -105,6 +106,10 @@ func LoginHandler() http.Handler {
func LogoutHandler() http.Handler { func LogoutHandler() http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
conn := getConn(r) conn := getConn(r)
conn.MustExec(r.Context(), "select logout()") conn.MustExec(r.Context(), "select logout()")
http.SetCookie(w, createSessionCookie("", -24*time.Hour)) http.SetCookie(w, createSessionCookie("", -24*time.Hour))
@ -140,6 +145,7 @@ type AppUser struct {
LoggedIn bool LoggedIn bool
Role string Role string
Language language.Tag Language language.Tag
CsrfToken string
} }
func CheckLogin(db *Db, next http.Handler) http.Handler { func CheckLogin(db *Db, next http.Handler) http.Handler {
@ -158,9 +164,9 @@ func CheckLogin(db *Db, next http.Handler) http.Handler {
LoggedIn: false, LoggedIn: false,
Role: defaultRole, Role: defaultRole,
} }
row := conn.QueryRow(ctx, "select coalesce(email, ''), role, lang_tag from user_profile") row := conn.QueryRow(ctx, "select coalesce(email, ''), role, lang_tag, csrf_token from user_profile")
var langTag string var langTag string
if err := row.Scan(&user.Email, &user.Role, &langTag); err != nil { if err := row.Scan(&user.Email, &user.Role, &langTag, &user.CsrfToken); err != nil {
panic(err) panic(err)
} }
user.LoggedIn = user.Email != "" user.LoggedIn = user.Email != ""
@ -171,6 +177,16 @@ func CheckLogin(db *Db, next http.Handler) http.Handler {
}) })
} }
func verifyCsrfTokenValid(r *http.Request) error {
user := getUser(r)
token := r.FormValue(csrfTokenField)
if user.CsrfToken == token {
return nil
}
locale := getLocale(r)
return errors.New(locale.Get("Cross-site request forgery detected."))
}
func getUser(r *http.Request) *AppUser { func getUser(r *http.Request) *AppUser {
return r.Context().Value(ContextUserKey).(*AppUser) return r.Context().Value(ContextUserKey).(*AppUser)
} }

View File

@ -104,6 +104,10 @@ func ProfileHandler() http.Handler {
http.Error(w, err.Error(), http.StatusBadRequest) http.Error(w, err.Error(), http.StatusBadRequest)
return return
} }
if err := verifyCsrfTokenValid(r); err != nil {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
if ok := form.Validate(); ok { if ok := form.Validate(); ok {
//goland:noinspection SqlWithoutWhere //goland:noinspection SqlWithoutWhere
cookie := conn.MustGetText(r.Context(), "", "update user_profile set name = $1, email = $2, lang_tag = $3 returning build_cookie()", form.Name, form.Email, form.Language) cookie := conn.MustGetText(r.Context(), "", "update user_profile set name = $1, email = $2, lang_tag = $3 returning build_cookie()", form.Name, form.Email, form.Language)

View File

@ -1,6 +1,7 @@
package pkg package pkg
import ( import (
"fmt"
"html/template" "html/template"
"io" "io"
"net/http" "net/http"
@ -13,6 +14,7 @@ func templateFile(name string) string {
func mustRenderTemplate(wr io.Writer, r *http.Request, layout string, filename string, data interface{}) { func mustRenderTemplate(wr io.Writer, r *http.Request, layout string, filename string, data interface{}) {
locale := getLocale(r) locale := getLocale(r)
company := getCompany(r) company := getCompany(r)
user := getUser(r)
t := template.New(filename) t := template.New(filename)
t.Funcs(template.FuncMap{ t.Funcs(template.FuncMap{
"gettext": locale.Get, "gettext": locale.Get,
@ -26,6 +28,9 @@ func mustRenderTemplate(wr io.Writer, r *http.Request, layout string, filename s
} }
return "/company/" + company.Slug + uri return "/company/" + company.Slug + uri
}, },
"csrfToken": func() template.HTML {
return template.HTML(fmt.Sprintf(`<input type="hidden" name="%s" value="%s">`, csrfTokenField, user.CsrfToken))
},
"addInputAttr": func(attr string, field *InputField) *InputField { "addInputAttr": func(attr string, field *InputField) *InputField {
field.Attributes = append(field.Attributes, template.HTMLAttr(attr)) field.Attributes = append(field.Attributes, template.HTMLAttr(attr))
return field return field

View File

@ -5,7 +5,7 @@ reset client_min_messages;
begin; begin;
select plan(47); select plan(53);
set search_path to numerus, auth, public; set search_path to numerus, auth, public;
@ -50,6 +50,13 @@ select column_privs_are('user_profile', 'lang_tag', 'invoicer', array['SELECT',
select column_privs_are('user_profile', 'lang_tag', 'admin', array['SELECT', 'UPDATE']); select column_privs_are('user_profile', 'lang_tag', 'admin', array['SELECT', 'UPDATE']);
select column_privs_are('user_profile', 'lang_tag', 'authenticator', array[]::text[]); select column_privs_are('user_profile', 'lang_tag', 'authenticator', array[]::text[]);
select has_column('user_profile', 'csrf_token');
select col_type_is('user_profile', 'csrf_token', 'text');
select column_privs_are('user_profile', 'csrf_token', 'guest', array ['SELECT']);
select column_privs_are('user_profile', 'csrf_token', 'invoicer', array['SELECT']);
select column_privs_are('user_profile', 'csrf_token', 'admin', array['SELECT']);
select column_privs_are('user_profile', 'csrf_token', 'authenticator', array[]::text[]);
set client_min_messages to warning; set client_min_messages to warning;
truncate auth."user" cascade; truncate auth."user" cascade;
@ -62,14 +69,14 @@ values (1, 'demo@tandem.blog', 'Demo', 'test', 'invoicer', '44facbb30d8a419dfd4b
; ;
prepare profile as prepare profile as
select user_id, email, name, role, lang_tag select user_id, email, name, role, lang_tag, csrf_token
from user_profile; from user_profile;
select set_config('request.user.cookie', '', false); select set_config('request.user.cookie', '', false);
select results_eq( select results_eq(
'profile', 'profile',
$$ values (0, null::email, '', 'guest'::name, 'und') $$, $$ values (0, null::email, '', 'guest'::name, 'und', '') $$,
'Should be set up with the guest user when no user logged in yet.' 'Should be set up with the guest user when no user logged in yet.'
); );
@ -77,7 +84,7 @@ select set_cookie( '44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tand
select results_eq( select results_eq(
'profile', 'profile',
$$ values (1, 'demo@tandem.blog'::email, 'Demo', 'invoicer'::name, 'ca') $$, $$ values (1, 'demo@tandem.blog'::email, 'Demo', 'invoicer'::name, 'ca', '44facbb30d') $$,
'Should only see the profile of the first user' 'Should only see the profile of the first user'
); );
@ -104,7 +111,7 @@ select throws_ok(
select results_eq( select results_eq(
'profile', 'profile',
$$ values (1, 'demo+update@tandem.blog'::email, 'Demo Update', 'invoicer'::name, 'es') $$, $$ values (1, 'demo+update@tandem.blog'::email, 'Demo Update', 'invoicer'::name, 'es', '44facbb30d') $$,
'Should see the changed profile of the first user' 'Should see the changed profile of the first user'
); );
@ -114,7 +121,7 @@ select set_cookie( '12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tan
select results_eq( select results_eq(
'profile', 'profile',
$$ values (5, 'admin@tandem.blog'::email, 'Admin', 'admin'::name, 'es') $$, $$ values (5, 'admin@tandem.blog'::email, 'Admin', 'admin'::name, 'es', '12af4c88b5') $$,
'Should only see the profile of the second user' 'Should only see the profile of the second user'
); );
@ -141,7 +148,7 @@ select throws_ok(
select results_eq( select results_eq(
'profile', 'profile',
$$ values (5, 'admin+update@tandem.blog'::email, 'Admin Update', 'admin'::name, 'ca') $$, $$ values (5, 'admin+update@tandem.blog'::email, 'Admin Update', 'admin'::name, 'ca', '12af4c88b5') $$,
'Should see the changed profile of the first user' 'Should see the changed profile of the first user'
); );

View File

@ -8,6 +8,7 @@ select
, name , name
, role , role
, lang_tag , lang_tag
, csrf_token
from numerus.user_profile from numerus.user_profile
where false; where false;

View File

@ -28,6 +28,7 @@
</li> </li>
<li role="presentation"> <li role="presentation">
<form method="POST" action="/logout"> <form method="POST" action="/logout">
{{ csrfToken }}
<button type="submit" role="menuitem"> <button type="submit" role="menuitem">
<i class="ri-logout-circle-line"></i> <i class="ri-logout-circle-line"></i>
{{( pgettext "Logout" "action" )}} {{( pgettext "Logout" "action" )}}

View File

@ -3,9 +3,11 @@
{{- end }} {{- end }}
{{ define "content" }} {{ define "content" }}
{{- /*gotype: dev.tandem.ws/tandem/numerus/pkg.contactForm*/ -}}
<section class="dialog-content"> <section class="dialog-content">
<h2>{{(pgettext "New Contact" "title")}}</h2> <h2>{{(pgettext "New Contact" "title")}}</h2>
<form method="POST" action="{{ companyURI "/contacts" }}"> <form method="POST" action="{{ companyURI "/contacts" }}">
{{ csrfToken }}
{{ template "input-field" .BusinessName | addInputAttr "autofocus" }} {{ template "input-field" .BusinessName | addInputAttr "autofocus" }}
{{ template "input-field" .VATIN }} {{ template "input-field" .VATIN }}
{{ template "input-field" .TradeName }} {{ template "input-field" .TradeName }}

View File

@ -3,9 +3,11 @@
{{- end }} {{- end }}
{{ define "content" }} {{ define "content" }}
{{- /*gotype: dev.tandem.ws/tandem/numerus/pkg.profileForm*/ -}}
<section class="dialog-content"> <section class="dialog-content">
<h2>{{(pgettext "User Settings" "title")}}</h2> <h2>{{(pgettext "User Settings" "title")}}</h2>
<form method="POST"> <form method="POST">
{{ csrfToken }}
<fieldset class="full-width"> <fieldset class="full-width">
<legend>{{( pgettext "User Access Data" "title" )}}</legend> <legend>{{( pgettext "User Access Data" "title" )}}</legend>

View File

@ -5,8 +5,10 @@
{{ define "content" }} {{ define "content" }}
<section class="dialog-content"> <section class="dialog-content">
<h2>{{(pgettext "Tax Details" "title")}}</h2> <h2>{{(pgettext "Tax Details" "title")}}</h2>
{{- /*gotype: dev.tandem.ws/tandem/numerus/pkg.TaxDetailsPage*/ -}}
{{ with .DetailsForm }} {{ with .DetailsForm }}
<form id="details" method="POST"> <form id="details" method="POST">
{{ csrfToken }}
{{ template "input-field" .BusinessName }} {{ template "input-field" .BusinessName }}
{{ template "input-field" .VATIN }} {{ template "input-field" .VATIN }}
{{ template "input-field" .TradeName }} {{ template "input-field" .TradeName }}
@ -28,6 +30,7 @@
{{ end }} {{ end }}
<form id="newtax" method="POST" action="{{ companyURI "/tax" }}"> <form id="newtax" method="POST" action="{{ companyURI "/tax" }}">
{{ csrfToken }}
</form> </form>
<fieldset> <fieldset>
@ -49,6 +52,7 @@
<td>{{ .Rate }}</td> <td>{{ .Rate }}</td>
<td> <td>
<form method="POST" action="{{ companyURI "/tax"}}/{{ .Id }}"> <form method="POST" action="{{ companyURI "/tax"}}/{{ .Id }}">
{{ csrfToken }}
<input type="hidden" name="_method" value="DELETE"/> <input type="hidden" name="_method" value="DELETE"/>
<button class="icon" aria-label="{{( gettext "Delete tax" )}}" type="submit"><i <button class="icon" aria-label="{{( gettext "Delete tax" )}}" type="submit"><i
class="ri-delete-back-2-line"></i></button> class="ri-delete-back-2-line"></i></button>