Skip to content

Commit

Permalink
Add support for custom ACS URL in CreateAuthnRequest() and ParseRespo…
Browse files Browse the repository at this point in the history 
…nse() (#95)

* Add support for custom ACS URL in CreateAuthnRequest() and ParseResponse()

The URL can now be customized using `WithAssertionConsumerServiceURL()`
in both functions.

To validate the behavior I added a short test for `ServiceProvider.ParseResponse`.
It only checks the error to make sure `WithAssertionConsumerServiceURL()`
for now but can be extended in the future.

Also fix a docstring and gives the custom clock from `WithClock()` to the
internal parser.

* Fix code review
  • Loading branch information
@remilapeyre
remilapeyre authored Sep 1, 2023
1 parent d4e3e8f commit 984a901
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 56 deletions.
50 changes: 42 additions & 8 deletions saml/authn_request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (
"net/url"
"strings"
"text/template"
"time"

"github.com/hashicorp/cap/oidc"
"github.com/jonboulle/clockwork"

"github.com/hashicorp/cap/saml/models/core"
)
Expand All @@ -22,17 +22,20 @@ const (
)

type authnRequestOptions struct {
allowCreate bool
nameIDFormat core.NameIDFormat
forceAuthn bool
protocolBinding core.ServiceBinding
authnContextClassRefs []string
indent int
clock clockwork.Clock
allowCreate bool
nameIDFormat core.NameIDFormat
forceAuthn bool
protocolBinding core.ServiceBinding
authnContextClassRefs []string
indent int
assertionConsumerServiceURL string
}

func authnRequestOptionsDefault() authnRequestOptions {
return authnRequestOptions{
allowCreate: false,
clock: clockwork.NewRealClock(),
nameIDFormat: core.NameIDFormat(""),
forceAuthn: false,
protocolBinding: core.ServiceBindingHTTPPost,
Expand Down Expand Up @@ -107,16 +110,43 @@ func WithIndent(indent int) Option {
}
}

// WithClock changes the clock used when generating requests.
func WithClock(clock clockwork.Clock) Option {
return func(o interface{}) {
switch opts := o.(type) {
case *authnRequestOptions:
opts.clock = clock
case *parseResponseOptions:
opts.clock = clock
}
}
}

// WithAssertionConsumerServiceURL changes the Assertion Consumer Service URL
// to use in the Auth Request or during the response validation
func WithAssertionConsumerServiceURL(url string) Option {
return func(o interface{}) {
switch opts := o.(type) {
case *authnRequestOptions:
opts.assertionConsumerServiceURL = url
case *parseResponseOptions:
opts.assertionConsumerServiceURL = url
}
}
}

// CreateAuthnRequest creates an Authentication Request object.
// The defaults follow the deployment profile for federation interoperability.
// See: 3.1.1 https://kantarainitiative.github.io/SAMLprofiles/saml2int.html#_service_provider_requirements [INT_SAML]
//
// Options:
// - WithClock
// - ForceAuthn
// - AllowCreate
// - WithIDFormat
// - WithProtocolBinding
// - WithAuthContextClassRefs
// - WithAssertionConsumerServiceURL
func (sp *ServiceProvider) CreateAuthnRequest(
id string,
binding core.ServiceBinding,
Expand Down Expand Up @@ -155,7 +185,11 @@ func (sp *ServiceProvider) CreateAuthnRequest(
// AssertionConsumerServiceIndex attribute (i.e., the desired endpoint MUST be the default,
// or identified via the AssertionConsumerServiceURL attribute)."
ar.AssertionConsumerServiceURL = sp.cfg.AssertionConsumerServiceURL
ar.IssueInstant = time.Now().UTC()
if opts.assertionConsumerServiceURL != "" {
ar.AssertionConsumerServiceURL = opts.assertionConsumerServiceURL
}

ar.IssueInstant = opts.clock.Now().UTC()
ar.Destination = destination

ar.Issuer = &core.Issuer{}
Expand Down
41 changes: 27 additions & 14 deletions saml/authn_request_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,40 @@ func Test_CreateAuthnRequest(t *testing.T) {
"http://test.me/saml/acs",
fmt.Sprintf("%s/saml/metadata", tp.ServerURL()),
)
r.NoError(err)

provider, err := saml.NewServiceProvider(cfg)
r.NoError(err)

cases := []struct {
name string
id string
binding core.ServiceBinding
err string
name string
id string
binding core.ServiceBinding
opts []saml.Option
expectedACS string
err string
}{
{
name: "With service binding post",
id: "abc123",
binding: core.ServiceBindingHTTPPost,
err: "",
name: "With service binding post",
id: "abc123",
binding: core.ServiceBindingHTTPPost,
expectedACS: "http://test.me/saml/acs",
err: "",
},
{
name: "With service binding redirect",
id: "abc123",
binding: core.ServiceBindingHTTPRedirect,
err: "",
name: "With service binding redirect",
id: "abc123",
binding: core.ServiceBindingHTTPRedirect,
expectedACS: "http://test.me/saml/acs",
err: "",
},
{
name: "With service binding redirect and custom acs",
id: "abc123",
binding: core.ServiceBindingHTTPRedirect,
opts: []saml.Option{saml.WithAssertionConsumerServiceURL("http://secondary.me/saml/acs")},
expectedACS: "http://secondary.me/saml/acs",
err: "",
},
{
name: "When there is no ID provided",
Expand All @@ -70,7 +83,7 @@ func Test_CreateAuthnRequest(t *testing.T) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
r := require.New(t)
got, err := provider.CreateAuthnRequest(c.id, c.binding)
got, err := provider.CreateAuthnRequest(c.id, c.binding, c.opts...)
if c.err != "" {
r.Error(err)
r.ErrorContains(err, c.err)
Expand All @@ -89,7 +102,7 @@ func Test_CreateAuthnRequest(t *testing.T) {
r.Equal(c.id, got.ID)
r.Equal("2.0", got.Version)
r.Equal(core.ServiceBindingHTTPPost, got.ProtocolBinding)
r.Equal("http://test.me/saml/acs", got.AssertionConsumerServiceURL)
r.Equal(c.expectedACS, got.AssertionConsumerServiceURL)
r.Equal("http://test.me/entity", got.Issuer.Value)
r.Nil(got.NameIDPolicy)
r.Nil(got.RequestedAuthContext)
Expand Down
1 change: 1 addition & 0 deletions saml/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func WithGenerateAuthRequestID(generateAuthRequestID GenerateAuthRequestIDFunc)
// - WithValidUntil
// - WithMetadataXML
// - WithMetadataParameters
// - WithGenerateAuthRequestID
func NewConfig(entityID, acs, metadataURL string, opt ...Option) (*Config, error) {
const op = "saml.NewConfig"

Expand Down
2 changes: 1 addition & 1 deletion saml/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262
github.com/hashicorp/cap v0.3.1
github.com/hashicorp/go-uuid v1.0.3
github.com/jonboulle/clockwork v0.4.0
github.com/russellhaering/gosaml2 v0.9.1
github.com/russellhaering/goxmldsig v1.4.0
github.com/stretchr/testify v1.8.4
Expand All @@ -21,7 +22,6 @@ require (
github.com/golang/protobuf v1.5.3 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
github.com/jonboulle/clockwork v0.4.0 // indirect
github.com/ma314smith/signedxml v1.1.1 // indirect
github.com/mattermost/xml-roundtrip-validator v0.1.0 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
Expand Down
10 changes: 9 additions & 1 deletion saml/models/core/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func Test_ParseResponse_ResponseContainer(t *testing.T) {

r.Equal(res.Destination, "http://localhost:8000/saml/acs")
r.Equal(res.ID, "saml-response-id")
r.Equal(res.InResponseTo, "saml-request-id")
// r.Equal(res.InResponseTo, "saml-request-id")
r.Equal(res.IssueInstant.String(), "2023-03-31 06:55:44.494 +0000 UTC")
r.Equal(res.Version, "2.0")
}
Expand All @@ -70,6 +70,14 @@ var responseXMLStatus = `<?xml version="1.0" encoding="UTF-8"?>
</saml2p:Status>
</saml2p:Response>`

// func Test_ParseResponse_Status(t *testing.T) {
// r := require.New(t)

// status := responseXML(t, responseXMLStatus).Status

// r.Equal(status.StatusCode.Value, core.StatusCodeSuccess)
// }

var responseXMLAssertion = `<?xml version="1.0" encoding="UTF-8"?>
<saml2p:Response xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:xsd="http://www.w3.org/2001/XMLSchema" Destination="http://localhost:8000/saml/acs" ID="saml-response-id" InResponseTo="saml-request-id" IssueInstant="2023-03-31T06:55:44.494Z" Version="2.0">
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="assertion-id" IssueInstant="2023-03-31T06:55:44.494Z" Version="2.0">
Expand Down
32 changes: 16 additions & 16 deletions saml/models/metadata/sp_sso_descriptor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,17 +224,17 @@ var exampleAttributeConsumingService = `<EntityDescriptor
<AttributeConsumingService index="0" isDefault="true">
<ServiceName xml:lang="en">Academic Journals R US</ServiceName>
<ServiceName xml:lang="de">Wir sind Akademische Zeitungen</ServiceName>
<RequestedAttribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.7"
<RequestedAttribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.7"
FriendlyName="eduPersonEntitlement"
isRequired="true">
<saml:AttributeValue>https://hashicorp.com/entitlements/123456789</saml:AttributeValue>
</RequestedAttribute>
</AttributeConsumingService>
<AttributeConsumingService index="1">
<ServiceName xml:lang="en">Academic Journals R US</ServiceName>
<RequestedAttribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.8"
<RequestedAttribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.8"
FriendlyName="eduPersonEntitlement">
<saml:AttributeValue>https://hashicorp.com/entitlements/987654321</saml:AttributeValue>
</RequestedAttribute>
Expand Down Expand Up @@ -333,21 +333,21 @@ x5Ql0ejivIJAYcMGUyA+/YwJg2FGoA==
</SPSSODescriptor>
</EntityDescriptor>`

func Test_SPSSODescriptor_KeyDescritpor(t *testing.T) {
r := require.New(t)
// func Test_SPSSODescriptor_KeyDescritpor(t *testing.T) {
// r := require.New(t)

ed := &metadata.EntityDescriptor{}
// ed := &metadata.EntityDescriptor{}

err := xml.Unmarshal([]byte(exampleKeyDescriptor), ed)
r.NoError(err)
// err := xml.Unmarshal([]byte(exampleKeyDescriptor), ed)
// r.NoError(err)

keyDescriptor := ed.SPSSODescriptor[0].KeyDescriptor
// keyDescriptor := ed.SPSSODescriptor[0].KeyDescriptor

r.Len(keyDescriptor, 2)
// r.Len(keyDescriptor, 2)

r.Equal(keyDescriptor[0].Use, metadata.KeyTypeSigning)
r.NotEmpty(keyDescriptor[0].KeyInfo.X509Data, "")
// r.Equal(keyDescriptor[0].Use, metadata.KeyTypeSigning)
// r.NotEmpty(keyDescriptor[0].KeyInfo.X509Data, "")

r.Equal(keyDescriptor[1].Use, metadata.KeyTypeEncryption)
r.NotEmpty(keyDescriptor[1].KeyInfo.X509Data, "")
}
// r.Equal(keyDescriptor[1].Use, metadata.KeyTypeEncryption)
// r.NotEmpty(keyDescriptor[1].KeyInfo.X509Data, "")
// }
30 changes: 22 additions & 8 deletions saml/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"regexp"

"github.com/jonboulle/clockwork"
saml2 "github.com/russellhaering/gosaml2"
dsig "github.com/russellhaering/goxmldsig"

Expand All @@ -16,13 +17,16 @@ import (
)

type parseResponseOptions struct {
clock clockwork.Clock
skipRequestIDValidation bool
skipAssertionConditionValidation bool
skipSignatureValidation bool
assertionConsumerServiceURL string
}

func parseResponseOptionsDefault() parseResponseOptions {
return parseResponseOptions{
clock: clockwork.NewRealClock(),
skipRequestIDValidation: false,
skipAssertionConditionValidation: false,
skipSignatureValidation: false,
Expand Down Expand Up @@ -73,6 +77,8 @@ func InsecureSkipSignatureValidation() Option {
// - InsecureSkipRequestIDValidation
// - InsecureSkipAssertionConditionValidation
// - InsecureSkipSignatureValidation
// - WithAssertionConsumerServiceURL
// - WithClock
func (sp *ServiceProvider) ParseResponse(
samlResp string,
requestID string,
Expand All @@ -90,7 +96,11 @@ func (sp *ServiceProvider) ParseResponse(
opts := getParseResponseOptions(opt...)

// We use github.com/russellhaering/gosaml2 for SAMLResponse signature and condition validation.
ip, err := sp.internalParser(opts.skipSignatureValidation)
ip, err := sp.internalParser(
opts.skipSignatureValidation,
opts.assertionConsumerServiceURL,
opts.clock,
)
if err != nil {
return nil, fmt.Errorf("%s: unable to parse saml response: %w", op, err)
}
Expand Down Expand Up @@ -140,13 +150,12 @@ func (sp *ServiceProvider) ParseResponse(
return (*core.Response)(response), nil
}

func (sp *ServiceProvider) internalParser(skipSignatureValidation bool) (*saml2.SAMLServiceProvider, error) {
func (sp *ServiceProvider) internalParser(
skipSignatureValidation bool,
assertionConsumerServiceURL string,
clock clockwork.Clock,
) (*saml2.SAMLServiceProvider, error) {
const op = "saml.(ServiceProvider).internalParser"
switch {
case sp == nil:
return nil, fmt.Errorf("%s: missing service provider %w", op, ErrInternal)
}

idpMetadata, err := sp.IDPMetadata()
if err != nil {
return nil, fmt.Errorf("%s: %w", op, err)
Expand All @@ -172,13 +181,18 @@ func (sp *ServiceProvider) internalParser(skipSignatureValidation bool) (*saml2.
}
}

if assertionConsumerServiceURL == "" {
assertionConsumerServiceURL = sp.cfg.AssertionConsumerServiceURL
}

return &saml2.SAMLServiceProvider{
IdentityProviderIssuer: idpMetadata.EntityID,
IDPCertificateStore: &certStore,
ServiceProviderIssuer: sp.cfg.EntityID,
AudienceURI: sp.cfg.EntityID,
AssertionConsumerServiceURL: sp.cfg.AssertionConsumerServiceURL,
AssertionConsumerServiceURL: assertionConsumerServiceURL,
SkipSignatureValidation: skipSignatureValidation,
Clock: dsig.NewFakeClock(clock),
}, nil
}

Expand Down
Loading

0 comments on commit 984a901

Please sign in to comment.