Remove race condition on relay attack
Signed-off-by: Bryan Frimin <bryan@getprobo.com>
This commit is contained in:
@@ -16,12 +16,14 @@ package auth
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/crewjam/saml"
|
"github.com/crewjam/saml"
|
||||||
"github.com/getprobo/probo/pkg/coredata"
|
"github.com/getprobo/probo/pkg/coredata"
|
||||||
"github.com/getprobo/probo/pkg/gid"
|
"github.com/getprobo/probo/pkg/gid"
|
||||||
|
"github.com/jackc/pgx/v5/pgconn"
|
||||||
"go.gearno.de/kit/pg"
|
"go.gearno.de/kit/pg"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -33,18 +35,8 @@ func PreventReplayAttack(
|
|||||||
organizationID gid.GID,
|
organizationID gid.GID,
|
||||||
expiresAt time.Time,
|
expiresAt time.Time,
|
||||||
) error {
|
) error {
|
||||||
var assertion coredata.SAMLAssertion
|
|
||||||
exists, err := assertion.CheckExists(ctx, conn, assertionID)
|
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("cannot check assertion ID: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
if exists {
|
|
||||||
return coredata.ErrAssertionAlreadyUsed{AssertionID: assertionID}
|
|
||||||
}
|
|
||||||
|
|
||||||
now := time.Now()
|
now := time.Now()
|
||||||
assertion = coredata.SAMLAssertion{
|
assertion := coredata.SAMLAssertion{
|
||||||
ID: assertionID,
|
ID: assertionID,
|
||||||
OrganizationID: organizationID,
|
OrganizationID: organizationID,
|
||||||
UsedAt: now,
|
UsedAt: now,
|
||||||
@@ -52,6 +44,10 @@ func PreventReplayAttack(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if err := assertion.Insert(ctx, conn, scope); err != nil {
|
if err := assertion.Insert(ctx, conn, scope); err != nil {
|
||||||
|
var pgErr *pgconn.PgError
|
||||||
|
if errors.As(err, &pgErr) && pgErr.Code == "23505" && pgErr.ConstraintName == "auth_saml_assertions_pkey" {
|
||||||
|
return coredata.ErrAssertionAlreadyUsed{AssertionID: assertionID}
|
||||||
|
}
|
||||||
return fmt.Errorf("cannot store assertion ID: %w", err)
|
return fmt.Errorf("cannot store assertion ID: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -39,34 +39,6 @@ func (e ErrAssertionAlreadyUsed) Error() string {
|
|||||||
return fmt.Sprintf("assertion ID %q has already been used (replay attack)", e.AssertionID)
|
return fmt.Sprintf("assertion ID %q has already been used (replay attack)", e.AssertionID)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *SAMLAssertion) CheckExists(
|
|
||||||
ctx context.Context,
|
|
||||||
conn pg.Conn,
|
|
||||||
assertionID string,
|
|
||||||
) (bool, error) {
|
|
||||||
query := `
|
|
||||||
SELECT id
|
|
||||||
FROM auth_saml_assertions
|
|
||||||
WHERE id = @id
|
|
||||||
LIMIT 1
|
|
||||||
`
|
|
||||||
|
|
||||||
rows, err := conn.Query(ctx, query, pgx.NamedArgs{"id": assertionID})
|
|
||||||
if err != nil {
|
|
||||||
return false, fmt.Errorf("cannot query saml_assertions: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
_, err = pgx.CollectOneRow(rows, pgx.RowTo[string])
|
|
||||||
if err == nil {
|
|
||||||
return true, nil
|
|
||||||
}
|
|
||||||
if err == pgx.ErrNoRows {
|
|
||||||
return false, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
return false, fmt.Errorf("cannot collect saml_assertion: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
func (s *SAMLAssertion) Insert(
|
func (s *SAMLAssertion) Insert(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
conn pg.Conn,
|
conn pg.Conn,
|
||||||
|
|||||||
Reference in New Issue
Block a user