Removed unused token claim properties from API

config/crd/bases/toolchain.dev.openshift.com_masteruserrecords.yaml

@@ -64,11 +64,6 @@ spec:
                   be able to login (but the underlying UserAccounts still exists)
                   "false" is assumed by default
                 type: boolean
-              originalSub:
-                description: OriginalSub is an optional property temporarily introduced
-                  for the purpose of migrating the users to a new IdP provider client,
-                  and contains the user's "original-sub" claim
-                type: string
               propagatedClaims:
                 description: PropagatedClaims contains a selection of claim values
                   from the SSO Identity Provider which are intended to be "propagated"
@@ -118,10 +113,6 @@ spec:
                 x-kubernetes-list-map-keys:
                 - targetCluster
                 x-kubernetes-list-type: map
-              userID:
-                description: UserID is the user ID from RHD Identity Provider token
-                  (“sub” claim)
-                type: string
             type: object
           status:
             description: MasterUserRecordStatus defines the observed state of MasterUserRecord

config/crd/bases/toolchain.dev.openshift.com_usersignups.yaml

@@ -16,18 +16,18 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - jsonPath: .spec.username
+    - jsonPath: .spec.identityClaims.preferredUsername
       name: Username
       type: string
-    - jsonPath: .spec.givenName
+    - jsonPath: .spec.identityClaims.givenName
       name: First Name
       priority: 1
       type: string
-    - jsonPath: .spec.familyName
+    - jsonPath: .spec.identityClaims.familyName
       name: Last Name
       priority: 1
       type: string
-    - jsonPath: .spec.company
+    - jsonPath: .spec.identityClaims.company
       name: Company
       priority: 1
       type: string
@@ -79,15 +79,6 @@ spec:
           spec:
             description: UserSignupSpec defines the desired state of UserSignup
             properties:
-              company:
-                description: The user's company name, obtained from the identity provider.
-                type: string
-              familyName:
-                description: The user's last name, obtained from the identity provider.
-                type: string
-              givenName:
-                description: The user's first name, obtained from the identity provider.
-                type: string
               identityClaims:
                 description: IdentityClaims contains as-is claim values extracted
                   from the user's access token
@@ -129,11 +120,6 @@ spec:
                 - preferredUsername
                 - sub
                 type: object
-              originalSub:
-                description: OriginalSub is an optional property temporarily introduced
-                  for the purpose of migrating the users to a new IdP provider client,
-                  and contains the user's "original-sub" claim
-                type: string
               states:
                 description: States contains a number of values that reflect the desired
                   state of the UserSignup.
@@ -145,13 +131,6 @@ spec:
                 description: The cluster in which the user is provisioned in If not
                   set then the target cluster will be picked automatically
                 type: string
-              userid:
-                description: The user's user ID, obtained from the identity provider
-                  from the 'sub' (subject) claim
-                type: string
-              username:
-                description: The user's username, obtained from the identity provider.
-                type: string
             type: object
           status:
             description: UserSignupStatus defines the observed state of UserSignup

controllers/masteruserrecord/sync_test.go

@@ -101,9 +101,6 @@ func setupSynchronizerItems() (toolchainv1alpha1.MasterUserRecord, toolchainv1al
 			Labels: map[string]string{
 				toolchainv1alpha1.TierLabelKey: "base1ns",
 			},
-			Annotations: map[string]string{
-				toolchainv1alpha1.UserEmailAnnotationKey: "foo@bar.com",
-			},
 		},
 		Spec: toolchainv1alpha1.UserAccountSpec{
 			PropagatedClaims: toolchainv1alpha1.PropagatedClaims{
@@ -113,11 +110,7 @@ func setupSynchronizerItems() (toolchainv1alpha1.MasterUserRecord, toolchainv1al
 		},
 	}
 	record := toolchainv1alpha1.MasterUserRecord{
-		ObjectMeta: metav1.ObjectMeta{
-			Annotations: map[string]string{
-				toolchainv1alpha1.UserEmailAnnotationKey: "foo@bar.com",
-			},
-		},
+		ObjectMeta: metav1.ObjectMeta{},
 		Spec: toolchainv1alpha1.MasterUserRecordSpec{
 			PropagatedClaims: toolchainv1alpha1.PropagatedClaims{
 				UserID: "foo",

controllers/usersignup/usersignup_controller.go

@@ -111,20 +111,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		return reconcile.Result{}, nil
 	}
 
-	// TODO remove this section (and the referenced function) after migration has completed
-	// FROM HERE ---------
-	migrated, err := r.migrateUserSignupClaimsIfNecessary(ctx, userSignup)
-	if err != nil {
-		// Error during migration - requeue the request
-		return reconcile.Result{}, err
-	}
-
-	if migrated {
-		// If migration occurred, then queue the UserSignup for reconciliation again
-		return reconcile.Result{Requeue: true}, nil
-	}
-	// TO HERE ^^^^^^^^^^^^
-
 	if userSignup.GetLabels() == nil {
 		userSignup.Labels = make(map[string]string)
 	}
@@ -204,40 +190,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	return reconcile.Result{}, r.ensureNewMurIfApproved(ctx, config, userSignup)
 }
 
-// migrateUserSignupClaimsIfNecessary is a temporary function that will set the UserSignup's IdentityClaims based on the
-// existing property values
-func (r *Reconciler) migrateUserSignupClaimsIfNecessary(ctx context.Context, userSignup *toolchainv1alpha1.UserSignup) (bool, error) {
-
-	if userSignup.Spec.IdentityClaims.Sub == "" {
-		userSignup.Spec.IdentityClaims.Sub = userSignup.Spec.Userid
-		userSignup.Spec.IdentityClaims.FamilyName = userSignup.Spec.FamilyName
-		userSignup.Spec.IdentityClaims.GivenName = userSignup.Spec.GivenName
-		userSignup.Spec.IdentityClaims.OriginalSub = userSignup.Spec.OriginalSub
-		userSignup.Spec.IdentityClaims.PreferredUsername = userSignup.Spec.Username
-		userSignup.Spec.IdentityClaims.Company = userSignup.Spec.Company
-
-		if val, ok := userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey]; ok {
-			userSignup.Spec.IdentityClaims.UserID = val
-		}
-
-		if val, ok := userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey]; ok {
-			userSignup.Spec.IdentityClaims.AccountID = val
-		}
-
-		if val, ok := userSignup.Annotations[toolchainv1alpha1.UserSignupUserEmailAnnotationKey]; ok {
-			userSignup.Spec.IdentityClaims.Email = val
-		}
-
-		if err := r.Client.Update(ctx, userSignup); err != nil {
-			return false, err
-		}
-
-		return true, nil
-	}
-
-	return false, nil
-}
-
 // handleDeactivatedUserSignup defines the workflow for deactivated users
 //
 // If there is no MasterUserRecord created, yet the UserSignup is marked as Deactivated, set the status,
@@ -705,7 +657,8 @@ func (r *Reconciler) provisionMasterUserRecord(
 
 	// track the MUR creation as an account activation event in Segment
 	if r.SegmentClient != nil {
-		r.SegmentClient.TrackAccountActivation(compliantUsername, userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey])
+		r.SegmentClient.TrackAccountActivation(compliantUsername, userSignup.Spec.IdentityClaims.UserID,
+			userSignup.Spec.IdentityClaims.AccountID)
 	} else {
 		logger.Info("segment client not configured to track account activations")
 	}

controllers/usersignup/usersignup_controller_test.go

@@ -58,60 +58,6 @@ var event = testsocialevent.NewSocialEvent(test.HostOperatorNs, commonsocialeven
 	testsocialevent.WithUserTier(deactivate80Tier.Name),
 	testsocialevent.WithSpaceTier(base2NSTemplateTier.Name))
 
-func TestUserSignupMigration(t *testing.T) {
-	member := NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue)
-
-	userSignup := commonsignup.NewUserSignup(
-		commonsignup.ApprovedManually())
-
-	// Clear the identity claims
-	userSignup.Spec.IdentityClaims = toolchainv1alpha1.IdentityClaimsEmbedded{}
-
-	// Set some other properties
-	userSignup.Spec.GivenName = "John"
-	userSignup.Spec.FamilyName = "Smith"
-	userSignup.Spec.Company = "Acme"
-	userSignup.Spec.OriginalSub = uuid.Must(uuid.NewV4()).String()
-
-	userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey] = "123456"
-	userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey] = "999988"
-
-	// First reconcile so that we can check that an error is returned if the client update fails
-	r, req, fakeClient := prepareReconcile(t, userSignup.Name, NewGetMemberClusters(member), userSignup, baseNSTemplateTier)
-	fakeClient.MockUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error {
-		return fmt.Errorf("some error")
-	}
-
-	// Reconcile and confirm we get an error
-	_, err := r.Reconcile(context.TODO(), req)
-	require.Error(t, err)
-
-	// Reconcile so that the migration can occur
-	r, req, _ = prepareReconcile(t, userSignup.Name, NewGetMemberClusters(member), userSignup, baseNSTemplateTier)
-	res, err := r.Reconcile(context.TODO(), req)
-
-	// then
-	require.NoError(t, err)
-
-	// Confirm requeue
-	require.True(t, res.Requeue)
-
-	// Reload the UserSignup
-	reloaded := &toolchainv1alpha1.UserSignup{}
-	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: userSignup.Name, Namespace: req.Namespace}, reloaded)
-	require.NoError(t, err)
-
-	// Confirm the migration
-	require.Equal(t, userSignup.Spec.Username, reloaded.Spec.IdentityClaims.PreferredUsername)
-	require.Equal(t, userSignup.Spec.Company, reloaded.Spec.IdentityClaims.Company)
-	require.Equal(t, userSignup.Spec.FamilyName, reloaded.Spec.IdentityClaims.FamilyName)
-	require.Equal(t, userSignup.Spec.GivenName, reloaded.Spec.IdentityClaims.GivenName)
-	require.Equal(t, userSignup.Spec.Userid, reloaded.Spec.IdentityClaims.Sub)
-	require.Equal(t, userSignup.Spec.OriginalSub, reloaded.Spec.IdentityClaims.OriginalSub)
-	require.Equal(t, userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], reloaded.Spec.IdentityClaims.UserID)
-	require.Equal(t, userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey], reloaded.Spec.IdentityClaims.AccountID)
-}
-
 func TestUserSignupCreateMUROk(t *testing.T) {
 	member := NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue)
 	logf.SetLogger(zap.New(zap.UseDevMode(true)))

go.mod

@@ -114,3 +114,7 @@ require (
 )
 
 go 1.20
+
+replace github.com/codeready-toolchain/api => github.com/sbryzak/api v0.0.0-20240204045223-0c2f53dbc8f6
+
+replace github.com/codeready-toolchain/toolchain-common => github.com/sbryzak/toolchain-common v0.0.0-20240201055525-d57dc5ee80f5

go.sum

@@ -136,10 +136,6 @@ github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWH
 github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5/go.mod h1:h6jFvWxBdQXxjopDMZyH2UVceIRfR84bdzbkoKrsWNo=
 github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA=
 github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
-github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420 h1:rxbURSqAgiMCsyKDRwsHMjw7bIktK0IUU/OXpxwu0Zk=
-github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
-github.com/codeready-toolchain/toolchain-common v0.0.0-20240130202956-94befa4c786c h1:83NTlxL6LkpKyu39QxyheGV1tWRXSPOmFyu9fC0qUAA=
-github.com/codeready-toolchain/toolchain-common v0.0.0-20240130202956-94befa4c786c/go.mod h1:f+h8e03UjCldtgI1NsZm/yXq2b/uZv2jsz5p8OK9Z+c=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
@@ -552,6 +548,10 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
+github.com/sbryzak/api v0.0.0-20240204045223-0c2f53dbc8f6 h1:bRkGN6Jj2M47Nu9q+mCebbwvipnaTkXq+2qM0J9a+bQ=
+github.com/sbryzak/api v0.0.0-20240204045223-0c2f53dbc8f6/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
+github.com/sbryzak/toolchain-common v0.0.0-20240201055525-d57dc5ee80f5 h1:wDSLLwumk1CwMpSp36cKcAft8bxHb7IEovdCxabHHnU=
+github.com/sbryzak/toolchain-common v0.0.0-20240201055525-d57dc5ee80f5/go.mod h1:oZhXFgrGmPd77Z3vorZMO2vdAC1FEfXsaXOivs0KG5U=
 github.com/scylladb/go-set v1.0.2/go.mod h1:DkpGd78rljTxKAnTDPFqXSGxvETQnJyuSOQwsHycqfs=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/segmentio/analytics-go/v3 v3.2.1 h1:G+f90zxtc1p9G+WigVyTR0xNfOghOGs/PYAlljLOyeg=

test/segment/assertions.go

@@ -12,7 +12,7 @@ import (
 )
 
 func AssertMessageQueuedForProvisionedMur(t *testing.T, cl *segment.Client, us *toolchainv1alpha1.UserSignup, murName string) {
-	assertMessageQueued(t, cl, murName, us.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], us.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey], "account activated")
+	assertMessageQueued(t, cl, murName, us.Spec.IdentityClaims.UserID, us.Spec.IdentityClaims.AccountID, "account activated")
 }
 
 func assertMessageQueued(t *testing.T, cl *segment.Client, username, userID, accountID string, event string) {