From 5bf0872d14022d751009d741f2431452baaaa642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Tue, 3 Dec 2024 16:32:53 +0100 Subject: [PATCH] fix(short-url): redirect to main page if not found (#97347) --- pkg/api/short_url.go | 8 +- pkg/tests/api/shorturl/short_url_test.go | 130 +++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 pkg/tests/api/shorturl/short_url_test.go diff --git a/pkg/api/short_url.go b/pkg/api/short_url.go index 6a1292c65b8..5c97b2803d7 100644 --- a/pkg/api/short_url.go +++ b/pkg/api/short_url.go @@ -46,12 +46,16 @@ func (hs *HTTPServer) redirectFromShortURL(c *contextmodel.ReqContext) { shortURL, err := hs.ShortURLService.GetShortURLByUID(c.Req.Context(), c.SignedInUser, shortURLUID) if err != nil { + // If we didn't get the URL for whatever reason, we redirect to the + // main page, otherwise we get into an endless loops of redirects, as + // we would try to redirect again. if shorturls.ErrShortURLNotFound.Is(err) { - hs.log.Debug("Not redirecting short URL since not found") + hs.log.Debug("Not redirecting short URL since not found", "uid", shortURLUID) + c.Redirect(hs.Cfg.AppURL, 308) return } - hs.log.Error("Short URL redirection error", "err", err) + c.Redirect(hs.Cfg.AppURL, 307) return } diff --git a/pkg/tests/api/shorturl/short_url_test.go b/pkg/tests/api/shorturl/short_url_test.go new file mode 100644 index 00000000000..e598c36ea7a --- /dev/null +++ b/pkg/tests/api/shorturl/short_url_test.go @@ -0,0 +1,130 @@ +package shorturl + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/org/orgimpl" + "github.com/grafana/grafana/pkg/services/quota/quotaimpl" + "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/userimpl" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/tests/testinfra" + "github.com/grafana/grafana/pkg/tests/testsuite" +) + +func TestMain(m *testing.M) { + testsuite.Run(m) +} + +func TestShortURL(t *testing.T) { + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + AppModeProduction: true, + }) + + grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) + + username, password := "viewer", "viewer" + createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleEditor), + Password: user.Password(password), + Login: username, + }) + + c := client(grafanaListedAddr, username, password) + + // Create a valid short-urls + res, err := c.post("/api/short-urls", bytes.NewReader([]byte(`{"path":"explore"}`))) + require.NoError(t, err) + defer func() { + _ = res.Body.Close() + }() + require.Equal(t, http.StatusOK, res.StatusCode) + + bodyRaw, err := io.ReadAll(res.Body) + require.NoError(t, err) + + resParsed := struct { + UID string `json:"uid"` + }{} + err = json.Unmarshal(bodyRaw, &resParsed) + require.NoError(t, err) + + // If the go-to exists, it should be set in the location and the status should match 302. + res, err = c.get(fmt.Sprintf("/goto/%s", resParsed.UID)) + require.NoError(t, err) + defer func() { + _ = res.Body.Close() + }() + require.Equal(t, "http://localhost:3000/explore", res.Header.Get("Location")) + require.Equal(t, http.StatusFound, res.StatusCode) + + // If the go-to does not exist, it should redirect to the home page and return 308. + res, err = c.get("/goto/DoesNotExist") + require.NoError(t, err) + defer func() { + _ = res.Body.Close() + }() + require.Equal(t, "http://localhost:3000/", res.Header.Get("Location")) + require.Equal(t, http.StatusPermanentRedirect, res.StatusCode) +} + +func createUser(t *testing.T, db db.DB, cfg *setting.Cfg, cmd user.CreateUserCommand) int64 { + t.Helper() + + cfg.AutoAssignOrg = true + cfg.AutoAssignOrgId = 1 + + quotaService := quotaimpl.ProvideService(db, cfg) + orgService, err := orgimpl.ProvideService(db, cfg, quotaService) + require.NoError(t, err) + usrSvc, err := userimpl.ProvideService( + db, orgService, cfg, nil, nil, tracing.InitializeTracerForTest(), + quotaService, supportbundlestest.NewFakeBundleService(), + ) + require.NoError(t, err) + + u, err := usrSvc.Create(context.Background(), &cmd) + require.NoError(t, err) + return u.ID +} + +type apiClient struct { + url string +} + +func client(host, user, pass string) apiClient { + if len(user) == 0 && len(pass) == 0 { + return apiClient{url: fmt.Sprintf("http://%s", host)} + } + return apiClient{url: fmt.Sprintf("http://%s:%s@%s", user, pass, host)} +} + +func (a apiClient) get(path string) (*http.Response, error) { + u := fmt.Sprintf("%s%s", a.url, path) + // Setup a client that doesn't follow redirects. + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + // nolint:gosec + return client.Get(u) +} + +func (a apiClient) post(path string, body io.Reader) (*http.Response, error) { + u := fmt.Sprintf("%s%s", a.url, path) + // nolint:gosec + return http.Post(u, "application/json", body) +}