diff --git a/deploy/check_cookie.sql b/deploy/check_cookie.sql index f3f816a..887dec1 100644 --- a/deploy/check_cookie.sql +++ b/deploy/check_cookie.sql @@ -1,28 +1,34 @@ -- Deploy numerus:check_cookie to pg --- requires: schema_auth +-- requires: schema_public -- requires: user begin; set search_path to public, numerus, auth; -create or replace function check_cookie(input_cookie text) returns record as +create or replace function check_cookie(input_cookie text) returns name as $$ declare - value record; + uid text; + user_email text; + user_role name; begin - select email::text, role - into value + select user_id::text, email::text, role + into uid, user_email, user_role from "user" where email = split_part(input_cookie, '/', 2) and cookie_expires_at > current_timestamp and length(password) > 0 and cookie = split_part(input_cookie, '/', 1) ; - if value is null then - select '', 'guest'::name into value; + if user_role is null then + uid := '0'; + user_email := ''; + user_role := 'guest'::name; end if; - return value; + perform set_config('request.user.id', uid, false); + perform set_config('request.user.email', user_email, false); + return user_role; end; $$ language plpgsql diff --git a/deploy/logout.sql b/deploy/logout.sql index 4c74cbb..c0a4523 100644 --- a/deploy/logout.sql +++ b/deploy/logout.sql @@ -11,7 +11,7 @@ $$ update "user" set cookie = default , cookie_expires_at = default -where email = current_setting('request.user') +where user_id = current_setting('request.user.id', true)::integer $$ language sql security definer diff --git a/deploy/set_cookie.sql b/deploy/set_cookie.sql new file mode 100644 index 0000000..18ee78a --- /dev/null +++ b/deploy/set_cookie.sql @@ -0,0 +1,22 @@ +-- Deploy numerus:set_cookie to pg +-- requires: schema_public +-- requires: check_cookie + +begin; + +set search_path to public; + +create or replace function set_cookie(input_cookie text) returns void as +$$ + select set_config('role', check_cookie(input_cookie), false); +$$ +language sql +stable; + +comment on function set_cookie(text) is +'Sets the user information for the cookie and switches to its role'; + +revoke execute on function set_cookie(text) from public; +grant execute on function set_cookie(text) to authenticator; + +commit; diff --git a/pkg/db.go b/pkg/db.go index 5568a13..3ff8bf9 100644 --- a/pkg/db.go +++ b/pkg/db.go @@ -3,14 +3,13 @@ package pkg import ( "context" "log" - "net/http" "github.com/jackc/pgx/v4" "github.com/jackc/pgx/v4/pgxpool" ) type Db struct { - pool *pgxpool.Pool + *pgxpool.Pool } func NewDatabase(ctx context.Context, connString string) (*Db, error) { @@ -25,19 +24,14 @@ func NewDatabase(ctx context.Context, connString string) (*Db, error) { } config.BeforeAcquire = func(ctx context.Context, conn *pgx.Conn) bool { - if user, ok := ctx.Value(ContextUserKey).(*AppUser); ok { - batch := &pgx.Batch{} - batch.Queue("select set_config('request.user', $1, false)", user.Email) - batch.Queue("select set_config('role', $1, false)", user.Role) - br := conn.SendBatch(ctx, batch) - defer br.Close() - for i := 0; i < batch.Len(); i++ { - if _, err := br.Exec(); err != nil { - log.Printf("ERROR - Failed to set role: %v", err) - return false - } - } + cookie := "" + if value, ok := ctx.Value(ContextCookieKey).(string); ok { + cookie = value } + if _, err := conn.Exec(ctx, "select set_cookie($1)", cookie); err != nil { + log.Printf("ERROR - Failed to set role: %v", err) + return false + } return true } @@ -56,13 +50,21 @@ func NewDatabase(ctx context.Context, connString string) (*Db, error) { return &Db{pool}, nil } -func (db *Db) Close() { - db.pool.Close() +func (db *Db) Acquire(ctx context.Context) (*Conn, error) { + conn, err := db.Pool.Acquire(ctx) + if err != nil { + return nil, err + } + return &Conn{conn}, nil } -func (db *Db) Text(r *http.Request, def string, sql string, args ...interface{}) string { +type Conn struct { + *pgxpool.Conn +} + +func (c *Conn) Text(ctx context.Context, def string, sql string, args ...interface{}) string { var result string - if err := db.pool.QueryRow(r.Context(), sql, args...).Scan(&result); err != nil { + if err := c.Conn.QueryRow(ctx, sql, args...).Scan(&result); err != nil { if err == pgx.ErrNoRows { return def } @@ -72,8 +74,8 @@ func (db *Db) Text(r *http.Request, def string, sql string, args ...interface{}) return result } -func (db *Db) Exec(r *http.Request, sql string, args ...interface{}) { - if _, err := db.pool.Exec(r.Context(), sql, args...); err != nil { +func (c *Conn) Exec(ctx context.Context, sql string, args ...interface{}) { + if _, err := c.Conn.Exec(ctx, sql, args...); err != nil { panic(err) } } diff --git a/pkg/login.go b/pkg/login.go index 0f627a0..a037653 100644 --- a/pkg/login.go +++ b/pkg/login.go @@ -5,12 +5,12 @@ import ( "net" "net/http" "time" - - "github.com/jackc/pgx/v4" ) const ( ContextUserKey = "numerus-user" + ContextCookieKey = "numerus-cookie" + ContextConnKey = "numerus-database" sessionCookie = "numerus-session" defaultRole = "guest" ) @@ -27,7 +27,7 @@ type AppUser struct { Role string } -func LoginHandler(db *Db) http.Handler { +func LoginHandler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { user := getUser(r) if user.LoggedIn { @@ -40,7 +40,8 @@ func LoginHandler(db *Db) http.Handler { Password: r.FormValue("password"), } if r.Method == "POST" { - cookie := db.Text(r, "", "select login($1, $2, $3)", page.Email, page.Password, remoteAddr(r)) + conn := getConn(r) + cookie := conn.Text(r.Context(), "", "select login($1, $2, $3)", page.Email, page.Password, remoteAddr(r)) if cookie != "" { http.SetCookie(w, createSessionCookie(cookie, 8766*24*time.Hour)) http.Redirect(w, r, "/", http.StatusSeeOther) @@ -55,11 +56,12 @@ func LoginHandler(db *Db) http.Handler { }) } -func LogoutHandler(db *Db) http.Handler { +func LogoutHandler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { user := getUser(r) if user.LoggedIn { - db.Exec(r, "select logout()") + conn := getConn(r) + conn.Exec(r.Context(), "select logout()") http.SetCookie(w, createSessionCookie("", -24*time.Hour)) } http.Redirect(w, r, "/login", http.StatusSeeOther) @@ -87,22 +89,30 @@ func createSessionCookie(value string, duration time.Duration) *http.Cookie { func CheckLogin(db *Db, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var ctx = r.Context(); + if cookie, err := r.Cookie(sessionCookie); err == nil { + ctx = context.WithValue(ctx, ContextCookieKey, cookie.Value); + } + + conn, err := db.Acquire(ctx) + if err != nil { + panic(err); + } + defer conn.Release(); + ctx = context.WithValue(ctx, ContextConnKey, conn) + user := &AppUser{ Email: "", LoggedIn: false, Role: defaultRole, } - if cookie, err := r.Cookie(sessionCookie); err == nil { - row := db.pool.QueryRow(r.Context(), "select * from check_cookie($1) as (email text, role name)", cookie.Value) - if err := row.Scan(&user.Email, &user.Role); err != nil { - if err != pgx.ErrNoRows { - panic(err) - } - } else { - user.LoggedIn = user.Role != "guest" - } + row := conn.QueryRow(ctx, "select current_setting('request.user.email', true), current_user") + if err := row.Scan(&user.Email, &user.Role); err != nil { + panic(err) } - ctx := context.WithValue(r.Context(), ContextUserKey, user) + user.LoggedIn = user.Email != "" + ctx = context.WithValue(ctx, ContextUserKey, user) + next.ServeHTTP(w, r.WithContext(ctx)) }) } @@ -110,3 +120,7 @@ func CheckLogin(db *Db, next http.Handler) http.Handler { func getUser(r *http.Request) *AppUser { return r.Context().Value(ContextUserKey).(*AppUser) } + +func getConn(r *http.Request) *Conn { + return r.Context().Value(ContextConnKey).(*Conn) +} diff --git a/pkg/router.go b/pkg/router.go index 996ed7e..ca0ee74 100644 --- a/pkg/router.go +++ b/pkg/router.go @@ -7,8 +7,8 @@ import ( func NewRouter(db *Db) http.Handler { router := http.NewServeMux() router.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("web/static")))) - router.Handle("/login", LoginHandler(db)) - router.Handle("/logout", LogoutHandler(db)) + router.Handle("/login", LoginHandler()) + router.Handle("/logout", LogoutHandler()) router.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { user := getUser(r) if user.LoggedIn { diff --git a/revert/set_cookie.sql b/revert/set_cookie.sql new file mode 100644 index 0000000..5ef9a1f --- /dev/null +++ b/revert/set_cookie.sql @@ -0,0 +1,7 @@ +-- Revert numerus:set_cookie from pg + +begin; + +drop function if exists public.set_cookie(text); + +commit; diff --git a/sqitch.plan b/sqitch.plan index 82a0352..ed61103 100644 --- a/sqitch.plan +++ b/sqitch.plan @@ -2,17 +2,18 @@ %project=numerus %uri=https://numerus.tandem.blog/ -roles 2023-01-12T18:42:16Z jordi fita i mas # Add database roles -schema_auth [roles] 2023-01-12T19:15:55Z jordi fita i mas # Add authentication schema -schema_public [roles] 2023-01-12T19:24:29Z jordi fita i mas # Set privileges to public schema -schema_numerus [roles] 2023-01-12T22:57:22Z jordi fita i mas # Add application schema -extension_citext [schema_public] 2023-01-12T23:03:33Z jordi fita i mas # Add citext extension -email [schema_numerus extension_citext] 2023-01-12T23:09:59Z jordi fita i mas # Add email domain -user [roles schema_auth email] 2023-01-12T23:44:03Z jordi fita i mas # Create user table -ensure_role_exists [schema_auth user] 2023-01-12T23:57:59Z jordi fita i mas # Add trigger to ensure the user’s role exists -extension_pgcrypto [schema_auth] 2023-01-13T00:11:50Z jordi fita i mas # Add pgcrypto extension -encrypt_password [schema_auth user extension_pgcrypto] 2023-01-13T00:14:30Z jordi fita i mas # Add trigger to encrypt user’s password -login_attempt [schema_auth] 2023-01-17T14:05:49Z jordi fita i mas # Add table to log login attempts -login [roles schema_numerus schema_auth extension_pgcrypto email user login_attempt] 2023-01-13T00:32:32Z jordi fita i mas # Add function to login -check_cookie [schema_auth user] 2023-01-17T17:48:49Z jordi fita i mas # Add function to check if a user cookie is valid -logout [schema_auth user] 2023-01-17T19:10:21Z jordi fita i mas # Add function to logout +roles 2023-01-12T18:42:16Z jordi fita mas # Add database roles +schema_auth [roles] 2023-01-12T19:15:55Z jordi fita mas # Add authentication schema +schema_public [roles] 2023-01-12T19:24:29Z jordi fita mas # Set privileges to public schema +schema_numerus [roles] 2023-01-12T22:57:22Z jordi fita mas # Add application schema +extension_citext [schema_public] 2023-01-12T23:03:33Z jordi fita mas # Add citext extension +email [schema_numerus extension_citext] 2023-01-12T23:09:59Z jordi fita mas # Add email domain +user [roles schema_auth email] 2023-01-12T23:44:03Z jordi fita mas # Create user table +ensure_role_exists [schema_auth user] 2023-01-12T23:57:59Z jordi fita mas # Add trigger to ensure the user’s role exists +extension_pgcrypto [schema_auth] 2023-01-13T00:11:50Z jordi fita mas # Add pgcrypto extension +encrypt_password [schema_auth user extension_pgcrypto] 2023-01-13T00:14:30Z jordi fita mas # Add trigger to encrypt user’s password +login_attempt [schema_auth] 2023-01-17T14:05:49Z jordi fita mas # Add table to log login attempts +login [roles schema_numerus schema_auth extension_pgcrypto email user login_attempt] 2023-01-13T00:32:32Z jordi fita mas # Add function to login +check_cookie [schema_public user] 2023-01-17T17:48:49Z jordi fita mas # Add function to check if a user cookie is valid +logout [schema_auth user] 2023-01-17T19:10:21Z jordi fita mas # Add function to logout +set_cookie [schema_public check_cookie] 2023-01-19T11:00:22Z jordi fita mas # Add function to set the role based on the cookie diff --git a/test/check_cookie.sql b/test/check_cookie.sql index 5789045..6534ab9 100644 --- a/test/check_cookie.sql +++ b/test/check_cookie.sql @@ -5,13 +5,13 @@ reset client_min_messages; begin; -select plan(15); +select plan(21); set search_path to auth, numerus, public; select has_function('public', 'check_cookie', array ['text']); select function_lang_is('public', 'check_cookie', array ['text'], 'plpgsql'); -select function_returns('public', 'check_cookie', array ['text'], 'record'); +select function_returns('public', 'check_cookie', array ['text'], 'name'); select is_definer('public', 'check_cookie', array ['text']); select volatility_is('public', 'check_cookie', array ['text'], 'stable'); select function_privs_are('public', 'check_cookie', array ['text'], 'guest', array []::text[]); @@ -23,49 +23,88 @@ set client_min_messages to warning; truncate auth."user" cascade; reset client_min_messages; -insert into auth."user" (email, name, password, role, cookie, cookie_expires_at) -values ('demo@tandem.blog', 'Demo', 'test', 'invoicer', '44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e', current_timestamp + interval '1 month') - , ('admin@tandem.blog', 'Demo', 'test', 'admin', '12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524', current_timestamp + interval '1 month') +insert into auth."user" (user_id, email, name, password, role, cookie, cookie_expires_at) +values (1, 'demo@tandem.blog', 'Demo', 'test', 'invoicer', '44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e', current_timestamp + interval '1 month') + , (9, 'admin@tandem.blog', 'Demo', 'test', 'admin', '12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524', current_timestamp + interval '1 month') ; -select results_eq ( - $$ select * from check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tandem.blog') as (e text, r name) $$, - $$ values ('demo@tandem.blog', 'invoicer'::name) $$, +prepare user_info as +select current_setting('request.user.id', true)::integer, current_setting('request.user.email', true); + +select is ( + check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tandem.blog'), + 'invoicer'::name, 'Should validate the cookie for the first user' ); select results_eq ( - $$ select * from check_cookie('12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tandem.blog') as (e text, r name) $$, - $$ values ('admin@tandem.blog', 'admin'::name) $$, + 'user_info', + $$ values (1, 'demo@tandem.blog') $$, + 'Should have updated the settings with the user info' +); + +select is ( + check_cookie('12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tandem.blog'), + 'admin'::name, 'Should validate the cookie for the second user' ); select results_eq ( - $$ select * from check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/admin@tandem.blog') as (e text, r name) $$, - $$ values ('', 'guest'::name) $$, + 'user_info', + $$ values (9, 'admin@tandem.blog') $$, + 'Should have updated the settings with the other user info' +); + +select is ( + check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/admin@tandem.blog'), + 'guest'::name, 'Should only match with the correct email' ); select results_eq ( - $$ select * from check_cookie('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/admin@tandem.blog') as (e text, r name) $$, - $$ values ('', 'guest'::name) $$, + 'user_info', + $$ values (0, '') $$, + 'Should have updated the settings with a guest user' +); + +select is ( + check_cookie('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/admin@tandem.blog'), + 'guest'::name, 'Should only match with the correct cookie value' ); +select results_eq ( + 'user_info', + $$ values (0, '') $$, + 'Should have left the settings with a guest user' +); + update "user" set cookie_expires_at = current_timestamp - interval '1 minute'; -select results_eq ( - $$ select * from check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tandem.blog') as (e text, r name) $$, - $$ values ('', 'guest'::name) $$, +select is ( + check_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tandem.blog'), + 'guest'::name, 'Should not allow expired cookies' ); select results_eq ( - $$ select * from check_cookie('12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tandem.blog') as (e text, r name) $$, - $$ values ('', 'guest'::name) $$, + 'user_info', + $$ values (0, '') $$, + 'Should have left the settings with a guest user' +); + +select is ( + check_cookie('12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tandem.blog'), + 'guest'::name, 'Should not allow expired cookied for the other user as well' ); +select results_eq ( + 'user_info', + $$ values (0, '') $$, + 'Should have left the settings with a guest user' +); + select * from finish(); diff --git a/test/logout.sql b/test/logout.sql index 876f6a4..0bc0776 100644 --- a/test/logout.sql +++ b/test/logout.sql @@ -23,16 +23,16 @@ set client_min_messages to warning; truncate auth."user" cascade; reset client_min_messages; -insert into auth."user" (email, name, password, role, cookie, cookie_expires_at) -values ('info@tandem.blog', 'Tandem', 'test', 'invoicer', '8c23d4a8d777775f8fc507676a0d99d3dfa54b03b1b257c838', current_timestamp + interval '1 day') - , ('admin@tandem.blog', 'Admin', 'test', 'admin', '0169e5f668eec1e6749fd25388b057997358efa8dfd697961a', current_timestamp + interval '2 day') +insert into auth."user" (user_id, email, name, password, role, cookie, cookie_expires_at) +values (1, 'info@tandem.blog', 'Tandem', 'test', 'invoicer', '8c23d4a8d777775f8fc507676a0d99d3dfa54b03b1b257c838', current_timestamp + interval '1 day') + , (12, 'admin@tandem.blog', 'Admin', 'test', 'admin', '0169e5f668eec1e6749fd25388b057997358efa8dfd697961a', current_timestamp + interval '2 day') ; prepare user_cookies as select cookie, cookie_expires_at from "user" order by user_id ; -select set_config('request.user', 'nothing', false); +select set_config('request.user.id', '0', false); select lives_ok( $$ select * from logout() $$, 'Can logout “nobody”' ); select results_eq( @@ -43,7 +43,7 @@ select results_eq( 'Nothing changed' ); -select set_config('request.user', 'info@tandem.blog', false); +select set_config('request.user.id', '1', false); select lives_ok( $$ select * from logout() $$, 'Can logout the first user' ); select results_eq( @@ -54,7 +54,7 @@ select results_eq( 'The first user logged out' ); -select set_config('request.user', 'admin@tandem.blog', false); +select set_config('request.user.id', '12', false); select lives_ok( $$ select * from logout() $$, 'Can logout the second user' ); select results_eq( diff --git a/test/set_cookie.sql b/test/set_cookie.sql new file mode 100644 index 0000000..8e9a559 --- /dev/null +++ b/test/set_cookie.sql @@ -0,0 +1,76 @@ +-- Test set_cookie +set client_min_messages to warning; +create extension if not exists pgtap; +reset client_min_messages; + +begin; + +select plan(15); + +set search_path to auth, numerus, public; + +select has_function('public', 'set_cookie', array ['text']); +select function_lang_is('public', 'set_cookie', array ['text'], 'sql'); +select function_returns('public', 'set_cookie', array ['text'], 'void'); +select isnt_definer('public', 'set_cookie', array ['text']); +select volatility_is('public', 'set_cookie', array ['text'], 'stable'); +select function_privs_are('public', 'set_cookie', array ['text'], 'guest', array []::text[]); +select function_privs_are('public', 'set_cookie', array ['text'], 'invoicer', array []::text[]); +select function_privs_are('public', 'set_cookie', array ['text'], 'admin', array []::text[]); +select function_privs_are('public', 'set_cookie', array ['text'], 'authenticator', array ['EXECUTE']); + +set client_min_messages to warning; +truncate auth."user" cascade; +reset client_min_messages; + +insert into auth."user" (user_id, email, name, password, role, cookie, cookie_expires_at) +values (1, 'demo@tandem.blog', 'Demo', 'test', 'invoicer', '44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e', current_timestamp + interval '1 month') + , (5, 'admin@tandem.blog', 'Demo', 'test', 'admin', '12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524', current_timestamp + interval '1 month') +; + +prepare user_info as +select current_setting('request.user.id', true)::integer, current_setting('request.user.email', true), current_user; + +select lives_ok( + $$ select set_cookie('44facbb30d8a419dfd4bfbc44a4b5539d4970148dfc84bed0e/demo@tandem.blog') $$, + 'Should run ok for a valid cookie' +); + +select results_eq( + 'user_info', + $$ values (1, 'demo@tandem.blog', 'invoicer'::name) $$, + 'Should have updated the info with the correct user' +); + +reset role; + +select lives_ok( + $$ select set_cookie('12af4c88b528c2ad4222e3740496ecbc58e76e26f087657524/admin@tandem.blog') $$, + 'Should run ok for a different cookie' +); + +select results_eq( + 'user_info', + $$ values (5, 'admin@tandem.blog', 'admin'::name) $$, + 'Should have updated the info with the other user' +); + +reset role; + +select lives_ok( + $$ select set_cookie('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/admin@tandem.blog') $$, + 'Should run ok even for an invalid cookie' +); + +select results_eq( + 'user_info', + $$ values (0, '', 'guest'::name) $$, + 'Should have updated the info as a guest user' +); + +reset role; + +select * +from finish(); + +rollback; diff --git a/verify/set_cookie.sql b/verify/set_cookie.sql new file mode 100644 index 0000000..6f3fc21 --- /dev/null +++ b/verify/set_cookie.sql @@ -0,0 +1,7 @@ +-- Verify numerus:set_cookie on pg + +begin; + +select has_function_privilege('public.set_cookie(text)', 'execute'); + +rollback;