Verify signature ownership in esign accept/record-event flows
Any self-provisioned trust center visitor could accept another visitor's NDA signature or inject audit-trail events into it by supplying its GID, since AcceptSignature and RecordEvent trusted the client-supplied signature ID without checking it belonged to the caller (GHSA-22xj-f767-ppw6). SignerEmail/ActorEmail are always derived from the verified session identity, never client input, so comparing them against the signature's stored SignerEmail in pkg/esign/service.go closes the hole at its root without touching the resolver-level authorization already in place elsewhere. Adds an e2e regression test that self-provisions two trust center visitors through the real magic-link flow and confirms one cannot touch the other's signature. Signed-off-by: Bryan Frimin <bryan@probo.com>
This commit is contained in:
committed by
Sacha Al Himdani
parent
83e7b3bdd4
commit
f20c3d73d2
@@ -400,10 +400,16 @@ func (c *Client) inviteUser(profileID gid.GID) {
|
||||
}
|
||||
|
||||
func (c *Client) getActivationToken(email string) string {
|
||||
return c.pollForLinkToken(fmt.Sprintf("to:%s subject:\"Invitation to join\"", email))
|
||||
}
|
||||
|
||||
// pollForLinkToken polls mailpit for a message matching searchQuery and
|
||||
// returns the first "token" query parameter found among its links.
|
||||
func (c *Client) pollForLinkToken(searchQuery string) string {
|
||||
deadline := time.Now().Add(10 * time.Second)
|
||||
|
||||
for time.Now().Before(deadline) {
|
||||
searchMails, err := c.SearchMails(fmt.Sprintf("to:%s subject:\"Invitation to join\"", email))
|
||||
searchMails, err := c.SearchMails(searchQuery)
|
||||
require.NoError(c.T, err, "mailpit messages search failed")
|
||||
|
||||
for _, msg := range searchMails.Messages {
|
||||
@@ -414,9 +420,8 @@ func (c *Client) getActivationToken(email string) string {
|
||||
linkURL, err := url.Parse(link.URL)
|
||||
require.NoError(c.T, err, "mailpit link invalid URL")
|
||||
|
||||
query := linkURL.Query()
|
||||
if query.Get("token") != "" {
|
||||
return query.Get("token")
|
||||
if token := linkURL.Query().Get("token"); token != "" {
|
||||
return token
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -424,7 +429,7 @@ func (c *Client) getActivationToken(email string) string {
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
}
|
||||
|
||||
c.T.Logf("activation token not found")
|
||||
c.T.Logf("link token not found for query %q", searchQuery)
|
||||
c.T.FailNow()
|
||||
|
||||
return ""
|
||||
@@ -530,6 +535,66 @@ func NewClientWithNewSession(t testing.TB, from *Client) *Client {
|
||||
return client
|
||||
}
|
||||
|
||||
// SelfProvisionTrustCenterVisitor signs a brand-new email up as a trust
|
||||
// center visitor through the public magic-link flow (send + verify), the
|
||||
// same path a real visitor takes. It returns a Client bound to that
|
||||
// visitor's own session and cookie jar, scoped to no organization.
|
||||
func SelfProvisionTrustCenterVisitor(t testing.TB, trustCenterID string) *Client {
|
||||
t.Helper()
|
||||
|
||||
jar, err := cookiejar.New(nil)
|
||||
require.NoError(t, err, "cannot create cookie jar")
|
||||
|
||||
email := fmt.Sprintf("visitor-%s@e2e.probo.test", generateUniqueID())
|
||||
|
||||
visitor := &Client{
|
||||
T: t,
|
||||
baseURL: GetBaseURL(),
|
||||
mailpitBaseURL: GetMailpitBaseURL(),
|
||||
email: email,
|
||||
httpClient: &http.Client{
|
||||
Jar: jar,
|
||||
Timeout: 30 * time.Second,
|
||||
},
|
||||
}
|
||||
|
||||
visitor.sendMagicLink(trustCenterID, email)
|
||||
token := visitor.pollForLinkToken(fmt.Sprintf("to:%s", email))
|
||||
visitor.verifyMagicLink(trustCenterID, token)
|
||||
|
||||
return visitor
|
||||
}
|
||||
|
||||
func (c *Client) sendMagicLink(trustCenterID, email string) {
|
||||
const query = `
|
||||
mutation($input: SendMagicLinkInput!) {
|
||||
sendMagicLink(input: $input) {
|
||||
success
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err := c.ExecuteTrust(trustCenterID, query, map[string]any{
|
||||
"input": map[string]any{"email": email},
|
||||
}, nil)
|
||||
require.NoError(c.T, err, "sendMagicLink mutation failed")
|
||||
}
|
||||
|
||||
func (c *Client) verifyMagicLink(trustCenterID, token string) {
|
||||
const query = `
|
||||
mutation($input: VerifyMagicLinkInput!) {
|
||||
verifyMagicLink(input: $input) {
|
||||
continue
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err := c.ExecuteTrust(trustCenterID, query, map[string]any{
|
||||
"input": map[string]any{"token": token},
|
||||
}, nil)
|
||||
require.NoError(c.T, err, "verifyMagicLink mutation failed")
|
||||
}
|
||||
|
||||
func (c *Client) GetEmail() string {
|
||||
return c.email
|
||||
}
|
||||
|
||||
31
e2e/trust/main_test.go
Normal file
31
e2e/trust/main_test.go
Normal file
@@ -0,0 +1,31 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
|
||||
package trust_test
|
||||
|
||||
import (
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"go.probo.inc/probo/e2e/internal/testutil"
|
||||
)
|
||||
|
||||
func TestMain(m *testing.M) {
|
||||
testutil.Setup()
|
||||
|
||||
code := m.Run()
|
||||
|
||||
testutil.Teardown()
|
||||
os.Exit(code)
|
||||
}
|
||||
233
e2e/trust/trust_center_nda_signature_test.go
Normal file
233
e2e/trust/trust_center_nda_signature_test.go
Normal file
@@ -0,0 +1,233 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
|
||||
package trust_test
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.probo.inc/probo/e2e/internal/testutil"
|
||||
)
|
||||
|
||||
// minimalPDFBase64 is a minimal but structurally valid one-page PDF (no
|
||||
// visible content). Unlike a hand-rolled "%PDF-1.4 ... /Catalog" stub, it
|
||||
// has a real Pages tree, so it survives the watermark stamping that
|
||||
// ProvisionMember runs when creating a pending NDA signature.
|
||||
const minimalPDFBase64 = "JVBERi0xLjcKJeLjz9MKMSAwIG9iago8PC9QYWdlcyAyIDAgUi9UeXBlL0NhdGFsb2c+PgplbmRv" +
|
||||
"YmoKNCAwIG9iago8PC9GaWx0ZXIvRmxhdGVEZWNvZGUvTGVuZ3RoIDExPj4Kc3RyZWFtCnicAQAA" +
|
||||
"//8AAAABZW5kc3RyZWFtCmVuZG9iagoyMyAwIG9iago8PC9GaWx0ZXIvRmxhdGVEZWNvZGUvRmly" +
|
||||
"c3QgMTQvTGVuZ3RoIDE2Ni9OIDMvVHlwZS9PYmpTdG0+PgpzdHJlYW0KeJxczkHKwjAQBeCrzAn+" +
|
||||
"Sdr+ugmzaEEEEUp1V7qI7SAFSaSZit5epi6UZhPem2/x/sFADtsCMrCZdQ6rGISDJCjAQIO1nzgI" +
|
||||
"ZEtoOMV56jk5h7sYRD8Lud6IiPD8ujPW/spEzmHpE6vCPd8eLGPv8TRfRI1C++EqFl7FOQhYPIxD" +
|
||||
"anVW0+GRh9GX8dmaP/PzYBULsyo2q6L7TktE7wAAAP//bYpDg2VuZHN0cmVhbQplbmRvYmoKNiAw" +
|
||||
"IG9iago8PC9DcmVhdGlvbkRhdGUoRDoyMDE5MDcwNDEwMjcyOCswMicwMCcpL01vZERhdGUoRDoy" +
|
||||
"MDE5MDcwNDEwMjcyOCswMicwMCcpL1Byb2R1Y2VyKHBkZmNwdSB2MC4xLjI1KT4+CmVuZG9iagoy" +
|
||||
"MiAwIG9iago8PC9GaWx0ZXIvRmxhdGVEZWNvZGUvSURbPDEyNjNDMjQ4RDcyOUI5MTNGQzM5MkYy" +
|
||||
"NjQ2MTk1NDJBPiA8MGNiOTUzNjAyNzk4NWQ1NTQ5YzNlY2ZlNmE5MjM5ZTc+XS9JbmRleFswIDIy" +
|
||||
"IDIzIDFdL0luZm8gNiAwIFIvTGVuZ3RoIDcyL1Jvb3QgMSAwIFIvU2l6ZSAyNC9UeXBlL1hSZWYv" +
|
||||
"V1sxIDIgMl0+PgpzdHJlYW0KeJwkzEkKgDAUBNH62TgbR7yMFxS8c6RMLx70puAsJciQuEgSwV0v" +
|
||||
"ES//AhpppZNeBhllklmyLLLKJrsclh/4AgAA//9tzQSTZW5kc3RyZWFtCmVuZG9iagoKc3RhcnR4" +
|
||||
"cmVmCjUxMgolJUVPRg=="
|
||||
|
||||
// TestTrustCenter_AcceptElectronicSignature_RejectsForeignSignature is a
|
||||
// regression test for GHSA-22xj-f767-ppw6: any self-provisioned trust
|
||||
// center visitor could accept another visitor's NDA signature, or inject
|
||||
// audit-trail events into it, simply by knowing its GID.
|
||||
func TestTrustCenter_AcceptElectronicSignature_RejectsForeignSignature(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
owner := testutil.NewClient(t, testutil.RoleOwner)
|
||||
trustCenterID := lookupTrustCenterID(t, owner)
|
||||
|
||||
uploadTrustCenterNDA(t, owner, trustCenterID)
|
||||
activateTrustCenter(t, owner, trustCenterID)
|
||||
|
||||
victim := testutil.SelfProvisionTrustCenterVisitor(t, trustCenterID)
|
||||
attacker := testutil.SelfProvisionTrustCenterVisitor(t, trustCenterID)
|
||||
|
||||
victimSignatureID, victimSignatureStatus := viewerSignature(t, victim, trustCenterID)
|
||||
require.NotEmpty(t, victimSignatureID)
|
||||
require.Equal(t, "PENDING", victimSignatureStatus)
|
||||
|
||||
const acceptMutation = `
|
||||
mutation($input: AcceptElectronicSignatureInput!) {
|
||||
acceptElectronicSignature(input: $input) {
|
||||
signature { id status }
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err := attacker.ExecuteTrust(trustCenterID, acceptMutation, map[string]any{
|
||||
"input": map[string]any{"signatureId": victimSignatureID},
|
||||
}, nil)
|
||||
require.Error(t, err, "attacker must not be able to accept another visitor's signature")
|
||||
assertForbidden(t, err)
|
||||
|
||||
const recordEventMutation = `
|
||||
mutation($input: RecordSigningEventInput!) {
|
||||
recordSigningEvent(input: $input) {
|
||||
success
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err = attacker.ExecuteTrust(trustCenterID, recordEventMutation, map[string]any{
|
||||
"input": map[string]any{
|
||||
"signatureId": victimSignatureID,
|
||||
"eventType": "DOCUMENT_VIEWED",
|
||||
},
|
||||
}, nil)
|
||||
require.Error(t, err, "attacker must not be able to inject events into another visitor's signature")
|
||||
assertForbidden(t, err)
|
||||
|
||||
_, statusAfterAttack := viewerSignature(t, victim, trustCenterID)
|
||||
assert.Equal(t, "PENDING", statusAfterAttack, "attack attempts must not have mutated the victim's signature")
|
||||
|
||||
var acceptResult struct {
|
||||
AcceptElectronicSignature struct {
|
||||
Signature struct {
|
||||
ID string `json:"id"`
|
||||
Status string `json:"status"`
|
||||
} `json:"signature"`
|
||||
} `json:"acceptElectronicSignature"`
|
||||
}
|
||||
|
||||
err = victim.ExecuteTrust(trustCenterID, acceptMutation, map[string]any{
|
||||
"input": map[string]any{"signatureId": victimSignatureID},
|
||||
}, &acceptResult)
|
||||
require.NoError(t, err, "the legitimate signer must still be able to accept their own signature")
|
||||
assert.Equal(t, "ACCEPTED", acceptResult.AcceptElectronicSignature.Signature.Status)
|
||||
}
|
||||
|
||||
func lookupTrustCenterID(t *testing.T, owner *testutil.Client) string {
|
||||
t.Helper()
|
||||
|
||||
const query = `
|
||||
query($organizationId: ID!) {
|
||||
node(id: $organizationId) {
|
||||
... on Organization {
|
||||
trustCenter { id }
|
||||
}
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
var result struct {
|
||||
Node struct {
|
||||
TrustCenter struct {
|
||||
ID string `json:"id"`
|
||||
} `json:"trustCenter"`
|
||||
} `json:"node"`
|
||||
}
|
||||
|
||||
err := owner.Execute(query, map[string]any{
|
||||
"organizationId": owner.GetOrganizationID().String(),
|
||||
}, &result)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, result.Node.TrustCenter.ID)
|
||||
|
||||
return result.Node.TrustCenter.ID
|
||||
}
|
||||
|
||||
func uploadTrustCenterNDA(t *testing.T, owner *testutil.Client, trustCenterID string) {
|
||||
t.Helper()
|
||||
|
||||
const query = `
|
||||
mutation($input: UploadTrustCenterNDAInput!) {
|
||||
uploadTrustCenterNDA(input: $input) {
|
||||
trustCenter { id }
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
pdfContent, err := base64.StdEncoding.DecodeString(minimalPDFBase64)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = owner.ExecuteWithFile(query, map[string]any{
|
||||
"input": map[string]any{
|
||||
"trustCenterId": trustCenterID,
|
||||
"fileName": "nda.pdf",
|
||||
"file": nil,
|
||||
},
|
||||
}, "input.file", testutil.UploadFile{
|
||||
Filename: "nda.pdf",
|
||||
ContentType: "application/pdf",
|
||||
Content: pdfContent,
|
||||
}, nil)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func activateTrustCenter(t *testing.T, owner *testutil.Client, trustCenterID string) {
|
||||
t.Helper()
|
||||
|
||||
const query = `
|
||||
mutation($input: UpdateTrustCenterInput!) {
|
||||
updateTrustCenter(input: $input) {
|
||||
trustCenter { id active }
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err := owner.Execute(query, map[string]any{
|
||||
"input": map[string]any{
|
||||
"trustCenterId": trustCenterID,
|
||||
"active": true,
|
||||
},
|
||||
}, nil)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func viewerSignature(t *testing.T, visitor *testutil.Client, trustCenterID string) (string, string) {
|
||||
t.Helper()
|
||||
|
||||
const query = `
|
||||
query {
|
||||
currentTrustCenter {
|
||||
nonDisclosureAgreement {
|
||||
viewerSignature { id status }
|
||||
}
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
var result struct {
|
||||
CurrentTrustCenter struct {
|
||||
NonDisclosureAgreement struct {
|
||||
ViewerSignature struct {
|
||||
ID string `json:"id"`
|
||||
Status string `json:"status"`
|
||||
} `json:"viewerSignature"`
|
||||
} `json:"nonDisclosureAgreement"`
|
||||
} `json:"currentTrustCenter"`
|
||||
}
|
||||
|
||||
err := visitor.ExecuteTrust(trustCenterID, query, nil, &result)
|
||||
require.NoError(t, err)
|
||||
|
||||
sig := result.CurrentTrustCenter.NonDisclosureAgreement.ViewerSignature
|
||||
|
||||
return sig.ID, sig.Status
|
||||
}
|
||||
|
||||
func assertForbidden(t *testing.T, err error) {
|
||||
t.Helper()
|
||||
|
||||
gqlErrs, ok := err.(testutil.GraphQLErrors)
|
||||
require.True(t, ok, "expected a GraphQL error, got %T: %v", err, err)
|
||||
require.NotEmpty(t, gqlErrs)
|
||||
assert.Equal(t, "FORBIDDEN", gqlErrs[0].Code())
|
||||
}
|
||||
@@ -18,4 +18,5 @@ import "errors"
|
||||
|
||||
var (
|
||||
ErrElectronicSignatureNotFound = errors.New("electronic signature not found")
|
||||
ErrSignatureAccessDenied = errors.New("signature does not belong to the caller")
|
||||
)
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"go.gearno.de/crypto/uuid"
|
||||
@@ -327,6 +328,10 @@ func (s *Service) AcceptSignature(ctx context.Context, scope coredata.Scoper, re
|
||||
return fmt.Errorf("cannot load electronic signature: %w", err)
|
||||
}
|
||||
|
||||
if !strings.EqualFold(signature.SignerEmail, req.SignerEmail.String()) {
|
||||
return ErrSignatureAccessDenied
|
||||
}
|
||||
|
||||
if signature.Status != coredata.ElectronicSignatureStatusPending &&
|
||||
signature.Status != coredata.ElectronicSignatureStatusFailed {
|
||||
return fmt.Errorf("cannot accept electronic signature in status %s", signature.Status)
|
||||
@@ -387,6 +392,10 @@ func (s *Service) RecordEvent(ctx context.Context, scope coredata.Scoper, req *R
|
||||
return fmt.Errorf("cannot load electronic signature: %w", err)
|
||||
}
|
||||
|
||||
if !strings.EqualFold(signature.SignerEmail, req.ActorEmail.String()) {
|
||||
return ErrSignatureAccessDenied
|
||||
}
|
||||
|
||||
return s.recordEvent(ctx, tx, scope, req)
|
||||
},
|
||||
)
|
||||
|
||||
@@ -48,6 +48,10 @@ func (r *mutationResolver) AcceptElectronicSignature(ctx context.Context, input
|
||||
return nil, gqlutils.NotFoundf(ctx, "electronic signature %q not found", input.SignatureID)
|
||||
}
|
||||
|
||||
if errors.Is(err, esign.ErrSignatureAccessDenied) {
|
||||
return nil, gqlutils.Forbiddenf(ctx, "cannot accept electronic signature")
|
||||
}
|
||||
|
||||
r.logger.ErrorCtx(ctx, "cannot accept electronic signature", log.Error(err))
|
||||
|
||||
return nil, gqlutils.Internal(ctx)
|
||||
@@ -85,6 +89,10 @@ func (r *mutationResolver) RecordSigningEvent(ctx context.Context, input types.R
|
||||
return nil, gqlutils.NotFoundf(ctx, "electronic signature %q not found", input.SignatureID)
|
||||
}
|
||||
|
||||
if errors.Is(err, esign.ErrSignatureAccessDenied) {
|
||||
return nil, gqlutils.Forbiddenf(ctx, "cannot record signing event")
|
||||
}
|
||||
|
||||
r.logger.ErrorCtx(ctx, "cannot record signing event", log.Error(err))
|
||||
|
||||
return nil, gqlutils.Internal(ctx)
|
||||
|
||||
Reference in New Issue
Block a user