From 5bd105888df675e63730ffeb1ef029263cd39d2c Mon Sep 17 00:00:00 2001 From: Bruce MacDonald Date: Tue, 5 Oct 2021 22:49:18 -0400 Subject: [PATCH 1/2] Align user and group lifecycles - remove direct group assignment to users in config - only create group when it is active at the source - reimport config on group creation - update tests - clear role, users, and groups on first config application --- internal/registry/_testdata/infra.yaml | 15 --- internal/registry/api.go | 2 +- internal/registry/api_test.go | 15 +-- internal/registry/config.go | 117 +++++++---------- internal/registry/config_test.go | 174 +++++++++++-------------- internal/registry/data.go | 51 +++++--- internal/registry/data_test.go | 30 +---- 7 files changed, 161 insertions(+), 243 deletions(-) diff --git a/internal/registry/_testdata/infra.yaml b/internal/registry/_testdata/infra.yaml index 5bd5df04e6..3d0d66a480 100644 --- a/internal/registry/_testdata/infra.yaml +++ b/internal/registry/_testdata/infra.yaml @@ -61,10 +61,6 @@ groups: - name: cluster-CCC users: - - email: woz@example.com - groups: - - ios-developers - - mac-admins - email: admin@example.com roles: - name: admin @@ -85,18 +81,7 @@ users: - name: cluster-CCC namespaces: - infrahq - - email: user@example.com - groups: - - ios-developers - roles: - - name: reader - kind: cluster-role - destinations: - - name: cluster-AAA - - name: cluster-UNKNOWN - email: unknown@example.com - groups: - - adversaries roles: - name: writer kind: cluster-role diff --git a/internal/registry/api.go b/internal/registry/api.go index 632fe2fbb1..f536fe8d2f 100644 --- a/internal/registry/api.go +++ b/internal/registry/api.go @@ -198,7 +198,7 @@ func (a *Api) ListUsers(w http.ResponseWriter, r *http.Request) { func (a *Api) ListGroups(w http.ResponseWriter, r *http.Request) { var groups []Group - if err := a.db.Preload("Source").Where(&Group{Active: true}).Find(&groups).Error; err != nil { + if err := a.db.Preload("Source").Find(&groups).Error; err != nil { logging.L.Error(err.Error()) sendApiError(w, http.StatusInternalServerError, "could not list groups") diff --git a/internal/registry/api_test.go b/internal/registry/api_test.go index 6535a6346d..006ae9f6df 100644 --- a/internal/registry/api_test.go +++ b/internal/registry/api_test.go @@ -650,8 +650,6 @@ func TestListRolesForDestinationReturnsRolesFromConfig(t *testing.T) { // roles from direct user assignment assert.Equal(t, 1, len(returnedUserRoles["admin"])) assert.True(t, containsUser(returnedUserRoles["admin"], adminUser.Email)) - assert.Equal(t, 1, len(returnedUserRoles["reader"])) - assert.True(t, containsUser(returnedUserRoles["reader"], standardUser.Email)) } func TestListRolesOnlyFindsForSpecificDestination(t *testing.T) { @@ -714,14 +712,6 @@ func TestListRolesForUnknownDestination(t *testing.T) { func TestListGroups(t *testing.T) { a := &Api{db: db} - if err := db.Model(Group{}).Where(&Group{Name: "heroes"}).Update("active", true).Error; err != nil { - t.Fatal(err) - } - - if err := db.Model(Group{}).Where(&Group{Name: "villains"}).Update("active", true).Error; err != nil { - t.Fatal(err) - } - r := httptest.NewRequest(http.MethodGet, "/v1/groups", nil) w := httptest.NewRecorder() @@ -740,8 +730,9 @@ func TestListGroups(t *testing.T) { groupSources[g.Name] = g.Source } - assert.Equal(t, "okta", groupSources["heroes"]) - assert.Equal(t, "okta", groupSources["villains"]) + // these groups were created specifically in the configuration test setup + assert.Equal(t, "okta", groupSources["ios-developers"]) + assert.Equal(t, "okta", groupSources["mac-admins"]) } func TestCreateAPIKey(t *testing.T) { diff --git a/internal/registry/config.go b/internal/registry/config.go index 50dfe60a7e..9dfc53e0db 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -76,7 +76,7 @@ func ImportSources(db *gorm.DB, sources []ConfigSource) error { db.First(&existing, &Source{Type: SourceTypeOkta}) if existing.Id != "" { - logging.L.Warn("overriding existing okta source settings, only one okta source is supported") + logging.L.Warn("overriding existing okta source settings with configuration settings") } var source Source @@ -111,8 +111,8 @@ func ImportSources(db *gorm.DB, sources []ConfigSource) error { return nil } -func ApplyGroupMappings(db *gorm.DB, configGroups []ConfigGroupMapping) (groupIds []string, err error) { - for _, g := range configGroups { +func ImportGroupMapping(db *gorm.DB, groups []ConfigGroupMapping) error { + for _, g := range groups { // get the source from the datastore that this group specifies var source Source // Assumes that only one type of each source can exist @@ -120,46 +120,48 @@ func ApplyGroupMappings(db *gorm.DB, configGroups []ConfigGroupMapping) (groupId if srcReadErr != nil { if errors.Is(srcReadErr, gorm.ErrRecordNotFound) { // skip this source, it will need to be added in the config and re-applied - logging.L.Debug("skipping group with source in config that does not exist: " + g.Source) + logging.L.Sugar().Debugf("skipping group '%s' with source '%s' in config that does not exist", g.Name, g.Source) continue } - err = srcReadErr + return srcReadErr + } + + var group Group - return + grpReadErr := db.Where(&Group{Name: g.Name, SourceId: source.Id}).First(&group).Error + if grpReadErr != nil { + if errors.Is(grpReadErr, gorm.ErrRecordNotFound) { + // skip this group, if they're created these roles will be added later + logging.L.Debug("skipping group in config import that has not yet been provisioned") + continue + } + + return grpReadErr } // import the roles on this group from the datastore var roles []Role - roles, err = importRoles(db, g.Roles) - if err != nil { - return - } - - var group Group - // Group names must be unique for mapping purposes - err = db.FirstOrCreate(&group, &Group{Name: g.Name, SourceId: source.Id}).Error + roles, err := importRoles(db, g.Roles) if err != nil { - return + return err } // add the new group associations to the roles for i, role := range roles { if db.Model(&group).Where(&Role{Id: role.Id}).Association("Roles").Count() == 0 { if err = db.Model(&group).Where(&Role{Id: role.Id}).Association("Roles").Append(&roles[i]); err != nil { - return + return err } } } - - groupIds = append(groupIds, group.Id) } - return groupIds, err + return nil } -func ApplyUserMapping(db *gorm.DB, users []ConfigUserMapping) error { +func ImportUserMapping(db *gorm.DB, users []ConfigUserMapping) error { for _, u := range users { var user User @@ -174,29 +176,7 @@ func ApplyUserMapping(db *gorm.DB, users []ConfigUserMapping) error { return usrReadErr } - // add the user to groups, these declarations can be overridden by external group syncing - for _, gName := range u.Groups { - // Assumes that only one group can exist with a given name, regardless of sources - var group Group - - grpReadErr := db.Where(&Group{Name: gName}).First(&group).Error - if grpReadErr != nil { - if errors.Is(grpReadErr, gorm.ErrRecordNotFound) { - logging.L.Debug("skipping unknown group \"" + gName + "\" on user") - continue - } - - return grpReadErr - } - - if db.Model(&user).Where(&Group{Id: group.Id}).Association("Groups").Count() == 0 { - if err := db.Model(&user).Where(&Group{Id: group.Id}).Association("Groups").Append(&group); err != nil { - return err - } - } - } - - // add roles to user + // add direct user to role mappings roles, err := importRoles(db, u.Roles) if err != nil { return err @@ -215,29 +195,6 @@ func ApplyUserMapping(db *gorm.DB, users []ConfigUserMapping) error { return nil } -// ImportMappings imports the group and user role mappings and removes previously created roles if they no longer exist -func ImportMappings(db *gorm.DB, groups []ConfigGroupMapping, users []ConfigUserMapping) error { - // gorm blocks global delete by default: https://gorm.io/docs/delete.html#Block-Global-Delete - if err := db.Where("1 = 1").Delete(&Role{}).Error; err != nil { - return err - } - - grpIdsToKeep, err := ApplyGroupMappings(db, groups) - if err != nil { - return err - } - - if len(grpIdsToKeep) == 0 { - logging.L.Debug("no valid groups found in configuration") - } - - if err := db.Where("1 = 1").Not(grpIdsToKeep).Delete(&Group{}).Error; err != nil { - return err - } - - return ApplyUserMapping(db, users) -} - // ImportConfig tries to import all valid fields in a config file func ImportConfig(db *gorm.DB, bs []byte) error { var config Config @@ -248,14 +205,36 @@ func ImportConfig(db *gorm.DB, bs []byte) error { initialConfig = config return db.Transaction(func(tx *gorm.DB) error { + // gorm blocks global delete by default: https://gorm.io/docs/delete.html#Block-Global-Delete + if err := tx.Where("1 = 1").Delete(&Role{}).Error; err != nil { + return err + } + + var users []User + if err := tx.Find(&users).Error; err != nil { + return err + } + if err := tx.Where("1 = 1").Delete(&users).Error; err != nil { + return err + } + + var groups []Group + if err := tx.Find(&groups).Error; err != nil { + return err + } + if err := tx.Where("1 = 1").Delete(&groups).Error; err != nil { + return err + } + if err := ImportSources(tx, config.Sources); err != nil { return err } - // Need to import of group/user mappings together because they both rely on roles - if err := ImportMappings(tx, config.Groups, config.Users); err != nil { + + if err := ImportGroupMapping(tx, config.Groups); err != nil { return err } - return nil + + return ImportUserMapping(tx, config.Users) }) } diff --git a/internal/registry/config_test.go b/internal/registry/config_test.go index 85c63991ad..e6d3ec0700 100644 --- a/internal/registry/config_test.go +++ b/internal/registry/config_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" "gorm.io/gorm" ) @@ -15,9 +16,9 @@ var db *gorm.DB var ( fakeOktaSource = Source{Type: "okta", Domain: "test.example.com", ClientSecret: "okta-secrets/client-secret", ApiToken: "okta-secrets/api-token"} - adminUser = User{Email: "admin@example.com"} - standardUser = User{Email: "user@example.com"} - iosDevUser = User{Email: "woz@example.com"} + adminUser = User{Id: "001", Email: "admin@example.com"} + standardUser = User{Id: "002", Email: "user@example.com"} + iosDevUser = User{Id: "003", Email: "woz@example.com"} clusterA = Destination{Name: "cluster-AAA"} clusterB = Destination{Name: "cluster-BBB"} clusterC = Destination{Name: "cluster-CCC"} @@ -84,69 +85,58 @@ func setup() error { return err } - return ImportConfig(db, confFile) -} + iosDevGroup := &Group{Name: "ios-developers", SourceId: fakeOktaSource.Id} -func TestMain(m *testing.M) { - err := setup() + err = db.Create(&iosDevGroup).Error if err != nil { - fmt.Println("Could not initialize test data: ", err) - os.Exit(1) + return err } - code := m.Run() - os.Exit(code) -} + iosDevGroupUsers := []User{iosDevUser, standardUser} -func TestImportCurrentValidConfig(t *testing.T) { - confFile, err := ioutil.ReadFile("_testdata/infra.yaml") + err = db.Model(&iosDevGroup).Association("Users").Replace(iosDevGroupUsers) if err != nil { - t.Fatal(err) + return err } - assert.NoError(t, ImportConfig(db, confFile)) -} - -func TestGroupsForExistingSourcesAreCreated(t *testing.T) { - var groups []Group - - db.Preload("Roles").Find(&groups) - assert.Equal(t, 4, len(groups), "Exactly four groups should be created from the test config, the other group has an invalid source") - - groupNames := make(map[string]Group) - for _, g := range groups { - groupNames[g.Name] = g + err = db.Create(&Group{Name: "mac-admins", SourceId: fakeOktaSource.Id}).Error + if err != nil { + return err } - assert.NotNil(t, groupNames["ios-developers"]) - assert.NotNil(t, groupNames["mac-admins"]) - assert.NotNil(t, groupNames["heroes"]) - assert.NotNil(t, groupNames["villains"]) - - iosDevRoleDests := make(map[string]bool) - for _, iosGroupRole := range groupNames["ios-developers"].Roles { - iosDevRoleDests[iosGroupRole.DestinationId] = true + // need to do the config import here so that the manual test modifications are not deleted + var config Config + if err := yaml.Unmarshal(confFile, &config); err != nil { + return err } - assert.True(t, iosDevRoleDests[clusterA.Id]) - assert.Len(t, groupNames["mac-admins"].Roles, 1) - assert.Contains(t, groupNames["mac-admins"].Roles[0].DestinationId, clusterB.Id) -} + initialConfig = config -func TestGroupsForUnknownSourcesAreNotCreated(t *testing.T) { - var groups []Group + return db.Transaction(func(tx *gorm.DB) error { + if err := ImportSources(tx, config.Sources); err != nil { + return err + } + + if err := ImportGroupMapping(tx, config.Groups); err != nil { + return err + } - db.Find(&groups) + return ImportUserMapping(tx, config.Users) + }) +} - assert.Equal(t, 4, len(groups), "Exactly four groups should be created from the test config, the other group has an invalid source") - group1 := groups[0] - group2 := groups[1] +func TestMain(m *testing.M) { + err := setup() + if err != nil { + fmt.Println("Could not initialize test data: ", err) + os.Exit(1) + } - assert.NotEqual(t, "unknown", group1.Name, "A group was made for a source that does not exist") - assert.NotEqual(t, "unknown", group2.Name, "A group was made for a source that does not exist") + code := m.Run() + os.Exit(code) } -func TestUsersForExistingUsersAndDestinationsAreCreated(t *testing.T) { +func TestRolesForExistingUsersAndDestinationsAreCreated(t *testing.T) { isAdminAdminA, err := containsUserRoleForDestination(db, adminUser, clusterA.Id, "admin") if err != nil { t.Error(err) @@ -161,20 +151,6 @@ func TestUsersForExistingUsersAndDestinationsAreCreated(t *testing.T) { assert.True(t, isAdminAdminB, "admin@example.com should have the admin role in cluster-BBB") - isStandardWriterA, err := containsUserRoleForDestination(db, standardUser, clusterA.Id, "writer") - if err != nil { - t.Error(err) - } - - assert.True(t, isStandardWriterA, "user@example.com should have the writer role in cluster-AAA") - - isStandardReaderA, err := containsUserRoleForDestination(db, standardUser, clusterA.Id, "reader") - if err != nil { - t.Error(err) - } - - assert.True(t, isStandardReaderA, "user@example.com should have the reader role in cluster-AAA") - unkownUser := User{Id: "0", Email: "unknown@example.com"} isUnknownUserGrantedRole, err := containsUserRoleForDestination(db, unkownUser, clusterA.Id, "writer") @@ -209,43 +185,6 @@ func TestExistingSourceIsOverridden(t *testing.T) { assert.Equal(t, fakeOktaSource.Domain, importedOkta.Domain) } -func containsUserRoleForDestination(db *gorm.DB, user User, destinationId string, roleName string) (bool, error) { - var roles []Role - - err := db.Preload("Destination").Preload("Groups").Preload("Users").Find(&roles, &Role{Name: roleName, DestinationId: destinationId}).Error - if err != nil { - return false, err - } - - // check direct role-user relations - for _, role := range roles { - for _, roleU := range role.Users { - if roleU.Email == user.Email { - return true, nil - } - } - } - - // check user groups-roles - var groups []Group - if err := db.Model(&user).Association("Groups").Find(&groups); err != nil { - return false, err - } - - for i := range groups { - var groupRoles []Role - if err := db.Model(&groups[i]).Association("Roles").Find(&groupRoles, &Role{Name: roleName, DestinationId: destinationId}); err != nil { - return false, err - } - - if len(groupRoles) > 0 { - return true, nil - } - } - - return false, nil -} - func TestClusterRolesAreAppliedToGroup(t *testing.T) { var group Group if err := db.Preload("Roles").Where(&Group{Name: "ios-developers"}).First(&group).Error; err != nil { @@ -363,3 +302,40 @@ func TestCleanupDomain(t *testing.T) { expected := "dev123123.okta.com" require.Equal(t, expected, s.Domain) } + +func containsUserRoleForDestination(db *gorm.DB, user User, destinationId string, roleName string) (bool, error) { + var roles []Role + + err := db.Preload("Destination").Preload("Groups").Preload("Users").Find(&roles, &Role{Name: roleName, DestinationId: destinationId}).Error + if err != nil { + return false, err + } + + // check direct role-user relations + for _, role := range roles { + for _, roleU := range role.Users { + if roleU.Email == user.Email { + return true, nil + } + } + } + + // check user groups-roles + var groups []Group + if err := db.Model(&user).Association("Groups").Find(&groups); err != nil { + return false, err + } + + for i := range groups { + var groupRoles []Role + if err := db.Model(&groups[i]).Association("Roles").Find(&groupRoles, &Role{Name: roleName, DestinationId: destinationId}); err != nil { + return false, err + } + + if len(groupRoles) > 0 { + return true, nil + } + } + + return false, nil +} diff --git a/internal/registry/data.go b/internal/registry/data.go index e7765cdded..0f3a601c45 100644 --- a/internal/registry/data.go +++ b/internal/registry/data.go @@ -60,7 +60,6 @@ type Group struct { Name string SourceId string Source Source `gorm:"foreignKey:SourceId;references:Id"` - Active bool // used to determine which groups actually exist at the source Roles []Role `gorm:"many2many:groups_roles"` Users []User `gorm:"many2many:groups_users"` @@ -141,7 +140,7 @@ func (u *User) BeforeCreate(tx *gorm.DB) (err error) { } func (u *User) AfterCreate(tx *gorm.DB) error { - return ApplyUserMapping(tx, initialConfig.Users) + return ImportUserMapping(tx, initialConfig.Users) } // TODO (jmorganca): use foreign constraints instead? @@ -166,11 +165,11 @@ func (d *Destination) BeforeCreate(tx *gorm.DB) (err error) { } func (d *Destination) AfterCreate(tx *gorm.DB) error { - if _, err := ApplyGroupMappings(tx, initialConfig.Groups); err != nil { + if err := ImportGroupMapping(tx, initialConfig.Groups); err != nil { return err } - return ApplyUserMapping(tx, initialConfig.Users) + return ImportUserMapping(tx, initialConfig.Users) } // TODO (jmorganca): use foreign constraints instead? @@ -194,6 +193,18 @@ func (g *Group) BeforeCreate(tx *gorm.DB) (err error) { return nil } +func (g *Group) AfterCreate(tx *gorm.DB) error { + return ImportGroupMapping(tx, initialConfig.Groups) +} + +func (g *Group) BeforeDelete(tx *gorm.DB) error { + if err := tx.Model(g).Association("Users").Clear(); err != nil { + return err + } + + return tx.Model(g).Association("Roles").Clear() +} + func (s *Source) BeforeCreate(tx *gorm.DB) (err error) { if s.Id == "" { s.Id = generate.MathRandString(IdLen) @@ -300,7 +311,7 @@ func (s *Source) SyncUsers(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) e } } - // Delete users not in source + // Remove users from source that no longer exist in the identity provider var toDelete []User if err := tx.Not("email IN ?", emails).Find(&toDelete).Error; err != nil { return err @@ -354,22 +365,18 @@ func (s *Source) SyncGroups(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) activeIDs = append(activeIDs, group.Id) } - // these groups no longer exist in the source so remove their users, but leave the entity in case they are re-created - var inactiveGroups []Group - err := tx.Where(&Group{SourceId: s.Id}).Not(activeIDs).Find(&inactiveGroups).Error - if err != nil { - return err - } - err = tx.Model(&inactiveGroups).Association("Users").Clear() - if err != nil { - return err - } - err = tx.Model(&inactiveGroups).Update("active", false).Error + // Delete source groups not in response + var toDelete []Group + + err := tx.Where(&Group{SourceId: s.Id}).Not(activeIDs).Find(&toDelete).Error if err != nil { return err } - if len(activeIDs) > 0 { - tx.Model(&Group{}).Where(activeIDs).Update("active", true) + + for i := range toDelete { + if err := tx.Delete(&toDelete[i]).Error; err != nil { + return err + } } return nil @@ -543,11 +550,15 @@ func NewDB(dbpath string) (*gorm.DB, error) { return nil, err } - if err := db.AutoMigrate(&Destination{}); err != nil { + if err := db.AutoMigrate(&Role{}); err != nil { return nil, err } - if err := db.AutoMigrate(&Role{}); err != nil { + if err := db.AutoMigrate(&Group{}); err != nil { + return nil, err + } + + if err := db.AutoMigrate(&Destination{}); err != nil { return nil, err } diff --git a/internal/registry/data_test.go b/internal/registry/data_test.go index 0c268473a9..7cc82100d5 100644 --- a/internal/registry/data_test.go +++ b/internal/registry/data_test.go @@ -26,26 +26,12 @@ func TestSyncGroupsClearsOnlySource(t *testing.T) { t.Fatal(err) } - // the total amount of groups for all sources should not change, just the users on the groups var groups []Group if err := db.Preload("Users").Find(&groups).Error; err != nil { t.Fatal(err) } - assert.Len(t, groups, 4) - - for _, group := range groups { - var src Source - if err := db.Where(&Source{Id: group.SourceId}).First(&src).Error; err != nil { - t.Fatal(err) - } - - if src.Type == SourceTypeOkta { - // these groups are part of the okta source and should be cleared - assert.Len(t, group.Users, 0) - assert.False(t, group.Active) - } - } + assert.Len(t, groups, 0) } func TestSyncGroupsFromOktaIgnoresUnknownUsers(t *testing.T) { @@ -73,7 +59,7 @@ func TestSyncGroupsFromOktaIgnoresUnknownUsers(t *testing.T) { assert.Equal(t, heroGroup.Users[0].Email, "woz@example.com") } -func TestSyncGroupsFromOktaRepopulatesEmptyGroups(t *testing.T) { +func TestSyncGroupsFromOktaRecreatesGroups(t *testing.T) { mockGroups := make(map[string][]string) mockGroups["heroes"] = []string{"woz@example.com"} testOkta := new(mocks.Okta) @@ -96,15 +82,6 @@ func TestSyncGroupsFromOktaRepopulatesEmptyGroups(t *testing.T) { assert.Len(t, heroGroup.Users, 1) assert.Equal(t, heroGroup.Users[0].Email, "woz@example.com") - assert.True(t, heroGroup.Active) - - var villainGroup Group - if err := db.Preload("Users").Where(&Group{Name: "villains", SourceId: fakeOktaSource.Id}).First(&villainGroup).Error; err != nil { - t.Fatal(err) - } - - assert.Len(t, villainGroup.Users, 0) - assert.False(t, villainGroup.Active) mockGroups["villains"] = []string{"user@example.com"} @@ -118,12 +95,11 @@ func TestSyncGroupsFromOktaRepopulatesEmptyGroups(t *testing.T) { assert.Len(t, heroGroup.Users, 1) assert.Equal(t, heroGroup.Users[0].Email, "woz@example.com") - assert.True(t, heroGroup.Active) + var villainGroup Group if err := db.Preload("Users").Where(&Group{Name: "villains", SourceId: fakeOktaSource.Id}).First(&villainGroup).Error; err != nil { t.Fatal(err) } assert.Len(t, villainGroup.Users, 1) - assert.True(t, villainGroup.Active) } From 26314903413c3b960af6fe1de3e11a9108ec5c53 Mon Sep 17 00:00:00 2001 From: Bruce MacDonald Date: Fri, 8 Oct 2021 11:16:34 -0400 Subject: [PATCH 2/2] improve source sync logging - wrap sync errors with context - make sync its own function - time sync events --- internal/registry/data.go | 35 ++++++++++----------- internal/registry/okta.go | 6 ++++ internal/registry/registry.go | 59 +++++++++++++++++++++++------------ internal/timer/timer.go | 8 +++++ 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/internal/registry/data.go b/internal/registry/data.go index 0f3a601c45..f77556f155 100644 --- a/internal/registry/data.go +++ b/internal/registry/data.go @@ -292,12 +292,12 @@ func (s *Source) SyncUsers(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) e case "okta": apiToken, err := k8s.GetSecret(s.ApiToken) if err != nil { - return err + return fmt.Errorf("sync okta users api token: %w", err) } emails, err = okta.Emails(s.Domain, s.ClientId, apiToken) if err != nil { - return err + return fmt.Errorf("sync okta users: %w", err) } default: return nil @@ -307,19 +307,19 @@ func (s *Source) SyncUsers(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) e // Create users in source for _, email := range emails { if err := s.CreateUser(tx, &User{}, email); err != nil { - return err + return fmt.Errorf("create user from okta: %w", err) } } // Remove users from source that no longer exist in the identity provider var toDelete []User if err := tx.Not("email IN ?", emails).Find(&toDelete).Error; err != nil { - return err + return fmt.Errorf("sync okta delete emails: %w", err) } for _, td := range toDelete { if err := s.DeleteUser(tx, td); err != nil { - return err + return fmt.Errorf("sync okta delete users: %w", err) } } @@ -334,12 +334,12 @@ func (s *Source) SyncGroups(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) case "okta": apiToken, err := k8s.GetSecret(s.ApiToken) if err != nil { - return err + return fmt.Errorf("sync okta groups api secret: %w", err) } groupEmails, err = okta.Groups(s.Domain, s.ClientId, apiToken) if err != nil { - return err + return fmt.Errorf("sync okta groups: %w", err) } default: return nil @@ -351,31 +351,28 @@ func (s *Source) SyncGroups(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) var group Group if err := tx.FirstOrCreate(&group, Group{Name: groupName, SourceId: s.Id}).Error; err != nil { logging.L.Sugar().Debug("could not find or create okta group: " + groupName) - return err + return fmt.Errorf("sync create okta group: %w", err) } var users []User - err := tx.Where("email IN ?", emails).Find(&users).Error - if err != nil { - return err + if err := tx.Where("email IN ?", emails).Find(&users).Error; err != nil { + return fmt.Errorf("sync okta group emails: %w", err) } - err = tx.Model(&group).Association("Users").Replace(users) - if err != nil { - return err + + if err := tx.Model(&group).Association("Users").Replace(users); err != nil { + return fmt.Errorf("sync okta replace with %d group users: %w", len(users), err) } activeIDs = append(activeIDs, group.Id) } // Delete source groups not in response var toDelete []Group - - err := tx.Where(&Group{SourceId: s.Id}).Not(activeIDs).Find(&toDelete).Error - if err != nil { - return err + if err := tx.Where(&Group{SourceId: s.Id}).Not(activeIDs).Find(&toDelete).Error; err != nil { + return fmt.Errorf("sync okta find inactive not in %d active: %w", len(activeIDs), err) } for i := range toDelete { if err := tx.Delete(&toDelete[i]).Error; err != nil { - return err + return fmt.Errorf("sync okta delete user: %w", err) } } diff --git a/internal/registry/okta.go b/internal/registry/okta.go index f20a967c4d..88adfe25c7 100644 --- a/internal/registry/okta.go +++ b/internal/registry/okta.go @@ -3,7 +3,9 @@ package registry import ( "context" "errors" + "time" + timer "github.com/infrahq/infra/internal/timer" "github.com/okta/okta-sdk-golang/v2/okta" "golang.org/x/oauth2" "gopkg.in/square/go-jose.v2/jwt" @@ -29,6 +31,8 @@ func (o *oktaImplementation) ValidateOktaConnection(domain string, clientID stri } func (o *oktaImplementation) Emails(domain string, clientID string, apiToken string) ([]string, error) { + defer timer.LogTimeElapsed(time.Now(), "okta user sync") + ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(3), okta.WithToken(apiToken)) if err != nil { return nil, err @@ -71,6 +75,8 @@ func (o *oktaImplementation) Emails(domain string, clientID string, apiToken str // Groups retrieves groups that exist in Okta for the configured InfraHQ group-role mappings and returns a map of group names to user lists func (o *oktaImplementation) Groups(domain string, clientID string, apiToken string) (map[string][]string, error) { + defer timer.LogTimeElapsed(time.Now(), "okta group sync") + ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(3), okta.WithToken(apiToken)) if err != nil { return nil, err diff --git a/internal/registry/registry.go b/internal/registry/registry.go index f4de44befb..28b905436b 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -21,7 +21,9 @@ import ( "github.com/infrahq/infra/internal/kubernetes" "github.com/infrahq/infra/internal/logging" timer "github.com/infrahq/infra/internal/timer" + "go.uber.org/zap" "golang.org/x/crypto/acme/autocert" + "gorm.io/gorm" ) type Options struct { @@ -40,6 +42,33 @@ const ( engineApiKeyName = "engine" ) +// syncSources polls every known source for users and groups +func syncSources(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta, logger *zap.Logger) { + var sources []Source + if err := db.Find(&sources).Error; err != nil { + logger.Sugar().Errorf("could not find sync sources: %w", err) + } + + for _, s := range sources { + switch s.Type { + case "okta": + logger.Sugar().Debug("synchronizing okta source") + + err := s.SyncUsers(db, k8s, okta) + if err != nil { + logger.Sugar().Errorf("sync okta users: %w", err) + } + + err = s.SyncGroups(db, k8s, okta) + if err != nil { + logger.Sugar().Errorf("sync okta groups: %w", err) + } + default: + logger.Sugar().Errorf("skipped validating unknown source type %s", s.Type) + } + } +} + func Run(options Options) error { db, err := NewDB(options.DBPath) if err != nil { @@ -81,18 +110,22 @@ func Run(options Options) error { zapLogger.Warn("skipped importing empty config") } - // validate any existing or imported sources okta := NewOkta() + // validate any existing or imported sources var sources []Source if err := db.Find(&sources).Error; err != nil { - zapLogger.Error(err.Error()) + zapLogger.Sugar().Error("find sources to validate: %w", err) } for _, s := range sources { - err = s.Validate(db, k8s, okta) - if err != nil { - zapLogger.Error(err.Error()) + switch s.Type { + case "okta": + if err := s.Validate(db, k8s, okta); err != nil { + zapLogger.Sugar().Errorf("could not validate okta: %w", err) + } + default: + zapLogger.Sugar().Errorf("skipped validating unknown source type %s", s.Type) } } @@ -116,21 +149,7 @@ func Run(options Options) error { syncSourcesTimer := timer.NewTimer() defer syncSourcesTimer.Stop() syncSourcesTimer.Start(interval, func() { - var sources []Source - if err := db.Find(&sources).Error; err != nil { - zapLogger.Error(err.Error()) - } - - for _, s := range sources { - err = s.SyncUsers(db, k8s, okta) - if err != nil { - zapLogger.Error(err.Error()) - } - err = s.SyncGroups(db, k8s, okta) - if err != nil { - zapLogger.Error(err.Error()) - } - } + syncSources(db, k8s, okta, zapLogger) }) // schedule destination sync job diff --git a/internal/timer/timer.go b/internal/timer/timer.go index 518331aa1b..33f130ad3e 100644 --- a/internal/timer/timer.go +++ b/internal/timer/timer.go @@ -2,6 +2,8 @@ package timer import ( "time" + + "github.com/infrahq/infra/internal/logging" ) type Timer struct { @@ -36,3 +38,9 @@ func (t *Timer) Start(interval time.Duration, sync func()) { func (t *Timer) Stop() { t.stop <- true } + +// LogTimeElapsed logs the amount of time since this function was defered at the debug level +func LogTimeElapsed(start time.Time, task string) { + elapsed := time.Since(start) + logging.L.Sugar().Debugf("%s in %s", task, elapsed) +}