Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed unused token claim properties from API #522

Merged
merged 9 commits into from
Feb 7, 2024
Merged
10 changes: 0 additions & 10 deletions config/crd/bases/toolchain.dev.openshift.com_useraccounts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@ spec:
description: If set to true then the corresponding user should not
be able to login "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"
Expand Down Expand Up @@ -93,11 +88,6 @@ spec:
- email
- sub
type: object
userID:
description: UserID is the user ID from RHD Identity Provider token
(“sub” claim) Is to be used to create Identity and UserIdentityMapping
resources
type: string
type: object
status:
description: UserAccountStatus defines the observed state of UserAccount
Expand Down
12 changes: 6 additions & 6 deletions controllers/useraccount/useraccount_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,11 +390,11 @@ func setLabelsAndAnnotations(object metav1.Object, userAcc *toolchainv1alpha1.Us

if isUserResource {
annotations := object.GetAnnotations()
if _, exists := annotations[toolchainv1alpha1.UserEmailAnnotationKey]; !exists {
if _, exists := annotations[toolchainv1alpha1.EmailUserAnnotationKey]; !exists {
if annotations == nil {
annotations = map[string]string{}
}
annotations[toolchainv1alpha1.UserEmailAnnotationKey] = userAcc.Spec.PropagatedClaims.Email
annotations[toolchainv1alpha1.EmailUserAnnotationKey] = userAcc.Spec.PropagatedClaims.Email
object.SetAnnotations(annotations)
changed = true
}
Expand All @@ -406,8 +406,8 @@ func setLabelsAndAnnotations(object metav1.Object, userAcc *toolchainv1alpha1.Us
if annotations == nil {
annotations = map[string]string{}
}
annotations[toolchainv1alpha1.SSOUserIDAnnotationKey] = userAcc.Spec.PropagatedClaims.UserID
annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey] = userAcc.Spec.PropagatedClaims.AccountID
annotations[toolchainv1alpha1.UserIDUserAnnotationKey] = userAcc.Spec.PropagatedClaims.UserID
annotations[toolchainv1alpha1.AccountIDUserAnnotationKey] = userAcc.Spec.PropagatedClaims.AccountID
object.SetAnnotations(annotations)
changed = true

Expand All @@ -416,8 +416,8 @@ func setLabelsAndAnnotations(object metav1.Object, userAcc *toolchainv1alpha1.Us
// Delete the UserID and AccountID annotations if they don't exist in the UserAccount
if !set && object.GetAnnotations() != nil {
annotations = object.GetAnnotations()
delete(annotations, toolchainv1alpha1.SSOUserIDAnnotationKey)
delete(annotations, toolchainv1alpha1.SSOAccountIDAnnotationKey)
delete(annotations, toolchainv1alpha1.UserIDUserAnnotationKey)
delete(annotations, toolchainv1alpha1.AccountIDUserAnnotationKey)
object.SetAnnotations(annotations)
changed = true
}
Expand Down
26 changes: 13 additions & 13 deletions controllers/useraccount/useraccount_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestReconcile(t *testing.T) {
toolchainv1alpha1.ProviderLabelKey: toolchainv1alpha1.ProviderLabelValue,
},
Annotations: map[string]string{
toolchainv1alpha1.UserEmailAnnotationKey: userAcc.Spec.PropagatedClaims.Email,
toolchainv1alpha1.EmailUserAnnotationKey: userAcc.Spec.PropagatedClaims.Email,
},
},
Identities: []string{
Expand Down Expand Up @@ -136,8 +136,8 @@ func TestReconcile(t *testing.T) {
user.UID = preexistingUser.UID // we have to set UID for the obtained user because the fake client doesn't set it

checkMapping(t, user, preexistingIdentity, preexistingIdentityForSsoUserAnnotation)
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey])
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.UserIDUserAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.AccountIDUserAnnotationKey])

// Check the identity is not created yet
assertIdentityNotFound(t, r, userAcc, config.Auth().Idp())
Expand All @@ -154,8 +154,8 @@ func TestReconcile(t *testing.T) {

// Check the created/updated user
user := assertUser(t, r, userAcc)
require.NotContains(t, user.Annotations, toolchainv1alpha1.SSOUserIDAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.SSOAccountIDAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.UserIDUserAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.AccountIDUserAnnotationKey)

t.Run("reset UserID and reconcile again", func(t *testing.T) {
userAcc.Spec.PropagatedClaims.UserID = "123456"
Expand All @@ -168,8 +168,8 @@ func TestReconcile(t *testing.T) {

// Check the created/updated user
user := assertUser(t, r, userAcc)
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey])
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.UserIDUserAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.AccountIDUserAnnotationKey])

t.Run("test missing AccountID annotation propagates to User", func(t *testing.T) {
// Remove the AccountID annotation from the UserAccount and reconcile again
Expand All @@ -183,8 +183,8 @@ func TestReconcile(t *testing.T) {

// Check the created/updated user
user := assertUser(t, r, userAcc)
require.NotContains(t, user.Annotations, toolchainv1alpha1.SSOUserIDAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.SSOAccountIDAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.UserIDUserAnnotationKey)
require.NotContains(t, user.Annotations, toolchainv1alpha1.AccountIDUserAnnotationKey)

t.Run("reset AccountID annotation and reconcile again", func(t *testing.T) {
userAcc.Spec.PropagatedClaims.AccountID = "987654"
Expand All @@ -197,8 +197,8 @@ func TestReconcile(t *testing.T) {

// Check the created/updated user
user := assertUser(t, r, userAcc)
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey])
require.Equal(t, "123456", user.Annotations[toolchainv1alpha1.UserIDUserAnnotationKey])
require.Equal(t, "987654", user.Annotations[toolchainv1alpha1.AccountIDUserAnnotationKey])
})
})
})
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func TestCreateIdentitiesOKWhenSSOUserIDAnnotationPresent(t *testing.T) {
toolchainv1alpha1.ProviderLabelKey: toolchainv1alpha1.ProviderLabelValue,
},
Annotations: map[string]string{
toolchainv1alpha1.UserEmailAnnotationKey: userAcc.Spec.PropagatedClaims.Email,
toolchainv1alpha1.EmailUserAnnotationKey: userAcc.Spec.PropagatedClaims.Email,
},
},
Identities: []string{
Expand Down Expand Up @@ -1551,7 +1551,7 @@ func assertUser(t *testing.T, r *Reconciler, userAcc *toolchainv1alpha1.UserAcco
assert.Equal(t, toolchainv1alpha1.ProviderLabelValue, user.Labels[toolchainv1alpha1.ProviderLabelKey])

assert.NotNil(t, user.Annotations)
assert.Equal(t, userAcc.Spec.PropagatedClaims.Email, user.Annotations[toolchainv1alpha1.UserEmailAnnotationKey])
assert.Equal(t, userAcc.Spec.PropagatedClaims.Email, user.Annotations[toolchainv1alpha1.EmailUserAnnotationKey])

assert.Empty(t, user.OwnerReferences) // User has no explicit owner reference.// Check the user identity mapping
return user
Expand Down
6 changes: 5 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module github.com/codeready-toolchain/member-operator

require (
github.com/codeready-toolchain/api v0.0.0-20240103194050-d5c7803671c1
github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420
github.com/codeready-toolchain/toolchain-common v0.0.0-20240126111814-12ab087b62d2
github.com/go-logr/logr v1.2.3
github.com/google/go-cmp v0.5.9
Expand Down Expand Up @@ -110,3 +110,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
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,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-20240103194050-d5c7803671c1 h1:R+5BmQrz9hBfj6QFL+ojExD6CiZ5EuuVUbeb3pqxdds=
github.com/codeready-toolchain/api v0.0.0-20240103194050-d5c7803671c1/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240126111814-12ab087b62d2 h1:wObz6g5TFOzn7AZF6hsd7vI+GQRytxQcVAL6/DnNex8=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240126111814-12ab087b62d2/go.mod h1:1oIpmgqMMIir4IjrVkmBaC3GXsObl0vmOFmnYhpbSAQ=
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=
Expand Down Expand Up @@ -537,6 +533,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/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
Expand Down
Loading