From aa5425b8be0466abec7e1164c3172c78cb48996b Mon Sep 17 00:00:00 2001 From: Na'aman Hirschfeld Date: Mon, 15 Jan 2024 12:36:29 +0100 Subject: [PATCH] chore: updated webhook to log redirect logic and updated status codes --- .../internal/api/webhooks.go | 70 +++++++++++-------- .../internal/api/webhooks_test.go | 10 +-- shared/go/testutils/http.go | 8 ++- 3 files changed, 51 insertions(+), 37 deletions(-) diff --git a/services/dashboard-backend/internal/api/webhooks.go b/services/dashboard-backend/internal/api/webhooks.go index 3739c558..c9957764 100644 --- a/services/dashboard-backend/internal/api/webhooks.go +++ b/services/dashboard-backend/internal/api/webhooks.go @@ -8,6 +8,7 @@ import ( "github.com/basemind-ai/monorepo/shared/go/db/models" "github.com/basemind-ai/monorepo/shared/go/exc" "github.com/basemind-ai/monorepo/shared/go/urlutils" + "github.com/rs/zerolog/log" "net/http" ) @@ -25,24 +26,19 @@ func handleUserInvitationWebhook(w http.ResponseWriter, r *http.Request) { return } - invitation, retrievalErr := db.GetQueries(). - RetrieveProjectInvitationByID(r.Context(), *invitationID) - if retrievalErr != nil { - apierror.BadRequest("invitation does not exist").Render(w) - return - } - cfg := config.Get(r.Context()) // TODO: when we support localisation, we should pass locale as a query param as well. redirectURL := fmt.Sprintf("%s/en/sign-in", cfg.FrontendBaseURL) - if exc.MustResult( - db.GetQueries().CheckUserProjectExists(r.Context(), models.CheckUserProjectExistsParams{ - Email: invitation.Email, - ProjectID: invitation.ProjectID, - }), - ) { - http.Redirect(w, r, redirectURL, http.StatusSeeOther) + invitation, retrievalErr := db.GetQueries(). + RetrieveProjectInvitationByID(r.Context(), *invitationID) + + if retrievalErr != nil { + log.Debug(). + Err(retrievalErr). + Str("redirectURL", redirectURL). + Msg("failed to retrieve invitation, redirecting to sign in page") + http.Redirect(w, r, redirectURL, http.StatusPermanentRedirect) return } @@ -51,26 +47,38 @@ func handleUserInvitationWebhook(w http.ResponseWriter, r *http.Request) { queries := db.GetQueries().WithTx(tx) - userAccount, userRetrievalErr := queries.RetrieveUserAccountByEmail( - r.Context(), - invitation.Email, - ) - if userRetrievalErr != nil { - createdUserAccount := exc.MustResult( - queries.CreateUserAccount(r.Context(), models.CreateUserAccountParams{ - Email: invitation.Email, - }), + if !exc.MustResult( + db.GetQueries().CheckUserProjectExists(r.Context(), models.CheckUserProjectExistsParams{ + Email: invitation.Email, + ProjectID: invitation.ProjectID, + }), + ) { + userAccount, userRetrievalErr := queries.RetrieveUserAccountByEmail( + r.Context(), + invitation.Email, ) - userAccount = createdUserAccount + if userRetrievalErr != nil { + createdUserAccount := exc.MustResult( + queries.CreateUserAccount(r.Context(), models.CreateUserAccountParams{ + Email: invitation.Email, + }), + ) + userAccount = createdUserAccount + } + + exc.MustResult(queries.CreateUserProject(r.Context(), models.CreateUserProjectParams{ + ProjectID: invitation.ProjectID, + UserID: userAccount.ID, + Permission: invitation.Permission, + })) } - exc.MustResult(queries.CreateUserProject(r.Context(), models.CreateUserProjectParams{ - ProjectID: invitation.ProjectID, - UserID: userAccount.ID, - Permission: invitation.Permission, - })) exc.Must(queries.DeleteProjectInvitation(r.Context(), invitation.ID)) exc.Must(tx.Commit(r.Context())) - - http.Redirect(w, r, redirectURL, http.StatusSeeOther) + http.Redirect(w, r, redirectURL, http.StatusPermanentRedirect) + log.Debug(). + Str("redirectURL", redirectURL). + Str("email", invitation.Email). + Str("projectId", db.UUIDToString(&invitation.ProjectID)). + Msg("user invitation handled") } diff --git a/services/dashboard-backend/internal/api/webhooks_test.go b/services/dashboard-backend/internal/api/webhooks_test.go index d8c66eb3..33b76af2 100644 --- a/services/dashboard-backend/internal/api/webhooks_test.go +++ b/services/dashboard-backend/internal/api/webhooks_test.go @@ -73,7 +73,7 @@ func TestWebhooksAPI(t *testing.T) { signedURL, ) assert.NoError(t, requestErr) - assert.Equal(t, http.StatusOK, response.StatusCode) + assert.Equal(t, http.StatusPermanentRedirect, response.StatusCode) userAccount, retrievalErr := db.GetQueries(). RetrieveUserAccountByEmail(context.TODO(), email) @@ -112,7 +112,7 @@ func TestWebhooksAPI(t *testing.T) { signedURL, ) assert.NoError(t, requestErr) - assert.Equal(t, http.StatusOK, response.StatusCode) + assert.Equal(t, http.StatusPermanentRedirect, response.StatusCode) exists, checkErr := db.GetQueries(). CheckUserProjectExists(context.TODO(), models.CheckUserProjectExistsParams{ @@ -149,7 +149,7 @@ func TestWebhooksAPI(t *testing.T) { signedURL, ) assert.NoError(t, requestErr) - assert.Equal(t, http.StatusOK, response.StatusCode) + assert.Equal(t, http.StatusPermanentRedirect, response.StatusCode) }) t.Run( @@ -167,7 +167,7 @@ func TestWebhooksAPI(t *testing.T) { ) t.Run( - "responds with 400 BAD REQUEST when no invite exists with the given ID", + "responds with 308 Permanent Redirect and redirect the user when no invite exists with the given ID", func(t *testing.T) { signedURL := createSignedURL( "b50e5477-f74a-4e80-be29-ae67eb6ada95", @@ -178,7 +178,7 @@ func TestWebhooksAPI(t *testing.T) { signedURL, ) assert.NoError(t, requestErr) - assert.Equal(t, http.StatusBadRequest, response.StatusCode) + assert.Equal(t, http.StatusPermanentRedirect, response.StatusCode) }, ) diff --git a/shared/go/testutils/http.go b/shared/go/testutils/http.go index bd5cd270..e801d8cd 100644 --- a/shared/go/testutils/http.go +++ b/shared/go/testutils/http.go @@ -15,5 +15,11 @@ func CreateTestHTTPClient(t *testing.T, handler http.Handler) httpclient.Client server.Close() }) - return httpclient.New(server.URL, server.Client()) + client := server.Client() + // we want to test that redirects are returned correctly by the API + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + } + + return httpclient.New(server.URL, client) }