From cbd574db1480e3afc6e4d484a03286ba655f5368 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 4 Sep 2023 12:23:04 +0300 Subject: [PATCH 01/27] implement jumpcloud idp manager --- go.mod | 1 + go.sum | 2 + management/server/idp/idp.go | 5 + management/server/idp/jumpcloud.go | 347 ++++++++++++++++++++++++ management/server/idp/jumpcloud_test.go | 46 ++++ 5 files changed, 401 insertions(+) create mode 100644 management/server/idp/jumpcloud.go create mode 100644 management/server/idp/jumpcloud_test.go diff --git a/go.mod b/go.mod index 7ecf61584ce..8be1599970a 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( require ( fyne.io/fyne/v2 v2.1.4 + github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible github.com/c-robinson/iplib v1.0.3 github.com/cilium/ebpf v0.10.0 github.com/coreos/go-iptables v0.7.0 diff --git a/go.sum b/go.sum index 1eb9d243dee..25182ca85df 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/PuerkitoBio/purell v1.0.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= +github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible h1:hqcTK6ZISdip65SR792lwYJTa/axESA0889D3UlZbLo= +github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible/go.mod h1:6B1nuc1MUs6c62ODZDl7hVE5Pv7O2XGSkgg2olnq34I= github.com/XiaoMi/pegasus-go-client v0.0.0-20210427083443-f3b6b08bc4c2 h1:pami0oPhVosjOu/qRHepRmdjD6hGILF7DBr+qQZeP10= github.com/XiaoMi/pegasus-go-client v0.0.0-20210427083443-f3b6b08bc4c2/go.mod h1:jNIx5ykW1MroBuaTja9+VpglmaJOUzezumfhLlER3oY= github.com/akavel/rsrc v0.8.0/go.mod h1:uLoCtb9J+EyAqh+26kdrTgmzRBFPGOolLWKpdxkKq+c= diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 48afd5c32ea..90b98051088 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -171,6 +171,11 @@ func NewManager(config Config, appMetrics telemetry.AppMetrics) (Manager, error) } return NewGoogleWorkspaceManager(googleClientConfig, appMetrics) + case "jumpcloud": + jumpcloudConfig := JumpCloudClientConfig{ + APIToken: config.ExtraConfig["ApiToken"], + } + return NewJumpCloudManager(jumpcloudConfig, appMetrics) default: return nil, fmt.Errorf("invalid manager type: %s", config.ManagerType) } diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go new file mode 100644 index 00000000000..028e20687c9 --- /dev/null +++ b/management/server/idp/jumpcloud.go @@ -0,0 +1,347 @@ +package idp + +import ( + "context" + "fmt" + "net/http" + "strings" + "time" + + v1 "github.com/TheJumpCloud/jcapi-go/v1" + + "github.com/netbirdio/netbird/management/server/telemetry" +) + +const ( + contentType = "application/json" + accept = "application/json" +) + +// JumpCloudManager JumpCloud manager client instance. +type JumpCloudManager struct { + client *v1.APIClient + apiToken string + httpClient ManagerHTTPClient + credentials ManagerCredentials + helper ManagerHelper + appMetrics telemetry.AppMetrics +} + +// JumpCloudClientConfig JumpCloud manager client configurations. +type JumpCloudClientConfig struct { + APIToken string +} + +// JumpCloudCredentials JumpCloud authentication information. +type JumpCloudCredentials struct { + clientConfig JumpCloudClientConfig + helper ManagerHelper + httpClient ManagerHTTPClient + appMetrics telemetry.AppMetrics +} + +type JumpCloudAttribute struct { + Name string `json:"name"` + Value any `json:"value"` +} + +// NewJumpCloudManager creates a new instance of the JumpCloudManager. +func NewJumpCloudManager(config JumpCloudClientConfig, appMetrics telemetry.AppMetrics) (*JumpCloudManager, error) { + httpTransport := http.DefaultTransport.(*http.Transport).Clone() + httpTransport.MaxIdleConns = 5 + + httpClient := &http.Client{ + Timeout: 10 * time.Second, + Transport: httpTransport, + } + + helper := JsonParser{} + + if config.APIToken == "" { + return nil, fmt.Errorf("jumpCloud IdP configuration is incomplete, ApiToken is missing") + } + + client := v1.NewAPIClient(v1.NewConfiguration()) + credentials := &JumpCloudCredentials{ + clientConfig: config, + httpClient: httpClient, + helper: helper, + appMetrics: appMetrics, + } + + return &JumpCloudManager{ + client: client, + apiToken: config.APIToken, + httpClient: httpClient, + credentials: credentials, + helper: helper, + appMetrics: appMetrics, + }, nil +} + +// Authenticate retrieves access token to use the JumpCloud user API. +func (jc *JumpCloudCredentials) Authenticate() (JWTToken, error) { + return JWTToken{}, nil +} + +// UpdateUserAppMetadata updates user app metadata based on userID and metadata map. +func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + updateReq := map[string]any{ + "body": v1.Systemuserput{ + Attributes: []interface{}{ + JumpCloudAttribute{ + Name: "wtAccountID", + Value: appMetadata.WTAccountID, + }, + JumpCloudAttribute{ + Name: "wtPendingInvite", + Value: appMetadata.WTPendingInvite, + }, + }, + }, + } + + _, resp, err := jm.client.SystemusersApi.SystemusersPut(authCtx, userID, contentType, accept, updateReq) + if err != nil { + return err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() + } + + return nil +} + +// GetUserDataByID requests user data from JumpCloud via ID. +func (jm *JumpCloudManager) GetUserDataByID(userID string, _ AppMetadata) (*UserData, error) { + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + + user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetUserDataByID() + } + + return parseJumpCloudUser(user), nil +} + +// GetAccount returns all the users for a given profile. +func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + searchFilter := map[string]interface{}{ + "searchFilter": map[string]interface{}{ + "filter": []string{accountID}, + "fields": []string{"wtAccountID"}, + }, + } + + usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetAccount() + } + + usersData := make([]*UserData, 0) + for _, user := range usersList.Results { + userData := parseJumpCloudUser(user) + usersData = append(usersData, userData) + } + + return usersData, nil +} + +// GetAllAccounts gets all registered accounts with corresponding user data. +// It returns a list of users indexed by accountID. +func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + + usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetAllAccounts() + } + + indexedUsers := make(map[string][]*UserData) + for _, user := range usersList.Results { + userData := parseJumpCloudUser(user) + + accountID := userData.AppMetadata.WTAccountID + if accountID != "" { + if _, ok := indexedUsers[accountID]; !ok { + indexedUsers[accountID] = make([]*UserData, 0) + } + indexedUsers[accountID] = append(indexedUsers[accountID], userData) + } + } + + return indexedUsers, nil +} + +// CreateUser creates a new user in JumpCloud Idp and sends an invitation. +func (jm *JumpCloudManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { + var firstName, lastName string + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + + fields := strings.Fields(name) + if n := len(fields); n > 0 { + firstName = strings.Join(fields[:n-1], " ") + lastName = fields[n-1] + } + createUserReq := map[string]any{ + "body": v1.Systemuserputpost{ + Username: firstName, + Email: email, + Firstname: firstName, + Lastname: lastName, + Activated: true, + Attributes: []any{ + JumpCloudAttribute{ + Name: "wtAccountID", + Value: accountID, + }, + JumpCloudAttribute{ + Name: "wtPendingInvite", + Value: true, + }, + }, + }, + } + + user, resp, err := jm.client.SystemusersApi.SystemusersPost(authCtx, contentType, accept, createUserReq) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to create user %s, statusCode %d", email, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountCreateUser() + } + + return parseJumpCloudUser(user), nil +} + +// GetUserByEmail searches users with a given email. +// If no users have been found, this function returns an empty list. +func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { + authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ + Key: jm.apiToken, + }) + searchFilter := map[string]interface{}{ + "searchFilter": map[string]interface{}{ + "filter": []string{email}, + "fields": []string{"email"}, + }, + } + + usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get user %s, statusCode %d", email, resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountGetUserByEmail() + } + + usersData := make([]*UserData, 0) + for _, user := range usersList.Results { + userData := parseJumpCloudUser(user) + usersData = append(usersData, userData) + } + + return usersData, nil +} + +// InviteUserByID resend invitations to users who haven't activated, +// their accounts prior to the expiration period. +func (jm *JumpCloudManager) InviteUserByID(_ string) error { + return fmt.Errorf("method InviteUserByID not implemented") +} + +// parseJumpCloudUser parse JumpCloud system user returned from API V1 to UserData. +func parseJumpCloudUser(user v1.Systemuserreturn) *UserData { + appMetadata := AppMetadata{} + names := []string{user.Firstname, user.Middlename, user.Lastname} + + for _, attribute := range user.Attributes { + if jcAttribute, ok := attribute.(map[string]any); ok { + if jcAttribute["name"] == "wtAccountID" { + appMetadata.WTAccountID = jcAttribute["value"].(string) + } + + if jcAttribute["name"] == "wtPendingInvite" { + if value, ok := jcAttribute["value"].(bool); ok { + appMetadata.WTPendingInvite = &value + } + } + } + } + + return &UserData{ + Email: user.Email, + Name: strings.Join(names, " "), + ID: user.Id, + AppMetadata: appMetadata, + } +} diff --git a/management/server/idp/jumpcloud_test.go b/management/server/idp/jumpcloud_test.go new file mode 100644 index 00000000000..1bfdcefcc70 --- /dev/null +++ b/management/server/idp/jumpcloud_test.go @@ -0,0 +1,46 @@ +package idp + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" +) + +func TestNewJumpCloudManager(t *testing.T) { + type test struct { + name string + inputConfig JumpCloudClientConfig + assertErrFunc require.ErrorAssertionFunc + assertErrFuncMessage string + } + + defaultTestConfig := JumpCloudClientConfig{ + APIToken: "test123", + } + + testCase1 := test{ + name: "Good Configuration", + inputConfig: defaultTestConfig, + assertErrFunc: require.NoError, + assertErrFuncMessage: "shouldn't return error", + } + + testCase2Config := defaultTestConfig + testCase2Config.APIToken = "" + + testCase2 := test{ + name: "Missing APIToken Configuration", + inputConfig: testCase2Config, + assertErrFunc: require.Error, + assertErrFuncMessage: "should return error when field empty", + } + + for _, testCase := range []test{testCase1, testCase2} { + t.Run(testCase.name, func(t *testing.T) { + _, err := NewJumpCloudManager(testCase.inputConfig, &telemetry.MockAppMetrics{}) + testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) + }) + } +} From f70c8387bb8cec590609d2fa4c65ccea0a03d07d Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 4 Sep 2023 19:55:10 +0300 Subject: [PATCH 02/27] Refactor JumpCloudManager for cleaner authentication context usage --- management/server/idp/jumpcloud.go | 75 +++++++++++++++--------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index 028e20687c9..718ff3040b4 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -54,7 +54,6 @@ func NewJumpCloudManager(config JumpCloudClientConfig, appMetrics telemetry.AppM Timeout: 10 * time.Second, Transport: httpTransport, } - helper := JsonParser{} if config.APIToken == "" { @@ -84,30 +83,38 @@ func (jc *JumpCloudCredentials) Authenticate() (JWTToken, error) { return JWTToken{}, nil } -// UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ +func (jm *JumpCloudManager) authenticationContext() context.Context { + return context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ Key: jm.apiToken, }) - updateReq := map[string]any{ +} + +// UpdateUserAppMetadata updates user app metadata based on userID and metadata map. +func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { + attributes := []interface{}{ + JumpCloudAttribute{ + Name: "wtAccountID", + Value: appMetadata.WTAccountID, + }, + } + if appMetadata.WTPendingInvite != nil { + attributes = append(attributes, JumpCloudAttribute{ + Name: "wtPendingInvite", + Value: appMetadata.WTPendingInvite, + }) + } + updateReq := map[string]interface{}{ "body": v1.Systemuserput{ - Attributes: []interface{}{ - JumpCloudAttribute{ - Name: "wtAccountID", - Value: appMetadata.WTAccountID, - }, - JumpCloudAttribute{ - Name: "wtPendingInvite", - Value: appMetadata.WTPendingInvite, - }, - }, + Attributes: attributes, }, } + authCtx := jm.authenticationContext() _, resp, err := jm.client.SystemusersApi.SystemusersPut(authCtx, userID, contentType, accept, updateReq) if err != nil { return err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -125,14 +132,12 @@ func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata App // GetUserDataByID requests user data from JumpCloud via ID. func (jm *JumpCloudManager) GetUserDataByID(userID string, _ AppMetadata) (*UserData, error) { - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ - Key: jm.apiToken, - }) - + authCtx := jm.authenticationContext() user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -150,9 +155,6 @@ func (jm *JumpCloudManager) GetUserDataByID(userID string, _ AppMetadata) (*User // GetAccount returns all the users for a given profile. func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ - Key: jm.apiToken, - }) searchFilter := map[string]interface{}{ "searchFilter": map[string]interface{}{ "filter": []string{accountID}, @@ -160,10 +162,12 @@ func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { }, } - usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -177,7 +181,7 @@ func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { } usersData := make([]*UserData, 0) - for _, user := range usersList.Results { + for _, user := range userList.Results { userData := parseJumpCloudUser(user) usersData = append(usersData, userData) } @@ -188,11 +192,8 @@ func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ - Key: jm.apiToken, - }) - - usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) if err != nil { return nil, err } @@ -209,7 +210,7 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { } indexedUsers := make(map[string][]*UserData) - for _, user := range usersList.Results { + for _, user := range userList.Results { userData := parseJumpCloudUser(user) accountID := userData.AppMetadata.WTAccountID @@ -227,9 +228,6 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { // CreateUser creates a new user in JumpCloud Idp and sends an invitation. func (jm *JumpCloudManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { var firstName, lastName string - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ - Key: jm.apiToken, - }) fields := strings.Fields(name) if n := len(fields); n > 0 { @@ -256,10 +254,12 @@ func (jm *JumpCloudManager) CreateUser(email, name, accountID, invitedByEmail st }, } + authCtx := jm.authenticationContext() user, resp, err := jm.client.SystemusersApi.SystemusersPost(authCtx, contentType, accept, createUserReq) if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -278,9 +278,6 @@ func (jm *JumpCloudManager) CreateUser(email, name, accountID, invitedByEmail st // GetUserByEmail searches users with a given email. // If no users have been found, this function returns an empty list. func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { - authCtx := context.WithValue(context.Background(), v1.ContextAPIKey, v1.APIKey{ - Key: jm.apiToken, - }) searchFilter := map[string]interface{}{ "searchFilter": map[string]interface{}{ "filter": []string{email}, @@ -288,10 +285,12 @@ func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { }, } - usersList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + authCtx := jm.authenticationContext() + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -305,7 +304,7 @@ func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { } usersData := make([]*UserData, 0) - for _, user := range usersList.Results { + for _, user := range userList.Results { userData := parseJumpCloudUser(user) usersData = append(usersData, userData) } From 3b0bd1201c4f4b817c65d9950d1f22a19faa0d5c Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 4 Sep 2023 22:13:28 +0300 Subject: [PATCH 03/27] update user attributes without replacing other custom attributes --- management/server/idp/jumpcloud.go | 63 ++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index 718ff3040b4..7ebbef33932 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -91,26 +91,21 @@ func (jm *JumpCloudManager) authenticationContext() context.Context { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - attributes := []interface{}{ - JumpCloudAttribute{ - Name: "wtAccountID", - Value: appMetadata.WTAccountID, - }, - } - if appMetadata.WTPendingInvite != nil { - attributes = append(attributes, JumpCloudAttribute{ - Name: "wtPendingInvite", - Value: appMetadata.WTPendingInvite, - }) + authCtx := jm.authenticationContext() + user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) + if err != nil { + return err } + defer resp.Body.Close() + + attributes := buildUserAttributesUpdatePayload(user, appMetadata) updateReq := map[string]interface{}{ "body": v1.Systemuserput{ Attributes: attributes, }, } - authCtx := jm.authenticationContext() - _, resp, err := jm.client.SystemusersApi.SystemusersPut(authCtx, userID, contentType, accept, updateReq) + _, resp, err = jm.client.SystemusersApi.SystemusersPut(authCtx, userID, contentType, accept, updateReq) if err != nil { return err } @@ -197,6 +192,7 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { if jm.appMetrics != nil { @@ -344,3 +340,44 @@ func parseJumpCloudUser(user v1.Systemuserreturn) *UserData { AppMetadata: appMetadata, } } + +// buildUserAttributesUpdatePayload constructs the updated user attributes based on system user and application metadata. +func buildUserAttributesUpdatePayload(user v1.Systemuserreturn, appMetadata AppMetadata) (userAttributes []interface{}) { + var isAccountIDFound, isPendingInviteFound bool + + for _, attribute := range user.Attributes { + if jcAttribute, ok := attribute.(JumpCloudAttribute); ok { + if jcAttribute.Name == "wtAccountID" { + jcAttribute.Value = appMetadata.WTAccountID + isAccountIDFound = true + } + + if jcAttribute.Name == "wtPendingInvite" { + if appMetadata.WTPendingInvite != nil { + jcAttribute.Value = appMetadata.WTPendingInvite + isPendingInviteFound = true + } + } + + attribute = jcAttribute + } + + userAttributes = append(userAttributes, attribute) + } + + if !isAccountIDFound { + userAttributes = append(userAttributes, JumpCloudAttribute{ + Name: "wtAccountID", + Value: appMetadata.WTAccountID, + }) + } + + if !isPendingInviteFound && appMetadata.WTPendingInvite != nil { + userAttributes = append(userAttributes, JumpCloudAttribute{ + Name: "wtPendingInvite", + Value: appMetadata.WTPendingInvite, + }) + } + + return userAttributes +} From 8e95904d1549ed39a242453db73c7f02769f354f Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 5 Sep 2023 11:23:58 +0300 Subject: [PATCH 04/27] add support for OIDC providers that may not support the audience in authorization --- infrastructure_files/base.setup.env | 13 ++++++++++++- infrastructure_files/configure.sh | 6 ++++++ infrastructure_files/docker-compose.yml.tmpl | 2 +- .../docker-compose.yml.tmpl.traefik | 2 +- infrastructure_files/management.json.tmpl | 2 +- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/infrastructure_files/base.setup.env b/infrastructure_files/base.setup.env index 4bcec128df9..f610a9691bc 100644 --- a/infrastructure_files/base.setup.env +++ b/infrastructure_files/base.setup.env @@ -46,6 +46,14 @@ NETBIRD_TOKEN_SOURCE=${NETBIRD_TOKEN_SOURCE:-accessToken} # PKCE authorization flow NETBIRD_AUTH_PKCE_REDIRECT_URL_PORTS=${NETBIRD_AUTH_PKCE_REDIRECT_URL_PORTS:-"53000"} NETBIRD_AUTH_PKCE_USE_ID_TOKEN=${NETBIRD_AUTH_PKCE_USE_ID_TOKEN:-false} +NETBIRD_AUTH_PKCE_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + +# Dashboard + +# The default setting is to transmit the audience to the IDP during authorization. However, +# if your IDP does not have this capability, you can turn this off by setting it to false. +NETBIRD_DASH_AUTH_USE_AUDIENCE=${NETBIRD_DASH_AUTH_USE_AUDIENCE:-true} +NETBIRD_DASH_AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE # exports export NETBIRD_DOMAIN @@ -86,4 +94,7 @@ export NETBIRD_TOKEN_SOURCE export NETBIRD_AUTH_DEVICE_AUTH_SCOPE export NETBIRD_AUTH_DEVICE_AUTH_USE_ID_TOKEN export NETBIRD_AUTH_PKCE_AUTHORIZATION_ENDPOINT -export NETBIRD_AUTH_PKCE_USE_ID_TOKEN \ No newline at end of file +export NETBIRD_AUTH_PKCE_USE_ID_TOKEN +export NETBIRD_AUTH_PKCE_AUDIENCE +export NETBIRD_DASH_AUTH_USE_AUDIENCE +export NETBIRD_DASH_AUTH_AUDIENCE \ No newline at end of file diff --git a/infrastructure_files/configure.sh b/infrastructure_files/configure.sh index 4e568b2fef4..3db79906827 100755 --- a/infrastructure_files/configure.sh +++ b/infrastructure_files/configure.sh @@ -164,6 +164,12 @@ done export NETBIRD_AUTH_PKCE_REDIRECT_URLS=${REDIRECT_URLS%,} +# Remove audience for providers that do not support it +if [ "$NETBIRD_DASH_AUTH_USE_AUDIENCE" = "false" ]; then + export NETBIRD_DASH_AUTH_AUDIENCE=none + export NETBIRD_AUTH_PKCE_AUDIENCE= +fi + env | grep NETBIRD envsubst docker-compose.yml diff --git a/infrastructure_files/docker-compose.yml.tmpl b/infrastructure_files/docker-compose.yml.tmpl index 3e7eb1df651..955586c47fa 100644 --- a/infrastructure_files/docker-compose.yml.tmpl +++ b/infrastructure_files/docker-compose.yml.tmpl @@ -12,7 +12,7 @@ services: - NETBIRD_MGMT_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT - NETBIRD_MGMT_GRPC_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT # OIDC - - AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + - AUTH_AUDIENCE=$NETBIRD_DASH_AUTH_AUDIENCE - AUTH_CLIENT_ID=$NETBIRD_AUTH_CLIENT_ID - AUTH_CLIENT_SECRET=$NETBIRD_AUTH_CLIENT_SECRET - AUTH_AUTHORITY=$NETBIRD_AUTH_AUTHORITY diff --git a/infrastructure_files/docker-compose.yml.tmpl.traefik b/infrastructure_files/docker-compose.yml.tmpl.traefik index 6d371081693..e7c45712657 100644 --- a/infrastructure_files/docker-compose.yml.tmpl.traefik +++ b/infrastructure_files/docker-compose.yml.tmpl.traefik @@ -12,7 +12,7 @@ services: - NETBIRD_MGMT_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT - NETBIRD_MGMT_GRPC_API_ENDPOINT=$NETBIRD_MGMT_API_ENDPOINT # OIDC - - AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE + - AUTH_AUDIENCE=$NETBIRD_DASH_AUTH_AUDIENCE - AUTH_CLIENT_ID=$NETBIRD_AUTH_CLIENT_ID - AUTH_CLIENT_SECRET=$NETBIRD_AUTH_CLIENT_SECRET - AUTH_AUTHORITY=$NETBIRD_AUTH_AUTHORITY diff --git a/infrastructure_files/management.json.tmpl b/infrastructure_files/management.json.tmpl index e74b93b32ac..e185faa6ebd 100644 --- a/infrastructure_files/management.json.tmpl +++ b/infrastructure_files/management.json.tmpl @@ -62,7 +62,7 @@ }, "PKCEAuthorizationFlow": { "ProviderConfig": { - "Audience": "$NETBIRD_AUTH_AUDIENCE", + "Audience": "$NETBIRD_AUTH_PKCE_AUDIENCE", "ClientID": "$NETBIRD_AUTH_CLIENT_ID", "ClientSecret": "$NETBIRD_AUTH_CLIENT_SECRET", "AuthorizationEndpoint": "$NETBIRD_AUTH_PKCE_AUTHORIZATION_ENDPOINT", From fd4a97655db19717981ee317e53555a5681188c1 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 5 Sep 2023 11:26:01 +0300 Subject: [PATCH 05/27] Implement PKCE token verification via Client ID for providers lacking audience support --- client/internal/auth/pkce_flow.go | 8 +++++++- client/internal/pkce_auth.go | 3 --- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/client/internal/auth/pkce_flow.go b/client/internal/auth/pkce_flow.go index d15d493738a..eb86f2d3a4e 100644 --- a/client/internal/auth/pkce_flow.go +++ b/client/internal/auth/pkce_flow.go @@ -200,7 +200,13 @@ func (p *PKCEAuthorizationFlow) handleOAuthToken(token *oauth2.Token) (TokenInfo tokenInfo.IDToken = idToken } - if err := isValidAccessToken(tokenInfo.GetTokenToUse(), p.providerConfig.Audience); err != nil { + // if a provider doesn't support an audience, use the Client ID for token verification + audience := p.providerConfig.Audience + if audience == "" { + audience = p.providerConfig.ClientID + } + + if err := isValidAccessToken(tokenInfo.GetTokenToUse(), audience); err != nil { return TokenInfo{}, fmt.Errorf("validate access token failed with error: %v", err) } diff --git a/client/internal/pkce_auth.go b/client/internal/pkce_auth.go index 2efbae97b5f..a35dacc77be 100644 --- a/client/internal/pkce_auth.go +++ b/client/internal/pkce_auth.go @@ -106,9 +106,6 @@ func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL func isPKCEProviderConfigValid(config PKCEAuthProviderConfig) error { errorMSGFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator" - if config.Audience == "" { - return fmt.Errorf(errorMSGFormat, "Audience") - } if config.ClientID == "" { return fmt.Errorf(errorMSGFormat, "Client ID") } From 45eeda4f0a6a9a5ec6256215021cd3aff853c7af Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 6 Sep 2023 14:22:13 +0300 Subject: [PATCH 06/27] include an option to turn off transmitting the audience to the IDP --- infrastructure_files/setup.env.example | 3 +++ 1 file changed, 3 insertions(+) diff --git a/infrastructure_files/setup.env.example b/infrastructure_files/setup.env.example index 9b03ccd2d77..f9ad638465f 100644 --- a/infrastructure_files/setup.env.example +++ b/infrastructure_files/setup.env.example @@ -8,6 +8,9 @@ NETBIRD_DOMAIN="" # e.g., https://example.eu.auth0.com/.well-known/openid-configuration # ------------------------------------------- NETBIRD_AUTH_OIDC_CONFIGURATION_ENDPOINT="" +# The default setting is to transmit the audience to the IDP during authorization. However, +# if your IDP does not have this capability, you can turn this off by setting it to false. +#NETBIRD_DASH_AUTH_USE_AUDIENCE=false NETBIRD_AUTH_AUDIENCE="" # e.g. netbird-client NETBIRD_AUTH_CLIENT_ID="" From 50ecf6f4da351c3305ec03b3270ba5f15db01f33 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Sep 2023 12:49:35 +0300 Subject: [PATCH 07/27] wip: Handle user metadata without transmitting them to Identity Provider --- management/server/account.go | 32 ++++ management/server/idp/authentik.go | 233 +++++++++++++++-------------- management/server/idp/idp.go | 8 + 3 files changed, 159 insertions(+), 114 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 4c707af3aff..d08e25fd41f 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" b64 "encoding/base64" + "errors" "fmt" "hash/crc32" "math/rand" @@ -938,6 +939,37 @@ func (am *DefaultAccountManager) newAccount(userID, domain string) (*Account, er func (am *DefaultAccountManager) warmupIDPCache() error { userData, err := am.idpManager.GetAllAccounts() if err != nil { + var idpErr *idp.Error + if errors.As(err, &idpErr) { + log.Warnf("failed to fetch all accounts from idp: %v", idpErr) + + log.Info("warming cache using store") + accounts := am.Store.GetAllAccounts() + + for _, account := range accounts { + data := make([]*idp.UserData, 0) + + for _, user := range account.Users { + // fetch user info with idp manager + userData, err := am.idpManager.GetUserDataByID(user.Id, idp.AppMetadata{WTAccountID: account.Id}) + if err != nil { + return err + } + + // update user metadata as are not stored in upstream idp + // Note (self-hosted does not support pendingInvite and invitedBy status) + userData.AppMetadata.WTAccountID = account.Id + data = append(data, userData) + } + + err = am.cacheManager.Set(am.ctx, account.Id, data, cacheStore.WithExpiration(cacheEntryExpiration())) + if err != nil { + return err + } + } + return nil + } + return err } diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index 0898f1c9422..c57efef1a59 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -12,9 +12,10 @@ import ( "time" "github.com/golang-jwt/jwt" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" "goauthentik.io/api/v3" + + "github.com/netbirdio/netbird/management/server/telemetry" ) // AuthentikManager authentik manager client instance. @@ -210,47 +211,48 @@ func (ac *AuthentikCredentials) Authenticate() (JWTToken, error) { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. func (am *AuthentikManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - ctx, err := am.authenticationContext() - if err != nil { - return err - } - - userPk, err := strconv.ParseInt(userID, 10, 32) - if err != nil { - return err - } - - var pendingInvite bool - if appMetadata.WTPendingInvite != nil { - pendingInvite = *appMetadata.WTPendingInvite - } - - patchedUserReq := api.PatchedUserRequest{ - Attributes: map[string]interface{}{ - wtAccountID: appMetadata.WTAccountID, - wtPendingInvite: pendingInvite, - }, - } - _, resp, err := am.apiClient.CoreApi.CoreUsersPartialUpdate(ctx, int32(userPk)). - PatchedUserRequest(patchedUserReq). - Execute() - if err != nil { - return err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusOK { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) - } - - return nil + //ctx, err := am.authenticationContext() + //if err != nil { + // return err + //} + // + //userPk, err := strconv.ParseInt(userID, 10, 32) + //if err != nil { + // return err + //} + // + //var pendingInvite bool + //if appMetadata.WTPendingInvite != nil { + // pendingInvite = *appMetadata.WTPendingInvite + //} + // + //patchedUserReq := api.PatchedUserRequest{ + // Attributes: map[string]interface{}{ + // wtAccountID: appMetadata.WTAccountID, + // wtPendingInvite: pendingInvite, + // }, + //} + //_, resp, err := am.apiClient.CoreApi.CoreUsersPartialUpdate(ctx, int32(userPk)). + // PatchedUserRequest(patchedUserReq). + // Execute() + //if err != nil { + // return err + //} + //defer resp.Body.Close() + // + //if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() + //} + // + //if resp.StatusCode != http.StatusOK { + // if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountRequestStatusError() + // } + // return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) + //} + // + //return nil + return &Error{"UpdateUserAppMetadata is not implemented"} } // GetUserDataByID requests user data from authentik via ID. @@ -287,83 +289,86 @@ func (am *AuthentikManager) GetUserDataByID(userID string, appMetadata AppMetada // GetAccount returns all the users for a given profile. func (am *AuthentikManager) GetAccount(accountID string) ([]*UserData, error) { - ctx, err := am.authenticationContext() - if err != nil { - return nil, err - } - - accountFilter := fmt.Sprintf("{%q:%q}", wtAccountID, accountID) - userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Attributes(accountFilter).Execute() - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountGetAccount() - } - - if resp.StatusCode != http.StatusOK { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) - } - - users := make([]*UserData, 0) - for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } - users = append(users, userData) - } - - return users, nil + //ctx, err := am.authenticationContext() + //if err != nil { + // return nil, err + //} + // + //accountFilter := fmt.Sprintf("{%q:%q}", wtAccountID, accountID) + //userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Attributes(accountFilter).Execute() + //if err != nil { + // return nil, err + //} + //defer resp.Body.Close() + // + //if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountGetAccount() + //} + // + //if resp.StatusCode != http.StatusOK { + // if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountRequestStatusError() + // } + // return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) + //} + // + //users := make([]*UserData, 0) + //for _, user := range userList.Results { + // userData, err := parseAuthentikUser(user) + // if err != nil { + // return nil, err + // } + // users = append(users, userData) + //} + // + //return users, nil + return nil, &Error{"GetAccount is not implemented"} } // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { - ctx, err := am.authenticationContext() - if err != nil { - return nil, err - } - - userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountGetAllAccounts() - } - - if resp.StatusCode != http.StatusOK { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) - } - - indexedUsers := make(map[string][]*UserData) - for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } - } - - return indexedUsers, nil + //ctx, err := am.authenticationContext() + //if err != nil { + // return nil, err + //} + // + //userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() + //if err != nil { + // return nil, err + //} + //defer resp.Body.Close() + // + //if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountGetAllAccounts() + //} + // + //if resp.StatusCode != http.StatusOK { + // if am.appMetrics != nil { + // am.appMetrics.IDPMetrics().CountRequestStatusError() + // } + // return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) + //} + // + //indexedUsers := make(map[string][]*UserData) + //for _, user := range userList.Results { + // userData, err := parseAuthentikUser(user) + // if err != nil { + // return nil, err + // } + // + // accountID := userData.AppMetadata.WTAccountID + // if accountID != "" { + // if _, ok := indexedUsers[accountID]; !ok { + // indexedUsers[accountID] = make([]*UserData, 0) + // } + // indexedUsers[accountID] = append(indexedUsers[accountID], userData) + // } + //} + // + //return indexedUsers, nil + + return nil, &Error{"GetAllAccounts is not implemented"} } // CreateUser creates a new user in authentik Idp and sends an invitation. diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 3c1f4c327a4..f3758743da5 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -9,6 +9,14 @@ import ( "github.com/netbirdio/netbird/management/server/telemetry" ) +type Error struct { + message string +} + +func (e *Error) Error() string { + return e.message +} + // Manager idp manager interface type Manager interface { UpdateUserAppMetadata(userId string, appMetadata AppMetadata) error From 8293c95b1b485475d31ec6102129e742097a7298 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Sep 2023 17:12:35 +0300 Subject: [PATCH 08/27] wip: load user account into cache for idp with no GetAccount support --- management/server/account.go | 39 ++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index d08e25fd41f..74b2bdfb305 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -951,9 +951,12 @@ func (am *DefaultAccountManager) warmupIDPCache() error { for _, user := range account.Users { // fetch user info with idp manager + // verify the validity of this user before adding to the cache userData, err := am.idpManager.GetUserDataByID(user.Id, idp.AppMetadata{WTAccountID: account.Id}) if err != nil { - return err + // delete/do not add user to the cache + log.Debugf("user with id: %s does not exist", user.Id) + continue } // update user metadata as are not stored in upstream idp @@ -1026,6 +1029,9 @@ func (am *DefaultAccountManager) addAccountIDToIDPAppMeta(userID string, account err = am.idpManager.UpdateUserAppMetadata(userID, idp.AppMetadata{WTAccountID: account.Id}) if err != nil { + // TODO: with self hosted with idp which do not implement this method + // store the attributes and tobe available in next cache refresh. + // FIX: unable to access dashboard after sending user invite. return status.Errorf(status.Internal, "updating user's app metadata failed with: %v", err) } // refresh cache to reflect the update @@ -1039,7 +1045,36 @@ func (am *DefaultAccountManager) addAccountIDToIDPAppMeta(userID string, account func (am *DefaultAccountManager) loadAccount(_ context.Context, accountID interface{}) ([]*idp.UserData, error) { log.Debugf("account %s not found in cache, reloading", accountID) - return am.idpManager.GetAccount(fmt.Sprintf("%v", accountID)) + account, err := am.idpManager.GetAccount(fmt.Sprintf("%v", accountID)) + if err != nil { + var idpErr *idp.Error + if errors.As(err, &idpErr) { + acc, err := am.Store.GetAccount(fmt.Sprintf("%v", accountID)) + if err != nil { + return nil, err + } + + data := make([]*idp.UserData, 0) + + for _, user := range acc.Users { + // fetch user info with idp manager + userData, err := am.idpManager.GetUserDataByID(user.Id, idp.AppMetadata{}) + if err != nil { + return nil, err + } + + // update user metadata as are not stored in upstream idp + // Note (self-hosted does not support pendingInvite and invitedBy status) + userData.AppMetadata.WTAccountID = fmt.Sprintf("%v", accountID) + data = append(data, userData) + } + + return account, nil + } + return nil, err + } + + return account, nil } func (am *DefaultAccountManager) lookupUserInCacheByEmail(email string, accountID string) (*idp.UserData, error) { From b36bcbcd164c7a62baaa91cfbacd1319e668457d Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 20 Sep 2023 11:53:35 +0300 Subject: [PATCH 09/27] Add DeleteUser method to JumpCloud IdP manager. --- management/server/idp/jumpcloud.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index 7ebbef33932..777ad467360 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -314,6 +314,29 @@ func (jm *JumpCloudManager) InviteUserByID(_ string) error { return fmt.Errorf("method InviteUserByID not implemented") } +// DeleteUser from jumpCloud directory +func (jm *JumpCloudManager) DeleteUser(userID string) error { + authCtx := jm.authenticationContext() + _, resp, err := jm.client.SystemusersApi.SystemusersDelete(authCtx, userID, contentType, accept, nil) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountRequestStatusError() + } + return fmt.Errorf("unable to delete user, statusCode %d", resp.StatusCode) + } + + if jm.appMetrics != nil { + jm.appMetrics.IDPMetrics().CountDeleteUser() + } + + return nil +} + // parseJumpCloudUser parse JumpCloud system user returned from API V1 to UserData. func parseJumpCloudUser(user v1.Systemuserreturn) *UserData { appMetadata := AppMetadata{} From 519f18bbad46386c09de53775335429d37c5ae96 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Fri, 22 Sep 2023 15:19:07 +0300 Subject: [PATCH 10/27] Add compatibility for the IDP lacking AppMetadata update capabilities --- management/server/account.go | 79 ++++++++---------------------------- 1 file changed, 18 insertions(+), 61 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 72ba73d7da4..c466dfcb372 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" b64 "encoding/base64" - "errors" "fmt" "hash/crc32" "math/rand" @@ -926,42 +925,29 @@ func (am *DefaultAccountManager) newAccount(userID, domain string) (*Account, er func (am *DefaultAccountManager) warmupIDPCache() error { userData, err := am.idpManager.GetAllAccounts() if err != nil { - var idpErr *idp.Error - if errors.As(err, &idpErr) { - log.Warnf("failed to fetch all accounts from idp: %v", idpErr) - - log.Info("warming cache using store") - accounts := am.Store.GetAllAccounts() - - for _, account := range accounts { - data := make([]*idp.UserData, 0) - - for _, user := range account.Users { - // fetch user info with idp manager - // verify the validity of this user before adding to the cache - userData, err := am.idpManager.GetUserDataByID(user.Id, idp.AppMetadata{WTAccountID: account.Id}) - if err != nil { - // delete/do not add user to the cache - log.Debugf("user with id: %s does not exist", user.Id) - continue - } + return err + } - // update user metadata as are not stored in upstream idp - // Note (self-hosted does not support pendingInvite and invitedBy status) - userData.AppMetadata.WTAccountID = account.Id - data = append(data, userData) + // If the Identity Provider does not support writing AppMetadata, + // in cases like this, we expect it to return all users in an "unset" field. + // We iterate over the users in the "unset" field, look up their AccountID in our store, and + // update their AppMetadata with the AccountID. + if unsetData, ok := userData["unset"]; ok { + for _, user := range unsetData { + accountID, err := am.Store.GetAccountByUser(user.ID) + if err == nil { + data := userData[accountID.Id] + if data == nil { + data = make([]*idp.UserData, 0) } - err = am.cacheManager.Set(am.ctx, account.Id, data, cacheStore.WithExpiration(cacheEntryExpiration())) - if err != nil { - return err - } + user.AppMetadata.WTAccountID = accountID.Id + + userData[accountID.Id] = append(data, user) } - return nil } - - return err } + delete(userData, "unset") for accountID, users := range userData { err = am.cacheManager.Set(am.ctx, accountID, users, cacheStore.WithExpiration(cacheEntryExpiration())) @@ -1032,36 +1018,7 @@ func (am *DefaultAccountManager) addAccountIDToIDPAppMeta(userID string, account func (am *DefaultAccountManager) loadAccount(_ context.Context, accountID interface{}) ([]*idp.UserData, error) { log.Debugf("account %s not found in cache, reloading", accountID) - account, err := am.idpManager.GetAccount(fmt.Sprintf("%v", accountID)) - if err != nil { - var idpErr *idp.Error - if errors.As(err, &idpErr) { - acc, err := am.Store.GetAccount(fmt.Sprintf("%v", accountID)) - if err != nil { - return nil, err - } - - data := make([]*idp.UserData, 0) - - for _, user := range acc.Users { - // fetch user info with idp manager - userData, err := am.idpManager.GetUserDataByID(user.Id, idp.AppMetadata{}) - if err != nil { - return nil, err - } - - // update user metadata as are not stored in upstream idp - // Note (self-hosted does not support pendingInvite and invitedBy status) - userData.AppMetadata.WTAccountID = fmt.Sprintf("%v", accountID) - data = append(data, userData) - } - - return account, nil - } - return nil, err - } - - return account, nil + return am.idpManager.GetAccount(fmt.Sprintf("%v", accountID)) } func (am *DefaultAccountManager) lookupUserInCacheByEmail(email string, accountID string) (*idp.UserData, error) { From a3f6de011502bf9344ac786259d3be2a49040e12 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Fri, 22 Sep 2023 15:20:32 +0300 Subject: [PATCH 11/27] Refactor Authentik IdP manager --- management/server/idp/authentik.go | 230 ++++++++--------------------- 1 file changed, 58 insertions(+), 172 deletions(-) diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index cf670685400..fcbcea96426 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -210,49 +210,8 @@ func (ac *AuthentikCredentials) Authenticate() (JWTToken, error) { } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (am *AuthentikManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - //ctx, err := am.authenticationContext() - //if err != nil { - // return err - //} - // - //userPk, err := strconv.ParseInt(userID, 10, 32) - //if err != nil { - // return err - //} - // - //var pendingInvite bool - //if appMetadata.WTPendingInvite != nil { - // pendingInvite = *appMetadata.WTPendingInvite - //} - // - //patchedUserReq := api.PatchedUserRequest{ - // Attributes: map[string]interface{}{ - // wtAccountID: appMetadata.WTAccountID, - // wtPendingInvite: pendingInvite, - // }, - //} - //_, resp, err := am.apiClient.CoreApi.CoreUsersPartialUpdate(ctx, int32(userPk)). - // PatchedUserRequest(patchedUserReq). - // Execute() - //if err != nil { - // return err - //} - //defer resp.Body.Close() - // - //if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - //} - // - //if resp.StatusCode != http.StatusOK { - // if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountRequestStatusError() - // } - // return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) - //} - // - //return nil - return &Error{"UpdateUserAppMetadata is not implemented"} +func (am *AuthentikManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { + return nil } // GetUserDataByID requests user data from authentik via ID. @@ -284,135 +243,86 @@ func (am *AuthentikManager) GetUserDataByID(userID string, appMetadata AppMetada return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) } - return parseAuthentikUser(*user) + userData := parseAuthentikUser(*user) + userData.AppMetadata = appMetadata + + return userData, nil } // GetAccount returns all the users for a given profile. func (am *AuthentikManager) GetAccount(accountID string) ([]*UserData, error) { - //ctx, err := am.authenticationContext() - //if err != nil { - // return nil, err - //} - // - //accountFilter := fmt.Sprintf("{%q:%q}", wtAccountID, accountID) - //userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Attributes(accountFilter).Execute() - //if err != nil { - // return nil, err - //} - //defer resp.Body.Close() - // - //if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountGetAccount() - //} - // - //if resp.StatusCode != http.StatusOK { - // if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountRequestStatusError() - // } - // return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) - //} - // - //users := make([]*UserData, 0) - //for _, user := range userList.Results { - // userData, err := parseAuthentikUser(user) - // if err != nil { - // return nil, err - // } - // users = append(users, userData) - //} - // - //return users, nil - return nil, &Error{"GetAccount is not implemented"} -} - -// GetAllAccounts gets all registered accounts with corresponding user data. -// It returns a list of users indexed by accountID. -func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { - //ctx, err := am.authenticationContext() - //if err != nil { - // return nil, err - //} - // - //userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() - //if err != nil { - // return nil, err - //} - //defer resp.Body.Close() - // - //if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountGetAllAccounts() - //} - // - //if resp.StatusCode != http.StatusOK { - // if am.appMetrics != nil { - // am.appMetrics.IDPMetrics().CountRequestStatusError() - // } - // return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) - //} - // - //indexedUsers := make(map[string][]*UserData) - //for _, user := range userList.Results { - // userData, err := parseAuthentikUser(user) - // if err != nil { - // return nil, err - // } - // - // accountID := userData.AppMetadata.WTAccountID - // if accountID != "" { - // if _, ok := indexedUsers[accountID]; !ok { - // indexedUsers[accountID] = make([]*UserData, 0) - // } - // indexedUsers[accountID] = append(indexedUsers[accountID], userData) - // } - //} - // - //return indexedUsers, nil - - return nil, &Error{"GetAllAccounts is not implemented"} -} - -// CreateUser creates a new user in authentik Idp and sends an invitation. -func (am *AuthentikManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { ctx, err := am.authenticationContext() if err != nil { return nil, err } - groupID, err := am.getUserGroupByName("netbird") + userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() if err != nil { return nil, err } + defer resp.Body.Close() - defaultBoolValue := true - createUserRequest := api.UserRequest{ - Email: &email, - Name: name, - IsActive: &defaultBoolValue, - Groups: []string{groupID}, - Username: email, - Attributes: map[string]interface{}{ - wtAccountID: accountID, - wtPendingInvite: &defaultBoolValue, - }, + if am.appMetrics != nil { + am.appMetrics.IDPMetrics().CountGetAccount() + } + + if resp.StatusCode != http.StatusOK { + if am.appMetrics != nil { + am.appMetrics.IDPMetrics().CountRequestStatusError() + } + return nil, fmt.Errorf("unable to get account %s users, statusCode %d", accountID, resp.StatusCode) + } + + users := make([]*UserData, 0) + for _, user := range userList.Results { + userData := parseAuthentikUser(user) + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) + } + + return users, nil +} + +// GetAllAccounts gets all registered accounts with corresponding user data. +// It returns a list of users indexed by accountID. +func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { + ctx, err := am.authenticationContext() + if err != nil { + return nil, err } - user, resp, err := am.apiClient.CoreApi.CoreUsersCreate(ctx).UserRequest(createUserRequest).Execute() + + userList, resp, err := am.apiClient.CoreApi.CoreUsersList(ctx).Execute() if err != nil { return nil, err } defer resp.Body.Close() if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountCreateUser() + am.appMetrics.IDPMetrics().CountGetAllAccounts() } - if resp.StatusCode != http.StatusCreated { + if resp.StatusCode != http.StatusOK { if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountRequestStatusError() } - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) + return nil, fmt.Errorf("unable to get all accounts, statusCode %d", resp.StatusCode) } - return parseAuthentikUser(*user) + indexedUsers := make(map[string][]*UserData) + for _, user := range userList.Results { + userData := parseAuthentikUser(user) + + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], userData) + } + + return indexedUsers, nil +} + +// CreateUser creates a new user in authentik Idp and sends an invitation. +func (am *AuthentikManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -442,11 +352,7 @@ func (am *AuthentikManager) GetUserByEmail(email string) ([]*UserData, error) { users := make([]*UserData, 0) for _, user := range userList.Results { - userData, err := parseAuthentikUser(user) - if err != nil { - return nil, err - } - users = append(users, userData) + users = append(users, parseAuthentikUser(user)) } return users, nil @@ -539,30 +445,10 @@ func (am *AuthentikManager) getUserGroupByName(name string) (string, error) { return group.Pk, nil } -func parseAuthentikUser(user api.User) (*UserData, error) { - var attributes struct { - AccountID string `json:"wt_account_id"` - PendingInvite bool `json:"wt_pending_invite"` - } - - helper := JsonParser{} - buf, err := helper.Marshal(user.Attributes) - if err != nil { - return nil, err - } - - err = helper.Unmarshal(buf, &attributes) - if err != nil { - return nil, err - } - +func parseAuthentikUser(user api.User) *UserData { return &UserData{ Email: *user.Email, Name: user.Name, ID: strconv.FormatInt(int64(user.Pk), 10), - AppMetadata: AppMetadata{ - WTAccountID: attributes.AccountID, - WTPendingInvite: &attributes.PendingInvite, - }, - }, nil + } } From 7aa72f6ab658a9d668b3c998ef35898815c88497 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Fri, 22 Sep 2023 17:11:02 +0300 Subject: [PATCH 12/27] cleanup --- management/server/account.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index c466dfcb372..dce0a9d81e1 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1002,9 +1002,6 @@ func (am *DefaultAccountManager) addAccountIDToIDPAppMeta(userID string, account err = am.idpManager.UpdateUserAppMetadata(userID, idp.AppMetadata{WTAccountID: account.Id}) if err != nil { - // TODO: with self hosted with idp which do not implement this method - // store the attributes and tobe available in next cache refresh. - // FIX: unable to access dashboard after sending user invite. return status.Errorf(status.Internal, "updating user's app metadata failed with: %v", err) } // refresh cache to reflect the update From 62d585302f441e5d51b026cc875be4d579a3bb61 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 25 Sep 2023 18:29:19 +0300 Subject: [PATCH 13/27] Refactor Zitadel IDP manager --- management/server/idp/zitadel.go | 249 +++----------------------- management/server/idp/zitadel_test.go | 117 +----------- 2 files changed, 27 insertions(+), 339 deletions(-) diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go index e42b9a5064b..f1c6f1579e3 100644 --- a/management/server/idp/zitadel.go +++ b/management/server/idp/zitadel.go @@ -1,13 +1,10 @@ package idp import ( - "encoding/base64" - "encoding/json" "fmt" "io" "net/http" "net/url" - "strconv" "strings" "sync" "time" @@ -68,12 +65,6 @@ type zitadelUser struct { type zitadelAttributes map[string][]map[string]any -// zitadelMetadata holds additional user data. -type zitadelMetadata struct { - Key string `json:"key"` - Value string `json:"value"` -} - // zitadelProfile represents an zitadel user profile response. type zitadelProfile struct { ID string `json:"id"` @@ -82,7 +73,6 @@ type zitadelProfile struct { PreferredLoginName string `json:"preferredLoginName"` LoginNames []string `json:"loginNames"` Human *zitadelUser `json:"human"` - Metadata []zitadelMetadata } // NewZitadelManager creates a new instance of the ZitadelManager. @@ -235,42 +225,8 @@ func (zc *ZitadelCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in zitadel Idp and sends an invite. -func (zm *ZitadelManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - payload, err := buildZitadelCreateUserRequestPayload(email, name) - if err != nil { - return nil, err - } - - body, err := zm.post("users/human/_import", payload) - if err != nil { - return nil, err - } - - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountCreateUser() - } - - var result struct { - UserID string `json:"userId"` - } - err = zm.helper.Unmarshal(body, &result) - if err != nil { - return nil, err - } - - invite := true - appMetadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - // Add metadata to new user - err = zm.UpdateUserAppMetadata(result.UserID, appMetadata) - if err != nil { - return nil, err - } - - return zm.GetUserDataByID(result.UserID, appMetadata) +func (zm *ZitadelManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -308,12 +264,6 @@ func (zm *ZitadelManager) GetUserByEmail(email string) ([]*UserData, error) { users := make([]*UserData, 0) for _, profile := range profiles.Result { - metadata, err := zm.getUserMetadata(profile.ID) - if err != nil { - return nil, err - } - profile.Metadata = metadata - users = append(users, profile.userData()) } @@ -337,18 +287,15 @@ func (zm *ZitadelManager) GetUserDataByID(userID string, appMetadata AppMetadata return nil, err } - metadata, err := zm.getUserMetadata(userID) - if err != nil { - return nil, err - } - profile.User.Metadata = metadata + userData := profile.User.userData() + userData.AppMetadata = appMetadata - return profile.User.userData(), nil + return userData, nil } // GetAccount returns all the users for a given profile. -func (zm *ZitadelManager) GetAccount(accountID string) ([]*UserData, error) { - accounts, err := zm.GetAllAccounts() +func (zm *ZitadelManager) GetAccount(_ string) ([]*UserData, error) { + body, err := zm.post("users/_search", "") if err != nil { return nil, err } @@ -357,7 +304,18 @@ func (zm *ZitadelManager) GetAccount(accountID string) ([]*UserData, error) { zm.appMetrics.IDPMetrics().CountGetAccount() } - return accounts[accountID], nil + var profiles struct{ Result []zitadelProfile } + err = zm.helper.Unmarshal(body, &profiles) + if err != nil { + return nil, err + } + + users := make([]*UserData, 0) + for _, profile := range profiles.Result { + users = append(users, profile.userData()) + } + + return users, nil } // GetAllAccounts gets all registered accounts with corresponding user data. @@ -380,22 +338,8 @@ func (zm *ZitadelManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Result { - // fetch user metadata - metadata, err := zm.getUserMetadata(profile.ID) - if err != nil { - return nil, err - } - profile.Metadata = metadata - - userData := profile.userData() - accountID := userData.AppMetadata.WTAccountID - - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], profile.userData()) } return indexedUsers, nil @@ -403,42 +347,7 @@ func (zm *ZitadelManager) GetAllAccounts() (map[string][]*UserData, error) { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. // Metadata values are base64 encoded. -func (zm *ZitadelManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - if appMetadata.WTPendingInvite == nil { - appMetadata.WTPendingInvite = new(bool) - } - pendingInviteBuf := strconv.AppendBool([]byte{}, *appMetadata.WTPendingInvite) - - wtAccountIDValue := base64.StdEncoding.EncodeToString([]byte(appMetadata.WTAccountID)) - wtPendingInviteValue := base64.StdEncoding.EncodeToString(pendingInviteBuf) - - metadata := zitadelAttributes{ - "metadata": { - { - "key": wtAccountID, - "value": wtAccountIDValue, - }, - { - "key": wtPendingInvite, - "value": wtPendingInviteValue, - }, - }, - } - payload, err := zm.helper.Marshal(metadata) - if err != nil { - return err - } - - resource := fmt.Sprintf("users/%s/metadata/_bulk", userID) - _, err = zm.post(resource, string(payload)) - if err != nil { - return err - } - - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - +func (zm *ZitadelManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -460,24 +369,6 @@ func (zm *ZitadelManager) DeleteUser(userID string) error { } return nil - -} - -// getUserMetadata requests user metadata from zitadel via ID. -func (zm *ZitadelManager) getUserMetadata(userID string) ([]zitadelMetadata, error) { - resource := fmt.Sprintf("users/%s/metadata/_search", userID) - body, err := zm.post(resource, "") - if err != nil { - return nil, err - } - - var metadata struct{ Result []zitadelMetadata } - err = zm.helper.Unmarshal(body, &metadata) - if err != nil { - return nil, err - } - - return metadata.Result, nil } // post perform Post requests. @@ -517,38 +408,7 @@ func (zm *ZitadelManager) post(resource string, body string) ([]byte, error) { } // delete perform Delete requests. -func (zm *ZitadelManager) delete(resource string) error { - jwtToken, err := zm.credentials.Authenticate() - if err != nil { - return err - } - - reqURL := fmt.Sprintf("%s/%s", zm.managementEndpoint, resource) - req, err := http.NewRequest(http.MethodDelete, reqURL, nil) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - resp, err := zm.httpClient.Do(req) - if err != nil { - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountRequestError() - } - - return err - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - if zm.appMetrics != nil { - zm.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return fmt.Errorf("unable to delete %s, statusCode %d", reqURL, resp.StatusCode) - } - +func (zm *ZitadelManager) delete(_ string) error { return nil } @@ -588,38 +448,13 @@ func (zm *ZitadelManager) get(resource string, q url.Values) ([]byte, error) { return io.ReadAll(resp.Body) } -// value returns string represented by the base64 string value. -func (zm zitadelMetadata) value() string { - value, err := base64.StdEncoding.DecodeString(zm.Value) - if err != nil { - return "" - } - - return string(value) -} - // userData construct user data from zitadel profile. func (zp zitadelProfile) userData() *UserData { var ( - email string - name string - wtAccountIDValue string - wtPendingInviteValue bool + email string + name string ) - for _, metadata := range zp.Metadata { - if metadata.Key == wtAccountID { - wtAccountIDValue = metadata.value() - } - - if metadata.Key == wtPendingInvite { - value, err := strconv.ParseBool(metadata.value()) - if err == nil { - wtPendingInviteValue = value - } - } - } - // Obtain the email for the human account and the login name, // for the machine account. if zp.Human != nil { @@ -636,39 +471,5 @@ func (zp zitadelProfile) userData() *UserData { Email: email, Name: name, ID: zp.ID, - AppMetadata: AppMetadata{ - WTAccountID: wtAccountIDValue, - WTPendingInvite: &wtPendingInviteValue, - }, - } -} - -func buildZitadelCreateUserRequestPayload(email string, name string) (string, error) { - var firstName, lastName string - - words := strings.Fields(name) - if n := len(words); n > 0 { - firstName = strings.Join(words[:n-1], " ") - lastName = words[n-1] } - - req := &zitadelUser{ - UserName: name, - Profile: zitadelUserInfo{ - FirstName: strings.TrimSpace(firstName), - LastName: strings.TrimSpace(lastName), - DisplayName: name, - }, - Email: zitadelEmail{ - Email: email, - IsEmailVerified: false, - }, - } - - str, err := json.Marshal(req) - if err != nil { - return "", err - } - - return string(str), nil } diff --git a/management/server/idp/zitadel_test.go b/management/server/idp/zitadel_test.go index b558bba733d..9931af7af4c 100644 --- a/management/server/idp/zitadel_test.go +++ b/management/server/idp/zitadel_test.go @@ -7,9 +7,10 @@ import ( "testing" "time" - "github.com/netbirdio/netbird/management/server/telemetry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/telemetry" ) func TestNewZitadelManager(t *testing.T) { @@ -296,98 +297,6 @@ func TestZitadelAuthenticate(t *testing.T) { } } -func TestZitadelUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: "{\"metadata\":[{\"key\":\"wt_account_id\",\"value\":\"b2s=\"},{\"key\":\"wt_pending_invite\",\"value\":\"ZmFsc2U=\"}]}", - appMetadata: appMetadata, - statusCode: 200, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: "{\"metadata\":[{\"key\":\"wt_account_id\",\"value\":\"b2s=\"},{\"key\":\"wt_pending_invite\",\"value\":\"dHJ1ZQ==\"}]}", - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 200, - helper: JsonParser{}, - managerCreds: &mockZitadelCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &ZitadelManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} - func TestZitadelProfile(t *testing.T) { type azureProfileTest struct { name string @@ -418,16 +327,6 @@ func TestZitadelProfile(t *testing.T) { IsEmailVerified: true, }, }, - Metadata: []zitadelMetadata{ - { - Key: "wt_account_id", - Value: "MQ==", - }, - { - Key: "wt_pending_invite", - Value: "ZmFsc2U=", - }, - }, }, expectedUserData: UserData{ ID: "test1", @@ -451,16 +350,6 @@ func TestZitadelProfile(t *testing.T) { "machine", }, Human: nil, - Metadata: []zitadelMetadata{ - { - Key: "wt_account_id", - Value: "MQ==", - }, - { - Key: "wt_pending_invite", - Value: "dHJ1ZQ==", - }, - }, }, expectedUserData: UserData{ ID: "test2", @@ -480,8 +369,6 @@ func TestZitadelProfile(t *testing.T) { assert.Equal(t, testCase.expectedUserData.ID, userData.ID, "User id should match") assert.Equal(t, testCase.expectedUserData.Email, userData.Email, "User email should match") assert.Equal(t, testCase.expectedUserData.Name, userData.Name, "User name should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTAccountID, userData.AppMetadata.WTAccountID, "Account id should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTPendingInvite, userData.AppMetadata.WTPendingInvite, "Pending invite should match") }) } } From 7706319aab474bfba5932092b7a5c5946e2def08 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 25 Sep 2023 19:22:48 +0300 Subject: [PATCH 14/27] Refactor Keycloak IDP manager --- management/server/idp/keycloak.go | 263 ++++--------------------- management/server/idp/keycloak_test.go | 114 ----------- 2 files changed, 33 insertions(+), 344 deletions(-) diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index d65a78ae3f0..bf9a5ecd67c 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -1,12 +1,10 @@ package idp import ( - "encoding/json" "fmt" "io" "net/http" "net/url" - "path" "strconv" "strings" "sync" @@ -51,28 +49,10 @@ type KeycloakCredentials struct { appMetrics telemetry.AppMetrics } -// keycloakUserCredential describe the authentication method for, -// newly created user profile. -type keycloakUserCredential struct { - Type string `json:"type"` - Value string `json:"value"` - Temporary bool `json:"temporary"` -} - // keycloakUserAttributes holds additional user data fields. type keycloakUserAttributes map[string][]string -// createUserRequest is a user create request. -type keycloakCreateUserRequest struct { - Email string `json:"email"` - Username string `json:"username"` - Enabled bool `json:"enabled"` - EmailVerified bool `json:"emailVerified"` - Credentials []keycloakUserCredential `json:"credentials"` - Attributes keycloakUserAttributes `json:"attributes"` -} - -// keycloakProfile represents an keycloak user profile response. +// keycloakProfile represents a keycloak user profile response. type keycloakProfile struct { ID string `json:"id"` CreatedTimestamp int64 `json:"createdTimestamp"` @@ -230,62 +210,8 @@ func (kc *KeycloakCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in keycloak Idp and sends an invite. -func (km *KeycloakManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - jwtToken, err := km.credentials.Authenticate() - if err != nil { - return nil, err - } - - invite := true - appMetadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - payloadString, err := buildKeycloakCreateUserRequestPayload(email, name, appMetadata) - if err != nil { - return nil, err - } - - reqURL := fmt.Sprintf("%s/users", km.adminEndpoint) - payload := strings.NewReader(payloadString) - - req, err := http.NewRequest(http.MethodPost, reqURL, payload) - if err != nil { - return nil, err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountCreateUser() - } - - resp, err := km.httpClient.Do(req) - if err != nil { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestError() - } - - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) - } - - locationHeader := resp.Header.Get("location") - userID, err := extractUserIDFromLocationHeader(locationHeader) - if err != nil { - return nil, err - } - - return km.GetUserDataByID(userID, appMetadata) +func (km *KeycloakManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -319,7 +245,7 @@ func (km *KeycloakManager) GetUserByEmail(email string) ([]*UserData, error) { } // GetUserDataByID requests user data from keycloak via ID. -func (km *KeycloakManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { +func (km *KeycloakManager) GetUserDataByID(userID string, _ AppMetadata) (*UserData, error) { body, err := km.get("users/"+userID, nil) if err != nil { return nil, err @@ -338,12 +264,9 @@ func (km *KeycloakManager) GetUserDataByID(userID string, appMetadata AppMetadat return profile.userData(), nil } -// GetAccount returns all the users for a given profile. +// GetAccount returns all the users for a given account profile. func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { - q := url.Values{} - q.Add("q", wtAccountID+":"+accountID) - - body, err := km.get("users", q) + profiles, err := km.fetchAllUserProfiles() if err != nil { return nil, err } @@ -352,15 +275,12 @@ func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { km.appMetrics.IDPMetrics().CountGetAccount() } - profiles := make([]keycloakProfile, 0) - err = km.helper.Unmarshal(body, &profiles) - if err != nil { - return nil, err - } - users := make([]*UserData, 0) for _, profile := range profiles { - users = append(users, profile.userData()) + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } return users, nil @@ -369,15 +289,7 @@ func (km *KeycloakManager) GetAccount(accountID string) ([]*UserData, error) { // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (km *KeycloakManager) GetAllAccounts() (map[string][]*UserData, error) { - totalUsers, err := km.totalUsersCount() - if err != nil { - return nil, err - } - - q := url.Values{} - q.Add("max", fmt.Sprint(*totalUsers)) - - body, err := km.get("users", q) + profiles, err := km.fetchAllUserProfiles() if err != nil { return nil, err } @@ -386,78 +298,17 @@ func (km *KeycloakManager) GetAllAccounts() (map[string][]*UserData, error) { km.appMetrics.IDPMetrics().CountGetAllAccounts() } - profiles := make([]keycloakProfile, 0) - err = km.helper.Unmarshal(body, &profiles) - if err != nil { - return nil, err - } - indexedUsers := make(map[string][]*UserData) for _, profile := range profiles { - userData := profile.userData() - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], profile.userData()) } return indexedUsers, nil } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (km *KeycloakManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - jwtToken, err := km.credentials.Authenticate() - if err != nil { - return err - } - - attrs := keycloakUserAttributes{} - attrs.Set(wtAccountID, appMetadata.WTAccountID) - if appMetadata.WTPendingInvite != nil { - attrs.Set(wtPendingInvite, strconv.FormatBool(*appMetadata.WTPendingInvite)) - } else { - attrs.Set(wtPendingInvite, "false") - } - - reqURL := fmt.Sprintf("%s/users/%s", km.adminEndpoint, userID) - data, err := km.helper.Marshal(map[string]any{ - "attributes": attrs, - }) - if err != nil { - return err - } - payload := strings.NewReader(string(data)) - - req, err := http.NewRequest(http.MethodPut, reqURL, payload) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - log.Debugf("updating IdP metadata for user %s", userID) - - resp, err := km.httpClient.Do(req) - if err != nil { - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountRequestError() - } - return err - } - defer resp.Body.Close() - - if km.appMetrics != nil { - km.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusNoContent { - return fmt.Errorf("unable to update the appMetadata, statusCode %d", resp.StatusCode) - } - +func (km *KeycloakManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -467,7 +318,7 @@ func (km *KeycloakManager) InviteUserByID(_ string) error { return fmt.Errorf("method InviteUserByID not implemented") } -// DeleteUser from Keycloack +// DeleteUser from Keycloak by user ID. func (km *KeycloakManager) DeleteUser(userID string) error { jwtToken, err := km.credentials.Authenticate() if err != nil { @@ -475,7 +326,6 @@ func (km *KeycloakManager) DeleteUser(userID string) error { } reqURL := fmt.Sprintf("%s/users/%s", km.adminEndpoint, url.QueryEscape(userID)) - req, err := http.NewRequest(http.MethodDelete, reqURL, nil) if err != nil { return err @@ -508,32 +358,27 @@ func (km *KeycloakManager) DeleteUser(userID string) error { return nil } -func buildKeycloakCreateUserRequestPayload(email string, name string, appMetadata AppMetadata) (string, error) { - attrs := keycloakUserAttributes{} - attrs.Set(wtAccountID, appMetadata.WTAccountID) - attrs.Set(wtPendingInvite, strconv.FormatBool(*appMetadata.WTPendingInvite)) - - req := &keycloakCreateUserRequest{ - Email: email, - Username: name, - Enabled: true, - EmailVerified: true, - Credentials: []keycloakUserCredential{ - { - Type: "password", - Value: GeneratePassword(8, 1, 1, 1), - Temporary: false, - }, - }, - Attributes: attrs, - } - - str, err := json.Marshal(req) +func (km *KeycloakManager) fetchAllUserProfiles() ([]keycloakProfile, error) { + totalUsers, err := km.totalUsersCount() + if err != nil { + return nil, err + } + + q := url.Values{} + q.Add("max", fmt.Sprint(*totalUsers)) + + body, err := km.get("users", q) + if err != nil { + return nil, err + } + + profiles := make([]keycloakProfile, 0) + err = km.helper.Unmarshal(body, &profiles) if err != nil { - return "", err + return nil, err } - return string(str), nil + return profiles, nil } // get perform Get requests. @@ -588,53 +433,11 @@ func (km *KeycloakManager) totalUsersCount() (*int, error) { return &count, nil } -// extractUserIDFromLocationHeader extracts the user ID from the location, -// header once the user is created successfully -func extractUserIDFromLocationHeader(locationHeader string) (string, error) { - userURL, err := url.Parse(locationHeader) - if err != nil { - return "", err - } - - return path.Base(userURL.Path), nil -} - // userData construct user data from keycloak profile. func (kp keycloakProfile) userData() *UserData { - accountID := kp.Attributes.Get(wtAccountID) - pendingInvite, err := strconv.ParseBool(kp.Attributes.Get(wtPendingInvite)) - if err != nil { - pendingInvite = false - } - return &UserData{ Email: kp.Email, Name: kp.Username, ID: kp.ID, - AppMetadata: AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &pendingInvite, - }, - } -} - -// Set sets the key to value. It replaces any existing -// values. -func (ka keycloakUserAttributes) Set(key, value string) { - ka[key] = []string{value} -} - -// Get returns the first value associated with the given key. -// If there are no values associated with the key, Get returns -// the empty string. -func (ka keycloakUserAttributes) Get(key string) string { - if ka == nil { - return "" - } - - values := ka[key] - if len(values) == 0 { - return "" } - return values[0] } diff --git a/management/server/idp/keycloak_test.go b/management/server/idp/keycloak_test.go index 0c33fc13754..9b6c1d3c63c 100644 --- a/management/server/idp/keycloak_test.go +++ b/management/server/idp/keycloak_test.go @@ -84,15 +84,6 @@ func TestNewKeycloakManager(t *testing.T) { } } -type mockKeycloakCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockKeycloakCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestKeycloakRequestJWTToken(t *testing.T) { type requestJWTTokenTest struct { @@ -316,108 +307,3 @@ func TestKeycloakAuthenticate(t *testing.T) { }) } } - -func TestKeycloakUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Status Code", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"false\"]}}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"false\"]}}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase5 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: fmt.Sprintf("{\"attributes\":{\"wt_account_id\":[\"%s\"],\"wt_pending_invite\":[\"true\"]}}", appMetadata.WTAccountID), - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockKeycloakCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4, updateUserAppMetadataTestCase5} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &KeycloakManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} From 5adebea24a07255735d75d7025f4b900caeed269 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 26 Sep 2023 12:00:36 +0300 Subject: [PATCH 15/27] Refactor Okta IDP manager --- management/server/idp/okta.go | 173 +++-------------------------- management/server/idp/okta_test.go | 44 ++------ 2 files changed, 25 insertions(+), 192 deletions(-) diff --git a/management/server/idp/okta.go b/management/server/idp/okta.go index 0e93c494c54..83ac6aacd34 100644 --- a/management/server/idp/okta.go +++ b/management/server/idp/okta.go @@ -8,9 +8,9 @@ import ( "strings" "time" - "github.com/netbirdio/netbird/management/server/telemetry" "github.com/okta/okta-sdk-golang/v2/okta" - "github.com/okta/okta-sdk-golang/v2/okta/query" + + "github.com/netbirdio/netbird/management/server/telemetry" ) // OktaManager okta manager client instance. @@ -76,11 +76,6 @@ func NewOktaManager(config OktaClientConfig, appMetrics telemetry.AppMetrics) (* return nil, err } - err = updateUserProfileSchema(client) - if err != nil { - return nil, err - } - credentials := &OktaCredentials{ clientConfig: config, httpClient: httpClient, @@ -103,49 +98,8 @@ func (oc *OktaCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in okta Idp and sends an invitation. -func (om *OktaManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - var ( - sendEmail = true - activate = true - userProfile = okta.UserProfile{ - "email": email, - "login": email, - wtAccountID: accountID, - wtPendingInvite: true, - } - ) - - fields := strings.Fields(name) - if n := len(fields); n > 0 { - userProfile["firstName"] = strings.Join(fields[:n-1], " ") - userProfile["lastName"] = fields[n-1] - } - - user, resp, err := om.client.User.CreateUser(context.Background(), - okta.CreateUserRequest{ - Profile: &userProfile, - }, - &query.Params{ - Activate: &activate, - SendEmail: &sendEmail, - }, - ) - if err != nil { - return nil, err - } - - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountCreateUser() - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to create user, statusCode %d", resp.StatusCode) - } - - return parseOktaUser(user) +func (om *OktaManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserDataByID requests user data from keycloak via ID. @@ -166,7 +120,13 @@ func (om *OktaManager) GetUserDataByID(userID string, appMetadata AppMetadata) ( return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode) } - return parseOktaUser(user) + userData, err := parseOktaUser(user) + if err != nil { + return nil, err + } + userData.AppMetadata = appMetadata + + return userData, nil } // GetUserByEmail searches users with a given email. @@ -200,8 +160,7 @@ func (om *OktaManager) GetUserByEmail(email string) ([]*UserData, error) { // GetAccount returns all the users for a given profile. func (om *OktaManager) GetAccount(accountID string) ([]*UserData, error) { - search := fmt.Sprintf("profile.wt_account_id eq %q", accountID) - users, resp, err := om.client.User.ListUsers(context.Background(), &query.Params{Search: search}) + users, resp, err := om.client.User.ListUsers(context.Background(), nil) if err != nil { return nil, err } @@ -223,6 +182,7 @@ func (om *OktaManager) GetAccount(accountID string) ([]*UserData, error) { if err != nil { return nil, err } + userData.AppMetadata.WTAccountID = accountID list = append(list, userData) } @@ -256,13 +216,8 @@ func (om *OktaManager) GetAllAccounts() (map[string][]*UserData, error) { return nil, err } - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], userData) } return indexedUsers, nil @@ -270,46 +225,6 @@ func (om *OktaManager) GetAllAccounts() (map[string][]*UserData, error) { // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. func (om *OktaManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - user, resp, err := om.client.User.GetUser(context.Background(), userID) - if err != nil { - return err - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user, statusCode %d", resp.StatusCode) - } - - profile := *user.Profile - - if appMetadata.WTPendingInvite != nil { - profile[wtPendingInvite] = *appMetadata.WTPendingInvite - } - - if appMetadata.WTAccountID != "" { - profile[wtAccountID] = appMetadata.WTAccountID - } - - user.Profile = &profile - _, resp, err = om.client.User.UpdateUser(context.Background(), userID, *user, nil) - if err != nil { - fmt.Println(err.Error()) - return err - } - - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusOK { - if om.appMetrics != nil { - om.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user, statusCode %d", resp.StatusCode) - } - return nil } @@ -341,60 +256,12 @@ func (om *OktaManager) DeleteUser(userID string) error { return nil } -// updateUserProfileSchema updates the Okta user schema to include custom fields, -// wt_account_id and wt_pending_invite. -func updateUserProfileSchema(client *okta.Client) error { - // Ensure Okta doesn't enforce user input for these fields, as they are solely used by Netbird - userPermissions := []*okta.UserSchemaAttributePermission{{Action: "HIDE", Principal: "SELF"}} - - _, resp, err := client.UserSchema.UpdateUserProfile( - context.Background(), - "default", - okta.UserSchema{ - Definitions: &okta.UserSchemaDefinitions{ - Custom: &okta.UserSchemaPublic{ - Id: "#custom", - Type: "object", - Properties: map[string]*okta.UserSchemaAttribute{ - wtAccountID: { - MaxLength: 100, - MinLength: 1, - Required: new(bool), - Scope: "NONE", - Title: "Wt Account Id", - Type: "string", - Permissions: userPermissions, - }, - wtPendingInvite: { - Required: new(bool), - Scope: "NONE", - Title: "Wt Pending Invite", - Type: "boolean", - Permissions: userPermissions, - }, - }, - }, - }, - }) - if err != nil { - return err - } - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("unable to update user profile schema, statusCode %d", resp.StatusCode) - } - - return nil -} - // parseOktaUserToUserData parse okta user to UserData. func parseOktaUser(user *okta.User) (*UserData, error) { var oktaUser struct { - Email string `json:"email"` - FirstName string `json:"firstName"` - LastName string `json:"lastName"` - AccountID string `json:"wt_account_id"` - PendingInvite bool `json:"wt_pending_invite"` + Email string `json:"email"` + FirstName string `json:"firstName"` + LastName string `json:"lastName"` } if user == nil { @@ -418,9 +285,5 @@ func parseOktaUser(user *okta.User) (*UserData, error) { Email: oktaUser.Email, Name: strings.Join([]string{oktaUser.FirstName, oktaUser.LastName}, " "), ID: user.Id, - AppMetadata: AppMetadata{ - WTAccountID: oktaUser.AccountID, - WTPendingInvite: &oktaUser.PendingInvite, - }, }, nil } diff --git a/management/server/idp/okta_test.go b/management/server/idp/okta_test.go index 02c28b3aefb..20df246f82a 100644 --- a/management/server/idp/okta_test.go +++ b/management/server/idp/okta_test.go @@ -1,31 +1,28 @@ package idp import ( + "testing" + "github.com/okta/okta-sdk-golang/v2/okta" "github.com/stretchr/testify/assert" - "testing" ) func TestParseOktaUser(t *testing.T) { type parseOktaUserTest struct { name string - invite bool inputProfile *okta.User expectedUserData *UserData assertErrFunc assert.ErrorAssertionFunc } parseOktaTestCase1 := parseOktaUserTest{ - name: "Good Request", - invite: true, + name: "Good Request", inputProfile: &okta.User{ Id: "123", Profile: &okta.UserProfile{ - "email": "test@example.com", - "firstName": "John", - "lastName": "Doe", - "wt_account_id": "456", - "wt_pending_invite": true, + "email": "test@example.com", + "firstName": "John", + "lastName": "Doe", }, }, expectedUserData: &UserData{ @@ -41,36 +38,17 @@ func TestParseOktaUser(t *testing.T) { parseOktaTestCase2 := parseOktaUserTest{ name: "Invalid okta user", - invite: true, inputProfile: nil, expectedUserData: nil, assertErrFunc: assert.Error, } - parseOktaTestCase3 := parseOktaUserTest{ - name: "Invalid pending invite type", - invite: false, - inputProfile: &okta.User{ - Id: "123", - Profile: &okta.UserProfile{ - "email": "test@example.com", - "firstName": "John", - "lastName": "Doe", - "wt_account_id": "456", - "wt_pending_invite": "true", - }, - }, - expectedUserData: nil, - assertErrFunc: assert.Error, - } - - for _, testCase := range []parseOktaUserTest{parseOktaTestCase1, parseOktaTestCase2, parseOktaTestCase3} { + for _, testCase := range []parseOktaUserTest{parseOktaTestCase1, parseOktaTestCase2} { t.Run(testCase.name, func(t *testing.T) { userData, err := parseOktaUser(testCase.inputProfile) testCase.assertErrFunc(t, err, testCase.assertErrFunc) if err == nil { - testCase.expectedUserData.AppMetadata.WTPendingInvite = &testCase.invite assert.True(t, userDataEqual(testCase.expectedUserData, userData), "user data should match") } }) @@ -83,13 +61,5 @@ func userDataEqual(a, b *UserData) bool { if a.Email != b.Email || a.Name != b.Name || a.ID != b.ID { return false } - if a.AppMetadata.WTAccountID != b.AppMetadata.WTAccountID { - return false - } - - if a.AppMetadata.WTPendingInvite != nil && b.AppMetadata.WTPendingInvite != nil && - *a.AppMetadata.WTPendingInvite != *b.AppMetadata.WTPendingInvite { - return false - } return true } From c10ebdb227fa200da360b2cd39015af0f116fd1f Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 26 Sep 2023 12:27:23 +0300 Subject: [PATCH 16/27] Refactor Azure IDP manager --- management/server/idp/azure.go | 300 +++------------------------- management/server/idp/azure_test.go | 159 +-------------- 2 files changed, 34 insertions(+), 425 deletions(-) diff --git a/management/server/idp/azure.go b/management/server/idp/azure.go index 22e6825ae82..0870c821563 100644 --- a/management/server/idp/azure.go +++ b/management/server/idp/azure.go @@ -1,7 +1,6 @@ package idp import ( - "encoding/json" "fmt" "io" "net/http" @@ -11,19 +10,13 @@ import ( "time" "github.com/golang-jwt/jwt" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" -) - -const ( - // azure extension properties template - wtAccountIDTpl = "extension_%s_wt_account_id" - wtPendingInviteTpl = "extension_%s_wt_pending_invite" - profileFields = "id,displayName,mail,userPrincipalName" - extensionFields = "id,name,targetObjects" + "github.com/netbirdio/netbird/management/server/telemetry" ) +const profileFields = "id,displayName,mail,userPrincipalName" + // AzureManager azure manager client instance. type AzureManager struct { ClientID string @@ -58,21 +51,6 @@ type AzureCredentials struct { // azureProfile represents an azure user profile. type azureProfile map[string]any -// passwordProfile represent authentication method for, -// newly created user profile. -type passwordProfile struct { - ForceChangePasswordNextSignIn bool `json:"forceChangePasswordNextSignIn"` - Password string `json:"password"` -} - -// azureExtension represent custom attribute, -// that can be added to user objects in Azure Active Directory (AD). -type azureExtension struct { - Name string `json:"name"` - DataType string `json:"dataType"` - TargetObjects []string `json:"targetObjects"` -} - // NewAzureManager creates a new instance of the AzureManager. func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) (*AzureManager, error) { httpTransport := http.DefaultTransport.(*http.Transport).Clone() @@ -115,7 +93,7 @@ func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) appMetrics: appMetrics, } - manager := &AzureManager{ + return &AzureManager{ ObjectID: config.ObjectID, ClientID: config.ClientID, GraphAPIEndpoint: config.GraphAPIEndpoint, @@ -123,14 +101,7 @@ func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) credentials: credentials, helper: helper, appMetrics: appMetrics, - } - - err := manager.configureAppMetadata() - if err != nil { - return nil, err - } - - return manager, nil + }, nil } // jwtStillValid returns true if the token still valid and have enough time to be used and get a response from azure. @@ -236,44 +207,14 @@ func (ac *AzureCredentials) Authenticate() (JWTToken, error) { } // CreateUser creates a new user in azure AD Idp. -func (am *AzureManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - payload, err := buildAzureCreateUserRequestPayload(email, name, accountID, am.ClientID) - if err != nil { - return nil, err - } - - body, err := am.post("users", payload) - if err != nil { - return nil, err - } - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountCreateUser() - } - - var profile azureProfile - err = am.helper.Unmarshal(body, &profile) - if err != nil { - return nil, err - } - - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - profile[wtAccountIDField] = accountID - - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - profile[wtPendingInviteField] = true - - return profile.userData(am.ClientID), nil +func (am *AzureManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserDataByID requests user data from keycloak via ID. func (am *AzureManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users/"+userID, q) if err != nil { @@ -290,18 +231,17 @@ func (am *AzureManager) GetUserDataByID(userID string, appMetadata AppMetadata) return nil, err } - return profile.userData(am.ClientID), nil + userData := profile.userData() + userData.AppMetadata = appMetadata + + return userData, nil } // GetUserByEmail searches users with a given email. // If no users have been found, this function returns an empty list. func (am *AzureManager) GetUserByEmail(email string) ([]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users/"+email, q) if err != nil { @@ -319,20 +259,15 @@ func (am *AzureManager) GetUserByEmail(email string) ([]*UserData, error) { } users := make([]*UserData, 0) - users = append(users, profile.userData(am.ClientID)) + users = append(users, profile.userData()) return users, nil } // GetAccount returns all the users for a given profile. func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) - q.Add("$filter", fmt.Sprintf("%s eq '%s'", wtAccountIDField, accountID)) + q.Add("$select", profileFields) body, err := am.get("users", q) if err != nil { @@ -351,7 +286,10 @@ func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { users := make([]*UserData, 0) for _, profile := range profiles.Value { - users = append(users, profile.userData(am.ClientID)) + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } return users, nil @@ -360,12 +298,8 @@ func (am *AzureManager) GetAccount(accountID string) ([]*UserData, error) { // GetAllAccounts gets all registered accounts with corresponding user data. // It returns a list of users indexed by accountID. func (am *AzureManager) GetAllAccounts() (map[string][]*UserData, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - selectFields := strings.Join([]string{profileFields, wtAccountIDField, wtPendingInviteField}, ",") - q := url.Values{} - q.Add("$select", selectFields) + q.Add("$select", profileFields) body, err := am.get("users", q) if err != nil { @@ -384,67 +318,17 @@ func (am *AzureManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Value { - userData := profile.userData(am.ClientID) - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + userData := profile.userData() + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], userData) } return indexedUsers, nil } // UpdateUserAppMetadata updates user app metadata based on userID. -func (am *AzureManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - jwtToken, err := am.credentials.Authenticate() - if err != nil { - return err - } - - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - - data, err := am.helper.Marshal(map[string]any{ - wtAccountIDField: appMetadata.WTAccountID, - wtPendingInviteField: appMetadata.WTPendingInvite, - }) - if err != nil { - return err - } - payload := strings.NewReader(string(data)) - - reqURL := fmt.Sprintf("%s/users/%s", am.GraphAPIEndpoint, userID) - req, err := http.NewRequest(http.MethodPatch, reqURL, payload) - if err != nil { - return err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - log.Debugf("updating idp metadata for user %s", userID) - - resp, err := am.httpClient.Do(req) - if err != nil { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestError() - } - return err - } - defer resp.Body.Close() - - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - - if resp.StatusCode != http.StatusNoContent { - return fmt.Errorf("unable to update the appMetadata, statusCode %d", resp.StatusCode) - } - +func (am *AzureManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -454,7 +338,7 @@ func (am *AzureManager) InviteUserByID(_ string) error { return fmt.Errorf("method InviteUserByID not implemented") } -// DeleteUser from Azure +// DeleteUser from Azure. func (am *AzureManager) DeleteUser(userID string) error { jwtToken, err := am.credentials.Authenticate() if err != nil { @@ -491,81 +375,6 @@ func (am *AzureManager) DeleteUser(userID string) error { return nil } -func (am *AzureManager) getUserExtensions() ([]azureExtension, error) { - q := url.Values{} - q.Add("$select", extensionFields) - - resource := fmt.Sprintf("applications/%s/extensionProperties", am.ObjectID) - body, err := am.get(resource, q) - if err != nil { - return nil, err - } - - var extensions struct{ Value []azureExtension } - err = am.helper.Unmarshal(body, &extensions) - if err != nil { - return nil, err - } - - return extensions.Value, nil -} - -func (am *AzureManager) createUserExtension(name string) (*azureExtension, error) { - extension := azureExtension{ - Name: name, - DataType: "string", - TargetObjects: []string{"User"}, - } - - payload, err := am.helper.Marshal(extension) - if err != nil { - return nil, err - } - - resource := fmt.Sprintf("applications/%s/extensionProperties", am.ObjectID) - body, err := am.post(resource, string(payload)) - if err != nil { - return nil, err - } - - var userExtension azureExtension - err = am.helper.Unmarshal(body, &userExtension) - if err != nil { - return nil, err - } - - return &userExtension, nil -} - -// configureAppMetadata sets up app metadata extensions if they do not exists. -func (am *AzureManager) configureAppMetadata() error { - wtAccountIDField := extensionName(wtAccountIDTpl, am.ClientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, am.ClientID) - - extensions, err := am.getUserExtensions() - if err != nil { - return err - } - - // If the wt_account_id extension does not already exist, create it. - if !hasExtension(extensions, wtAccountIDField) { - _, err = am.createUserExtension(wtAccountID) - if err != nil { - return err - } - } - - // If the wt_pending_invite extension does not already exist, create it. - if !hasExtension(extensions, wtPendingInviteField) { - _, err = am.createUserExtension(wtPendingInvite) - if err != nil { - return err - } - } - - return nil -} - // get perform Get requests. func (am *AzureManager) get(resource string, q url.Values) ([]byte, error) { jwtToken, err := am.credentials.Authenticate() @@ -639,7 +448,7 @@ func (am *AzureManager) post(resource string, body string) ([]byte, error) { } // userData construct user data from keycloak profile. -func (ap azureProfile) userData(clientID string) *UserData { +func (ap azureProfile) userData() *UserData { id, ok := ap["id"].(string) if !ok { id = "" @@ -655,66 +464,9 @@ func (ap azureProfile) userData(clientID string) *UserData { name = "" } - accountIDField := extensionName(wtAccountIDTpl, clientID) - accountID, ok := ap[accountIDField].(string) - if !ok { - accountID = "" - } - - pendingInviteField := extensionName(wtPendingInviteTpl, clientID) - pendingInvite, ok := ap[pendingInviteField].(bool) - if !ok { - pendingInvite = false - } - return &UserData{ Email: email, Name: name, ID: id, - AppMetadata: AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &pendingInvite, - }, - } -} - -func buildAzureCreateUserRequestPayload(email, name, accountID, clientID string) (string, error) { - wtAccountIDField := extensionName(wtAccountIDTpl, clientID) - wtPendingInviteField := extensionName(wtPendingInviteTpl, clientID) - - req := &azureProfile{ - "accountEnabled": true, - "displayName": name, - "mailNickName": strings.Join(strings.Split(name, " "), ""), - "userPrincipalName": email, - "passwordProfile": passwordProfile{ - ForceChangePasswordNextSignIn: true, - Password: GeneratePassword(8, 1, 1, 1), - }, - wtAccountIDField: accountID, - wtPendingInviteField: true, - } - - str, err := json.Marshal(req) - if err != nil { - return "", err - } - - return string(str), nil -} - -func extensionName(extensionTpl, clientID string) string { - clientID = strings.ReplaceAll(clientID, "-", "") - return fmt.Sprintf(extensionTpl, clientID) -} - -// hasExtension checks whether a given extension by name, -// exists in an list of extensions. -func hasExtension(extensions []azureExtension, name string) bool { - for _, ext := range extensions { - if ext.Name == name { - return true - } } - return false } diff --git a/management/server/idp/azure_test.go b/management/server/idp/azure_test.go index 9d845ffbeef..fee8c45a0a8 100644 --- a/management/server/idp/azure_test.go +++ b/management/server/idp/azure_test.go @@ -124,206 +124,63 @@ func TestAzureAuthenticate(t *testing.T) { } } -func TestAzureUpdateUserAppMetadata(t *testing.T) { - type updateUserAppMetadataTest struct { - name string - inputReqBody string - expectedReqBody string - appMetadata AppMetadata - statusCode int - helper ManagerHelper - managerCreds ManagerCredentials - assertErrFunc assert.ErrorAssertionFunc - assertErrFuncMessage string - } - - appMetadata := AppMetadata{WTAccountID: "ok"} - - updateUserAppMetadataTestCase1 := updateUserAppMetadataTest{ - name: "Bad Authentication", - expectedReqBody: "", - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - err: fmt.Errorf("error"), - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase2 := updateUserAppMetadataTest{ - name: "Bad Status Code", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":null}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 400, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase3 := updateUserAppMetadataTest{ - name: "Bad Response Parsing", - statusCode: 400, - helper: &mockJsonParser{marshalErrorString: "error"}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.Error, - assertErrFuncMessage: "should return error", - } - - updateUserAppMetadataTestCase4 := updateUserAppMetadataTest{ - name: "Good request", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":null}", appMetadata.WTAccountID), - appMetadata: appMetadata, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - invite := true - updateUserAppMetadataTestCase5 := updateUserAppMetadataTest{ - name: "Update Pending Invite", - expectedReqBody: fmt.Sprintf("{\"extension__wt_account_id\":\"%s\",\"extension__wt_pending_invite\":true}", appMetadata.WTAccountID), - appMetadata: AppMetadata{ - WTAccountID: "ok", - WTPendingInvite: &invite, - }, - statusCode: 204, - helper: JsonParser{}, - managerCreds: &mockAzureCredentials{ - jwtToken: JWTToken{}, - }, - assertErrFunc: assert.NoError, - assertErrFuncMessage: "shouldn't return error", - } - - for _, testCase := range []updateUserAppMetadataTest{updateUserAppMetadataTestCase1, updateUserAppMetadataTestCase2, - updateUserAppMetadataTestCase3, updateUserAppMetadataTestCase4, updateUserAppMetadataTestCase5} { - t.Run(testCase.name, func(t *testing.T) { - reqClient := mockHTTPClient{ - resBody: testCase.inputReqBody, - code: testCase.statusCode, - } - - manager := &AzureManager{ - httpClient: &reqClient, - credentials: testCase.managerCreds, - helper: testCase.helper, - } - - err := manager.UpdateUserAppMetadata("1", testCase.appMetadata) - testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage) - - assert.Equal(t, testCase.expectedReqBody, reqClient.reqBody, "request body should match") - }) - } -} - func TestAzureProfile(t *testing.T) { type azureProfileTest struct { name string - clientID string invite bool inputProfile azureProfile expectedUserData UserData } azureProfileTestCase1 := azureProfileTest{ - name: "Good Request", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, + name: "Good Request", + invite: false, inputProfile: azureProfile{ "id": "test1", "displayName": "John Doe", "userPrincipalName": "test1@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": false, }, expectedUserData: UserData{ Email: "test1@test.com", Name: "John Doe", ID: "test1", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, }, } azureProfileTestCase2 := azureProfileTest{ - name: "Missing User ID", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: true, + name: "Missing User ID", + invite: true, inputProfile: azureProfile{ "displayName": "John Doe", "userPrincipalName": "test2@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": true, }, expectedUserData: UserData{ Email: "test2@test.com", Name: "John Doe", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, }, } azureProfileTestCase3 := azureProfileTest{ - name: "Missing User Name", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, + name: "Missing User Name", + invite: false, inputProfile: azureProfile{ "id": "test3", "userPrincipalName": "test3@test.com", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_account_id": "1", - "extension_25d0b095048440d29fd303f8f4abbb3c_wt_pending_invite": false, }, expectedUserData: UserData{ ID: "test3", Email: "test3@test.com", - AppMetadata: AppMetadata{ - WTAccountID: "1", - }, - }, - } - - azureProfileTestCase4 := azureProfileTest{ - name: "Missing Extension Fields", - clientID: "25d0b095-0484-40d2-9fd3-03f8f4abbb3c", - invite: false, - inputProfile: azureProfile{ - "id": "test4", - "displayName": "John Doe", - "userPrincipalName": "test4@test.com", - }, - expectedUserData: UserData{ - ID: "test4", - Name: "John Doe", - Email: "test4@test.com", - AppMetadata: AppMetadata{}, }, } - for _, testCase := range []azureProfileTest{azureProfileTestCase1, azureProfileTestCase2, azureProfileTestCase3, azureProfileTestCase4} { + for _, testCase := range []azureProfileTest{azureProfileTestCase1, azureProfileTestCase2, azureProfileTestCase3} { t.Run(testCase.name, func(t *testing.T) { testCase.expectedUserData.AppMetadata.WTPendingInvite = &testCase.invite - userData := testCase.inputProfile.userData(testCase.clientID) + userData := testCase.inputProfile.userData() assert.Equal(t, testCase.expectedUserData.ID, userData.ID, "User id should match") assert.Equal(t, testCase.expectedUserData.Email, userData.Email, "User email should match") assert.Equal(t, testCase.expectedUserData.Name, userData.Name, "User name should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTAccountID, userData.AppMetadata.WTAccountID, "Account id should match") - assert.Equal(t, testCase.expectedUserData.AppMetadata.WTPendingInvite, userData.AppMetadata.WTPendingInvite, "Pending invite should match") }) } } From de1fa8b26b62cc7bb42e58cf986d8797aea2f921 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 26 Sep 2023 12:28:06 +0300 Subject: [PATCH 17/27] Remove unused types declarations --- management/server/idp/azure_test.go | 9 --------- management/server/idp/zitadel.go | 7 +++++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/management/server/idp/azure_test.go b/management/server/idp/azure_test.go index fee8c45a0a8..b4dc96b23cd 100644 --- a/management/server/idp/azure_test.go +++ b/management/server/idp/azure_test.go @@ -8,15 +8,6 @@ import ( "github.com/stretchr/testify/assert" ) -type mockAzureCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockAzureCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestAzureJwtStillValid(t *testing.T) { type jwtStillValidTest struct { name string diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go index f1c6f1579e3..cc1ded91c32 100644 --- a/management/server/idp/zitadel.go +++ b/management/server/idp/zitadel.go @@ -294,7 +294,7 @@ func (zm *ZitadelManager) GetUserDataByID(userID string, appMetadata AppMetadata } // GetAccount returns all the users for a given profile. -func (zm *ZitadelManager) GetAccount(_ string) ([]*UserData, error) { +func (zm *ZitadelManager) GetAccount(accountID string) ([]*UserData, error) { body, err := zm.post("users/_search", "") if err != nil { return nil, err @@ -312,7 +312,10 @@ func (zm *ZitadelManager) GetAccount(_ string) ([]*UserData, error) { users := make([]*UserData, 0) for _, profile := range profiles.Result { - users = append(users, profile.userData()) + userData := profile.userData() + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } return users, nil From 1c4e6b24139f9763f25f2fd6bba30788eb5798a1 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 26 Sep 2023 13:40:23 +0300 Subject: [PATCH 18/27] Refactor Google Workspace IdP manager --- management/server/idp/google_workspace.go | 170 +++------------------- 1 file changed, 21 insertions(+), 149 deletions(-) diff --git a/management/server/idp/google_workspace.go b/management/server/idp/google_workspace.go index 40854e59860..eee15f663d1 100644 --- a/management/server/idp/google_workspace.go +++ b/management/server/idp/google_workspace.go @@ -5,15 +5,14 @@ import ( "encoding/base64" "fmt" "net/http" - "strings" "time" - "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" "golang.org/x/oauth2/google" admin "google.golang.org/api/admin/directory/v1" - "google.golang.org/api/googleapi" "google.golang.org/api/option" + + "github.com/netbirdio/netbird/management/server/telemetry" ) // GoogleWorkspaceManager Google Workspace manager client instance. @@ -73,17 +72,13 @@ func NewGoogleWorkspaceManager(config GoogleWorkspaceClientConfig, appMetrics te } service, err := admin.NewService(context.Background(), - option.WithScopes(admin.AdminDirectoryUserScope, admin.AdminDirectoryUserschemaScope), + option.WithScopes(admin.AdminDirectoryUserScope), option.WithCredentials(adminCredentials), ) if err != nil { return nil, err } - if err = configureAppMetadataSchema(service, config.CustomerID); err != nil { - return nil, err - } - return &GoogleWorkspaceManager{ usersService: service.Users, CustomerID: config.CustomerID, @@ -95,27 +90,7 @@ func NewGoogleWorkspaceManager(config GoogleWorkspaceClientConfig, appMetrics te } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (gm *GoogleWorkspaceManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - metadata, err := gm.helper.Marshal(appMetadata) - if err != nil { - return err - } - - user := &admin.User{ - CustomSchemas: map[string]googleapi.RawMessage{ - "app_metadata": metadata, - }, - } - - _, err = gm.usersService.Update(userID, user).Do() - if err != nil { - return err - } - - if gm.appMetrics != nil { - gm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - +func (gm *GoogleWorkspaceManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } @@ -130,23 +105,23 @@ func (gm *GoogleWorkspaceManager) GetUserDataByID(userID string, appMetadata App gm.appMetrics.IDPMetrics().CountGetUserDataByID() } - return parseGoogleWorkspaceUser(user) + userData := parseGoogleWorkspaceUser(user) + userData.AppMetadata = appMetadata + + return userData, nil } // GetAccount returns all the users for a given profile. func (gm *GoogleWorkspaceManager) GetAccount(accountID string) ([]*UserData, error) { - query := fmt.Sprintf("app_metadata.wt_account_id=\"%s\"", accountID) - usersList, err := gm.usersService.List().Customer(gm.CustomerID).Query(query).Projection("full").Do() + usersList, err := gm.usersService.List().Customer(gm.CustomerID).Projection("full").Do() if err != nil { return nil, err } usersData := make([]*UserData, 0) for _, user := range usersList.Users { - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } + userData := parseGoogleWorkspaceUser(user) + userData.AppMetadata.WTAccountID = accountID usersData = append(usersData, userData) } @@ -168,61 +143,16 @@ func (gm *GoogleWorkspaceManager) GetAllAccounts() (map[string][]*UserData, erro indexedUsers := make(map[string][]*UserData) for _, user := range usersList.Users { - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], parseGoogleWorkspaceUser(user)) } return indexedUsers, nil } // CreateUser creates a new user in Google Workspace and sends an invitation. -func (gm *GoogleWorkspaceManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - invite := true - metadata := AppMetadata{ - WTAccountID: accountID, - WTPendingInvite: &invite, - } - - username := &admin.UserName{} - fields := strings.Fields(name) - if n := len(fields); n > 0 { - username.GivenName = strings.Join(fields[:n-1], " ") - username.FamilyName = fields[n-1] - } - - payload, err := gm.helper.Marshal(metadata) - if err != nil { - return nil, err - } - - user := &admin.User{ - Name: username, - PrimaryEmail: email, - CustomSchemas: map[string]googleapi.RawMessage{ - "app_metadata": payload, - }, - Password: GeneratePassword(8, 1, 1, 1), - } - user, err = gm.usersService.Insert(user).Do() - if err != nil { - return nil, err - } - - if gm.appMetrics != nil { - gm.appMetrics.IDPMetrics().CountCreateUser() - } - - return parseGoogleWorkspaceUser(user) +func (gm *GoogleWorkspaceManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -237,13 +167,8 @@ func (gm *GoogleWorkspaceManager) GetUserByEmail(email string) ([]*UserData, err gm.appMetrics.IDPMetrics().CountGetUserByEmail() } - userData, err := parseGoogleWorkspaceUser(user) - if err != nil { - return nil, err - } - users := make([]*UserData, 0) - users = append(users, userData) + users = append(users, parseGoogleWorkspaceUser(user)) return users, nil } @@ -281,7 +206,6 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) creds, err := google.CredentialsFromJSON( context.Background(), decodeKey, - admin.AdminDirectoryUserschemaScope, admin.AdminDirectoryUserScope, ) if err == nil { @@ -294,7 +218,6 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) creds, err = google.FindDefaultCredentials( context.Background(), - admin.AdminDirectoryUserschemaScope, admin.AdminDirectoryUserScope, ) if err != nil { @@ -304,62 +227,11 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) return creds, nil } -// configureAppMetadataSchema create a custom schema for managing app metadata fields in Google Workspace. -func configureAppMetadataSchema(service *admin.Service, customerID string) error { - schemaList, err := service.Schemas.List(customerID).Do() - if err != nil { - return err - } - - // checks if app_metadata schema is already created - for _, schema := range schemaList.Schemas { - if schema.SchemaName == "app_metadata" { - return nil - } - } - - // create new app_metadata schema - appMetadataSchema := &admin.Schema{ - SchemaName: "app_metadata", - Fields: []*admin.SchemaFieldSpec{ - { - FieldName: "wt_account_id", - FieldType: "STRING", - MultiValued: false, - }, - { - FieldName: "wt_pending_invite", - FieldType: "BOOL", - MultiValued: false, - }, - }, - } - _, err = service.Schemas.Insert(customerID, appMetadataSchema).Do() - if err != nil { - return err - } - - return nil -} - // parseGoogleWorkspaceUser parse google user to UserData. -func parseGoogleWorkspaceUser(user *admin.User) (*UserData, error) { - var appMetadata AppMetadata - - // Get app metadata from custom schemas - if user.CustomSchemas != nil { - rawMessage := user.CustomSchemas["app_metadata"] - helper := JsonParser{} - - if err := helper.Unmarshal(rawMessage, &appMetadata); err != nil { - return nil, err - } - } - +func parseGoogleWorkspaceUser(user *admin.User) *UserData { return &UserData{ - ID: user.Id, - Email: user.PrimaryEmail, - Name: user.Name.FullName, - AppMetadata: appMetadata, - }, nil + ID: user.Id, + Email: user.PrimaryEmail, + Name: user.Name.FullName, + } } From b155ee28f27ae514928d9b2d8e91b11e26c1724d Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 26 Sep 2023 15:13:07 +0300 Subject: [PATCH 19/27] Remove unused function and variables --- management/server/idp/authentik.go | 34 ------------------------- management/server/idp/azure.go | 36 --------------------------- management/server/idp/keycloak.go | 5 ---- management/server/idp/zitadel_test.go | 9 ------- 4 files changed, 84 deletions(-) diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index fcbcea96426..69eeec995fb 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -411,40 +411,6 @@ func (am *AuthentikManager) authenticationContext() (context.Context, error) { return context.WithValue(context.Background(), api.ContextAPIKeys, value), nil } -// getUserGroupByName retrieves the user group for assigning new users. -// If the group is not found, a new group with the specified name will be created. -func (am *AuthentikManager) getUserGroupByName(name string) (string, error) { - ctx, err := am.authenticationContext() - if err != nil { - return "", err - } - - groupList, resp, err := am.apiClient.CoreApi.CoreGroupsList(ctx).Name(name).Execute() - if err != nil { - return "", err - } - defer resp.Body.Close() - - if groupList != nil { - if len(groupList.Results) > 0 { - return groupList.Results[0].Pk, nil - } - } - - createGroupRequest := api.GroupRequest{Name: name} - group, resp, err := am.apiClient.CoreApi.CoreGroupsCreate(ctx).GroupRequest(createGroupRequest).Execute() - if err != nil { - return "", err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - return "", fmt.Errorf("unable to create user group, statusCode: %d", resp.StatusCode) - } - - return group.Pk, nil -} - func parseAuthentikUser(user api.User) *UserData { return &UserData{ Email: *user.Email, diff --git a/management/server/idp/azure.go b/management/server/idp/azure.go index 0870c821563..bf7ca6ba9b5 100644 --- a/management/server/idp/azure.go +++ b/management/server/idp/azure.go @@ -411,42 +411,6 @@ func (am *AzureManager) get(resource string, q url.Values) ([]byte, error) { return io.ReadAll(resp.Body) } -// post perform Post requests. -func (am *AzureManager) post(resource string, body string) ([]byte, error) { - jwtToken, err := am.credentials.Authenticate() - if err != nil { - return nil, err - } - - reqURL := fmt.Sprintf("%s/%s", am.GraphAPIEndpoint, resource) - req, err := http.NewRequest(http.MethodPost, reqURL, strings.NewReader(body)) - if err != nil { - return nil, err - } - req.Header.Add("authorization", "Bearer "+jwtToken.AccessToken) - req.Header.Add("content-type", "application/json") - - resp, err := am.httpClient.Do(req) - if err != nil { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestError() - } - - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - if am.appMetrics != nil { - am.appMetrics.IDPMetrics().CountRequestStatusError() - } - - return nil, fmt.Errorf("unable to post %s, statusCode %d", reqURL, resp.StatusCode) - } - - return io.ReadAll(resp.Body) -} - // userData construct user data from keycloak profile. func (ap azureProfile) userData() *UserData { id, ok := ap["id"].(string) diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index bf9a5ecd67c..0cef5c6ae27 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -16,11 +16,6 @@ import ( "github.com/netbirdio/netbird/management/server/telemetry" ) -const ( - wtAccountID = "wt_account_id" - wtPendingInvite = "wt_pending_invite" -) - // KeycloakManager keycloak manager client instance. type KeycloakManager struct { adminEndpoint string diff --git a/management/server/idp/zitadel_test.go b/management/server/idp/zitadel_test.go index 9931af7af4c..9a771b36a7c 100644 --- a/management/server/idp/zitadel_test.go +++ b/management/server/idp/zitadel_test.go @@ -64,15 +64,6 @@ func TestNewZitadelManager(t *testing.T) { } } -type mockZitadelCredentials struct { - jwtToken JWTToken - err error -} - -func (mc *mockZitadelCredentials) Authenticate() (JWTToken, error) { - return mc.jwtToken, mc.err -} - func TestZitadelRequestJWTToken(t *testing.T) { type requestJWTTokenTest struct { From cebf9d93a9af3031d6a70ed1851eb10155135ec6 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 27 Sep 2023 10:45:21 +0300 Subject: [PATCH 20/27] Initialize a user data slice with defined capacity --- management/server/account.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/account.go b/management/server/account.go index dce0a9d81e1..3247631e168 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -938,7 +938,7 @@ func (am *DefaultAccountManager) warmupIDPCache() error { if err == nil { data := userData[accountID.Id] if data == nil { - data = make([]*idp.UserData, 0) + data = make([]*idp.UserData, 1) } user.AppMetadata.WTAccountID = accountID.Id From 7c5c9a2a4290f4a034b89823f1c57987d482f68d Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 27 Sep 2023 12:00:28 +0300 Subject: [PATCH 21/27] Initialize a user data slice with defined capacity --- management/server/account.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index dce0a9d81e1..2ee664a653c 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -938,11 +938,10 @@ func (am *DefaultAccountManager) warmupIDPCache() error { if err == nil { data := userData[accountID.Id] if data == nil { - data = make([]*idp.UserData, 0) + data = make([]*idp.UserData, 0, 1) } user.AppMetadata.WTAccountID = accountID.Id - userData[accountID.Id] = append(data, user) } } From e04fb2e11a0e62f330d7084ab78a0fd81173590b Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Wed, 27 Sep 2023 12:02:05 +0300 Subject: [PATCH 22/27] Initialize a user data slice with defined capacity --- management/server/account.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/account.go b/management/server/account.go index 3247631e168..c340ea0734f 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -938,7 +938,7 @@ func (am *DefaultAccountManager) warmupIDPCache() error { if err == nil { data := userData[accountID.Id] if data == nil { - data = make([]*idp.UserData, 1) + data = make([]*idp.UserData, 0, 1) } user.AppMetadata.WTAccountID = accountID.Id From 940c09b40f64bf35bcfe19b02d83bdc9390201e7 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 2 Oct 2023 00:58:26 +0300 Subject: [PATCH 23/27] Refactor JumpCloud IdP with new user cache implementation for self-hosted IdP --- management/server/idp/jumpcloud.go | 189 +++-------------------------- 1 file changed, 20 insertions(+), 169 deletions(-) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index 777ad467360..ced8eb5777d 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -40,11 +40,6 @@ type JumpCloudCredentials struct { appMetrics telemetry.AppMetrics } -type JumpCloudAttribute struct { - Name string `json:"name"` - Value any `json:"value"` -} - // NewJumpCloudManager creates a new instance of the JumpCloudManager. func NewJumpCloudManager(config JumpCloudClientConfig, appMetrics telemetry.AppMetrics) (*JumpCloudManager, error) { httpTransport := http.DefaultTransport.(*http.Transport).Clone() @@ -90,43 +85,12 @@ func (jm *JumpCloudManager) authenticationContext() context.Context { } // UpdateUserAppMetadata updates user app metadata based on userID and metadata map. -func (jm *JumpCloudManager) UpdateUserAppMetadata(userID string, appMetadata AppMetadata) error { - authCtx := jm.authenticationContext() - user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) - if err != nil { - return err - } - defer resp.Body.Close() - - attributes := buildUserAttributesUpdatePayload(user, appMetadata) - updateReq := map[string]interface{}{ - "body": v1.Systemuserput{ - Attributes: attributes, - }, - } - - _, resp, err = jm.client.SystemusersApi.SystemusersPut(authCtx, userID, contentType, accept, updateReq) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - if jm.appMetrics != nil { - jm.appMetrics.IDPMetrics().CountRequestStatusError() - } - return fmt.Errorf("unable to update user %s, statusCode %d", userID, resp.StatusCode) - } - - if jm.appMetrics != nil { - jm.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() - } - +func (jm *JumpCloudManager) UpdateUserAppMetadata(_ string, _ AppMetadata) error { return nil } // GetUserDataByID requests user data from JumpCloud via ID. -func (jm *JumpCloudManager) GetUserDataByID(userID string, _ AppMetadata) (*UserData, error) { +func (jm *JumpCloudManager) GetUserDataByID(userID string, appMetadata AppMetadata) (*UserData, error) { authCtx := jm.authenticationContext() user, resp, err := jm.client.SystemusersApi.SystemusersGet(authCtx, userID, contentType, accept, nil) if err != nil { @@ -145,20 +109,16 @@ func (jm *JumpCloudManager) GetUserDataByID(userID string, _ AppMetadata) (*User jm.appMetrics.IDPMetrics().CountGetUserDataByID() } - return parseJumpCloudUser(user), nil + userData := parseJumpCloudUser(user) + userData.AppMetadata = appMetadata + + return userData, nil } // GetAccount returns all the users for a given profile. func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { - searchFilter := map[string]interface{}{ - "searchFilter": map[string]interface{}{ - "filter": []string{accountID}, - "fields": []string{"wtAccountID"}, - }, - } - authCtx := jm.authenticationContext() - userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, searchFilter) + userList, resp, err := jm.client.SearchApi.SearchSystemusersPost(authCtx, contentType, accept, nil) if err != nil { return nil, err } @@ -175,13 +135,15 @@ func (jm *JumpCloudManager) GetAccount(accountID string) ([]*UserData, error) { jm.appMetrics.IDPMetrics().CountGetAccount() } - usersData := make([]*UserData, 0) + users := make([]*UserData, 0) for _, user := range userList.Results { userData := parseJumpCloudUser(user) - usersData = append(usersData, userData) + userData.AppMetadata.WTAccountID = accountID + + users = append(users, userData) } - return usersData, nil + return users, nil } // GetAllAccounts gets all registered accounts with corresponding user data. @@ -207,68 +169,16 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, user := range userList.Results { - userData := parseJumpCloudUser(user) - - accountID := userData.AppMetadata.WTAccountID - if accountID != "" { - if _, ok := indexedUsers[accountID]; !ok { - indexedUsers[accountID] = make([]*UserData, 0) - } - indexedUsers[accountID] = append(indexedUsers[accountID], userData) - } + accountID := "unset" + indexedUsers[accountID] = append(indexedUsers[accountID], parseJumpCloudUser(user)) } return indexedUsers, nil } // CreateUser creates a new user in JumpCloud Idp and sends an invitation. -func (jm *JumpCloudManager) CreateUser(email, name, accountID, invitedByEmail string) (*UserData, error) { - var firstName, lastName string - - fields := strings.Fields(name) - if n := len(fields); n > 0 { - firstName = strings.Join(fields[:n-1], " ") - lastName = fields[n-1] - } - createUserReq := map[string]any{ - "body": v1.Systemuserputpost{ - Username: firstName, - Email: email, - Firstname: firstName, - Lastname: lastName, - Activated: true, - Attributes: []any{ - JumpCloudAttribute{ - Name: "wtAccountID", - Value: accountID, - }, - JumpCloudAttribute{ - Name: "wtPendingInvite", - Value: true, - }, - }, - }, - } - - authCtx := jm.authenticationContext() - user, resp, err := jm.client.SystemusersApi.SystemusersPost(authCtx, contentType, accept, createUserReq) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - if jm.appMetrics != nil { - jm.appMetrics.IDPMetrics().CountRequestStatusError() - } - return nil, fmt.Errorf("unable to create user %s, statusCode %d", email, resp.StatusCode) - } - - if jm.appMetrics != nil { - jm.appMetrics.IDPMetrics().CountCreateUser() - } - - return parseJumpCloudUser(user), nil +func (jm *JumpCloudManager) CreateUser(_, _, _, _ string) (*UserData, error) { + return nil, fmt.Errorf("method CreateUser not implemented") } // GetUserByEmail searches users with a given email. @@ -301,8 +211,7 @@ func (jm *JumpCloudManager) GetUserByEmail(email string) ([]*UserData, error) { usersData := make([]*UserData, 0) for _, user := range userList.Results { - userData := parseJumpCloudUser(user) - usersData = append(usersData, userData) + usersData = append(usersData, parseJumpCloudUser(user)) } return usersData, nil @@ -339,68 +248,10 @@ func (jm *JumpCloudManager) DeleteUser(userID string) error { // parseJumpCloudUser parse JumpCloud system user returned from API V1 to UserData. func parseJumpCloudUser(user v1.Systemuserreturn) *UserData { - appMetadata := AppMetadata{} names := []string{user.Firstname, user.Middlename, user.Lastname} - - for _, attribute := range user.Attributes { - if jcAttribute, ok := attribute.(map[string]any); ok { - if jcAttribute["name"] == "wtAccountID" { - appMetadata.WTAccountID = jcAttribute["value"].(string) - } - - if jcAttribute["name"] == "wtPendingInvite" { - if value, ok := jcAttribute["value"].(bool); ok { - appMetadata.WTPendingInvite = &value - } - } - } - } - return &UserData{ - Email: user.Email, - Name: strings.Join(names, " "), - ID: user.Id, - AppMetadata: appMetadata, - } -} - -// buildUserAttributesUpdatePayload constructs the updated user attributes based on system user and application metadata. -func buildUserAttributesUpdatePayload(user v1.Systemuserreturn, appMetadata AppMetadata) (userAttributes []interface{}) { - var isAccountIDFound, isPendingInviteFound bool - - for _, attribute := range user.Attributes { - if jcAttribute, ok := attribute.(JumpCloudAttribute); ok { - if jcAttribute.Name == "wtAccountID" { - jcAttribute.Value = appMetadata.WTAccountID - isAccountIDFound = true - } - - if jcAttribute.Name == "wtPendingInvite" { - if appMetadata.WTPendingInvite != nil { - jcAttribute.Value = appMetadata.WTPendingInvite - isPendingInviteFound = true - } - } - - attribute = jcAttribute - } - - userAttributes = append(userAttributes, attribute) + Email: user.Email, + Name: strings.Join(names, " "), + ID: user.Id, } - - if !isAccountIDFound { - userAttributes = append(userAttributes, JumpCloudAttribute{ - Name: "wtAccountID", - Value: appMetadata.WTAccountID, - }) - } - - if !isPendingInviteFound && appMetadata.WTPendingInvite != nil { - userAttributes = append(userAttributes, JumpCloudAttribute{ - Name: "wtPendingInvite", - Value: appMetadata.WTPendingInvite, - }) - } - - return userAttributes } From cc215aca941ce5fee6bf5597594ac4d054386ad8 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 2 Oct 2023 01:05:26 +0300 Subject: [PATCH 24/27] Remove unused custom error and refactor unset account id into constant --- management/server/idp/authentik.go | 4 +--- management/server/idp/azure.go | 4 +--- management/server/idp/google_workspace.go | 4 ++-- management/server/idp/idp.go | 10 +++------- management/server/idp/keycloak.go | 4 ++-- management/server/idp/okta.go | 3 +-- management/server/idp/zitadel.go | 4 ++-- 7 files changed, 12 insertions(+), 21 deletions(-) diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index 69eeec995fb..315333dd2da 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -312,9 +312,7 @@ func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, user := range userList.Results { userData := parseAuthentikUser(user) - - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], userData) + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil diff --git a/management/server/idp/azure.go b/management/server/idp/azure.go index bf7ca6ba9b5..068b96d7812 100644 --- a/management/server/idp/azure.go +++ b/management/server/idp/azure.go @@ -319,9 +319,7 @@ func (am *AzureManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Value { userData := profile.userData() - - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], userData) + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil diff --git a/management/server/idp/google_workspace.go b/management/server/idp/google_workspace.go index eee15f663d1..e7df67c1af2 100644 --- a/management/server/idp/google_workspace.go +++ b/management/server/idp/google_workspace.go @@ -143,8 +143,8 @@ func (gm *GoogleWorkspaceManager) GetAllAccounts() (map[string][]*UserData, erro indexedUsers := make(map[string][]*UserData) for _, user := range usersList.Users { - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], parseGoogleWorkspaceUser(user)) + userData := parseGoogleWorkspaceUser(user) + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 76eaade749b..3955d1d687b 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -9,13 +9,9 @@ import ( "github.com/netbirdio/netbird/management/server/telemetry" ) -type Error struct { - message string -} - -func (e *Error) Error() string { - return e.message -} +const ( + unsetAccountID = "unset" +) // Manager idp manager interface type Manager interface { diff --git a/management/server/idp/keycloak.go b/management/server/idp/keycloak.go index 0cef5c6ae27..c056116ae9f 100644 --- a/management/server/idp/keycloak.go +++ b/management/server/idp/keycloak.go @@ -295,8 +295,8 @@ func (km *KeycloakManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles { - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], profile.userData()) + userData := profile.userData() + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil diff --git a/management/server/idp/okta.go b/management/server/idp/okta.go index 83ac6aacd34..4790d8785d5 100644 --- a/management/server/idp/okta.go +++ b/management/server/idp/okta.go @@ -216,8 +216,7 @@ func (om *OktaManager) GetAllAccounts() (map[string][]*UserData, error) { return nil, err } - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], userData) + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go index cc1ded91c32..8540ecdb924 100644 --- a/management/server/idp/zitadel.go +++ b/management/server/idp/zitadel.go @@ -341,8 +341,8 @@ func (zm *ZitadelManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, profile := range profiles.Result { - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], profile.userData()) + userData := profile.userData() + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil From 21bf9b335afbc0627a99f6c80229a19155f65de4 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 2 Oct 2023 01:09:25 +0300 Subject: [PATCH 25/27] Refactor --- management/server/idp/jumpcloud.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index ced8eb5777d..0311e727c3f 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -169,8 +169,8 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, user := range userList.Results { - accountID := "unset" - indexedUsers[accountID] = append(indexedUsers[accountID], parseJumpCloudUser(user)) + userData := parseJumpCloudUser(user) + indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) } return indexedUsers, nil From 39acc49cc4c267184456a3c1f35abc91f9351e6c Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 3 Oct 2023 18:27:48 +0300 Subject: [PATCH 26/27] Update map key to correct constant --- management/server/idp/jumpcloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/idp/jumpcloud.go b/management/server/idp/jumpcloud.go index 0311e727c3f..0115b404982 100644 --- a/management/server/idp/jumpcloud.go +++ b/management/server/idp/jumpcloud.go @@ -170,7 +170,7 @@ func (jm *JumpCloudManager) GetAllAccounts() (map[string][]*UserData, error) { indexedUsers := make(map[string][]*UserData) for _, user := range userList.Results { userData := parseJumpCloudUser(user) - indexedUsers[unsetAccountID] = append(indexedUsers[unsetAccountID], userData) + indexedUsers[UnsetAccountID] = append(indexedUsers[UnsetAccountID], userData) } return indexedUsers, nil From fb368aedbb1678d9859bc2d54c339d14cf8f9076 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Tue, 3 Oct 2023 18:20:21 +0200 Subject: [PATCH 27/27] Add missing IDP manager check for jumpcloud --- management/server/idp/idp.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 3b6a633c89e..7adb76f4044 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -176,7 +176,11 @@ func NewManager(config Config, appMetrics telemetry.AppMetrics) (Manager, error) CustomerID: config.ExtraConfig["CustomerId"], } return NewGoogleWorkspaceManager(googleClientConfig, appMetrics) - + case "jumpcloud": + jumpcloudConfig := JumpCloudClientConfig{ + APIToken: config.ExtraConfig["ApiToken"], + } + return NewJumpCloudManager(jumpcloudConfig, appMetrics) default: return nil, fmt.Errorf("invalid manager type: %s", config.ManagerType) }