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.

Sign up for GitHub

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

set approved to false after user sign up deactivated #801

Merged
merged 8 commits into from
Oct 13, 2023
Merged

set approved to false after user sign up deactivated #801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
edc60bb
set approved to false after user sign up deactivated
mmulholla Sep 28, 2023
5e0ab0e
remove misleading comments
mmulholla Sep 29, 2023
d4eba97
more test fixes
mmulholla Oct 4, 2023
45a9791
fix failure in metrics test
mmulholla Oct 10, 2023
eefb76d
Merge branch 'master' into RemoveUserSignUpApprovalAfterDeactivation
mfrancisc Oct 11, 2023
412ea16
Fix go fmt error
mmulholla Oct 11, 2023
69c0537
Merge branch 'master' into RemoveUserSignUpApprovalAfterDeactivation
mfrancisc Oct 11, 2023
e80240b
Merge branch 'master' into RemoveUserSignUpApprovalAfterDeactivation
MatousJobanek Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/e2e/e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ func TestE2EFlow(t *testing.T) {

// Wait until the UserSignup is deactivated
_, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin(), wait.ManuallyDeactivated())...))
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ManuallyDeactivated())...))
require.NoError(t, err)

// Ensure the first identity is deleted
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/parallel/registration_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func TestSignupOK(t *testing.T) {
})
require.NoError(t, err)
_, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin(), wait.DeactivatedWithoutPreDeactivation())...),
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.DeactivatedWithoutPreDeactivation())...),
wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueDeactivated))
require.NoError(t, err)
require.NoError(t, hostAwait.WaitUntilSpaceAndSpaceBindingsDeleted(t, userSignup.Status.CompliantUsername))
Expand Down Expand Up @@ -593,7 +593,7 @@ func TestPhoneVerification(t *testing.T) {

// Ensure the UserSignup is deactivated
_, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin(), wait.ManuallyDeactivated())...))
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ManuallyDeactivated())...))
require.NoError(t, err)

// Now attempt the verification again
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/user_management_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (s *userManagementTestSuite) TestUserDeactivation() {
require.NoError(t, err)

userSignupMember1, err = hostAwait.WaitForUserSignup(t, userSignupMember1.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.ApprovedByAdmin(), wait.Deactivated())...),
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Deactivated())...),
wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueDeactivated))
require.NoError(t, err)
require.True(t, states.Deactivated(userSignupMember1), "usersignup should be deactivated")
Expand Down Expand Up @@ -502,7 +502,7 @@ func (s *userManagementTestSuite) TestUserDeactivation() {

// Since the config for retention days is set to 0, the account should be deactivated again immediately
userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin(), wait.DeactivatedWithoutNotification())...),
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.DeactivatedWithoutNotification())...),
wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueDeactivated))
require.NoError(t, err)
require.True(t, states.Deactivated(userSignup), "usersignup should be deactivated")
Expand Down
2 changes: 1 addition & 1 deletion test/metrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestVerificationRequiredMetric(t *testing.T) {
// when reactivating the user
InvokeEndpoint(t, "POST", route+"/api/v1/signup", token0, "", http.StatusAccepted)
userSignup, err = hostAwait.WaitForUserSignup(t, identity0.Username,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.VerificationRequired())...),
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.VerificationRequired(), wait.ApprovedDeactivated())...),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we should improve in the future as well. Instead of keeping the old "Deactivated' reason we could add a different one. But it's fine for now 👍

wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueNotReady))

// then
Expand Down
4 changes: 1 addition & 3 deletions testsupport/deactivation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ func DeactivateAndCheckUser(t *testing.T, awaitilities wait.Awaitilities, userSi
assert.Equal(t, "userdeactivated", notification.Spec.Template)
assert.Equal(t, userSignup.Spec.Userid, notification.Spec.Context["UserID"])

// We wait for the "Approved()" condition status here because it doesn't specify a reason for the approval,
// and the reason should not be necessary for the purpose of this test.
userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin(), wait.DeactivatedWithoutPreDeactivation())...),
wait.UntilUserSignupHasConditions(wait.ConditionSet(wait.Default(), wait.DeactivatedWithoutPreDeactivation())...),
wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueDeactivated))
require.NoError(t, err)
require.True(t, states.Deactivated(userSignup), "usersignup should be deactivated")
Expand Down
30 changes: 30 additions & 0 deletions testsupport/wait/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ func ApprovedByAdmin() []toolchainv1alpha1.Condition {
}
}

func ApprovedDeactivated() []toolchainv1alpha1.Condition {
return []toolchainv1alpha1.Condition{
{
Type: toolchainv1alpha1.UserSignupApproved,
Status: corev1.ConditionFalse,
Reason: toolchainv1alpha1.UserSignupUserDeactivatedReason,
},
}
}

func PendingApprovalNoCluster() []toolchainv1alpha1.Condition {
return []toolchainv1alpha1.Condition{
{
Expand Down Expand Up @@ -230,6 +240,11 @@ func DeactivatedWithoutPreDeactivation() []toolchainv1alpha1.Condition {
Status: corev1.ConditionTrue,
Reason: "NotificationCRCreated",
},
{
Type: toolchainv1alpha1.UserSignupApproved,
Status: corev1.ConditionFalse,
Reason: toolchainv1alpha1.UserSignupUserDeactivatedReason,
},
}
}

Expand All @@ -245,6 +260,11 @@ func DeactivatedWithoutNotification() []toolchainv1alpha1.Condition {
Status: corev1.ConditionFalse,
Reason: "UserIsActive",
},
{
Type: toolchainv1alpha1.UserSignupApproved,
Status: corev1.ConditionFalse,
Reason: toolchainv1alpha1.UserSignupUserDeactivatedReason,
},
}
}

Expand All @@ -260,6 +280,11 @@ func ManuallyDeactivated() []toolchainv1alpha1.Condition {
Status: corev1.ConditionTrue,
Reason: "NotificationCRCreated",
},
{
Type: toolchainv1alpha1.UserSignupApproved,
Status: corev1.ConditionFalse,
Reason: toolchainv1alpha1.UserSignupUserDeactivatedReason,
},
}
}

Expand All @@ -280,6 +305,11 @@ func Deactivated() []toolchainv1alpha1.Condition {
Status: corev1.ConditionTrue,
Reason: "NotificationCRCreated",
},
{
Type: toolchainv1alpha1.UserSignupApproved,
Status: corev1.ConditionFalse,
Reason: toolchainv1alpha1.UserSignupUserDeactivatedReason,
},
}
}

Expand Down