Repository: supabase/gotrue
Pull request #596 (2022-08-09 16:01:19)

feat(saml): Add attribute mapping


Patch:
diff --git a/api/saml.go b/api/saml.go
index c79c5c43..d59a517d 100644
--- a/api/saml.go
+++ b/api/saml.go
@@ -51,14 +51,12 @@ func (a *API) getSAMLServiceProvider(identityProvider *saml.EntityDescriptor, id
 		panic(err)
 	}
 
-	// TODO: figure out idpInitiated properties for higher security
-
 	provider := samlsp.DefaultServiceProvider(samlsp.Options{
-		URL:         *externalURL,
-		Key:         a.config.SAML.RSAPrivateKey,
-		Certificate: a.config.SAML.Certificate,
-		//SignRequest:       !idpInitiated,
-		AllowIDPInitiated: true,
+		URL:               *externalURL,
+		Key:               a.config.SAML.RSAPrivateKey,
+		Certificate:       a.config.SAML.Certificate,
+		SignRequest:       true,
+		AllowIDPInitiated: idpInitiated,
 		IDPMetadata:       identityProvider,
 	})
 
@@ -67,10 +65,19 @@ func (a *API) getSAMLServiceProvider(identityProvider *saml.EntityDescriptor, id
 
 // SAMLMetadata serves GoTrue's SAML Service Provider metadata file.
 func (a *API) SAMLMetadata(w http.ResponseWriter, r *http.Request) error {
-	serviceProvider := a.getSAMLServiceProvider(nil /* <- identityProvider */, false /* <- idpInitiated */)
+	serviceProvider := a.getSAMLServiceProvider(nil, true)
 
 	metadata := serviceProvider.Metadata()
 
+	for i := range metadata.SPSSODescriptors {
+		// we set this to false since the IdP initiated flow can only
+		// sign the Assertion, and not the full Request
+		// unfortunately this is hardcoded in the crewjam library if
+		// signatures (instead of encryption) are supported
+		// https://github.com/crewjam/saml/blob/v0.4.8/service_provider.go#L217
+		metadata.SPSSODescriptors[i].AuthnRequestsSigned = nil
+	}
+
 	metadataXML, err := xml.Marshal(metadata)
 	if err != nil {
 		return err
@@ -210,29 +217,61 @@ func (a *API) samlCallback(ctx context.Context, r *http.Request) (*provider.User
 		return nil, nil, badRequestError("SAML Assertion did not contain a persistent Subject Identifier attribute or Subject NameID uniquely identifying this user")
 	}
 
+	claims := assertion.Process(ssoProvider.SAMLProvider.AttributeMapping)
+
+	email, ok := claims["email"].(string)
+	if !ok || email == "" {
+		email = assertion.Email()
+	}
+
+	if email == "" {
+		return nil, nil, badRequestError("SAML Assertion did not contain a mapping for the email claim")
+	} else {
+		claims["email"] = email
+	}
+
+	jsonClaims, err := json.Marshal(claims)
+	if err != nil {
+		return nil, nil, internalServerError("Mapped claims from provider could not be serialized into JSON").WithInternalError(err)
+	}
+
+	providerClaims := &provider.Claims{}
+	if err := json.Unmarshal(jsonClaims, providerClaims); err != nil {
+		return nil, nil, internalServerError("Mapped claims from provider could not be deserialized from JSON").WithInternalError(err)
+	}
+
+	providerClaims.Subject = userID
+	providerClaims.Issuer = ssoProvider.SAMLProvider.EntityID
+	providerClaims.Email = email
+	providerClaims.EmailVerified = true
+
+	providerClaimsMap, err := providerClaims.ToMap()
+	if err != nil {
+		return nil, nil, internalServerError("Parsed provider claims could not be turned into a map").WithInternalError(err)
+	}
+
+	// remove all of the parsed claims, so that the rest can go into CustomClaims
+	for key := range providerClaimsMap {
+		delete(claims, key)
+	}
+
+	providerClaims.CustomClaims = claims
+
 	var userProvidedData provider.UserProvidedData
 	var cond models.GrantAuthenticatedConditions
 
-	// TODO: get emails from attribute mapping
-	emails := assertion.Emails()
-	for _, email := range emails {
-		userProvidedData.Emails = append(userProvidedData.Emails, provider.Email{
-			Email:    email,
-			Verified: true,
-			Primary:  true,
-		})
-	}
+	userProvidedData.Emails = append(userProvidedData.Emails, provider.Email{
+		Email:    email,
+		Verified: true,
+		Primary:  true,
+	})
 
 	userProvidedData.Provider.Type = "saml"
 	userProvidedData.Provider.ID = ssoProvider.ID.String()
 	userProvidedData.Provider.SAMLEntityID = ssoProvider.SAMLProvider.EntityID
 	userProvidedData.Provider.SAMLInitiatedBy = initiatedBy
 
-	userProvidedData.Metadata = &provider.Claims{
-		Subject:       userID,
-		Email:         userProvidedData.Emails[0].Email,
-		EmailVerified: true,
-	}
+	userProvidedData.Metadata = providerClaims
 
 	cond.SSOProviderID = ssoProvider.ID
 	cond.InitiatedByProvider = initiatedBy == "idp"
@@ -259,12 +298,12 @@ func (a *API) adminListSAMLIdPs(w http.ResponseWriter, r *http.Request) error {
 }
 
 type CreateSAMLIdPParams struct {
-	Name             string               `json:"name"`
-	Description      string               `json:"description"`
-	MetadataURL      string               `json:"metadata_url"`
-	MetadataXML      string               `json:"metadata_xml"`
-	Domains          []string             `json:"domains"`
-	AttributeMapping SAMLAttributeMapping `json:"attribute_mapping"`
+	Name             string                      `json:"name"`
+	Description      string                      `json:"description"`
+	MetadataURL      string                      `json:"metadata_url"`
+	MetadataXML      string                      `json:"metadata_xml"`
+	Domains          []string                    `json:"domains"`
+	AttributeMapping models.SAMLAttributeMapping `json:"attribute_mapping"`
 }
 
 func (p *CreateSAMLIdPParams) validate(forUpdate bool) error {
@@ -281,6 +320,8 @@ func (p *CreateSAMLIdPParams) validate(forUpdate bool) error {
 		if metadataURL.Scheme != "https" {
 			return badRequestError("metadata_url is not a HTTPS URL")
 		}
+	} else if !p.AttributeMapping.HasKey("email") {
+		return badRequestError("email key must be definied in a SAML Attribute Mapping")
 	}
 
 	return nil
@@ -535,31 +576,13 @@ func (a *API) adminDeleteSAMLIdP(w http.ResponseWriter, r *http.Request) error {
 		return err
 	}
 
-	// TODO: destroy all sessions coming from this provider
-
 	return sendJSON(w, http.StatusOK, provider)
 }
 
-type SAMLAttributeMapping struct {
-	Keys SAMLAttributeMappingSet `json:"keys,omitempty"`
-}
-
-type SAMLAttributeMappingValue struct {
-	Name  string   `json:"name,omitempty"`
-	Names []string `json:"names,omitempty"`
-
-	IsEmail    bool `json:"is_email,omitempty"`
-	IsVerified bool `json:"is_verified,omitempty"`
-
-	Default interface{} `json:"default,omitempty"`
-}
-
 type SAMLAssertion struct {
 	*saml.Assertion
 }
 
-type SAMLAttributeMappingSet map[string]SAMLAttributeMappingValue
-
 const (
 	SAMLSubjectIDAttributeName = "urn:oasis:names:tc:SAML:attribute:subject-id"
 )
@@ -637,33 +660,75 @@ func (a *SAMLAssertion) SubjectID() (string, bool) {
 	return a.Subject.NameID.Value, isPersistent
 }
 
-const (
-	SAMLAttributeNameEmail = "urn:oid:0.9.2342.19200300.100.1.3"
-)
-
-// Emails returns all SAMLAttributeNameEmail attribute values.
-func (a *SAMLAssertion) Emails() []string {
-	values := a.Attribute(SAMLAttributeNameEmail)
+// Email returns the best guess for an email address.
+func (a *SAMLAssertion) Email() string {
+	attributeNames := []string{
+		"urn:oid:0.9.2342.19200300.100.1.3",
+		"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
+		"http://schemas.xmlsoap.org/claims/EmailAddress",
+		"mail",
+		"email",
+	}
 
-	var emails []string
+	for _, name := range attributeNames {
+		for _, attr := range a.Attribute(name) {
+			if attr.Value != "" {
+				return attr.Value
+			}
+		}
+	}
 
-	for _, value := range values {
-		emails = append(emails, value.Value)
+	if a.Subject.NameID.Format == string(saml.EmailAddressNameIDFormat) {
+		return a.Subject.NameID.Value
 	}
 
-	return emails
+	return ""
 }
 
-func (a *SAMLAssertion) Process(set SAMLAttributeMappingSet) map[string]interface{} {
-	return nil
+// Process processes this assertion according to the SAMLAttributeMapping. Never returns nil.
+func (a *SAMLAssertion) Process(mapping models.SAMLAttributeMapping) map[string]interface{} {
+	ret := make(map[string]interface{})
+
+	for key, mapper := range mapping.Keys {
+		names := []string{mapper.Name}
+		names = append(names, mapper.Names...)
+
+		setKey := false
+
+		for _, name := range names {
+			for _, attr := range a.Attribute(name) {
+				if attr.Value != "" {
+					ret[key] = attr.Value
+					setKey = true
+					break
+				}
+			}
+
+			if setKey {
+				break
+			}
+		}
+
+		if !setKey && mapper.Default != nil {
+			ret[key] = mapper.Default
+		}
+	}
+
+	return ret
 }
 
 func (a *SAMLAssertion) NotBefore() time.Time {
-	// TODO: extract this from the assertion conditions
-	return time.Now().UTC()
+	if a.Conditions != nil && !a.Conditions.NotBefore.IsZero() {
+		return a.Conditions.NotBefore.UTC()
+	}
+
+	return time.Time{}
 }
 
 func (a *SAMLAssertion) NotAfter() time.Time {
-	// TODO: extract this from the assertion conditions
-	return time.Now().UTC().AddDate(0, 0, 1)
+	if a.Conditions != nil && !a.Conditions.NotOnOrAfter.IsZero() {
+		return a.Conditions.NotOnOrAfter.UTC()
+	}
+
+	return time.Time{}
 }
diff --git a/migrations/20220804104030_add_saml.up.sql b/migrations/20220808120830_add_saml.up.sql
similarity index 99%
rename from migrations/20220804104030_add_saml.up.sql
rename to migrations/20220808120830_add_saml.up.sql
index 1231815f..6614b92a 100644
--- a/migrations/20220804104030_add_saml.up.sql
+++ b/migrations/20220808120830_add_saml.up.sql
@@ -30,6 +30,7 @@ create table if not exists auth.saml_providers (
 	entity_id text not null unique,
 	metadata_xml text not null,
 	metadata_url text null,
+	attribute_mapping jsonb null,
 	created_at timestamptz null,
 	updated_at timestamptz null,
 	primary key (id),
diff --git a/models/refresh_token.go b/models/refresh_token.go
index 1d9dccec..93d8c9e0 100644
--- a/models/refresh_token.go
+++ b/models/refresh_token.go
@@ -22,8 +22,8 @@ type RefreshToken struct {
 	UserID uuid.UUID `db:"user_id"`
 	User   *User     `belongs_to:"users"`
 
-	SSOSession   *SSOSession `belongs_to:"sso_sessions"`
-	SSOSessionID uuid.UUID   `db:"sso_session_id"`
+	SSOSession   *SSOSession        `belongs_to:"sso_sessions"`
+	SSOSessionID storage.NullString `db:"sso_session_id"`
 
 	Revoked   bool      `db:"revoked"`
 	CreatedAt time.Time `db:"created_at"`
@@ -127,7 +127,7 @@ func createRefreshToken(tx *storage.Connection, user *User, oldToken *RefreshTok
 		}
 
 		token.SSOSession = &ssoSession
-		token.SSOSessionID = ssoSession.ID
+		token.SSOSessionID = storage.NullString(ssoSession.ID.String())
 	}
 
 	if err := tx.Create(token); err != nil {
@@ -138,7 +138,7 @@ func createRefreshToken(tx *storage.Connection, user *User, oldToken *RefreshTok
 		return nil, errors.Wrap(err, "error loading refresh token after create")
 	}
 
-	if token.SSOSessionID != (uuid.UUID{}) {
+	if token.SSOSessionID != storage.NullString("") {
 		if err := tx.Eager().Q().Where("id = ?", token.SSOSessionID).First(token.SSOSession); err != nil {
 			return nil, errors.Wrap(err, "error loading SSO session for refresh token after create")
 		}
diff --git a/models/sso.go b/models/sso.go
index ef4fdb67..b4fa9d90 100644
--- a/models/sso.go
+++ b/models/sso.go
@@ -2,6 +2,8 @@ package models
 
 import (
 	"database/sql"
+	"database/sql/driver"
+	"encoding/json"
 	"strings"
 	"time"
 
@@ -30,6 +32,63 @@ func (p SSOProvider) Type() string {
 	return "saml"
 }
 
+type SAMLAttribute struct {
+	Name    string      `json:"name,omitempty"`
+	Names   []string    `json:"names,omitempty"`
+	Default interface{} `json:"default,omitempty"`
+}
+
+func (a SAMLAttribute) HasName() bool {
+	if a.Name != "" {
+		return true
+	}
+
+	for _, name := range a.Names {
+		if name != "" {
+			return true
+		}
+	}
+
+	return false
+}
+
+type SAMLAttributeMapping struct {
+	Keys map[string]SAMLAttribute `json:"keys,omitempty"`
+}
+
+func (m SAMLAttributeMapping) HasKey(key string) bool {
+	attr, ok := m.Keys[key]
+	if !ok {
+		return false
+	}
+
+	if !attr.HasName() {
+		return false
+	}
+
+	return true
+}
+
+func (m *SAMLAttributeMapping) Scan(src interface{}) error {
+	b, ok := src.([]byte)
+	if !ok {
+		return errors.New("scan source was not []byte")
+	}
+	err := json.Unmarshal(b, m)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (m SAMLAttributeMapping) Value() (driver.Value, error) {
+	b, err := json.Marshal(m)
+	if err != nil {
+		return nil, err
+	}
+	return string(b), nil
+}
+
 type SAMLProvider struct {
 	ID uuid.UUID `db:"id" json:"-"`
 
@@ -40,6 +99,8 @@ type SAMLProvider struct {
 	MetadataXML string `db:"metadata_xml" json:"metadata_xml,omitempty"`
 	MetadataURL string `db:"metadata_url" json:"metadata_url,omitempty"`
 
+	AttributeMapping SAMLAttributeMapping `db:"attribute_mapping" json:"attribute_mapping,omitempty"`
+
 	CreatedAt time.Time `db:"created_at" json:"-"`
 	UpdatedAt time.Time `db:"updated_at" json:"-"`
 }


Test patch:
diff --git a/api/token_test.go b/api/token_test.go
index 972702b8..56a3ac3a 100644
--- a/api/token_test.go
+++ b/api/token_test.go
@@ -53,7 +53,7 @@ func (ts *TokenTestSuite) SetupTest() {
 	u.BannedUntil = nil
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user")
 
-	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u)
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u, nil)
 	require.NoError(ts.T(), err, "Error creating refresh token")
 }
 
@@ -156,7 +156,7 @@ func (ts *TokenTestSuite) TestTokenRefreshTokenRotation() {
 	t := time.Now()
 	u.EmailConfirmedAt = &t
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving foo user")
-	first, err := models.GrantAuthenticatedUser(ts.API.db, u)
+	first, err := models.GrantAuthenticatedUser(ts.API.db, u, nil)
 	require.NoError(ts.T(), err)
 	second, err := models.GrantRefreshTokenSwap(ts.API.db, u, first)
 	require.NoError(ts.T(), err)
@@ -248,7 +248,7 @@ func (ts *TokenTestSuite) createBannedUser() *models.User {
 	u.BannedUntil = &t
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test banned user")
 
-	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u)
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u, nil)
 	require.NoError(ts.T(), err, "Error creating refresh token")
 
 	return u
diff --git a/models/refresh_token_test.go b/models/refresh_token_test.go
index e5e8790d..0372aeef 100644
--- a/models/refresh_token_test.go
+++ b/models/refresh_token_test.go
@@ -37,7 +37,7 @@ func TestRefreshToken(t *testing.T) {
 
 func (ts *RefreshTokenTestSuite) TestGrantAuthenticatedUser() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, nil)
 	require.NoError(ts.T(), err)
 
 	require.NotEmpty(ts.T(), r.Token)
@@ -46,7 +46,7 @@ func (ts *RefreshTokenTestSuite) TestGrantAuthenticatedUser() {
 
 func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, nil)
 	require.NoError(ts.T(), err)
 
 	s, err := GrantRefreshTokenSwap(ts.db, u, r)
@@ -64,7 +64,7 @@ func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
 
 func (ts *RefreshTokenTestSuite) TestLogout() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, nil)
 	require.NoError(ts.T(), err)
 
 	require.NoError(ts.T(), Logout(ts.db, uuid.Nil, u.ID))
diff --git a/models/user_test.go b/models/user_test.go
index f87d55fe..3f3e39d3 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -152,7 +152,7 @@ func (ts *UserTestSuite) TestFindUserByRecoveryToken() {
 
 func (ts *UserTestSuite) TestFindUserWithRefreshToken() {
 	u := ts.createUser()
-	r, err := GrantAuthenticatedUser(ts.db, u)
+	r, err := GrantAuthenticatedUser(ts.db, u, nil)
 	require.NoError(ts.T(), err)
 
 	n, nr, err := FindUserWithRefreshToken(ts.db, r.Token)