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

Reorder checks in response validation #197

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
58 changes: 29 additions & 29 deletions validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
"github.com/russellhaering/gosaml2/types"
)

//ErrParsing indicates that the value present in an assertion could not be
//parsed. It can be inspected for the specific tag name, the contents, and the
//intended type.
// ErrParsing indicates that the value present in an assertion could not be
// parsed. It can be inspected for the specific tag name, the contents, and the
// intended type.
type ErrParsing struct {
Tag, Value, Type string
}
Expand All @@ -32,14 +32,14 @@ func (ep ErrParsing) Error() string {
return fmt.Sprintf("Error parsing %s tag value as type %s", ep.Tag, ep.Value)
}

//Oft-used messages
// Oft-used messages
const (
ReasonUnsupported = "Unsupported"
ReasonExpired = "Expired"
)

//ErrInvalidValue indicates that the expected value did not match the received
//value.
// ErrInvalidValue indicates that the expected value did not match the received
// value.
type ErrInvalidValue struct {
Key, Expected, Actual string
Reason string
Expand All @@ -52,13 +52,13 @@ func (e ErrInvalidValue) Error() string {
return fmt.Sprintf("%s %s value, Expected: %s, Actual: %s", e.Reason, e.Key, e.Expected, e.Actual)
}

//Well-known methods of subject confirmation
// Well-known methods of subject confirmation
const (
SubjMethodBearer = "urn:oasis:names:tc:SAML:2.0:cm:bearer"
)

//VerifyAssertionConditions inspects an assertion element and makes sure that
//all SAML2 contracts are upheld.
// VerifyAssertionConditions inspects an assertion element and makes sure that
// all SAML2 contracts are upheld.
func (sp *SAMLServiceProvider) VerifyAssertionConditions(assertion *types.Assertion) (*WarningInfo, error) {
warningInfo := &WarningInfo{}
now := sp.Clock.Now()
Expand Down Expand Up @@ -131,32 +131,14 @@ func (sp *SAMLServiceProvider) VerifyAssertionConditions(assertion *types.Assert
return warningInfo, nil
}

//Validate ensures that the assertion passed is valid for the current Service
//Provider.
// Validate ensures that the assertion passed is valid for the current Service
// Provider.
func (sp *SAMLServiceProvider) Validate(response *types.Response) error {
err := sp.validateResponseAttributes(response)
if err != nil {
return err
}

if len(response.Assertions) == 0 {
return ErrMissingAssertion
}

issuer := response.Issuer
if issuer == nil {
// FIXME?: SAML Core 2.0 Section 3.2.2 has Response.Issuer as [Optional]
return ErrMissingElement{Tag: IssuerTag}
}

if sp.IdentityProviderIssuer != "" && response.Issuer.Value != sp.IdentityProviderIssuer {
return ErrInvalidValue{
Key: IssuerTag,
Expected: sp.IdentityProviderIssuer,
Actual: response.Issuer.Value,
}
}

status := response.Status
if status == nil {
return ErrMissingElement{Tag: StatusTag}
Expand All @@ -175,6 +157,24 @@ func (sp *SAMLServiceProvider) Validate(response *types.Response) error {
}
}

if len(response.Assertions) == 0 {
return ErrMissingAssertion
}

issuer := response.Issuer
if issuer == nil {
// FIXME?: SAML Core 2.0 Section 3.2.2 has Response.Issuer as [Optional]
return ErrMissingElement{Tag: IssuerTag}
}

if sp.IdentityProviderIssuer != "" && response.Issuer.Value != sp.IdentityProviderIssuer {
return ErrInvalidValue{
Key: IssuerTag,
Expected: sp.IdentityProviderIssuer,
Actual: response.Issuer.Value,
}
}

for _, assertion := range response.Assertions {
issuer = assertion.Issuer
if issuer == nil {
Expand Down