diff --git a/internal/api/sso.go b/internal/api/sso.go index e50d1a369..10034075c 100644 --- a/internal/api/sso.go +++ b/internal/api/sso.go @@ -91,8 +91,6 @@ func (a *API) SingleSignOn(w http.ResponseWriter, r *http.Request) error { return internalServerError("Error parsing SAML Metadata for SAML provider").WithInternalError(err) } - // TODO: fetch new metadata if validUntil < time.Now() - serviceProvider := a.getSAMLServiceProvider(entityDescriptor, false /* <- idpInitiated */) authnRequest, err := serviceProvider.MakeAuthenticationRequest( @@ -104,6 +102,12 @@ func (a *API) SingleSignOn(w http.ResponseWriter, r *http.Request) error { return internalServerError("Error creating SAML Authentication Request").WithInternalError(err) } + // Some IdPs do not support the use of the `persistent` NameID format, + // and require a different format to be sent to work. + if ssoProvider.SAMLProvider.NameIDFormat != nil { + authnRequest.NameIDPolicy.Format = ssoProvider.SAMLProvider.NameIDFormat + } + relayState := models.SAMLRelayState{ SSOProviderID: ssoProvider.ID, RequestID: authnRequest.ID, diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 6e52fc8ff..fae328c4f 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/url" + "strings" "unicode/utf8" "github.com/crewjam/saml" @@ -74,6 +75,7 @@ type CreateSSOProviderParams struct { MetadataXML string `json:"metadata_xml"` Domains []string `json:"domains"` AttributeMapping models.SAMLAttributeMapping `json:"attribute_mapping"` + NameIDFormat string `json:"name_id_format"` } func (p *CreateSSOProviderParams) validate(forUpdate bool) error { @@ -94,8 +96,22 @@ func (p *CreateSSOProviderParams) validate(forUpdate bool) error { } } - // TODO validate p.AttributeMapping - // TODO validate domains + switch p.NameIDFormat { + case "", + string(saml.PersistentNameIDFormat), + string(saml.EmailAddressNameIDFormat), + string(saml.TransientNameIDFormat), + string(saml.UnspecifiedNameIDFormat): + // it's valid + + default: + return badRequestError(ErrorCodeValidationFailed, "name_id_format must be unspecified or one of %v", strings.Join([]string{ + string(saml.PersistentNameIDFormat), + string(saml.EmailAddressNameIDFormat), + string(saml.TransientNameIDFormat), + string(saml.UnspecifiedNameIDFormat), + }, ", ")) + } return nil } @@ -217,6 +233,10 @@ func (a *API) adminSSOProvidersCreate(w http.ResponseWriter, r *http.Request) er provider.SAMLProvider.MetadataURL = ¶ms.MetadataURL } + if params.NameIDFormat != "" { + provider.SAMLProvider.NameIDFormat = ¶ms.NameIDFormat + } + provider.SAMLProvider.AttributeMapping = params.AttributeMapping for _, domain := range params.Domains { @@ -335,6 +355,21 @@ func (a *API) adminSSOProvidersUpdate(w http.ResponseWriter, r *http.Request) er provider.SAMLProvider.AttributeMapping = params.AttributeMapping } + nameIDFormat := "" + if provider.SAMLProvider.NameIDFormat != nil { + nameIDFormat = *provider.SAMLProvider.NameIDFormat + } + + if params.NameIDFormat != nameIDFormat { + modified = true + + if params.NameIDFormat == "" { + provider.SAMLProvider.NameIDFormat = nil + } else { + provider.SAMLProvider.NameIDFormat = ¶ms.NameIDFormat + } + } + if modified { if err := db.Transaction(func(tx *storage.Connection) error { if terr := tx.Eager().Update(provider); terr != nil { diff --git a/internal/models/sso.go b/internal/models/sso.go index d56c8962d..bbdd138ce 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -115,6 +115,8 @@ type SAMLProvider struct { AttributeMapping SAMLAttributeMapping `db:"attribute_mapping" json:"attribute_mapping,omitempty"` + NameIDFormat *string `db:"name_id_format" json:"name_id_format,omitempty"` + CreatedAt time.Time `db:"created_at" json:"-"` UpdatedAt time.Time `db:"updated_at" json:"-"` } diff --git a/migrations/20240314092811_add_saml_name_id_format.up.sql b/migrations/20240314092811_add_saml_name_id_format.up.sql new file mode 100644 index 000000000..0196250d2 --- /dev/null +++ b/migrations/20240314092811_add_saml_name_id_format.up.sql @@ -0,0 +1,3 @@ +do $$ begin +alter table {{ index .Options "Namespace" }}.saml_providers add column if not exists name_id_format text null; +end $$