Merge pull request #57 from vshn/display-name-sync

feat: Sync display name to Keycloak

controllers/organization_controller.go

@@ -167,7 +167,7 @@ func buildKeycloakGroup(org *orgv1.Organization, memb *controlv1.OrganizationMem
 		groupMem = append(groupMem, u.Name)
 	}
 
-	return keycloak.NewGroup(org.Name).WithMemberNames(groupMem...)
+	return keycloak.NewGroup(org.Spec.DisplayName, org.Name).WithMemberNames(groupMem...)
 }
 
 // SetupWithManager sets up the controller with the Manager.

controllers/organization_controller_test.go

@@ -28,7 +28,7 @@ func Test_OrganizationController_Reconcile_Success(t *testing.T) {
 	ctx := context.Background()
 
 	c, keyMock, _ := prepareTest(t, fooOrg, fooMemb)
-	group := keycloak.NewGroup("foo").WithMemberNames("bar", "bar3")
+	group := keycloak.NewGroup("Foo Inc.", "foo").WithMemberNames("bar", "bar3")
 	keyMock.EXPECT().
 		PutGroup(gomock.Any(), group).
 		Return(group, nil).
@@ -59,7 +59,7 @@ func Test_OrganizationController_Reconcile_Failure(t *testing.T) {
 	ctx := context.Background()
 
 	c, keyMock, erMock := prepareTest(t, fooOrg, fooMemb)
-	group := keycloak.NewGroup("foo").WithMemberNames("bar", "bar3")
+	group := keycloak.NewGroup("Foo Inc.", "foo").WithMemberNames("bar", "bar3")
 	keyMock.EXPECT().
 		PutGroup(gomock.Any(), group).
 		Return(keycloak.Group{}, errors.New("create failed")).
@@ -95,7 +95,7 @@ func Test_OrganizationController_Reconcile_Member_Failure(t *testing.T) {
 	ctx := context.Background()
 
 	c, keyMock, erMock := prepareTest(t, fooOrg, fooMemb)
-	group := keycloak.NewGroup("foo").WithMemberNames("bar", "bar3")
+	group := keycloak.NewGroup("Foo Inc.", "foo").WithMemberNames("bar", "bar3")
 	keyMock.EXPECT().
 		PutGroup(gomock.Any(), group).
 		Return(keycloak.Group{}, &keycloak.MembershipSyncErrors{

controllers/periodic_syncer_test.go

@@ -45,12 +45,12 @@ func Test_Sync_Success(t *testing.T) {
 		},
 	)
 
-	barOrg := keycloak.NewGroup("bar")
+	barOrg := keycloak.NewGroup("Bar Inc.", "bar")
 	barOrg.Members = []keycloak.User{
 		{Username: "bar", DefaultOrganizationRef: "bar"},
 		{Username: "bar3", DefaultOrganizationRef: "bar-mss"},
 	}
-	barTeam := keycloak.NewGroup("bar", "bar-team")
+	barTeam := keycloak.NewGroup("Bar Team", "bar", "bar-team")
 	barTeam.Members = []keycloak.User{
 		{Username: "bar-tm-1"},
 		{Username: "bar-tm-2", DefaultOrganizationRef: "bar-outsourcing"},
@@ -135,8 +135,8 @@ func Test_Sync_Fail_Update(t *testing.T) {
 	// By not adding buzzMember manually we simulate an error while updating the members resource
 
 	groups := []keycloak.Group{
-		keycloak.NewGroup("buzz").WithMemberNames("buzz1", "buzz"),
-		keycloak.NewGroup("bar").WithMemberNames("bar", "bar3"),
+		keycloak.NewGroup("Buzz Inc.", "buzz").WithMemberNames("buzz1", "buzz"),
+		keycloak.NewGroup("Bar Inc.", "bar").WithMemberNames("bar", "bar3"),
 	}
 	keyMock.EXPECT().
 		ListGroups(gomock.Any()).
@@ -175,8 +175,8 @@ func Test_Sync_Skip_Existing(t *testing.T) {
 	c, keyMock, _ := prepareTest(t, fooOrg, fooMemb, barTeam) // We need to add barMember manually as there is no control API in the tests creating them
 
 	groups := []keycloak.Group{
-		keycloak.NewGroup("foo").WithMemberNames("foo", "foo2"),
-		keycloak.NewGroup("foo", "bar").WithMemberNames("updated-member-1", "updated-member-2"),
+		keycloak.NewGroup("Foo Inc.", "foo").WithMemberNames("foo", "foo2"),
+		keycloak.NewGroup("Foo Inc. Bar Team", "foo", "bar").WithMemberNames("updated-member-1", "updated-member-2"),
 	}
 	keyMock.EXPECT().
 		ListGroups(gomock.Any()).
@@ -226,7 +226,7 @@ func Test_Sync_Skip_ExistingUsers(t *testing.T) {
 
 	c, keyMock, _ := prepareTest(t, fooOrg, fooMemb, &subject)
 
-	fooGroup := keycloak.NewGroup("foo")
+	fooGroup := keycloak.NewGroup("Foo Inc.", "foo")
 	fooGroup.Members = []keycloak.User{
 		{
 			Username:               subject.Name,
@@ -257,8 +257,8 @@ func Test_Sync_Skip_UserInMultipleGroups(t *testing.T) {
 	keyMock.EXPECT().
 		ListGroups(gomock.Any()).
 		Return([]keycloak.Group{
-			keycloak.NewGroup("foo").WithMemberNames("in-multiple-groups"),
-			keycloak.NewGroup("foo", "bar").WithMemberNames("in-multiple-groups"),
+			keycloak.NewGroup("Foo Inc.", "foo").WithMemberNames("in-multiple-groups"),
+			keycloak.NewGroup("Foo Inc. Bar Team", "foo", "bar").WithMemberNames("in-multiple-groups"),
 		}, nil).
 		Times(1)
 

keycloak/client.go

@@ -15,16 +15,18 @@ type Group struct {
 	path []string
 
 	Members []User
+
+	displayName string
 }
 
 // NewGroup creates a new group.
-func NewGroup(path ...string) Group {
-	return Group{path: path}
+func NewGroup(displayName string, path ...string) Group {
+	return Group{path: path, displayName: displayName}
 }
 
 // NewGroupFromPath creates a new group.
-func NewGroupFromPath(path string) Group {
-	return NewGroup(strings.Split(strings.TrimPrefix(path, "/"), "/")...)
+func NewGroupFromPath(displayName string, path string) Group {
+	return NewGroup(displayName, strings.Split(strings.TrimPrefix(path, "/"), "/")...)
 }
 
 // WithMemberNames returns a copy of the group with given members added.
@@ -119,6 +121,7 @@ type GoCloak interface {
 	CreateGroup(ctx context.Context, accessToken, realm string, group gocloak.Group) (string, error)
 	CreateChildGroup(ctx context.Context, accessToken, realm, groupID string, group gocloak.Group) (string, error)
 	GetGroups(ctx context.Context, accessToken, realm string, params gocloak.GetGroupsParams) ([]*gocloak.Group, error)
+	UpdateGroup(ctx context.Context, accessToken, realm string, updatedGroup gocloak.Group) error
 	DeleteGroup(ctx context.Context, accessToken, realm, groupID string) error
 
 	GetGroupMembers(ctx context.Context, accessToken, realm, groupID string, params gocloak.GetGroupsParams) ([]*gocloak.User, error)
@@ -157,7 +160,7 @@ func NewClient(host, realm, username, password string) Client {
 // PutGroup creates the provided Keycloak group if it does not exist and adjusts the group members accordingly.
 // The method is idempotent.
 func (c Client) PutGroup(ctx context.Context, group Group) (Group, error) {
-	res := NewGroup(group.path...)
+	res := NewGroup(group.displayName, group.path...)
 	group = c.prependRoot(group)
 
 	token, err := c.login(ctx)
@@ -176,6 +179,14 @@ func (c Client) PutGroup(ctx context.Context, group Group) (Group, error) {
 			return res, err
 		}
 		found = &created
+	} else {
+		if getDisplayNameOfGroup(found) != group.displayName {
+			found.Attributes = setDisplayName(found.Attributes, group.displayName)
+			err := c.updateGroup(ctx, token, *found)
+			if err != nil {
+				return res, err
+			}
+		}
 	}
 
 	membErr := MembershipSyncErrors{}
@@ -212,8 +223,9 @@ func (c Client) PutGroup(ctx context.Context, group Group) (Group, error) {
 
 func (c Client) createGroup(ctx context.Context, token *gocloak.JWT, group Group) (gocloak.Group, error) {
 	toCreate := gocloak.Group{
-		Name: gocloak.StringP(group.BaseName()),
-		Path: gocloak.StringP(group.Path()),
+		Name:       gocloak.StringP(group.BaseName()),
+		Path:       gocloak.StringP(group.Path()),
+		Attributes: setDisplayName(nil, group.displayName),
 	}
 
 	if len(group.PathMembers()) == 1 {
@@ -223,7 +235,7 @@ func (c Client) createGroup(ctx context.Context, token *gocloak.JWT, group Group
 	}
 
 	p := group.PathMembers()
-	parent, err := c.getGroup(ctx, token, NewGroup(p[0:len(p)-1]...))
+	parent, err := c.getGroup(ctx, token, NewGroup(group.displayName, p[0:len(p)-1]...))
 	if err != nil {
 		return toCreate, fmt.Errorf("error finding parent group for %v: %w", group, err)
 	}
@@ -236,6 +248,11 @@ func (c Client) createGroup(ctx context.Context, token *gocloak.JWT, group Group
 	return toCreate, err
 }
 
+func (c Client) updateGroup(ctx context.Context, token *gocloak.JWT, group gocloak.Group) error {
+	err := c.Client.UpdateGroup(ctx, token.AccessToken, c.Realm, group)
+	return err
+}
+
 // DeleteGroup deletes the Keycloak group by name.
 // The method is idempotent and will not do anything if the group does not exits.
 func (c Client) DeleteGroup(ctx context.Context, path ...string) error {
@@ -245,7 +262,7 @@ func (c Client) DeleteGroup(ctx context.Context, path ...string) error {
 	}
 	defer c.logout(ctx, token)
 
-	found, err := c.getGroup(ctx, token, c.prependRoot(NewGroup(path...)))
+	found, err := c.getGroup(ctx, token, c.prependRoot(NewGroup("", path...)))
 	if err != nil {
 		return fmt.Errorf("failed finding group: %w", err)
 	}
@@ -488,7 +505,7 @@ func flatGroups(gcp []gocloak.Group) []Group {
 	var flatten func([]gocloak.Group)
 	flatten = func(groups []gocloak.Group) {
 		for _, g := range groups {
-			group := NewGroupFromPath(*g.Path)
+			group := NewGroupFromPath(getDisplayNameOfGroup(&g), *g.Path)
 			group.id = *g.ID
 			flat = append(flat, group)
 			if g.SubGroups != nil {
@@ -501,6 +518,30 @@ func flatGroups(gcp []gocloak.Group) []Group {
 	return flat
 }
 
+func getDisplayNameOfGroup(group *gocloak.Group) string {
+	if group.Attributes != nil {
+		displayNames, ok := (*group.Attributes)["displayName"]
+		if ok && len(displayNames) > 0 {
+			return displayNames[0]
+		}
+	}
+	return ""
+}
+
+func setDisplayName(attributes *map[string][]string, displayName string) *map[string][]string {
+	if attributes == nil {
+		attrMap := make(map[string][]string)
+		attributes = &attrMap
+	}
+	if displayName == "" {
+		delete(*attributes, "displayName")
+	} else {
+		(*attributes)["displayName"] = []string{displayName}
+	}
+	return attributes
+}
+
 var defaultParams = gocloak.GetGroupsParams{
-	Max: gocloak.IntP(-1),
+	Max:                 gocloak.IntP(-1),
+	BriefRepresentation: gocloak.BoolP(false), // required in order to get attributes when listing groups
 }

keycloak/client_delete_test.go

@@ -27,7 +27,7 @@ func TestDeleteGroup_simple(t *testing.T) {
 	mockLogin(mKeycloak, c)
 	mockGetGroups(mKeycloak, c, "foo-gmbh",
 		[]*gocloak.Group{
-			newGocloakGroup("foo-id", "foo-gmbh"),
+			newGocloakGroup("Foo Inc.", "foo-id", "foo-gmbh"),
 		})
 	mockDeleteGroup(mKeycloak, c, "foo-id")
 
@@ -47,7 +47,7 @@ func TestDeleteGroup_RootGroup(t *testing.T) {
 	mockLogin(mKeycloak, c)
 	mockGetGroups(mKeycloak, c, "foo-gmbh",
 		[]*gocloak.Group{
-			newGocloakGroup("foo-id", "root-group", "foo-gmbh"),
+			newGocloakGroup("Foo Inc.", "foo-id", "root-group", "foo-gmbh"),
 		})
 	mockDeleteGroup(mKeycloak, c, "foo-id")
 
@@ -69,7 +69,7 @@ func TestDeleteGroup_subgroup(t *testing.T) {
 	mockLogin(mKeycloak, c)
 	mockGetGroups(mKeycloak, c, "foo-gmbh",
 		[]*gocloak.Group{
-			newGocloakGroup("foo-id", "parent", "foo-gmbh"),
+			newGocloakGroup("Foo Inc.", "foo-id", "parent", "foo-gmbh"),
 		})
 	mockDeleteGroup(mKeycloak, c, "foo-id")
 

keycloak/client_list_test.go

@@ -25,11 +25,11 @@ func TestListGroups_simple(t *testing.T) {
 	}
 
 	gs := []*gocloak.Group{
-		newGocloakGroup("foo-id", "foo-gmbh"),
-		newGocloakGroup("bar-id", "bar-gmbh"),
+		newGocloakGroup("Foo Inc.", "foo-id", "foo-gmbh"),
+		newGocloakGroup("Bar Inc.", "bar-id", "bar-gmbh"),
 		func() *gocloak.Group {
-			g := newGocloakGroup("parent-id", "parent-gmbh")
-			g.SubGroups = &[]gocloak.Group{*newGocloakGroup("qux-id", "parent-gmbh", "qux-team")}
+			g := newGocloakGroup("", "parent-id", "parent-gmbh")
+			g.SubGroups = &[]gocloak.Group{*newGocloakGroup("Parent GmbH", "qux-id", "parent-gmbh", "qux-team")}
 			return g
 		}(),
 	}
@@ -75,13 +75,13 @@ func TestListGroups_RootGroup(t *testing.T) {
 	}
 
 	gs := []*gocloak.Group{
-		newGocloakGroup("foo-id", "foo-gmbh"),
+		newGocloakGroup("Foo Inc.", "foo-id", "foo-gmbh"),
 		func() *gocloak.Group {
-			g := newGocloakGroup("root-group-id", "root-group")
+			g := newGocloakGroup("", "root-group-id", "root-group")
 			g.SubGroups = &[]gocloak.Group{
 				func() gocloak.Group {
-					g := *newGocloakGroup("foo-gmbh-id", "root-group", "foo-gmbh")
-					g.SubGroups = &[]gocloak.Group{*newGocloakGroup("foo-team-id", "root-group", "foo-gmbh", "foo-team")}
+					g := *newGocloakGroup("Foo Inc.", "foo-gmbh-id", "root-group", "foo-gmbh")
+					g.SubGroups = &[]gocloak.Group{*newGocloakGroup("Foo Team", "foo-team-id", "root-group", "foo-gmbh", "foo-team")}
 					return g
 				}()}
 			return g
@@ -112,8 +112,8 @@ func TestListGroups_RootGroup_no_groups_under_root(t *testing.T) {
 	}
 
 	gs := []*gocloak.Group{
-		newGocloakGroup("foo-id", "foo-gmbh"),
-		newGocloakGroup("root-group-id", "root-group"),
+		newGocloakGroup("Foo Inc.", "foo-id", "foo-gmbh"),
+		newGocloakGroup("", "root-group-id", "root-group"),
 	}
 	mockLogin(mKeycloak, c)
 	mockListGroups(mKeycloak, c, gs)
@@ -134,7 +134,7 @@ func TestListGroups_RootGroup_RootNotFound(t *testing.T) {
 	}
 
 	gs := []*gocloak.Group{
-		newGocloakGroup("foo-id", "foo-gmbh"),
+		newGocloakGroup("Foo Inc.", "foo-id", "foo-gmbh"),
 	}
 	mockLogin(mKeycloak, c)
 	mockListGroups(mKeycloak, c, gs)