mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
fix(database): skip CREATE USER when role exists to avoid password in Postgres logs (#12538)
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary When `zitadel init` runs on every Helm upgrade/re-deploy, `VerifyUser` always executed `CREATE USER ... WITH PASSWORD '...'`. If the role already existed, PostgreSQL returned `42710` and logged the full statement—including the plaintext password—even though ZITADEL swallowed that error. This mirrors the existing `VerifyDatabase` catalog check: query `pg_roles` first and skip creation when the role exists. Also stops mutating the package-level `createUserStmt` when appending `WITH PASSWORD`. Fixes #12178 ## Changes - `cmd/initialise/verify_user.go`: check `SELECT EXISTS(... FROM pg_roles ...)` before `CREATE USER`; build password clause on a local statement copy - `cmd/initialise/verify_user_test.go`: cover catalog skip, catalog-check failure, and create paths with the new query ## Test plan - [x] `go test ./cmd/initialise/...` (passed) - [ ] Re-run init against an existing role and confirm Postgres logs no longer contain `CREATE USER ... WITH PASSWORD` <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-bc514793-d275-4d91-a71b-1b7566668b65?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-bc514793-d275-4d91-a71b-1b7566668b65&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: Silvan <[email protected]>
This commit is contained in:
co-authored by
Cursor Agent
Silvan
parent
464d1cb655
commit
0a18aba91c
@@ -6,6 +6,7 @@ import (
|
|||||||
_ "embed"
|
_ "embed"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
"github.com/spf13/viper"
|
"github.com/spf13/viper"
|
||||||
@@ -58,11 +59,36 @@ func VerifyUser(username, password string) func(context.Context, *database.DB) e
|
|||||||
logging.Info(ctx, "config.database.postgres.user.username is same as config.database.postgres.admin.username, skipping create user", "username", username)
|
logging.Info(ctx, "config.database.postgres.user.username is same as config.database.postgres.admin.username, skipping create user", "username", username)
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
logging.Info(ctx, "verify user", "username", username)
|
|
||||||
if password != "" {
|
// Check if the role already exists in the catalog before attempting CREATE USER.
|
||||||
createUserStmt += " WITH PASSWORD '" + password + "'"
|
// This avoids PostgreSQL logging the full CREATE USER statement (including the
|
||||||
|
// plaintext password) when the role already exists on re-deploy / re-init.
|
||||||
|
var exists bool
|
||||||
|
err = db.QueryRowContext(ctx, func(r *sql.Row) error {
|
||||||
|
return r.Scan(&exists)
|
||||||
|
}, "SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", username)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("unable to check if user exists: %w", err)
|
||||||
|
}
|
||||||
|
if exists {
|
||||||
|
logging.Info(ctx, "user already exists, skipping creation", "username", username)
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return exec(ctx, db, fmt.Sprintf(createUserStmt, username), []string{roleAlreadyExistsCode})
|
logging.Info(ctx, "verify user", "username", username)
|
||||||
|
// Format the username first so the password is never part of a fmt format string
|
||||||
|
// (passwords may contain '%' which would be interpreted as format verbs).
|
||||||
|
stmt := fmt.Sprintf(createUserStmt, username)
|
||||||
|
if password != "" {
|
||||||
|
stmt += " WITH PASSWORD " + quotePostgresLiteral(password)
|
||||||
|
}
|
||||||
|
|
||||||
|
return exec(ctx, db, stmt, []string{roleAlreadyExistsCode})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// quotePostgresLiteral returns s as a single-quoted PostgreSQL string literal,
|
||||||
|
// escaping embedded single quotes by doubling them.
|
||||||
|
func quotePostgresLiteral(s string) string {
|
||||||
|
return "'" + strings.ReplaceAll(s, "'", "''") + "'"
|
||||||
|
}
|
||||||
|
|||||||
@@ -32,6 +32,9 @@ func Test_verifyUser(t *testing.T) {
|
|||||||
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
{"postgres"},
|
{"postgres"},
|
||||||
}),
|
}),
|
||||||
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", nil, []string{"exists"}, [][]driver.Value{
|
||||||
|
{false},
|
||||||
|
}, "zitadel-user"),
|
||||||
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\"", sql.ErrTxDone),
|
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\"", sql.ErrTxDone),
|
||||||
),
|
),
|
||||||
username: "zitadel-user",
|
username: "zitadel-user",
|
||||||
@@ -46,6 +49,9 @@ func Test_verifyUser(t *testing.T) {
|
|||||||
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
{"postgres"},
|
{"postgres"},
|
||||||
}),
|
}),
|
||||||
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", nil, []string{"exists"}, [][]driver.Value{
|
||||||
|
{false},
|
||||||
|
}, "zitadel-user"),
|
||||||
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\"", nil),
|
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\"", nil),
|
||||||
),
|
),
|
||||||
username: "zitadel-user",
|
username: "zitadel-user",
|
||||||
@@ -60,6 +66,9 @@ func Test_verifyUser(t *testing.T) {
|
|||||||
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
{"postgres"},
|
{"postgres"},
|
||||||
}),
|
}),
|
||||||
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", nil, []string{"exists"}, [][]driver.Value{
|
||||||
|
{false},
|
||||||
|
}, "zitadel-user"),
|
||||||
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\" WITH PASSWORD 'password'", nil),
|
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\" WITH PASSWORD 'password'", nil),
|
||||||
),
|
),
|
||||||
username: "zitadel-user",
|
username: "zitadel-user",
|
||||||
@@ -68,19 +77,52 @@ func Test_verifyUser(t *testing.T) {
|
|||||||
targetErr: nil,
|
targetErr: nil,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "already exists",
|
name: "correct with password containing percent and quote",
|
||||||
args: args{
|
args: args{
|
||||||
db: prepareDB(t,
|
db: prepareDB(t,
|
||||||
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
{"postgres"},
|
{"postgres"},
|
||||||
}),
|
}),
|
||||||
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\" WITH PASSWORD 'password'", nil),
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", nil, []string{"exists"}, [][]driver.Value{
|
||||||
|
{false},
|
||||||
|
}, "zitadel-user"),
|
||||||
|
expectExec("-- replace zitadel-user with the name of the user\nCREATE USER \"zitadel-user\" WITH PASSWORD 'p%''ass'", nil),
|
||||||
),
|
),
|
||||||
username: "zitadel-user",
|
username: "zitadel-user",
|
||||||
password: "",
|
password: "p%'ass",
|
||||||
},
|
},
|
||||||
targetErr: nil,
|
targetErr: nil,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "already exists in catalog, skip creation",
|
||||||
|
args: args{
|
||||||
|
db: prepareDB(t,
|
||||||
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
|
{"postgres"},
|
||||||
|
}),
|
||||||
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", nil, []string{"exists"}, [][]driver.Value{
|
||||||
|
{true},
|
||||||
|
}, "zitadel-user"),
|
||||||
|
),
|
||||||
|
username: "zitadel-user",
|
||||||
|
password: "password",
|
||||||
|
},
|
||||||
|
targetErr: nil,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "catalog check fails",
|
||||||
|
args: args{
|
||||||
|
db: prepareDB(t,
|
||||||
|
expectQuery("SELECT current_user", nil, []string{"current_user"}, [][]driver.Value{
|
||||||
|
{"postgres"},
|
||||||
|
}),
|
||||||
|
expectQuery("SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", sql.ErrConnDone, []string{"exists"}, [][]driver.Value{}, "zitadel-user"),
|
||||||
|
),
|
||||||
|
username: "zitadel-user",
|
||||||
|
password: "password",
|
||||||
|
},
|
||||||
|
targetErr: sql.ErrConnDone,
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "same user, skip create",
|
name: "same user, skip create",
|
||||||
args: args{
|
args: args{
|
||||||
|
|||||||
Reference in New Issue
Block a user