Skip to content

Commit

Permalink
Removed unused token claim properties from API (#522)
Browse files Browse the repository at this point in the history
* Regenerated, temp replacement of api dependency

* update toolchain-common

* updated to use new annotations

* updated dependencies

* updated dependencies

* updated to latest api and toolchain-common
  • Loading branch information
sbryzak authored Feb 7, 2024
1 parent 7eff1e3 commit 5b1f7c1
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 35 deletions.
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
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module github.com/codeready-toolchain/member-operator

require (
github.com/codeready-toolchain/api v0.0.0-20240103194050-d5c7803671c1
github.com/codeready-toolchain/toolchain-common v0.0.0-20240126111814-12ab087b62d2
github.com/codeready-toolchain/api v0.0.0-20240207000013-661b63025269
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c
github.com/go-logr/logr v1.2.3
github.com/google/go-cmp v0.5.9
// using latest commit from 'github.com/openshift/api branch release-4.12'
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ 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/codeready-toolchain/api v0.0.0-20240207000013-661b63025269 h1:YS5Q6YsTYq9Fo8qA6NQOTWAcVg86VEwulT1UfNWknIQ=
github.com/codeready-toolchain/api v0.0.0-20240207000013-661b63025269/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c h1:WKjD9qRSLqQsi4XduuXjsaF8qFH4yj157qWdA5wOOtg=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c/go.mod h1:+COaw79DVTLSb2unqVwcBtYOg6sh7MbMHgXU1/ht2I8=
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

0 comments on commit 5b1f7c1

Please sign in to comment.