diff --git a/pkg/application/service/services.go b/pkg/application/service/services.go index 4aae2d35..c9b4f45e 100644 --- a/pkg/application/service/services.go +++ b/pkg/application/service/services.go @@ -8,15 +8,14 @@ import ( type SignupService interface { Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) - GetSignup(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) - GetUserSignupFromIdentifier(userID, username string) (*toolchainv1alpha1.UserSignup, error) - PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHash string) error + GetSignup(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + PhoneNumberAlreadyInUse(username, phoneNumberOrHash string) error } type VerificationService interface { - InitVerification(ctx *gin.Context, userID, username, e164PhoneNumber, countryCode string) error - VerifyPhoneCode(ctx *gin.Context, userID, username, code string) error - VerifyActivationCode(ctx *gin.Context, userID, username, code string) error + InitVerification(ctx *gin.Context, username, e164PhoneNumber, countryCode string) error + VerifyPhoneCode(ctx *gin.Context, username, code string) error + VerifyActivationCode(ctx *gin.Context, username, code string) error } type Services interface { diff --git a/pkg/controller/signup.go b/pkg/controller/signup.go index 66f4914b..3e2b7f4c 100644 --- a/pkg/controller/signup.go +++ b/pkg/controller/signup.go @@ -60,7 +60,6 @@ func (s *Signup) PostHandler(ctx *gin.Context) { // invokes the Verification service with an E.164 formatted phone number value derived from the country code and phone number // provided by the user. func (s *Signup) InitVerificationHandler(ctx *gin.Context) { - userID := ctx.GetString(context.SubKey) username := ctx.GetString(context.UsernameKey) // Read the Body content @@ -87,9 +86,9 @@ func (s *Signup) InitVerificationHandler(ctx *gin.Context) { } e164Number := phonenumbers.Format(number, phonenumbers.E164) - err = s.app.VerificationService().InitVerification(ctx, userID, username, e164Number, strconv.Itoa(countryCode)) + err = s.app.VerificationService().InitVerification(ctx, username, e164Number, strconv.Itoa(countryCode)) if err != nil { - log.Errorf(ctx, err, "Verification for %s could not be sent", userID) + log.Errorf(ctx, err, "Verification for %s could not be sent", username) e := &crterrors.Error{} switch { case errors.As(err, &e): @@ -100,7 +99,7 @@ func (s *Signup) InitVerificationHandler(ctx *gin.Context) { return } - log.Infof(ctx, "phone verification has been sent for userID %s", userID) + log.Infof(ctx, "phone verification has been sent for username %s", username) ctx.Status(http.StatusNoContent) ctx.Writer.WriteHeaderNow() } @@ -108,16 +107,15 @@ func (s *Signup) InitVerificationHandler(ctx *gin.Context) { // GetHandler returns the Signup resource func (s *Signup) GetHandler(ctx *gin.Context) { - // Get the UserSignup resource from the service by the userID - userID := ctx.GetString(context.SubKey) + // Get the UserSignup resource from the service by the username username := ctx.GetString(context.UsernameKey) - signupResource, err := s.app.SignupService().GetSignup(ctx, userID, username, true) + signupResource, err := s.app.SignupService().GetSignup(ctx, username, true) if err != nil { log.Error(ctx, err, "error getting UserSignup resource") crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting UserSignup resource") } if signupResource == nil { - log.Infof(ctx, "UserSignup resource for userID: %s, username: %s resource not found", userID, username) + log.Infof(ctx, "UserSignup resource for username '%s' resource not found", username) ctx.AbortWithStatus(http.StatusNotFound) } else { ctx.JSON(http.StatusOK, signupResource) @@ -134,10 +132,9 @@ func (s *Signup) VerifyPhoneCodeHandler(ctx *gin.Context) { return } - userID := ctx.GetString(context.SubKey) username := ctx.GetString(context.UsernameKey) - err := s.app.VerificationService().VerifyPhoneCode(ctx, userID, username, code) + err := s.app.VerificationService().VerifyPhoneCode(ctx, username, code) if err != nil { e := &crterrors.Error{} switch { @@ -167,10 +164,9 @@ func (s *Signup) VerifyActivationCodeHandler(ctx *gin.Context) { return } - userID := ctx.GetString(context.SubKey) username := ctx.GetString(context.UsernameKey) - err := s.app.VerificationService().VerifyActivationCode(ctx, userID, username, code) + err := s.app.VerificationService().VerifyActivationCode(ctx, username, code) if err != nil { log.Error(ctx, err, "error validating activation code") e := &crterrors.Error{} diff --git a/pkg/controller/signup_test.go b/pkg/controller/signup_test.go index 7b321680..20356b0f 100644 --- a/pkg/controller/signup_test.go +++ b/pkg/controller/signup_test.go @@ -193,7 +193,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() { Reason: "Provisioning", }, } - svc.MockGetSignup = func(_ *gin.Context, _, name string, _ bool) (*signup.Signup, error) { + svc.MockGetSignup = func(_ *gin.Context, name string, _ bool) (*signup.Signup, error) { if name == username { return expected, nil } @@ -220,7 +220,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() { ctx.Request = req ctx.Set(context.UsernameKey, username) - svc.MockGetSignup = func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + svc.MockGetSignup = func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, nil } @@ -237,7 +237,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() { ctx.Request = req ctx.Set(context.UsernameKey, username) - svc.MockGetSignup = func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + svc.MockGetSignup = func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, errors.New("oopsie woopsie") } @@ -451,7 +451,7 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { require.Equal(s.T(), "Internal Server Error", bodyParams["status"]) require.InDelta(s.T(), float64(500), bodyParams["code"], 0.01) - require.Equal(s.T(), "no user: error retrieving usersignup: ", bodyParams["message"]) + require.Equal(s.T(), "no user: error retrieving usersignup with username 'johnny@kubesaw'", bodyParams["message"]) require.Equal(s.T(), "error while verifying phone code", bodyParams["details"]) }) @@ -724,24 +724,19 @@ func initActivationCodeVerification(t *testing.T, handler gin.HandlerFunc, usern } type FakeSignupService struct { - MockGetSignup func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) - MockSignup func(ctx *gin.Context) (*crtapi.UserSignup, error) - MockGetUserSignupFromIdentifier func(userID, username string) (*crtapi.UserSignup, error) - MockPhoneNumberAlreadyInUse func(userID, username, value string) error + MockGetSignup func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) + MockSignup func(ctx *gin.Context) (*crtapi.UserSignup, error) + MockPhoneNumberAlreadyInUse func(username, value string) error } -func (m *FakeSignupService) GetSignup(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) { - return m.MockGetSignup(ctx, userID, username, checkUserSignupComplete) +func (m *FakeSignupService) GetSignup(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) { + return m.MockGetSignup(ctx, username, checkUserSignupComplete) } func (m *FakeSignupService) Signup(ctx *gin.Context) (*crtapi.UserSignup, error) { return m.MockSignup(ctx) } -func (m *FakeSignupService) GetUserSignupFromIdentifier(userID, username string) (*crtapi.UserSignup, error) { - return m.MockGetUserSignupFromIdentifier(userID, username) -} - -func (m *FakeSignupService) PhoneNumberAlreadyInUse(userID, username, e164phoneNumber string) error { - return m.MockPhoneNumberAlreadyInUse(userID, username, e164phoneNumber) +func (m *FakeSignupService) PhoneNumberAlreadyInUse(username, e164phoneNumber string) error { + return m.MockPhoneNumberAlreadyInUse(username, e164phoneNumber) } diff --git a/pkg/proxy/handlers/spacelister.go b/pkg/proxy/handlers/spacelister.go index 0137d88a..08c375d5 100644 --- a/pkg/proxy/handlers/spacelister.go +++ b/pkg/proxy/handlers/spacelister.go @@ -28,7 +28,7 @@ const ( type SpaceLister struct { namespaced.Client - GetSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + GetSignupFunc func(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) ProxyMetrics *metrics.ProxyMetrics } @@ -43,7 +43,7 @@ func NewSpaceLister(client namespaced.Client, app application.Application, proxy func (s *SpaceLister) GetProvisionedUserSignup(ctx echo.Context) (*signup.Signup, error) { username, _ := ctx.Get(context.UsernameKey).(string) - userSignup, err := s.GetSignupFunc(nil, "", username, false) + userSignup, err := s.GetSignupFunc(nil, username, false) if err != nil { ctx.Logger().Error(errs.Wrap(err, "error retrieving signup")) return nil, err diff --git a/pkg/proxy/handlers/spacelister_get_test.go b/pkg/proxy/handlers/spacelister_get_test.go index bab33b32..0b35487e 100644 --- a/pkg/proxy/handlers/spacelister_get_test.go +++ b/pkg/proxy/handlers/spacelister_get_test.go @@ -76,7 +76,7 @@ func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) { expectedErr string expectedErrCode int expectedWorkspace string - overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) mockFakeClient func(fakeClient *test.FakeClient) overrideGetMembersFunc func(conditions ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster overrideMemberClient *test.FakeClient @@ -270,7 +270,7 @@ func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) { expectedWs: nil, expectedErr: "signup error", expectedErrCode: 500, - overrideSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, expectedWorkspace: "dancelover", @@ -620,7 +620,7 @@ func TestGetUserWorkspace(t *testing.T) { workspaceRequest string expectedWorkspace func(t *testing.T, fakeClient *test.FakeClient) toolchainv1alpha1.Workspace mockFakeClient func(fakeClient *test.FakeClient) - overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) }{ "get robin workspace": { username: "robin", @@ -673,7 +673,7 @@ func TestGetUserWorkspace(t *testing.T) { username: "batman", workspaceRequest: "batman", expectedErr: "signup error", - overrideSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, expectedWorkspace: nil, diff --git a/pkg/proxy/handlers/spacelister_list_test.go b/pkg/proxy/handlers/spacelister_list_test.go index f0da39fb..f83ec7d2 100644 --- a/pkg/proxy/handlers/spacelister_list_test.go +++ b/pkg/proxy/handlers/spacelister_list_test.go @@ -122,7 +122,7 @@ func TestHandleSpaceListRequest(t *testing.T) { expectedErr string expectedErrCode int expectedWorkspace string - overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) mockFakeClient func(fakeClient *test.FakeClient) }{ "dancelover lists spaces": { @@ -181,7 +181,7 @@ func TestHandleSpaceListRequest(t *testing.T) { expectedWs: nil, expectedErr: "signup error", expectedErrCode: 500, - overrideSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, }, diff --git a/pkg/proxy/members.go b/pkg/proxy/members.go index f575b80d..02c8e472 100644 --- a/pkg/proxy/members.go +++ b/pkg/proxy/members.go @@ -37,19 +37,19 @@ func NewMemberClusters(client namespaced.Client, signupService service.SignupSer return si } -func (s *MemberClusters) GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { +func (s *MemberClusters) GetClusterAccess(username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { // if workspace is not provided then return the default space access if workspace == "" { - return s.getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName) + return s.getClusterAccessForDefaultWorkspace(username, proxyPluginName) } - return s.getSpaceAccess(userID, username, workspace, proxyPluginName, publicViewerEnabled) + return s.getSpaceAccess(username, workspace, proxyPluginName, publicViewerEnabled) } // getSpaceAccess retrieves space access for an user -func (s *MemberClusters) getSpaceAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { +func (s *MemberClusters) getSpaceAccess(username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { // retrieve the user's complaint name - complaintUserName, err := s.getUserSignupComplaintName(userID, username, publicViewerEnabled) + complaintUserName, err := s.getUserSignupComplaintName(username, publicViewerEnabled) if err != nil { return nil, err } @@ -65,14 +65,14 @@ func (s *MemberClusters) getSpaceAccess(userID, username, workspace, proxyPlugin return s.accessForSpace(space, complaintUserName, proxyPluginName) } -func (s *MemberClusters) getUserSignupComplaintName(userID, username string, publicViewerEnabled bool) (string, error) { +func (s *MemberClusters) getUserSignupComplaintName(username string, publicViewerEnabled bool) (string, error) { // if PublicViewer is enabled and the requested user is the PublicViewer, than no lookup is required if publicViewerEnabled && username == toolchainv1alpha1.KubesawAuthenticatedUsername { return username, nil } // retrieve the UserSignup from cache - userSignup, err := s.getSignupFromInformerForProvisionedUser(userID, username) + userSignup, err := s.getSignupFromInformerForProvisionedUser(username) if err != nil { return "", err } @@ -81,9 +81,9 @@ func (s *MemberClusters) getUserSignupComplaintName(userID, username string, pub } // getClusterAccessForDefaultWorkspace retrieves the cluster for the user's default workspace -func (s *MemberClusters) getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName string) (*access.ClusterAccess, error) { +func (s *MemberClusters) getClusterAccessForDefaultWorkspace(username, proxyPluginName string) (*access.ClusterAccess, error) { // retrieve the UserSignup from cache - userSignup, err := s.getSignupFromInformerForProvisionedUser(userID, username) + userSignup, err := s.getSignupFromInformerForProvisionedUser(username) if err != nil { return nil, err } @@ -92,10 +92,10 @@ func (s *MemberClusters) getClusterAccessForDefaultWorkspace(userID, username, p return s.accessForCluster(userSignup.APIEndpoint, userSignup.ClusterName, userSignup.CompliantUsername, proxyPluginName) } -func (s *MemberClusters) getSignupFromInformerForProvisionedUser(userID, username string) (*signup.Signup, error) { +func (s *MemberClusters) getSignupFromInformerForProvisionedUser(username string) (*signup.Signup, error) { // don't check for usersignup complete status, since it might cause the proxy blocking the request // and returning an error when quick transitions from ready to provisioning are happening. - userSignup, err := s.SignupService.GetSignup(nil, userID, username, false) + userSignup, err := s.SignupService.GetSignup(nil, username, false) if err != nil { return nil, err } diff --git a/pkg/proxy/members_test.go b/pkg/proxy/members_test.go index 5192920f..240ab706 100644 --- a/pkg/proxy/members_test.go +++ b/pkg/proxy/members_test.go @@ -116,12 +116,12 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { for k, tc := range tt { s.Run(k, func() { s.Run("signup service returns error", func() { - sc.MockGetSignup = func(_, _ string) (*signup.Signup, error) { + sc.MockGetSignup = func(_ string) (*signup.Signup, error) { return nil, errors.New("oopsi woopsi") } // when - _, err := members.GetClusterAccess("", "789-ready", tc.workspace, "", publicViewerEnabled) + _, err := members.GetClusterAccess("789-ready", tc.workspace, "", publicViewerEnabled) // then require.EqualError(s.T(), err, "oopsi woopsi") @@ -131,7 +131,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("username is not found", func() { // when - _, err := members.GetClusterAccess("", "unknown_username", tc.workspace, "", publicViewerEnabled) + _, err := members.GetClusterAccess("unknown_username", tc.workspace, "", publicViewerEnabled) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -139,7 +139,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("user is not provisioned yet", func() { // when - _, err := members.GetClusterAccess("", "456-not-ready", tc.workspace, "", publicViewerEnabled) + _, err := members.GetClusterAccess("456-not-ready", tc.workspace, "", publicViewerEnabled) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -165,7 +165,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { members := proxy.NewMemberClusters(nsClient, sc, commoncluster.GetMemberClusters) // when - _, err := members.GetClusterAccess("", "789-ready", "smith2", "", publicViewerEnabled) + _, err := members.GetClusterAccess("789-ready", "smith2", "", publicViewerEnabled) // then // original error is only logged so that it doesn't reveal information about a space that may not belong to the requestor @@ -174,7 +174,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("space not found", func() { // when - _, err := members.GetClusterAccess("", "789-ready", "unknown", "", publicViewerEnabled) // unknown workspace requested + _, err := members.GetClusterAccess("789-ready", "unknown", "", publicViewerEnabled) // unknown workspace requested // then require.EqualError(s.T(), err, "the requested space is not available") @@ -188,7 +188,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { }) s.Run("default workspace case", func() { // when - _, err := members.GetClusterAccess("", "789-ready", "", "", publicViewerEnabled) + _, err := members.GetClusterAccess("789-ready", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member clusters found") @@ -196,7 +196,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("workspace context case", func() { // when - _, err := members.GetClusterAccess("", "789-ready", "smith2", "", publicViewerEnabled) + _, err := members.GetClusterAccess("789-ready", "smith2", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member clusters found") @@ -209,7 +209,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { }) s.Run("default workspace case", func() { // when - _, err := members.GetClusterAccess("", "012-ready-unknown-cluster", "", "", publicViewerEnabled) + _, err := members.GetClusterAccess("012-ready-unknown-cluster", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member cluster found for the user") @@ -217,7 +217,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("workspace context case", func() { // when - _, err := members.GetClusterAccess("", "012-ready-unknown-cluster", "unknown-cluster", "", publicViewerEnabled) + _, err := members.GetClusterAccess("012-ready-unknown-cluster", "unknown-cluster", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member cluster found for space 'unknown-cluster'") @@ -278,7 +278,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { expectedToken := "abc123" // should match member 2 bearer token // when - ca, err := members.GetClusterAccess("", "789-ready", "", "tekton-results", publicViewerEnabled) + ca, err := members.GetClusterAccess("789-ready", "", "tekton-results", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -291,7 +291,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("cluster access correct when using workspace context", func() { // when - ca, err := members.GetClusterAccess("", "789-ready", "smith2", "tekton-results", publicViewerEnabled) // workspace-context specified + ca, err := members.GetClusterAccess("789-ready", "smith2", "tekton-results", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -319,7 +319,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { return memberClient.Client.Get(ctx, key, obj, opts...) } memberArray[0].Client = mC - ca, err := members.GetClusterAccess("", "789-ready", "teamspace", "tekton-results", publicViewerEnabled) // workspace-context specified + ca, err := members.GetClusterAccess("789-ready", "teamspace", "tekton-results", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -337,7 +337,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { expectedToken := "abc123" // should match member 2 bearer token // when - ca, err := members.GetClusterAccess("", "789-ready", "", "", publicViewerEnabled) + ca, err := members.GetClusterAccess("789-ready", "", "", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -350,7 +350,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("cluster access correct when using workspace context", func() { // when - ca, err := members.GetClusterAccess("", "789-ready", "smith2", "", publicViewerEnabled) // workspace-context specified + ca, err := members.GetClusterAccess("789-ready", "smith2", "", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -362,7 +362,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("another workspace on another cluster", func() { // when - ca, err := members.GetClusterAccess("", "789-ready", "teamspace", "", publicViewerEnabled) // workspace-context specified + ca, err := members.GetClusterAccess("789-ready", "teamspace", "", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -382,7 +382,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("user is public-viewer", func() { s.Run("has no default workspace", func() { // when - ca, err := members.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "", "", true) + ca, err := members.GetClusterAccess(toolchainv1alpha1.KubesawAuthenticatedUsername, "", "", true) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -395,7 +395,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { }) s.Run("public-viewer is disabled", func() { // when - ca, err := members.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "smith2", "", false) + ca, err := members.GetClusterAccess(toolchainv1alpha1.KubesawAuthenticatedUsername, "smith2", "", false) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -409,7 +409,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { expectedClusterAccess := access.NewClusterAccess(*expectedURL, "token", toolchainv1alpha1.KubesawAuthenticatedUsername) // when - clusterAccess, err := members.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "smith2", "", true) + clusterAccess, err := members.GetClusterAccess(toolchainv1alpha1.KubesawAuthenticatedUsername, "smith2", "", true) // then require.NoError(s.T(), err) @@ -418,7 +418,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("not-available space", func() { // when - clusterAccess, err := members.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "456-not-ready", "", true) + clusterAccess, err := members.GetClusterAccess(toolchainv1alpha1.KubesawAuthenticatedUsername, "456-not-ready", "", true) // then require.EqualError(s.T(), err, "the requested space is not available") @@ -427,7 +427,7 @@ func (s *TestMemberClustersSuite) TestGetClusterAccess() { s.Run("ready space with unknown cluster", func() { // when - clusterAccess, err := members.GetClusterAccess("", toolchainv1alpha1.KubesawAuthenticatedUsername, "012-ready-unknown-cluster", "", true) + clusterAccess, err := members.GetClusterAccess(toolchainv1alpha1.KubesawAuthenticatedUsername, "012-ready-unknown-cluster", "", true) // then require.EqualError(s.T(), err, "the requested space is not available") diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 36195b76..8cf590c3 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -265,7 +265,6 @@ func (p *Proxy) health(ctx echo.Context) error { func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, error) { // retrieve required information from the HTTP request - userID, _ := ctx.Get(context.SubKey).(string) username, _ := ctx.Get(context.UsernameKey).(string) proxyPluginName, workspaceName, err := getWorkspaceContext(ctx.Request()) if err != nil { @@ -278,7 +277,7 @@ func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, // if the target workspace is NOT explicitly declared in the HTTP request, // process the request against the user's home workspace if workspaceName == "" { - cluster, err := p.processHomeWorkspaceRequest(ctx, userID, username, proxyPluginName) + cluster, err := p.processHomeWorkspaceRequest(ctx, username, proxyPluginName) if err != nil { return "", nil, err } @@ -287,7 +286,7 @@ func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, // if the target workspace is explicitly declared in the HTTP request, // process the request against the declared workspace - cluster, err := p.processWorkspaceRequest(ctx, userID, username, workspaceName, proxyPluginName) + cluster, err := p.processWorkspaceRequest(ctx, username, workspaceName, proxyPluginName) if err != nil { return "", nil, err } @@ -295,10 +294,10 @@ func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, } // processHomeWorkspaceRequest process an HTTP Request targeting the user's home workspace. -func (p *Proxy) processHomeWorkspaceRequest(ctx echo.Context, userID, username, proxyPluginName string) (*access.ClusterAccess, error) { +func (p *Proxy) processHomeWorkspaceRequest(ctx echo.Context, username, proxyPluginName string) (*access.ClusterAccess, error) { // retrieves the ClusterAccess for the user and their home workspace members := NewMemberClusters(p.Client, p.signupService, p.getMembersFunc) - cluster, err := members.GetClusterAccess(userID, username, "", proxyPluginName, false) + cluster, err := members.GetClusterAccess(username, "", proxyPluginName, false) if err != nil { return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) } @@ -319,10 +318,10 @@ func (p *Proxy) processHomeWorkspaceRequest(ctx echo.Context, userID, username, } // processWorkspaceRequest process an HTTP Request targeting a specific workspace. -func (p *Proxy) processWorkspaceRequest(ctx echo.Context, userID, username, workspaceName, proxyPluginName string) (*access.ClusterAccess, error) { +func (p *Proxy) processWorkspaceRequest(ctx echo.Context, username, workspaceName, proxyPluginName string) (*access.ClusterAccess, error) { // check that the user is provisioned and the space exists. // if the PublicViewer support is enabled, user check is skipped. - if err := p.checkUserIsProvisionedAndSpaceExists(ctx, userID, username, workspaceName); err != nil { + if err := p.checkUserIsProvisionedAndSpaceExists(ctx, username, workspaceName); err != nil { return nil, err } @@ -338,13 +337,13 @@ func (p *Proxy) processWorkspaceRequest(ctx echo.Context, userID, username, work } // retrieve the ClusterAccess for the user and the target workspace - return p.getClusterAccess(ctx, userID, username, proxyPluginName, workspace) + return p.getClusterAccess(ctx, username, proxyPluginName, workspace) } // checkUserIsProvisionedAndSpaceExists checks that the user is provisioned and the Space exists. // If the PublicViewer support is enabled, User check is skipped. -func (p *Proxy) checkUserIsProvisionedAndSpaceExists(ctx echo.Context, userID, username, workspaceName string) error { - if err := p.checkUserIsProvisioned(ctx, userID, username); err != nil { +func (p *Proxy) checkUserIsProvisionedAndSpaceExists(ctx echo.Context, username, workspaceName string) error { + if err := p.checkUserIsProvisioned(ctx, username); err != nil { return crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) } if err := p.checkSpaceExists(workspaceName); err != nil { @@ -366,7 +365,7 @@ func (p *Proxy) checkSpaceExists(workspaceName string) error { // checkUserIsProvisioned checks whether the user is Approved, if they are not an error is returned. // If public-viewer is enabled, user validation is skipped. -func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, userID, username string) error { +func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, username string) error { // skip if public-viewer is enabled: read-only operations on community workspaces are always permitted. if context.IsPublicViewerEnabled(ctx) { return nil @@ -376,7 +375,7 @@ func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, userID, username string // // UserSignup complete status is not checked, since it might cause the proxy blocking the request // and returning an error when quick transitions from ready to provisioning are happening. - userSignup, err := p.signupService.GetSignup(nil, userID, username, false) + userSignup, err := p.signupService.GetSignup(nil, username, false) if err != nil { return err } @@ -395,9 +394,9 @@ func (p *Proxy) checkUserIsProvisioned(ctx echo.Context, userID, username string // if the user has access to it. // Access can be either direct (a SpaceBinding linking the user to the workspace exists) // or community (a SpaceBinding linking the PublicViewer user to the workspace exists). -func (p *Proxy) getClusterAccess(ctx echo.Context, userID, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) { +func (p *Proxy) getClusterAccess(ctx echo.Context, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) { // retrieve cluster access as requesting user or PublicViewer - cluster, err := p.getClusterAccessAsUserOrPublicViewer(ctx, userID, username, proxyPluginName, workspace) + cluster, err := p.getClusterAccessAsUserOrPublicViewer(ctx, username, proxyPluginName, workspace) if err != nil { return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) } @@ -410,11 +409,11 @@ func (p *Proxy) getClusterAccess(ctx echo.Context, userID, username, proxyPlugin // this function returns the ClusterAccess impersonating the PublicViewer user. // If requesting user does not exists and PublicViewer is disabled or does not have access to the workspace, // this function returns an error. -func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) { +func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, username, proxyPluginName string, workspace *toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) { // retrieve the requesting user's UserSignup - userSignup, err := p.signupService.GetSignup(nil, userID, username, false) + userSignup, err := p.signupService.GetSignup(nil, username, false) if err != nil { - log.Error(nil, err, fmt.Sprintf("error retrieving user signup for userID '%s' and username '%s'", userID, username)) + log.Error(nil, err, fmt.Sprintf("error retrieving user signup for username '%s'", username)) return nil, crterrors.NewInternalError(errs.New("unable to get user info"), "error retrieving user") } @@ -423,7 +422,6 @@ func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, u members := NewMemberClusters(p.Client, p.signupService, p.getMembersFunc) if publicViewerEnabled && !userHasDirectAccess(userSignup, workspace) { return members.GetClusterAccess( - toolchainv1alpha1.KubesawAuthenticatedUsername, toolchainv1alpha1.KubesawAuthenticatedUsername, workspace.Name, proxyPluginName, @@ -431,7 +429,7 @@ func (p *Proxy) getClusterAccessAsUserOrPublicViewer(ctx echo.Context, userID, u } // otherwise retrieve the ClusterAccess for the cluster hosting the workspace and the given user. - return members.GetClusterAccess(userID, username, workspace.Name, proxyPluginName, publicViewerEnabled) + return members.GetClusterAccess(username, workspace.Name, proxyPluginName, publicViewerEnabled) } // userHasDirectAccess checks if an UserSignup has access to a workspace. @@ -545,8 +543,8 @@ func customHTTPErrorHandler(cause error, ctx echo.Context) { } } -// addUserContext updates echo.Context with the User ID extracted from the Bearer token. -// To be used for storing the user ID and logging only. +// addUserContext updates echo.Context with the claims extracted from the Bearer token. +// To be used for storing the claims and logging only. func (p *Proxy) addUserContext() echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(ctx echo.Context) error { @@ -658,7 +656,7 @@ func (p *Proxy) extractUserToken(req *http.Request) (*auth.TokenClaims, error) { token, err := p.tokenParser.FromString(userToken) if err != nil { - return nil, crterrors.NewUnauthorizedError("unable to extract userID from token", err.Error()) + return nil, crterrors.NewUnauthorizedError("unable to extract claims from token", err.Error()) } return token, nil } diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 2bed79a6..0ce94a66 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -215,10 +215,10 @@ func (s *TestProxySuite) checkPlainHTTPErrors(proxy *Proxy) { require.NotNil(s.T(), resp) defer resp.Body.Close() assert.Equal(s.T(), http.StatusUnauthorized, resp.StatusCode) - s.assertResponseBody(resp, "invalid bearer token: unable to extract userID from token: token is malformed: token contains an invalid number of segments") + s.assertResponseBody(resp, "invalid bearer token: unable to extract claims from token: token is malformed: token contains an invalid number of segments") }) - s.Run("unauthorized if can't extract userID from a valid token", func() { + s.Run("unauthorized if can't extract claims from a valid token", func() { // when req, err := http.NewRequest("GET", "http://localhost:8081/api/mycoolworkspace/pods", nil) require.NoError(s.T(), err) @@ -231,7 +231,7 @@ func (s *TestProxySuite) checkPlainHTTPErrors(proxy *Proxy) { require.NotNil(s.T(), resp) defer resp.Body.Close() assert.Equal(s.T(), http.StatusUnauthorized, resp.StatusCode) - s.assertResponseBody(resp, "invalid bearer token: unable to extract userID from token: token does not comply to expected claims: subject missing") + s.assertResponseBody(resp, "invalid bearer token: unable to extract claims from token: token does not comply to expected claims: subject missing") }) s.Run("unauthorized if can't extract email from a valid token", func() { @@ -247,7 +247,7 @@ func (s *TestProxySuite) checkPlainHTTPErrors(proxy *Proxy) { require.NotNil(s.T(), resp) defer resp.Body.Close() assert.Equal(s.T(), http.StatusUnauthorized, resp.StatusCode) - s.assertResponseBody(resp, "invalid bearer token: unable to extract userID from token: token does not comply to expected claims: email missing") + s.assertResponseBody(resp, "invalid bearer token: unable to extract claims from token: token does not comply to expected claims: email missing") }) s.Run("unauthorized if workspace context is invalid", func() { @@ -353,7 +353,7 @@ func (s *TestProxySuite) checkWebsocketsError() { }, "not a jwt token": { ProtocolHeaders: []string{"base64url.bearer.authorization.k8s.io.dG9rZW4,dummy"}, - ExpectedError: "invalid bearer token: unable to extract userID from token: token is malformed: token contains an invalid number of segments", + ExpectedError: "invalid bearer token: unable to extract claims from token: token is malformed: token contains an invalid number of segments", }, "invalid token is not base64 encoded": { ProtocolHeaders: []string{"base64url.bearer.authorization.k8s.io.token,dummy"}, @@ -515,7 +515,7 @@ func (s *TestProxySuite) checkProxyOK(proxy *Proxy) { ExpectedProxyResponseStatus int Standalone bool // If true then the request is not expected to be forwarded to the kube api server - OverrideGetSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + OverrideGetSignupFunc func(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) ExpectedResponse *string }{ "plain http cors preflight request with no request method": { @@ -680,7 +680,7 @@ func (s *TestProxySuite) checkProxyOK(proxy *Proxy) { "Authorization": {"Bearer clusterSAToken"}, }, ExpectedProxyResponseStatus: http.StatusInternalServerError, - OverrideGetSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + OverrideGetSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("test error") }, ExpectedResponse: ptr("unable to retrieve user workspaces: test error"), diff --git a/pkg/signup/service/signup_service.go b/pkg/signup/service/signup_service.go index d5e4f9c9..dd070ecf 100644 --- a/pkg/signup/service/signup_service.go +++ b/pkg/signup/service/signup_service.go @@ -31,8 +31,6 @@ import ( ) const ( - DNS1123NameMaximumLength = 63 - // NoSpaceKey is the query key for specifying whether the UserSignup should be created without a Space NoSpaceKey = "no-space" ) @@ -64,7 +62,7 @@ func NewSignupService(client namespaced.Client, opts ...SignupServiceOption) ser return s } -// newUserSignup generates a new UserSignup resource with the specified username and userID. +// newUserSignup generates a new UserSignup resource with the specified username and available claims. // This resource then can be used to create a new UserSignup in the host cluster or to update the existing one. func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { username := ctx.GetString(context.UsernameKey) @@ -78,7 +76,7 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi } if isCRTAdmin(username) { - log.Info(ctx, fmt.Sprintf("A crtadmin user '%s' just tried to signup - the UserID is: '%s'", ctx.GetString(context.UsernameKey), ctx.GetString(context.SubKey))) + log.Info(ctx, fmt.Sprintf("A crtadmin user '%s' just tried to signup", ctx.GetString(context.UsernameKey))) return nil, apierrors.NewForbidden(schema.GroupResource{}, "", fmt.Errorf("failed to create usersignup for %s", username)) } @@ -233,28 +231,21 @@ func extractEmailHost(email string) string { return email[i+1:] } -// Signup reactivates the deactivated UserSignup resource or creates a new one with the specified username and userID +// Signup reactivates the deactivated UserSignup resource or creates a new one with the specified username // if doesn't exist yet. func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { - encodedUserID := signupcommon.EncodeUserIdentifier(ctx.GetString(context.SubKey)) + username := ctx.GetString(context.UsernameKey) + encodedUsername := signupcommon.EncodeUserIdentifier(username) // Retrieve UserSignup resource from the host cluster userSignup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(ctx, s.NamespacedName(encodedUserID), userSignup); err != nil { + if err := s.Get(ctx, s.NamespacedName(encodedUsername), userSignup); err != nil { if apierrors.IsNotFound(err) { - // The UserSignup could not be located by its encoded UserID, attempt to load it using its encoded PreferredUsername instead - encodedUsername := signupcommon.EncodeUserIdentifier(ctx.GetString(context.UsernameKey)) - if err := s.Get(ctx, s.NamespacedName(encodedUsername), userSignup); err != nil { - if apierrors.IsNotFound(err) { - // New Signup - log.WithValues(map[string]interface{}{"encoded_user_id": encodedUserID}).Info(ctx, "user not found, creating a new one") - return s.createUserSignup(ctx) - } - return nil, err - } - } else { - return nil, err + // New Signup + log.WithValues(map[string]interface{}{"encoded_username": encodedUsername}).Info(ctx, "user not found, creating a new one") + return s.createUserSignup(ctx) } + return nil, err } // Check UserSignup status to determine whether user signup is deactivated @@ -264,12 +255,11 @@ func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, e return s.reactivateUserSignup(ctx, userSignup) } - username := ctx.GetString(context.UsernameKey) return nil, apierrors.NewConflict(schema.GroupResource{}, "", fmt.Errorf( - "UserSignup [id: %s; username: %s]. Unable to create UserSignup because there is already an active UserSignup with such ID", encodedUserID, username)) + "UserSignup [username: %s]. Unable to create UserSignup because there is already an active UserSignup with such a username", username)) } -// createUserSignup creates a new UserSignup resource with the specified username and userID +// createUserSignup creates a new UserSignup resource with the specified username func (s *ServiceImpl) createUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { userSignup, err := s.newUserSignup(ctx) if err != nil { @@ -279,7 +269,7 @@ func (s *ServiceImpl) createUserSignup(ctx *gin.Context) (*toolchainv1alpha1.Use return userSignup, s.Create(ctx, userSignup) } -// reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username and userID +// reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { // Update the existing usersignup's spec and annotations/labels by new values from a freshly generated one. // We don't want to deal with merging/patching the usersignup resource @@ -310,24 +300,23 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain // The checkUserSignupCompleted was introduced in order to avoid checking the readiness of the complete condition on the UserSignup in certain situations, // such as proxy calls for example. // Returns nil, nil if the UserSignup resource is not found or if it's deactivated. -func (s *ServiceImpl) GetSignup(ctx *gin.Context, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { - return s.DoGetSignup(ctx, s.Client, userID, username, checkUserSignupCompleted) +func (s *ServiceImpl) GetSignup(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { + return s.DoGetSignup(ctx, s.Client, username, checkUserSignupCompleted) } -func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userID, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { +func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { var userSignup *toolchainv1alpha1.UserSignup err := signup.PollUpdateSignup(ctx, func() error { - // Retrieve UserSignup resource from the host cluster, using the specified UserID and username - var getError error - userSignup, getError = s.DoGetUserSignupFromIdentifier(cl, userID, username) - // If an error was returned, then return here - if getError != nil { - if apierrors.IsNotFound(getError) { + // Retrieve UserSignup resource from the host cluster + us := &toolchainv1alpha1.UserSignup{} + if err := cl.Get(gocontext.TODO(), cl.NamespacedName(signupcommon.EncodeUserIdentifier(username)), us); err != nil { + if apierrors.IsNotFound(err) { return nil } - return getError + return err } + userSignup = us // Otherwise if the returned userSignup is nil, return here also if userSignup == nil || ctx == nil { @@ -505,41 +494,15 @@ func (s *ServiceImpl) auditUserSignupAgainstClaims(ctx *gin.Context, userSignup return updated } -// GetUserSignupFromIdentifier is used to return the actual UserSignup resource instance, rather than the Signup DTO -func (s *ServiceImpl) GetUserSignupFromIdentifier(userID, username string) (*toolchainv1alpha1.UserSignup, error) { - return s.DoGetUserSignupFromIdentifier(s.Client, userID, username) -} - -// GetUserSignupFromIdentifier is used to return the actual UserSignup resource instance, rather than the Signup DTO -func (s *ServiceImpl) DoGetUserSignupFromIdentifier(cl namespaced.Client, userID, username string) (*toolchainv1alpha1.UserSignup, error) { - // Retrieve UserSignup resource from the host cluster - userSignup := &toolchainv1alpha1.UserSignup{} - if err := cl.Get(gocontext.TODO(), cl.NamespacedName(signupcommon.EncodeUserIdentifier(username)), userSignup); err != nil { - if apierrors.IsNotFound(err) { - // Capture any error here in a separate var, as we need to preserve the original - if err2 := cl.Get(gocontext.TODO(), cl.NamespacedName(signupcommon.EncodeUserIdentifier(userID)), userSignup); err2 != nil { - if apierrors.IsNotFound(err2) { - return nil, err - } - return nil, err2 - } - return userSignup, nil - } - return nil, err - } - - return userSignup, nil -} - var ( md5Matcher = regexp.MustCompile("(?i)[a-f0-9]{32}$") ) // PhoneNumberAlreadyInUse checks if the phone number has been banned. If so, return -// an internal server error. If not, check if an approved UserSignup with a different userID and username +// an internal server error. If not, check if an approved UserSignup with a different username // and email address exists. If so, return an internal server error. Otherwise, return without error. // Either the actual phone number, or the md5 hash of the phone number may be provided here. -func (s *ServiceImpl) PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHash string) error { +func (s *ServiceImpl) PhoneNumberAlreadyInUse(username, phoneNumberOrHash string) error { labelValue := hash.EncodeString(phoneNumberOrHash) if md5Matcher.Match([]byte(phoneNumberOrHash)) { labelValue = phoneNumberOrHash @@ -566,7 +529,7 @@ func (s *ServiceImpl) PhoneNumberAlreadyInUse(userID, username, phoneNumberOrHas for _, signup := range userSignups.Items { userSignup := signup // drop with go 1.22 - if userSignup.Spec.IdentityClaims.Sub != userID && userSignup.Spec.IdentityClaims.PreferredUsername != username && !states.Deactivated(&userSignup) { + if userSignup.Spec.IdentityClaims.PreferredUsername != username && !states.Deactivated(&userSignup) { return errors.NewForbiddenError("cannot re-register with phone number", "phone number already in use") } diff --git a/pkg/signup/service/signup_service_test.go b/pkg/signup/service/signup_service_test.go index f568cae4..bfc82ef1 100644 --- a/pkg/signup/service/signup_service_test.go +++ b/pkg/signup/service/signup_service_test.go @@ -240,7 +240,7 @@ func (s *TestSignupServiceSuite) TestGetSignupFailsWithNotFoundThenOtherError() c, _ := gin.CreateTestContext(httptest.NewRecorder()) // when - _, err := application.SignupService().GetSignup(c, "", "abc", true) + _, err := application.SignupService().GetSignup(c, "abc", true) // then require.EqualError(s.T(), err, "something quite unfortunate happened: something bad") @@ -434,7 +434,7 @@ func (s *TestSignupServiceSuite) TestFailsIfUserSignupNameAlreadyExists() { _, err := application.SignupService().Signup(ctx) // then - require.EqualError(s.T(), err, "Operation cannot be fulfilled on \"\": UserSignup [id: userid; username: jsmith@kubesaw]. Unable to create UserSignup because there is already an active UserSignup with such ID") + require.EqualError(s.T(), err, "Operation cannot be fulfilled on \"\": UserSignup [username: jsmith@kubesaw]. Unable to create UserSignup because there is already an active UserSignup with such a username") } func (s *TestSignupServiceSuite) TestFailsIfUserBanned() { @@ -495,7 +495,7 @@ func (s *TestSignupServiceSuite) TestPhoneNumberAlreadyInUseBannedUser() { _, application := testutil.PrepareInClusterApp(s.T(), bannedUser) // when - err := application.SignupService().PhoneNumberAlreadyInUse("", "jsmith", "+12268213044") + err := application.SignupService().PhoneNumberAlreadyInUse("jsmith", "+12268213044") // then require.EqualError(s.T(), err, "cannot re-register with phone number: phone number already in use") @@ -513,7 +513,7 @@ func (s *TestSignupServiceSuite) TestPhoneNumberAlreadyInUseUserSignup() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) // when - err := application.SignupService().PhoneNumberAlreadyInUse("", "jsmith", "+12268213044") + err := application.SignupService().PhoneNumberAlreadyInUse("jsmith", "+12268213044") // then require.EqualError(s.T(), err, "cannot re-register with phone number: phone number already in use") @@ -522,16 +522,10 @@ func (s *TestSignupServiceSuite) TestPhoneNumberAlreadyInUseUserSignup() { func (s *TestSignupServiceSuite) TestOKIfOtherUserBanned() { s.ServiceConfiguration(true, "", 5) - userID, err := uuid.NewV4() - require.NoError(s.T(), err) - - bannedUserID, err := uuid.NewV4() - require.NoError(s.T(), err) - bannedUser := &toolchainv1alpha1.BannedUser{ TypeMeta: v1.TypeMeta{}, ObjectMeta: v1.ObjectMeta{ - Name: bannedUserID.String(), + Name: "banneduser", Namespace: commontest.HostOperatorNs, Labels: map[string]string{ toolchainv1alpha1.BannedUserEmailHashLabelKey: "1df66fbb427ff7e64ac46af29cc74b71", @@ -541,12 +535,11 @@ func (s *TestSignupServiceSuite) TestOKIfOtherUserBanned() { Email: "jane.doe@gmail.com", }, } - require.NoError(s.T(), err) rr := httptest.NewRecorder() ctx, _ := gin.CreateTestContext(rr) - ctx.Set(context.UsernameKey, "jsmith") - ctx.Set(context.SubKey, userID.String()) + ctx.Set(context.UsernameKey, "jsmith@gmail") + ctx.Set(context.SubKey, "userid") ctx.Set(context.EmailKey, "jsmith@gmail.com") fakeClient, application := testutil.PrepareInClusterApp(s.T(), bannedUser) @@ -565,7 +558,7 @@ func (s *TestSignupServiceSuite) TestOKIfOtherUserBanned() { val := userSignups.Items[0] require.Equal(s.T(), commontest.HostOperatorNs, val.Namespace) - require.Equal(s.T(), "jsmith", val.Name) + require.Equal(s.T(), signupcommon.EncodeUserIdentifier("jsmith@gmail"), val.Name) require.False(s.T(), states.ApprovedManually(&val)) require.Equal(s.T(), "a7b1b413c1cbddbcd19a51222ef8e20a", val.Labels[toolchainv1alpha1.UserSignupUserEmailHashLabelKey]) } @@ -584,7 +577,7 @@ func (s *TestSignupServiceSuite) TestGetUserSignupFails() { } // when - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) // then require.EqualError(s.T(), err, "an error occurred") @@ -595,7 +588,7 @@ func (s *TestSignupServiceSuite) TestGetSignupNotFound() { _, application := testutil.PrepareInClusterApp(s.T()) // when - signup, err := application.SignupService().GetSignup(c, "", "does-not-exist", true) + signup, err := application.SignupService().GetSignup(c, "does-not-exist", true) // then require.Nil(s.T(), signup) @@ -614,13 +607,12 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusNotComplete() { testusersignup.SignupIncomplete("test_reason", "test_message"), testusersignup.ApprovedAutomaticallyAgo(0), ) - states.SetVerificationRequired(userSignupNotComplete, true) _, application := testutil.PrepareInClusterApp(s.T(), userSignupNotComplete) // when - response, err := application.SignupService().GetSignup(c, "", "not-complete@kubesaw", true) + response, err := application.SignupService().GetSignup(c, "not-complete@kubesaw", true) // then require.NoError(s.T(), err) @@ -656,7 +648,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusNotComplete() { // when // we set checkUserSignupCompleted to false - response, err := svc.GetSignup(c, "", "not-complete@kubesaw", false) + response, err := svc.GetSignup(c, "not-complete@kubesaw", false) // then require.NoError(s.T(), err) @@ -730,7 +722,7 @@ func (s *TestSignupServiceSuite) TestGetSignupNoStatusNotCompleteCondition() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) // when - response, err := application.SignupService().GetSignup(c, "", "bill", true) + response, err := application.SignupService().GetSignup(c, "bill", true) // then require.NoError(s.T(), err) @@ -765,7 +757,7 @@ func (s *TestSignupServiceSuite) TestGetSignupDeactivated() { c, _ := gin.CreateTestContext(httptest.NewRecorder()) // when - signup, err := application.SignupService().GetSignup(c, "", username, true) + signup, err := application.SignupService().GetSignup(c, username, true) // then require.Nil(s.T(), signup) @@ -790,7 +782,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusOK() { c, _ := gin.CreateTestContext(httptest.NewRecorder()) // when - response, err := application.SignupService().GetSignup(c, "", username, true) + response, err := application.SignupService().GetSignup(c, username, true) // then require.NoError(s.T(), err) @@ -868,7 +860,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusFailGetToolchainStatus() { _, application := testutil.PrepareInClusterApp(s.T(), us, mur, space) // when - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) // then require.EqualError(s.T(), err, fmt.Sprintf("error when retrieving ToolchainStatus to set Che Dashboard for completed UserSignup %s: toolchainstatuses.toolchain.dev.openshift.com \"toolchain-status\" not found", us.Name)) @@ -892,7 +884,7 @@ func (s *TestSignupServiceSuite) TestGetSignupMURGetFails() { } // when - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) // then require.EqualError(s.T(), err, fmt.Sprintf("error when retrieving MasterUserRecord for completed UserSignup %s: an error occurred", us.Name)) @@ -974,7 +966,7 @@ func (s *TestSignupServiceSuite) TestGetSignupReadyConditionStatus() { _, application := testutil.PrepareInClusterApp(s.T(), us, mur, space, toolchainStatus) // when - response, err := application.SignupService().GetSignup(c, "", username, true) + response, err := application.SignupService().GetSignup(c, username, true) // then require.NoError(s.T(), err) @@ -1004,7 +996,7 @@ func (s *TestSignupServiceSuite) TestGetSignupBannedUserEmail() { ctx.Set(context.EmailKey, "jsmith@gmail.com") // when - response, err := application.SignupService().GetSignup(ctx, "", "ted@kubesaw", true) + response, err := application.SignupService().GetSignup(ctx, "ted@kubesaw", true) // then // return not found signup @@ -1232,7 +1224,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() s.ServiceConfiguration(false, "", 5) - // Create a new UserSignup, set its UserID and AccountID annotations + // Create a new UserSignup username, userSignup := s.newUserSignupComplete() mur := &toolchainv1alpha1.MasterUserRecord{ @@ -1259,7 +1251,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() c.Set(context.UsernameKey, "cocochanel") fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, mur) - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) require.NoError(s.T(), err) modified := &toolchainv1alpha1.UserSignup{} @@ -1282,7 +1274,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() c, _ := gin.CreateTestContext(httptest.NewRecorder()) c.Set(context.GivenNameKey, "Jonathan") - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) require.NoError(s.T(), err) modified := &toolchainv1alpha1.UserSignup{} @@ -1308,7 +1300,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() c.Set(context.FamilyNameKey, "Smythe") c.Set(context.CompanyKey, "Red Hat") - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) require.NoError(s.T(), err) modified := &toolchainv1alpha1.UserSignup{} @@ -1336,7 +1328,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() c.Set(context.OriginalSubKey, "jsmythe-original-sub") c.Set(context.EmailKey, "jsmythe@redhat.com") - _, err := application.SignupService().GetSignup(c, "", username, true) + _, err := application.SignupService().GetSignup(c, username, true) require.NoError(s.T(), err) modified := &toolchainv1alpha1.UserSignup{} diff --git a/pkg/verification/service/verification_service.go b/pkg/verification/service/verification_service.go index 3da87ef1..b73bc8b4 100644 --- a/pkg/verification/service/verification_service.go +++ b/pkg/verification/service/verification_service.go @@ -12,6 +12,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/namespaced" signuppkg "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/pkg/verification/sender" + signupcommon "github.com/codeready-toolchain/toolchain-common/pkg/usersignup" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/application/service" @@ -64,15 +65,15 @@ func NewVerificationService(context servicecontext.ServiceContext, opts ...Verif // InitVerification sends a verification message to the specified user, using the Twilio service. If successful, // the user will receive a verification SMS. The UserSignup resource is updated with a number of annotations in order // to manage the phone verification process and protect against system abuse. -func (s *ServiceImpl) InitVerification(ctx *gin.Context, userID, username, e164PhoneNumber, countryCode string) error { - signup, err := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if err != nil { +func (s *ServiceImpl) InitVerification(ctx *gin.Context, username, e164PhoneNumber, countryCode string) error { + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { log.Error(ctx, err, "usersignup not found") return crterrors.NewNotFoundError(err, "usersignup not found") } log.Error(ctx, err, "error retrieving usersignup") - return crterrors.NewInternalError(err, fmt.Sprintf("error retrieving usersignup: %s", userID)) + return crterrors.NewInternalError(err, fmt.Sprintf("error retrieving usersignup with username '%s'", username)) } labelValues := map[string]string{} @@ -80,12 +81,12 @@ func (s *ServiceImpl) InitVerification(ctx *gin.Context, userID, username, e164P // check that verification is required before proceeding if !states.VerificationRequired(signup) { - log.Info(ctx, fmt.Sprintf("phone verification attempted for user without verification requirement: %s", userID)) + log.Info(ctx, fmt.Sprintf("phone verification attempted for user without verification requirement: '%s'", signup.Name)) return crterrors.NewBadRequest("forbidden request", "verification code will not be sent") } // Check if the provided phone number is already being used by another user - err = s.Services().SignupService().PhoneNumberAlreadyInUse(userID, username, e164PhoneNumber) + err := s.Services().SignupService().PhoneNumberAlreadyInUse(username, e164PhoneNumber) if err != nil { e := &crterrors.Error{} switch { @@ -168,8 +169,8 @@ func (s *ServiceImpl) InitVerification(ctx *gin.Context, userID, username, e164P } doUpdate := func() error { - signup, err := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if err != nil { + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { return err } if signup.Labels == nil { @@ -221,18 +222,18 @@ func generateVerificationCode() (string, error) { // VerifyPhoneCode validates the user's phone verification code. It updates the specified UserSignup value, so even // if an error is returned by this function the caller should still process changes to it -func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, userID, username, code string) (verificationErr error) { +func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, username, code string) (verificationErr error) { cfg := configuration.GetRegistrationServiceConfig() // If we can't even find the UserSignup, then die here - signup, lookupErr := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if lookupErr != nil { - if apierrors.IsNotFound(lookupErr) { - log.Error(ctx, lookupErr, "usersignup not found") - return crterrors.NewNotFoundError(lookupErr, "user not found") + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if apierrors.IsNotFound(err) { + log.Error(ctx, err, "usersignup not found") + return crterrors.NewNotFoundError(err, "user not found") } - log.Error(ctx, lookupErr, "error retrieving usersignup") - return crterrors.NewInternalError(lookupErr, fmt.Sprintf("error retrieving usersignup: %s", userID)) + log.Error(ctx, err, "error retrieving usersignup") + return crterrors.NewInternalError(err, fmt.Sprintf("error retrieving usersignup with username '%s'", username)) } // check if it's a reactivation @@ -262,7 +263,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, userID, username, code s annotationsToDelete := []string{} unsetVerificationRequired := false - err := s.Services().SignupService().PhoneNumberAlreadyInUse(userID, username, signup.Labels[toolchainv1alpha1.UserSignupUserPhoneHashLabelKey]) + err := s.Services().SignupService().PhoneNumberAlreadyInUse(username, signup.Labels[toolchainv1alpha1.UserSignupUserPhoneHashLabelKey]) if err != nil { log.Error(ctx, err, "phone number to verify already in use") return crterrors.NewBadRequest("phone number already in use", @@ -322,9 +323,9 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, userID, username, code s } doUpdate := func() error { - signup, err := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if err != nil { - log.Error(ctx, err, fmt.Sprintf("error getting signup from identifier. user_id: %s | username: %s", userID, username)) + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + log.Error(ctx, err, fmt.Sprintf("error getting signup with username '%s'", username)) return err } @@ -385,15 +386,15 @@ func checkRequiredManualApproval(ctx *gin.Context, signup *toolchainv1alpha1.Use // VerifyActivationCode verifies the activation code: // - checks that the SocialEvent resource named after the activation code exists // - checks that the SocialEvent has enough capacity to approve the user -func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, userID, username, code string) error { +func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, username, code string) error { log.Infof(ctx, "verifying activation code '%s'", code) // look-up the UserSignup - signup, err := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if err != nil { + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { return crterrors.NewNotFoundError(err, "user not found") } - return crterrors.NewInternalError(err, fmt.Sprintf("error retrieving usersignup: %s", userID)) + return crterrors.NewInternalError(err, fmt.Sprintf("error retrieving usersignup with username '%s'", username)) } annotationValues := map[string]string{} annotationsToDelete := []string{} @@ -402,8 +403,8 @@ func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, userID, username, c defer func() { doUpdate := func() error { - signup, err := s.Services().SignupService().GetUserSignupFromIdentifier(userID, username) - if err != nil { + signup := &toolchainv1alpha1.UserSignup{} + if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { return err } if unsetVerificationRequired { diff --git a/pkg/verification/service/verification_service_test.go b/pkg/verification/service/verification_service_test.go index 93d9208e..909c02d1 100644 --- a/pkg/verification/service/verification_service_test.go +++ b/pkg/verification/service/verification_service_test.go @@ -150,7 +150,7 @@ func (s *TestVerificationServiceSuite) TestInitVerification() { // Test the init verification for the first UserSignup ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", "johnny@kubesaw", "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, "johnny@kubesaw", "+1NUMBER", "1") require.NoError(s.T(), err) signup := &toolchainv1alpha1.UserSignup{} @@ -186,7 +186,7 @@ func (s *TestVerificationServiceSuite) TestInitVerification() { ctx, _ = gin.CreateTestContext(httptest.NewRecorder()) // for the second usersignup - err = application.VerificationService().InitVerification(ctx, "", "jsmith@kubesaw", "+61NUMBER", "1") + err = application.VerificationService().InitVerification(ctx, "jsmith@kubesaw", "+61NUMBER", "1") require.NoError(s.T(), err) signup2 := &toolchainv1alpha1.UserSignup{} @@ -260,8 +260,8 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { } ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") - require.EqualError(s.T(), err, "get failed: error retrieving usersignup: ", err.Error()) + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + require.EqualError(s.T(), err, "get failed: error retrieving usersignup with username 'johnny@kubesaw'", err.Error()) }) s.Run("when client UPDATE call fails indefinitely should return error", func() { @@ -274,7 +274,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { } ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "there was an error while updating your account - please wait a moment before "+ "trying again. If this error persists, please contact the Developer Sandbox team at devsandbox@redhat.com "+ "for assistance: error while verifying phone code") @@ -294,7 +294,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { } ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.NoError(s.T(), err) signup := &toolchainv1alpha1.UserSignup{} @@ -346,7 +346,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationPassesWhenMaxCountRea fakeClient, application := testutil.PrepareInClusterAppWithOption(s.T(), httpClientFactoryOption(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.NoError(s.T(), err) signup := &toolchainv1alpha1.UserSignup{} @@ -387,7 +387,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsWhenCountContain _, application := testutil.PrepareInClusterAppWithOption(s.T(), httpClientFactoryOption(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "daily limit exceeded: cannot generate new verification code") } @@ -413,7 +413,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsDailyCounterExce _, application := testutil.PrepareInClusterAppWithOption(s.T(), httpClientFactoryOption(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") + err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "daily limit exceeded: cannot generate new verification code", err.Error()) require.Empty(s.T(), userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) } @@ -445,7 +445,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsWhenPhoneNumberI fakeClient, application := testutil.PrepareInClusterAppWithOption(s.T(), httpClientFactoryOption(), alphaUserSignup, bravoUserSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") + err := application.VerificationService().InitVerification(ctx, bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") require.Error(s.T(), err) require.Equal(s.T(), "phone number already in use: cannot register using phone number: +19875551122", err.Error()) @@ -485,7 +485,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationOKWhenPhoneNumberInUs fakeClient, application := testutil.PrepareInClusterAppWithOption(s.T(), httpClientFactoryOption(), alphaUserSignup, bravoUserSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().InitVerification(ctx, "", bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") + err := application.VerificationService().InitVerification(ctx, bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") require.NoError(s.T(), err) // Reload bravoUserSignup @@ -515,7 +515,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.NoError(s.T(), err) signup := &toolchainv1alpha1.UserSignup{} @@ -539,7 +539,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", "employee085@kubesaw", "654321") + err := application.VerificationService().VerifyPhoneCode(ctx, "employee085@kubesaw", "654321") require.NoError(s.T(), err) signup := &toolchainv1alpha1.UserSignup{} @@ -562,7 +562,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.Error(s.T(), err) e := &crterrors.Error{} require.ErrorAs(s.T(), err, &e) @@ -583,7 +583,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") e := &crterrors.Error{} require.ErrorAs(s.T(), err, &e) require.Equal(s.T(), "expired: verification code expired", e.Error()) @@ -603,7 +603,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "too many verification attempts", err.Error()) }) @@ -620,7 +620,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "too many verification attempts", err.Error()) signup := &toolchainv1alpha1.UserSignup{} @@ -643,7 +643,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "parsing time \"ABC\" as \"2006-01-02T15:04:05.000Z07:00\": cannot parse \"ABC\" as \"2006\": error parsing expiry timestamp", err.Error()) }) @@ -742,7 +742,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) - err := application.VerificationService().VerifyPhoneCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "123456") + err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") // then signup := &toolchainv1alpha1.UserSignup{} @@ -776,7 +776,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.NoError(s.T(), err) @@ -794,7 +794,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.NoError(s.T(), err) @@ -814,7 +814,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.EqualError(s.T(), err, "too many verification attempts: 3") @@ -833,7 +833,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "invalid") + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "invalid") // then require.EqualError(s.T(), err, "invalid code: the provided code is invalid") @@ -852,7 +852,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, "invalid") + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "invalid") // then require.EqualError(s.T(), err, "invalid code: the provided code is invalid") @@ -871,7 +871,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.EqualError(s.T(), err, "invalid code: the event is full") @@ -890,7 +890,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.EqualError(s.T(), err, "invalid code: the provided code is invalid") @@ -909,7 +909,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) // when - err := application.VerificationService().VerifyActivationCode(ctx, "", userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) + err := application.VerificationService().VerifyActivationCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, event.Name) // then require.EqualError(s.T(), err, "invalid code: the provided code is invalid") diff --git a/test/fake/proxy.go b/test/fake/proxy.go index f0698ef9..c60c9155 100644 --- a/test/fake/proxy.go +++ b/test/fake/proxy.go @@ -31,29 +31,26 @@ func (m *SignupService) addSignup(identifier string, userSignup *signup.Signup) } type SignupService struct { - MockGetSignup func(userID, username string) (*signup.Signup, error) + MockGetSignup func(username string) (*signup.Signup, error) userSignups map[string]*signup.Signup } -func (m *SignupService) DefaultMockGetSignup() func(userID, username string) (*signup.Signup, error) { - return func(_, username string) (userSignup *signup.Signup, e error) { +func (m *SignupService) DefaultMockGetSignup() func(username string) (*signup.Signup, error) { + return func(username string) (userSignup *signup.Signup, e error) { return m.userSignups[username], nil } } -func (m *SignupService) GetSignup(_ *gin.Context, userID, username string, _ bool) (*signup.Signup, error) { - return m.MockGetSignup(userID, username) +func (m *SignupService) GetSignup(_ *gin.Context, username string, _ bool) (*signup.Signup, error) { + return m.MockGetSignup(username) } func (m *SignupService) Signup(_ *gin.Context) (*toolchainv1alpha1.UserSignup, error) { return nil, nil } -func (m *SignupService) GetUserSignupFromIdentifier(_, _ string) (*toolchainv1alpha1.UserSignup, error) { - return nil, nil -} func (m *SignupService) UpdateUserSignup(_ *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { return nil, nil } -func (m *SignupService) PhoneNumberAlreadyInUse(_, _, _ string) error { +func (m *SignupService) PhoneNumberAlreadyInUse(_, _ string) error { return nil }