Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancing signature validation in SAML Response #144 #145

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions saml/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ var (
ErrInvalidAudience = errors.New("invalid audience")
ErrMissingSubject = errors.New("subject missing")
ErrMissingAttributeStmt = errors.New("attribute statement missing")
ErrInvalidSignature = errors.New("invalid signature")
)
58 changes: 56 additions & 2 deletions saml/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type parseResponseOptions struct {
skipAssertionConditionValidation bool
skipSignatureValidation bool
assertionConsumerServiceURL string
validateResponseSignature bool
validateAssertionSignature bool
}

func parseResponseOptionsDefault() parseResponseOptions {
Expand All @@ -32,6 +34,8 @@ func parseResponseOptionsDefault() parseResponseOptions {
skipRequestIDValidation: false,
skipAssertionConditionValidation: false,
skipSignatureValidation: false,
validateResponseSignature: false,
validateAssertionSignature: false,
}
}

Expand Down Expand Up @@ -73,6 +77,24 @@ func InsecureSkipSignatureValidation() Option {
}
}

// ValidateResponseSignature enables signature validation to ensure the response is at least signed
func ValidateResponseSignature() Option {
return func(o interface{}) {
if o, ok := o.(*parseResponseOptions); ok {
o.validateResponseSignature = true
}
}
}

// ValidateAssertionSignature enables signature validation to ensure the assertion is at least signed
func ValidateAssertionSignature() Option {
return func(o interface{}) {
if o, ok := o.(*parseResponseOptions); ok {
o.validateAssertionSignature = true
}
}
}

// ParseResponse parses and validates a SAML Reponse.
//
// Options:
Expand All @@ -87,15 +109,18 @@ func (sp *ServiceProvider) ParseResponse(
opt ...Option,
) (*core.Response, error) {
const op = "saml.(ServiceProvider).ParseResponse"
opts := getParseResponseOptions(opt...)

switch {
case sp == nil:
return nil, fmt.Errorf("%s: missing service provider %w", op, ErrInternal)
case samlResp == "":
return nil, fmt.Errorf("%s: missing saml response: %w", op, ErrInvalidParameter)
case requestID == "":
return nil, fmt.Errorf("%s: missing request ID: %w", op, ErrInvalidParameter)
case opts.skipSignatureValidation && (opts.validateResponseSignature || opts.validateAssertionSignature):
return nil, fmt.Errorf("%s: option `skip signature validation` cannot be true with any validate signature option : %w", op, ErrInvalidParameter)
}
opts := getParseResponseOptions(opt...)

// We use github.com/russellhaering/gosaml2 for SAMLResponse signature and condition validation.
ip, err := sp.internalParser(
Expand Down Expand Up @@ -151,7 +176,17 @@ func (sp *ServiceProvider) ParseResponse(
}
}

return &core.Response{Response: *response}, nil
samlResponse := core.Response{Response: *response}
if opts.validateResponseSignature || opts.validateAssertionSignature {
// func ip.ValidateEncodedResponse(...) above only requires either `response or all its `assertions` are signed,
// but does not require both. The validateSignature function will validate either response or assertion
// or both is surely signed depending on the parse response options given.
if err := validateSignature(&samlResponse, op, opts); err != nil {
return nil, err
}
}

return &samlResponse, nil
}

func (sp *ServiceProvider) internalParser(
Expand Down Expand Up @@ -245,3 +280,22 @@ func parsePEMCertificate(cert []byte) (*x509.Certificate, error) {

return x509.ParseCertificate(block.Bytes)
}

func validateSignature(response *core.Response, op string, opts parseResponseOptions) error {
// validate child object assertions
for _, assert := range response.Assertions() {
// note: at one time func ip.ValidateEncodedResponse(...) above allows all signed or all unsigned
// assertions, and will give error if there is a mix of both. We are still looping on all assertions
// instead of retrieving signature for one assertion, so we do not depend on dependency implementation.
if !assert.SignatureValidated && opts.validateAssertionSignature {
return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
}
}

// validate root object response
if !response.SignatureValidated && opts.validateResponseSignature {
return fmt.Errorf("%s: %w", op, ErrInvalidSignature)
}

return nil
}
84 changes: 79 additions & 5 deletions saml/response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

var testExpiredResp = `PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz4KPHNhbWwycDpSZXNwb25zZSBEZXN0aW5hdGlvbj0iaHR0cDovL2xvY2FsaG9zdDo4MDAwL3NhbWwvYWNzIiBJRD0iXzg4NDljMmVlNTMyZmNkYjc4MWYyYTE3NzZlYWMzNzQxIiBJblJlc3BvbnNlVG89ImJjNWE1YmFhLTk0ZTAtNThhOC04NzJjLWU1MTQ5MWQyYjNlZSIgSXNzdWVJbnN0YW50PSIyMDIzLTA4LTI1VDE0OjMyOjUzLjY4MFoiIFZlcnNpb249IjIuMCIgeG1sbnM6c2FtbDJwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiIHhtbG5zOnhzZD0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiPjxzYW1sMjpJc3N1ZXIgeG1sbnM6c2FtbDI9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphc3NlcnRpb24iPmh0dHBzOi8vc2FtbHRlc3QuaWQvc2FtbC9pZHA8L3NhbWwyOklzc3Vlcj48ZHM6U2lnbmF0dXJlIHhtbG5zOmRzPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjIj48ZHM6U2lnbmVkSW5mbz48ZHM6Q2Fub25pY2FsaXphdGlvbk1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPjxkczpTaWduYXR1cmVNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzA0L3htbGRzaWctbW9yZSNyc2Etc2hhMjU2Ii8+PGRzOlJlZmVyZW5jZSBVUkk9IiNfODg0OWMyZWU1MzJmY2RiNzgxZjJhMTc3NmVhYzM3NDEiPjxkczpUcmFuc2Zvcm1zPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjZW52ZWxvcGVkLXNpZ25hdHVyZSIvPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzEwL3htbC1leGMtYzE0biMiPjxlYzpJbmNsdXNpdmVOYW1lc3BhY2VzIFByZWZpeExpc3Q9InhzZCIgeG1sbnM6ZWM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPjwvZHM6VHJhbnNmb3JtPjwvZHM6VHJhbnNmb3Jtcz48ZHM6RGlnZXN0TWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8wNC94bWxlbmMjc2hhMjU2Ii8+PGRzOkRpZ2VzdFZhbHVlPlJWNDg1dUtHSlptTkExbzU2Z3h4aytWWmt2eE1xdGxIWkEyaUhIOFpVMVE9PC9kczpEaWdlc3RWYWx1ZT48L2RzOlJlZmVyZW5jZT48L2RzOlNpZ25lZEluZm8+PGRzOlNpZ25hdHVyZVZhbHVlPmQzTHBjNmhjU0I3YndDek1yTzN3ZlpyTmlHazVnWjhyS1JLT1FFTkRQMnErcDMrTGtEbVNCdDZ6enl4bjMzTUNTSnQrZFBIcEYxNFlNQUsvTjNQbld3U1NVcDBqNWt6T2M5S2E1TmRpYW5FME5nWW5VMHFqaEZKYlRoQVF6N2hSb3dTNEo0OWhTLzZNdVNRMFo3bkJCQ2VEZ2VENlBZUkFwS012bE90a0JHUEphTFQybVJ5L2duUStDQzZ1ZFVkSnl2U2diOW40M2x2eGRhYVpXckRLM1dnYTk4WWxrY1JITHJtUEFBTThLeFlXbmtvcGlvNllJTlU0RDVtWmpzRXNuVWtINDFXZ2N3Z21TMnh6UDNJQ25OYzNXSDlOSHJWS3A5YXQyREJ3cllESXNlczZGWGdZcStpVVdLMjE5MWpXcElDM3FWQUIwY09pbG1SWHd0RUg3Zz09PC9kczpTaWduYXR1cmVWYWx1ZT48ZHM6S2V5SW5mbz48ZHM6WDUwOURhdGE+PGRzOlg1MDlDZXJ0aWZpY2F0ZT5NSUlERWpDQ0FmcWdBd0lCQWdJVkFNRUNRMXRqZ2hhZm01T3hXRGg5aHdaZnh0aFdNQTBHQ1NxR1NJYjNEUUVCQ3dVQU1CWXhGREFTCkJnTlZCQU1NQzNOaGJXeDBaWE4wTG1sa01CNFhEVEU0TURneU5ESXhNVFF3T1ZvWERUTTRNRGd5TkRJeE1UUXdPVm93RmpFVU1CSUcKQTFVRUF3d0xjMkZ0YkhSbGMzUXVhV1F3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRQzBaNFFYMU5GSwpzNzF1ZmJRd29Rb1c3cWtOQUpSSUFOR0E0aU0wVGhZZ2h1bDNwQytGd3JHdjM3YVR4V1hmQTFVRzluaktiYkRyZWlEQVpLbmdDZ3lqCnhqMHVKNGxBcmdrcjRBT0VqajV6WEE4MXVHSEFSZlVCY3R2UWNzWnBCSXhET3ZVVUltQWwrM05xTGdNR0YyZmt0eE1HN2tYM0dFVk4KYzFrbGJOM2RmWXNhdzVkVXJ3MjVEaGVMOW5wN0cvKzI4R3dIUHZMYjRhcHRPaU9OYkNhVnZoOVVNSEVBOUY3YzB6ZkYvY0w1Zk9wZApWYTU0d1RJMHUxMkNzRkt0NzhoNmxFR0c1alVzL3FYOWNsWm5jSk03RUZrTjNpbVBQeSswSEM4bnNwWGlIL01aVzhvMmNxV1JrcnczCk16QlpXM09qazVuUWo0MFY2TlViamI3a2ZlanpBZ01CQUFHalZ6QlZNQjBHQTFVZERnUVdCQlFUNlk5SjNUdy9oT0djOFBOVjdKRUUKNGsyWk5UQTBCZ05WSFJFRUxUQXJnZ3R6WVcxc2RHVnpkQzVwWklZY2FIUjBjSE02THk5ellXMXNkR1Z6ZEM1cFpDOXpZVzFzTDJsawpjREFOQmdrcWhraUc5dzBCQVFzRkFBT0NBUUVBU2szZ3VLZlRrVmhFYUlWdnhFUE5SMnczdld0M2Z3bXdKQ2NjVzk4WFhMV2dOYnUzCllhTWIyUlNuN1RoNHAzaCttZnlrMmRvbjZhdTdVeXpjMUpkMzlSTnY4MFRHNWlRb3hmQ2dwaHkxRlltbWRhU2ZPOHd2RHRIVFROaUwKQXJBeE9ZdHpmWWJ6YjVRck5OSC9nUUVOOFJKYUVmL2cvMUdUdzl4LzEwM2RTTUswUlh0bCtmUnMybmJsRDFKSktTUTNBZGh4Sy93ZQpQM2FVUHRMeFZWSjl3TU9RT2ZjeTAybCtoSE1iNnVBanNQT3BPVktxaTNNOFhtY1VaT3B4NHN3dGdHZGVvU3BlUnlydE12UndkY2NpCk5CcDlVWm9tZTQ0cVpBWUgxaXFycG1tanNmSTlwSkl0c2dXdTNrWFBqaFNmajFBSkdSMWw5Skd2SnJIa2kxaUhUQT09PC9kczpYNTA5Q2VydGlmaWNhdGU+PC9kczpYNTA5RGF0YT48L2RzOktleUluZm8+PC9kczpTaWduYXR1cmU+PHNhbWwycDpTdGF0dXM+PHNhbWwycDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz48L3NhbWwycDpTdGF0dXM+PHNhbWwyOkFzc2VydGlvbiBJRD0iXzM1ZWE5MGI3MTFkNmYzODUzNDVmMGRiZGQ3ZDBlZDViIiBJc3N1ZUluc3RhbnQ9IjIwMjMtMDgtMjVUMTQ6MzI6NTMuNjgwWiIgVmVyc2lvbj0iMi4wIiB4bWxuczpzYW1sMj0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiI+PHNhbWwyOklzc3Vlcj5odHRwczovL3NhbWx0ZXN0LmlkL3NhbWwvaWRwPC9zYW1sMjpJc3N1ZXI+PHNhbWwyOlN1YmplY3Q+PHNhbWwyOk5hbWVJRCBGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjEuMTpuYW1laWQtZm9ybWF0OmVtYWlsQWRkcmVzcyIgTmFtZVF1YWxpZmllcj0iaHR0cHM6Ly9zYW1sdGVzdC5pZC9zYW1sL2lkcCIgU1BOYW1lUXVhbGlmaWVyPSJodHRwOi8vc2FtbC5qdWx6L2V4YW1wbGUiIHhtbG5zOnNhbWwyPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIj5tc21pdGhAc2FtbHRlc3QuaWQ8L3NhbWwyOk5hbWVJRD48c2FtbDI6U3ViamVjdENvbmZpcm1hdGlvbiBNZXRob2Q9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpjbTpiZWFyZXIiPjxzYW1sMjpTdWJqZWN0Q29uZmlybWF0aW9uRGF0YSBBZGRyZXNzPSIxMDQuMjguMzkuMzQiIEluUmVzcG9uc2VUbz0iYmM1YTViYWEtOTRlMC01OGE4LTg3MmMtZTUxNDkxZDJiM2VlIiBOb3RPbk9yQWZ0ZXI9IjIwMjMtMDgtMjVUMTQ6Mzc6NTMuNjkzWiIgUmVjaXBpZW50PSJodHRwOi8vbG9jYWxob3N0OjgwMDAvc2FtbC9hY3MiLz48L3NhbWwyOlN1YmplY3RDb25maXJtYXRpb24+PC9zYW1sMjpTdWJqZWN0PjxzYW1sMjpDb25kaXRpb25zIE5vdEJlZm9yZT0iMjAyMy0wOC0yNVQxNDozMjo1My42ODBaIiBOb3RPbk9yQWZ0ZXI9IjIwMjMtMDgtMjVUMTQ6Mzc6NTMuNjgwWiI+PHNhbWwyOkF1ZGllbmNlUmVzdHJpY3Rpb24+PHNhbWwyOkF1ZGllbmNlPmh0dHA6Ly9zYW1sLmp1bHovZXhhbXBsZTwvc2FtbDI6QXVkaWVuY2U+PC9zYW1sMjpBdWRpZW5jZVJlc3RyaWN0aW9uPjwvc2FtbDI6Q29uZGl0aW9ucz48c2FtbDI6QXV0aG5TdGF0ZW1lbnQgQXV0aG5JbnN0YW50PSIyMDIzLTA4LTI1VDE0OjMxOjU2LjA2NFoiIFNlc3Npb25JbmRleD0iX2Y3MmE2M2VlMzc4MmI0N2M4OWY2MGU4MWFkZGUwYWIwIj48c2FtbDI6U3ViamVjdExvY2FsaXR5IEFkZHJlc3M9IjEwNC4yOC4zOS4zNCIvPjxzYW1sMjpBdXRobkNvbnRleHQ+PHNhbWwyOkF1dGhuQ29udGV4dENsYXNzUmVmPnVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphYzpjbGFzc2VzOlBhc3N3b3JkUHJvdGVjdGVkVHJhbnNwb3J0PC9zYW1sMjpBdXRobkNvbnRleHRDbGFzc1JlZj48L3NhbWwyOkF1dGhuQ29udGV4dD48L3NhbWwyOkF1dGhuU3RhdGVtZW50PjxzYW1sMjpBdHRyaWJ1dGVTdGF0ZW1lbnQ+PHNhbWwyOkF0dHJpYnV0ZSBGcmllbmRseU5hbWU9ImVkdVBlcnNvbkVudGl0bGVtZW50IiBOYW1lPSJ1cm46b2lkOjEuMy42LjEuNC4xLjU5MjMuMS4xLjEuNyIgTmFtZUZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmF0dHJuYW1lLWZvcm1hdDp1cmkiPjxzYW1sMjpBdHRyaWJ1dGVWYWx1ZT5BbWJhc3NhZG9yPC9zYW1sMjpBdHRyaWJ1dGVWYWx1ZT48c2FtbDI6QXR0cmlidXRlVmFsdWU+Tm9uZTwvc2FtbDI6QXR0cmlidXRlVmFsdWU+PC9zYW1sMjpBdHRyaWJ1dGU+PHNhbWwyOkF0dHJpYnV0ZSBOYW1lPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDphdHRyaWJ1dGU6c3ViamVjdC1pZCIgTmFtZUZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmF0dHJuYW1lLWZvcm1hdDp1cmkiPjxzYW1sMjpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiB4c2k6dHlwZT0ieHNkOnN0cmluZyI+bXNtaXRoQHNhbWx0ZXN0LmlkPC9zYW1sMjpBdHRyaWJ1dGVWYWx1ZT48L3NhbWwyOkF0dHJpYnV0ZT48c2FtbDI6QXR0cmlidXRlIEZyaWVuZGx5TmFtZT0idWlkIiBOYW1lPSJ1cm46b2lkOjAuOS4yMzQyLjE5MjAwMzAwLjEwMC4xLjEiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWU+bW9ydHk8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjxzYW1sMjpBdHRyaWJ1dGUgRnJpZW5kbHlOYW1lPSJ0ZWxlcGhvbmVOdW1iZXIiIE5hbWU9InVybjpvaWQ6Mi41LjQuMjAiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWU+KzEtNTU1LTU1NS01NTA1PC9zYW1sMjpBdHRyaWJ1dGVWYWx1ZT48L3NhbWwyOkF0dHJpYnV0ZT48c2FtbDI6QXR0cmlidXRlIEZyaWVuZGx5TmFtZT0icm9sZSIgTmFtZT0iaHR0cHM6Ly9zYW1sdGVzdC5pZC9hdHRyaWJ1dGVzL3JvbGUiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWUgeG1sbnM6eHNpPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYS1pbnN0YW5jZSIgeHNpOnR5cGU9InhzZDpzdHJpbmciPmphbml0b3JAc2FtbHRlc3QuaWQ8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjxzYW1sMjpBdHRyaWJ1dGUgRnJpZW5kbHlOYW1lPSJtYWlsIiBOYW1lPSJ1cm46b2lkOjAuOS4yMzQyLjE5MjAwMzAwLjEwMC4xLjMiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWU+bXNtaXRoQHNhbWx0ZXN0LmlkPC9zYW1sMjpBdHRyaWJ1dGVWYWx1ZT48L3NhbWwyOkF0dHJpYnV0ZT48c2FtbDI6QXR0cmlidXRlIEZyaWVuZGx5TmFtZT0ic24iIE5hbWU9InVybjpvaWQ6Mi41LjQuNCIgTmFtZUZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmF0dHJuYW1lLWZvcm1hdDp1cmkiPjxzYW1sMjpBdHRyaWJ1dGVWYWx1ZT5TbWl0aDwvc2FtbDI6QXR0cmlidXRlVmFsdWU+PC9zYW1sMjpBdHRyaWJ1dGU+PHNhbWwyOkF0dHJpYnV0ZSBGcmllbmRseU5hbWU9ImRpc3BsYXlOYW1lIiBOYW1lPSJ1cm46b2lkOjIuMTYuODQwLjEuMTEzNzMwLjMuMS4yNDEiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWU+TW9ydHkgU21pdGg8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjxzYW1sMjpBdHRyaWJ1dGUgRnJpZW5kbHlOYW1lPSJnaXZlbk5hbWUiIE5hbWU9InVybjpvaWQ6Mi41LjQuNDIiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6dXJpIj48c2FtbDI6QXR0cmlidXRlVmFsdWU+TW9ydGltZXI8L3NhbWwyOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDI6QXR0cmlidXRlPjwvc2FtbDI6QXR0cmlidXRlU3RhdGVtZW50Pjwvc2FtbDI6QXNzZXJ0aW9uPjwvc2FtbDJwOlJlc3BvbnNlPg==`

// TODO: add the ability to sign requests, so we can write more complete unit tests
func TestServiceProvider_ParseResponse(t *testing.T) {
t.Parallel()
const (
Expand Down Expand Up @@ -60,12 +59,87 @@ func TestServiceProvider_ParseResponse(t *testing.T) {
wantErrAs error
}{
{
name: "success",
name: "success - with both response and assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithResponseSigned()))),
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithResponseAndAssertionSigned()))),
opts: []saml.Option{},
requestID: testRequestId,
},
{
name: "success - with just response signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
opts: []saml.Option{},
requestID: testRequestId,
},
{
name: "success - with just assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustAssertionSigned()))),
opts: []saml.Option{},
requestID: testRequestId,
},
{
name: "success - with both options enabled of validating both signatures & with both response and assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithResponseAndAssertionSigned()))),
opts: []saml.Option{saml.ValidateResponseSignature(), saml.ValidateAssertionSignature()},
requestID: testRequestId,
},
{
name: "success - with option of validate response signature & with only response signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
opts: []saml.Option{saml.ValidateResponseSignature()},
requestID: testRequestId,
},
{
name: "success - with option of validate assertion signature & with only assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustAssertionSigned()))),
opts: []saml.Option{saml.ValidateAssertionSignature()},
requestID: testRequestId,
},
{
name: "missing signature",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t))),
opts: []saml.Option{},
requestID: testRequestId,
wantErrContains: "response and/or assertions must be signed",
},
{
name: "error-invalid-signature - with both options enabled of validating both signatures & with only response signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
opts: []saml.Option{saml.ValidateResponseSignature(), saml.ValidateAssertionSignature()},
requestID: testRequestId,
wantErrContains: "invalid signature",
},
{
name: "error-invalid-signature - with both options enabled of validating both signatures & with only assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustAssertionSigned()))),
opts: []saml.Option{saml.ValidateResponseSignature(), saml.ValidateAssertionSignature()},
requestID: testRequestId,
wantErrContains: "invalid signature",
},
{
name: "error-invalid-signature - with option of validate response signature & with only assertion signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustAssertionSigned()))),
opts: []saml.Option{saml.ValidateResponseSignature()},
requestID: testRequestId,
wantErrContains: "invalid signature",
},
{
name: "error-invalid-signature - with option of validate assertion signature & with just response signed",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithJustResponseSigned()))),
opts: []saml.Option{saml.ValidateAssertionSignature()},
requestID: testRequestId,
wantErrContains: "invalid signature",
},
{
name: "err-assertion-missing-attribute-stmt",
sp: testSp,
Expand Down Expand Up @@ -144,15 +218,15 @@ func TestServiceProvider_ParseResponse(t *testing.T) {
{
name: "err-in-response-to",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithResponseSigned()))),
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t, testprovider.WithResponseAndAssertionSigned()))),
requestID: "invalid-request-id",
wantErrContains: "doesn't match the expected requestID (invalid-request-id)",
},
{
name: "expired",
sp: testSp,
samlResp: base64.StdEncoding.EncodeToString([]byte(tp.SamlResponse(t,
testprovider.WithResponseSigned(),
testprovider.WithResponseAndAssertionSigned(),
testprovider.WithResponseExpired(),
))),
requestID: "request-id",
Expand Down
47 changes: 39 additions & 8 deletions saml/test/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,9 @@ func (p *TestProvider) parseRequestPost(request string) *core.AuthnRequest {
}

type responseOptions struct {
sign bool
expired bool
signResponseElem bool
signAssertionElem bool
expired bool
}

type ResponseOption func(*responseOptions)
Expand All @@ -450,9 +451,22 @@ func defaultResponseOptions() *responseOptions {
return &responseOptions{}
}

func WithResponseSigned() ResponseOption {
func WithResponseAndAssertionSigned() ResponseOption {
return func(o *responseOptions) {
o.sign = true
o.signResponseElem = true
o.signAssertionElem = true
}
}

func WithJustAssertionSigned() ResponseOption {
return func(o *responseOptions) {
o.signAssertionElem = true
}
}

func WithJustResponseSigned() ResponseOption {
return func(o *responseOptions) {
o.signResponseElem = true
}
}

Expand Down Expand Up @@ -544,13 +558,30 @@ func (p *TestProvider) SamlResponse(t *testing.T, opts ...ResponseOption) string
err = doc.ReadFromBytes(resp)
r.NoError(err)

if opt.sign {
if opt.signResponseElem || opt.signAssertionElem {
signCtx := dsig.NewDefaultSigningContext(p.keystore)

signed, err := signCtx.SignEnveloped(doc.Root())
r.NoError(err)
// sign child object assertions
// note we will sign the `assertion` first and then only the parent `response`, because the `response`
// signature is based on the entire contents of `response` (including `assertion` signature)
if opt.signAssertionElem {
responseEl := doc.SelectElement("Response")
for _, assert := range responseEl.FindElements("Assertion") {
signedAssert, err := signCtx.SignEnveloped(assert)
r.NoError(err)

// replace signed assert object
responseEl.RemoveChildAt(assert.Index())
responseEl.AddChild(signedAssert)
}
}

doc.SetRoot(signed)
// sign root object response
if opt.signResponseElem {
signed, err := signCtx.SignEnveloped(doc.Root())
r.NoError(err)
doc.SetRoot(signed)
}
}

result, err := doc.WriteToString()
Expand Down
Loading