From 58eb44e4f3c55dfb1acccf84e1340b41eaed879e Mon Sep 17 00:00:00 2001 From: Heitor Danilo Date: Tue, 7 Jan 2025 21:55:58 -0300 Subject: [PATCH] feat(api,enterprise): add support for SAML authentication SAML (Security Assertion Markup Language) is an XML-based SSO protocol that enables secure authentication between organizations. ShellHub can now acts as a SAML Service Provider (SP), allowing users to authenticate through their organization's Identity Provider (IdP). - Implement SAML SP functionality for external authentication - Support single IdP configuration at instance level - Extend `/info` endpoint to expose SAML authentication status - Add a `external_id` attributes to user's collection. Which defines the user's ID in the IdP. Only one IdP can be configured globally per ShellHub instance; this feature is enterprise-only. --- api/pkg/responses/system.go | 1 + api/services/system.go | 1 + api/store/mongo/migrations/main.go | 2 + api/store/mongo/migrations/migration_88.go | 73 ++++++++++ .../mongo/migrations/migration_88_test.go | 136 ++++++++++++++++++ api/store/mongo/migrations/migration_89.go | 61 ++++++++ .../mongo/migrations/migration_89_test.go | 117 +++++++++++++++ api/store/mongo/user_test.go | 3 +- gateway/nginx/conf.d/shellhub.conf | 10 ++ pkg/models/system.go | 44 +++++- pkg/models/user.go | 12 ++ 11 files changed, 458 insertions(+), 2 deletions(-) create mode 100644 api/store/mongo/migrations/migration_88.go create mode 100644 api/store/mongo/migrations/migration_88_test.go create mode 100644 api/store/mongo/migrations/migration_89.go create mode 100644 api/store/mongo/migrations/migration_89_test.go diff --git a/api/pkg/responses/system.go b/api/pkg/responses/system.go index 6cc01013a1b..e0eedf5b99d 100644 --- a/api/pkg/responses/system.go +++ b/api/pkg/responses/system.go @@ -9,6 +9,7 @@ type SystemInfo struct { type SystemAuthenticationInfo struct { Local bool `json:"local"` + SAML bool `json:"saml"` } type SystemEndpointsInfo struct { diff --git a/api/services/system.go b/api/services/system.go index 2b3d405245d..fad8496de8b 100644 --- a/api/services/system.go +++ b/api/services/system.go @@ -36,6 +36,7 @@ func (s *service) GetSystemInfo(ctx context.Context, req *requests.GetSystemInfo }, Authentication: &responses.SystemAuthenticationInfo{ Local: system.Authentication.Local.Enabled, + SAML: system.Authentication.SAML.Enabled, }, } diff --git a/api/store/mongo/migrations/main.go b/api/store/mongo/migrations/main.go index 65a4650a487..45294ed1180 100644 --- a/api/store/mongo/migrations/main.go +++ b/api/store/mongo/migrations/main.go @@ -97,6 +97,8 @@ func GenerateMigrations() []migrate.Migration { migration85, migration86, migration87, + migration88, + migration89, } } diff --git a/api/store/mongo/migrations/migration_88.go b/api/store/mongo/migrations/migration_88.go new file mode 100644 index 00000000000..055cf181098 --- /dev/null +++ b/api/store/mongo/migrations/migration_88.go @@ -0,0 +1,73 @@ +package migrations + +import ( + "context" + + "github.com/sirupsen/logrus" + migrate "github.com/xakep666/mongo-migrate" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo" +) + +var migration88 = migrate.Migration{ + Version: 88, + Description: "Adding an 'authentication.saml' attributes to system collection", + Up: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error { + logrus.WithFields(logrus.Fields{ + "component": "migration", + "version": 88, + "action": "Up", + }).Info("Applying migration") + + filter := bson.M{ + "authentication.saml": bson.M{"$exists": false}, + } + + update := bson.M{ + "$set": bson.M{ + "authentication.saml": bson.M{ + "enabled": false, + "idp": bson.M{ + "entity_id": "", + "signon_url": "", + "certificates": []string{}, + }, + "sp": bson.M{ + "sign_auth_requests": false, + "certificate": "", + "private_key": "", + }, + }, + }, + } + + _, err := db. + Collection("system"). + UpdateMany(ctx, filter, update) + + return err + }), + Down: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error { + logrus.WithFields(logrus.Fields{ + "component": "migration", + "version": 88, + "action": "Down", + }).Info("Reverting migration") + + filter := bson.M{ + "authentication.saml": bson.M{"$exists": true}, + } + + update := bson.M{ + "$unset": bson.M{ + "authentication.saml": "", + }, + } + + _, err := db. + Collection("system"). + UpdateMany(ctx, filter, update) + + return err + }), +} diff --git a/api/store/mongo/migrations/migration_88_test.go b/api/store/mongo/migrations/migration_88_test.go new file mode 100644 index 00000000000..76ac5bd5e27 --- /dev/null +++ b/api/store/mongo/migrations/migration_88_test.go @@ -0,0 +1,136 @@ +package migrations + +import ( + "context" + "testing" + + "github.com/shellhub-io/shellhub/pkg/envs" + envmock "github.com/shellhub-io/shellhub/pkg/envs/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + migrate "github.com/xakep666/mongo-migrate" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" +) + +func TestMigration88Up(t *testing.T) { + ctx := context.Background() + + mock := &envmock.Backend{} + envs.DefaultBackend = mock + + tests := []struct { + description string + setup func() error + }{ + { + description: "Apply up on migration 88", + setup: func() error { + _, err := c. + Database("test"). + Collection("system"). + InsertOne(ctx, map[string]interface{}{ + "authentication": map[string]interface{}{ + "local": map[string]interface{}{ + "enabled": true, + }, + }, + }) + + return err + }, + }, + } + + for _, tc := range tests { + t.Run(tc.description, func(tt *testing.T) { + tt.Cleanup(func() { + assert.NoError(tt, srv.Reset()) + }) + + assert.NoError(tt, tc.setup()) + + migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[87]) + require.NoError(tt, migrates.Up(context.Background(), migrate.AllAvailable)) + + query := c. + Database("test"). + Collection("system"). + FindOne(context.TODO(), bson.M{}) + + system := make(map[string]interface{}) + require.NoError(tt, query.Decode(&system)) + + saml, ok := system["authentication"].(map[string]interface{})["saml"].(map[string]interface{}) + require.Equal(tt, true, ok) + + enabled, ok := saml["enabled"] + require.Equal(tt, true, ok) + require.Equal(tt, false, enabled) + + idp, ok := saml["idp"].(map[string]interface{}) + require.Equal(tt, true, ok) + require.Equal(tt, map[string]interface{}{"entity_id": "", "signon_url": "", "certificates": primitive.A{}}, idp) + + sp, ok := saml["sp"].(map[string]interface{}) + require.Equal(tt, true, ok) + require.Equal(tt, map[string]interface{}{"sign_auth_requests": false, "certificate": "", "private_key": ""}, sp) + }) + } +} + +func TestMigration88Down(t *testing.T) { + ctx := context.Background() + + mock := &envmock.Backend{} + envs.DefaultBackend = mock + + mock.On("Get", "SHELLHUB_CLOUD").Return("false") + mock.On("Get", "SHELLHUB_ENTERPRISE").Return("false") + + tests := []struct { + description string + setup func() error + }{ + { + description: "Apply up on migration 88", + setup: func() error { + _, err := c. + Database("test"). + Collection("system"). + InsertOne(ctx, map[string]interface{}{ + "authentication": map[string]interface{}{ + "local": true, + }, + }) + + return err + }, + }, + } + + for _, tc := range tests { + t.Run(tc.description, func(tt *testing.T) { + tt.Cleanup(func() { + assert.NoError(tt, srv.Reset()) + }) + + assert.NoError(tt, tc.setup()) + + migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[87]) + require.NoError(tt, migrates.Up(context.Background(), migrate.AllAvailable)) + require.NoError(tt, migrates.Down(context.Background(), migrate.AllAvailable)) + + query := c. + Database("test"). + Collection("system"). + FindOne(context.TODO(), bson.M{}) + + system := make(map[string]interface{}) + require.NoError(tt, query.Decode(&system)) + + _, ok := system["authentication"].(map[string]interface{})["saml"] + require.Equal(tt, false, ok) + }) + } +} diff --git a/api/store/mongo/migrations/migration_89.go b/api/store/mongo/migrations/migration_89.go new file mode 100644 index 00000000000..93ec772a153 --- /dev/null +++ b/api/store/mongo/migrations/migration_89.go @@ -0,0 +1,61 @@ +package migrations + +import ( + "context" + + "github.com/sirupsen/logrus" + migrate "github.com/xakep666/mongo-migrate" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo" +) + +var migration89 = migrate.Migration{ + Version: 89, + Description: "Adding an external ID attribute to users collection", + Up: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error { + logrus.WithFields(logrus.Fields{ + "component": "migration", + "version": 89, + "action": "Up", + }).Info("Applying migration") + + filter := bson.M{ + "external_id": bson.M{"$exists": false}, + } + + update := bson.M{ + "$set": bson.M{ + "external_id": "", + }, + } + + _, err := db. + Collection("users"). + UpdateMany(ctx, filter, update) + + return err + }), + Down: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error { + logrus.WithFields(logrus.Fields{ + "component": "migration", + "version": 89, + "action": "Down", + }).Info("Reverting migration") + + filter := bson.M{ + "external_id": bson.M{"$exists": true}, + } + + update := bson.M{ + "$unset": bson.M{ + "external_id": "", + }, + } + + _, err := db. + Collection("users"). + UpdateMany(ctx, filter, update) + + return err + }), +} diff --git a/api/store/mongo/migrations/migration_89_test.go b/api/store/mongo/migrations/migration_89_test.go new file mode 100644 index 00000000000..5ccc5ca0fd3 --- /dev/null +++ b/api/store/mongo/migrations/migration_89_test.go @@ -0,0 +1,117 @@ +package migrations + +import ( + "context" + "testing" + + "github.com/shellhub-io/shellhub/pkg/envs" + envmock "github.com/shellhub-io/shellhub/pkg/envs/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + migrate "github.com/xakep666/mongo-migrate" + "go.mongodb.org/mongo-driver/bson" +) + +func TestMigration89Up(t *testing.T) { + ctx := context.Background() + + mock := &envmock.Backend{} + envs.DefaultBackend = mock + + cases := []struct { + description string + setup func() error + test func() error + }{ + { + description: "Success to apply up on migration 89", + setup: func() error { + _, err := c. + Database("test"). + Collection("users"). + InsertOne(ctx, map[string]interface{}{ + "name": "john doe", + }) + + return err + }, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(tt *testing.T) { + tt.Cleanup(func() { + assert.NoError(tt, srv.Reset()) + }) + + assert.NoError(tt, tc.setup()) + + migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[88]) + require.NoError(tt, migrates.Up(context.Background(), migrate.AllAvailable)) + + query := c. + Database("test"). + Collection("users"). + FindOne(context.TODO(), bson.M{"name": "john doe"}) + + user := make(map[string]interface{}) + require.NoError(tt, query.Decode(&user)) + + _, ok := user["external_id"] + require.Equal(tt, true, ok) + }) + } +} + +func TestMigration89Down(t *testing.T) { + ctx := context.Background() + + mock := &envmock.Backend{} + envs.DefaultBackend = mock + + cases := []struct { + description string + setup func() error + test func() error + }{ + { + description: "Success to apply up on migration 89", + setup: func() error { + _, err := c. + Database("test"). + Collection("users"). + InsertOne(ctx, map[string]interface{}{ + "name": "john doe", + "external_id": "unique_string", + }) + + return err + }, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + t.Cleanup(func() { + assert.NoError(t, srv.Reset()) + }) + + assert.NoError(t, tc.setup()) + + migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[88]) + require.NoError(t, migrates.Up(context.Background(), migrate.AllAvailable)) + require.NoError(t, migrates.Down(context.Background(), migrate.AllAvailable)) + + query := c. + Database("test"). + Collection("users"). + FindOne(context.TODO(), bson.M{"name": "john doe"}) + + user := make(map[string]interface{}) + require.NoError(t, query.Decode(&user)) + + _, ok := user["external_id"] + require.Equal(t, false, ok) + }) + } +} diff --git a/api/store/mongo/user_test.go b/api/store/mongo/user_test.go index e6fa43a9ea6..55d6f5995e5 100644 --- a/api/store/mongo/user_test.go +++ b/api/store/mongo/user_test.go @@ -292,6 +292,7 @@ func TestStore_UserCreateInvited(t *testing.T) { "created_at": primitive.NewDateTimeFromTime(now), "last_login": primitive.NewDateTimeFromTime(time.Time{}), "origin": nil, + "external_id": nil, "status": models.UserStatusInvited.String(), "max_namespaces": nil, "name": nil, @@ -300,7 +301,7 @@ func TestStore_UserCreateInvited(t *testing.T) { "recovery_email": nil, "email_marketing": nil, "password": nil, - "preferences": map[string]interface{}{"preferred_namespace": nil}, + "preferences": map[string]interface{}{"preferred_namespace": nil, "auth_methods": nil}, "mfa": map[string]interface{}{"enabled": nil, "secret": nil, "recovery_codes": nil}, }, tmpUser, diff --git a/gateway/nginx/conf.d/shellhub.conf b/gateway/nginx/conf.d/shellhub.conf index 4fc116eaa4d..9ec3b6de8fb 100644 --- a/gateway/nginx/conf.d/shellhub.conf +++ b/gateway/nginx/conf.d/shellhub.conf @@ -452,6 +452,16 @@ server { {{ end -}} {{ if $cfg.EnableEnterprise -}} + location /api/user/saml/auth { + {{ set_upstream "cloud-api" 8080 }} + + auth_request off; + proxy_set_header X-Real-IP $x_real_ip; + proxy_set_header X-Forwarded-Proto $x_forwarded_proto; + proxy_set_header X-Forwarded-Host $host; + proxy_pass http://upstream_router; + } + location /api/user/mfa { {{ set_upstream "cloud-api" 8080 }} diff --git a/pkg/models/system.go b/pkg/models/system.go index 3cdfc3b6a6a..4a5cafe3ee1 100644 --- a/pkg/models/system.go +++ b/pkg/models/system.go @@ -9,7 +9,8 @@ type System struct { } type SystemAuthentication struct { - Local *SystemAuthenticationLocal `json:"manual" bson:"manual"` + Local *SystemAuthenticationLocal `json:"local" bson:"local"` + SAML *SystemAuthenticationSAML `json:"saml" bson:"saml"` } type SystemAuthenticationLocal struct { @@ -17,3 +18,44 @@ type SystemAuthenticationLocal struct { // not. Enabled bool `json:"enabled" bool:"enabled"` } + +type SystemAuthenticationSAML struct { + // Enabled indicates whether SAML authentication is enabled. + Enabled bool `json:"enabled" bson:"enabled"` + Idp *SystemIdpSAML `json:"idp" bson:"idp"` + Sp *SystemSpSAML `json:"sp" bson:"sp"` +} + +type SystemIdpSAML struct { + EntityID string `json:"entity_id" bson:"entity_id"` + SignonURL string `json:"signon_url" bson:"signon_url"` + // Certificates is a list of X.509 certificates provided by the IdP. These certificates are used + // by the SP to validate the digital signatures of SAML assertions issued by the IdP. + Certificates []string `json:"certificates" bson:"certificates"` + // Mappings defines how IdP SAML attributes map to ShellHub attributes. + // + // Example: + // { + // "external_id": "user_id", + // "email": "emailaddress", + // "name": "displayName" + // } + Mappings map[string]string `json:"mappings" bson:"mappings"` +} + +type SystemSpSAML struct { + // SignRequests indicates whether ShellHub should sign authentication requests. + // If enabled, an X509 certificate is used to sign the request, and the IdP must authenticate + // the request using the corresponding public certificate. Enabling this option disables + // the "IdP-initiated" authentication pipeline. + SignAuthRequests bool `json:"sign_auth_requests" bson:"sign_auth_requests"` + // Certificate is an X509 certificate that the IdP uses to verify the authenticity of the + // authentication request signed by ShellHub. This certificate corresponds to the private key + // in the [SystemSpSAML.PrivateKey] and it is only populated when [SystemSpSAML.SignAuthRequests] + // is true. + Certificate string `json:"certificate" bson:"certificate"` + // PrivateKey is an encrypted private key used by ShellHub to sign authentication requests. + // The IdP verifies the signature using the [SystemSpSAML.Certificate]. It is only populated + // when [SystemSpSAML.SignAuthRequests] is true. + PrivateKey string `json:"-" bson:"private_key"` +} diff --git a/pkg/models/user.go b/pkg/models/user.go index e92d7e583a2..110239f6237 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -33,6 +33,9 @@ const ( // UserOriginLocal indicates that the user was created through the standard signup process, without // using third-party integrations like SSO IdPs. UserOriginLocal UserOrigin = "local" + + // UserOriginSAML indicates that the user was created using a SAML method. + UserOriginSAML UserOrigin = "SAML" ) func (o UserOrigin) String() string { @@ -44,6 +47,9 @@ type UserAuthMethod string const ( // UserAuthMethodLocal indicates that the user can authenticate using an email and password. UserAuthMethodLocal UserAuthMethod = "local" + + // UserAuthMethodManual indicates that the user can authenticate using a third-party SAML application. + UserAuthMethodSAML UserAuthMethod = "saml" ) func (a UserAuthMethod) String() string { @@ -54,6 +60,11 @@ type User struct { ID string `json:"id,omitempty" bson:"_id,omitempty"` // Origin specifies the the user's signup method. Origin UserOrigin `json:"-" bson:"origin"` + + // ExternalID represents the user's identifier in an external system. It is always empty when [User.Origin] + // is [UserOriginLocal]. + ExternalID string `json:"-" bson:"external_id"` + Status UserStatus `json:"status" bson:"status"` // MaxNamespaces represents the count of namespaces that the user can owns. MaxNamespaces int `json:"max_namespaces" bson:"max_namespaces"` @@ -175,6 +186,7 @@ type UserChanges struct { RecoveryEmail string `bson:"recovery_email,omitempty"` Password string `bson:"password,omitempty"` Status UserStatus `bson:"status,omitempty"` + ExternalID *string `bson:"external_id,omitempty"` PreferredNamespace *string `bson:"preferences.preferred_namespace,omitempty"` MaxNamespaces *int `bson:"max_namespaces,omitempty"` EmailMarketing *bool `bson:"email_marketing,omitempty"`