fix(sec): Cross-Tenant-IDOR bei POP3-Konten schließen
Gleiches Muster wie bei IMAP (730099d): domain_admin konnte POP3-Konten
fremder Tenants auflisten, löschen und Importe/Progress fremder Tenants
ansehen, da pop3_accounts keine tenant_id hatte und Store.List() für
Admins ungefiltert alle Konten lieferte.
- pop3_accounts: neue Spalte tenant_id (ALTER TABLE ADD COLUMN IF NOT EXISTS)
- Store.List() filtert nach tenant_id, außer für superadmin
- Store.Create() setzt tenant_id beim Anlegen
- delete/start-import/progress prüfen zusätzlich tenantAccessAllowed()
This commit is contained in:
@@ -400,7 +400,7 @@ func (s *Server) handleListPop3(w http.ResponseWriter, r *http.Request) {
|
|||||||
sess := sessionFromCtx(r.Context())
|
sess := sessionFromCtx(r.Context())
|
||||||
// SEC-03: Use HasRole to correctly check admin privileges (domain_admin, admin, superadmin).
|
// SEC-03: Use HasRole to correctly check admin privileges (domain_admin, admin, superadmin).
|
||||||
isAdmin := auth.HasRole(sess.Role, userstore.RoleDomainAdmin)
|
isAdmin := auth.HasRole(sess.Role, userstore.RoleDomainAdmin)
|
||||||
accounts, err := s.pop3Store.List(r.Context(), sess.Username, isAdmin)
|
accounts, err := s.pop3Store.List(r.Context(), sess.Username, isAdmin, sess.TenantID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeError(w, http.StatusInternalServerError, "failed to list POP3 accounts")
|
writeError(w, http.StatusInternalServerError, "failed to list POP3 accounts")
|
||||||
return
|
return
|
||||||
@@ -449,6 +449,7 @@ func (s *Server) handleCreatePop3(w http.ResponseWriter, r *http.Request) {
|
|||||||
TLS: req.TLS,
|
TLS: req.TLS,
|
||||||
TLSSkipVerify: req.TLSSkipVerify,
|
TLSSkipVerify: req.TLSSkipVerify,
|
||||||
Username: req.Username,
|
Username: req.Username,
|
||||||
|
TenantID: sess.TenantID,
|
||||||
}
|
}
|
||||||
|
|
||||||
created, err := s.pop3Store.Create(r.Context(), acc, req.Password)
|
created, err := s.pop3Store.Create(r.Context(), acc, req.Password)
|
||||||
@@ -611,6 +612,10 @@ func (s *Server) handlePop3Progress(w http.ResponseWriter, r *http.Request) {
|
|||||||
writeError(w, http.StatusForbidden, "access denied")
|
writeError(w, http.StatusForbidden, "access denied")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if !tenantAccessAllowed(sess, acc.TenantID) {
|
||||||
|
writeError(w, http.StatusForbidden, "access denied")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
writeJSON(w, http.StatusOK, acc)
|
writeJSON(w, http.StatusOK, acc)
|
||||||
}
|
}
|
||||||
|
|||||||
+16
-9
@@ -34,6 +34,7 @@ type Account struct {
|
|||||||
ProgressCurrent int `json:"progress_current"`
|
ProgressCurrent int `json:"progress_current"`
|
||||||
ProgressTotal int `json:"progress_total"`
|
ProgressTotal int `json:"progress_total"`
|
||||||
CreatedAt time.Time `json:"created_at"`
|
CreatedAt time.Time `json:"created_at"`
|
||||||
|
TenantID *int64 `json:"tenant_id,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// Store manages POP3 account persistence in PostgreSQL.
|
// Store manages POP3 account persistence in PostgreSQL.
|
||||||
@@ -62,6 +63,7 @@ CREATE TABLE IF NOT EXISTS pop3_accounts (
|
|||||||
created_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
created_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
||||||
);
|
);
|
||||||
CREATE INDEX IF NOT EXISTS idx_pop3_accounts_owner ON pop3_accounts (owner);
|
CREATE INDEX IF NOT EXISTS idx_pop3_accounts_owner ON pop3_accounts (owner);
|
||||||
|
ALTER TABLE pop3_accounts ADD COLUMN IF NOT EXISTS tenant_id INTEGER REFERENCES tenants(id);
|
||||||
`
|
`
|
||||||
|
|
||||||
// New creates a new Store, connects to PostgreSQL, and runs the schema migration.
|
// New creates a new Store, connects to PostgreSQL, and runs the schema migration.
|
||||||
@@ -93,10 +95,10 @@ func (s *Store) Create(ctx context.Context, acc Account, password string) (*Acco
|
|||||||
}
|
}
|
||||||
|
|
||||||
row := s.pool.QueryRow(ctx, `
|
row := s.pool.QueryRow(ctx, `
|
||||||
INSERT INTO pop3_accounts (owner, name, host, port, tls, tls_skip_verify, username, password_enc)
|
INSERT INTO pop3_accounts (owner, name, host, port, tls, tls_skip_verify, username, password_enc, tenant_id)
|
||||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
|
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)
|
||||||
RETURNING id, created_at`,
|
RETURNING id, created_at`,
|
||||||
acc.Owner, acc.Name, acc.Host, acc.Port, acc.TLS, acc.TLSSkipVerify, acc.Username, enc,
|
acc.Owner, acc.Name, acc.Host, acc.Port, acc.TLS, acc.TLSSkipVerify, acc.Username, enc, acc.TenantID,
|
||||||
)
|
)
|
||||||
|
|
||||||
if err := row.Scan(&acc.ID, &acc.CreatedAt); err != nil {
|
if err := row.Scan(&acc.ID, &acc.CreatedAt); err != nil {
|
||||||
@@ -111,7 +113,7 @@ func (s *Store) Create(ctx context.Context, acc Account, password string) (*Acco
|
|||||||
// selectColumns is the canonical column list used in all SELECT statements.
|
// selectColumns is the canonical column list used in all SELECT statements.
|
||||||
const selectColumns = ` id, owner, name, host, port, tls, tls_skip_verify, username,
|
const selectColumns = ` id, owner, name, host, port, tls, tls_skip_verify, username,
|
||||||
status, error_msg, last_import_at, last_import_count,
|
status, error_msg, last_import_at, last_import_count,
|
||||||
progress_current, progress_total, created_at `
|
progress_current, progress_total, created_at, tenant_id `
|
||||||
|
|
||||||
// scanner abstracts pgx.Row and pgx.Rows — both expose Scan(...any) error.
|
// scanner abstracts pgx.Row and pgx.Rows — both expose Scan(...any) error.
|
||||||
type scanner interface {
|
type scanner interface {
|
||||||
@@ -123,21 +125,26 @@ func scanRow(row scanner) (Account, error) {
|
|||||||
err := row.Scan(
|
err := row.Scan(
|
||||||
&a.ID, &a.Owner, &a.Name, &a.Host, &a.Port, &a.TLS, &a.TLSSkipVerify, &a.Username,
|
&a.ID, &a.Owner, &a.Name, &a.Host, &a.Port, &a.TLS, &a.TLSSkipVerify, &a.Username,
|
||||||
&a.Status, &a.ErrorMsg, &a.LastImportAt,
|
&a.Status, &a.ErrorMsg, &a.LastImportAt,
|
||||||
&a.LastImportCount, &a.ProgressCurrent, &a.ProgressTotal, &a.CreatedAt,
|
&a.LastImportCount, &a.ProgressCurrent, &a.ProgressTotal, &a.CreatedAt, &a.TenantID,
|
||||||
)
|
)
|
||||||
return a, err
|
return a, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// List returns POP3 accounts. Admins see all accounts; regular users see only their own.
|
// List returns POP3 accounts. Superadmins (tenantID == nil) see all accounts;
|
||||||
func (s *Store) List(ctx context.Context, owner string, isAdmin bool) ([]Account, error) {
|
// other admins (tenantID != nil) see all accounts within their own tenant;
|
||||||
|
// regular users see only their own accounts.
|
||||||
|
func (s *Store) List(ctx context.Context, owner string, isAdmin bool, tenantID *int64) ([]Account, error) {
|
||||||
var rows pgx.Rows
|
var rows pgx.Rows
|
||||||
var err error
|
var err error
|
||||||
|
|
||||||
q := `SELECT` + selectColumns + `FROM pop3_accounts`
|
q := `SELECT` + selectColumns + `FROM pop3_accounts`
|
||||||
|
|
||||||
if isAdmin {
|
switch {
|
||||||
|
case isAdmin && tenantID == nil:
|
||||||
rows, err = s.pool.Query(ctx, q+` ORDER BY id`)
|
rows, err = s.pool.Query(ctx, q+` ORDER BY id`)
|
||||||
} else {
|
case isAdmin:
|
||||||
|
rows, err = s.pool.Query(ctx, q+` WHERE tenant_id = $1 ORDER BY id`, *tenantID)
|
||||||
|
default:
|
||||||
rows, err = s.pool.Query(ctx, q+` WHERE owner = $1 ORDER BY id`, owner)
|
rows, err = s.pool.Query(ctx, q+` WHERE owner = $1 ORDER BY id`, owner)
|
||||||
}
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user