ShortURL: Fix tests and small refactor (#108871)

pkg/api/api.go

@@ -176,7 +176,6 @@ func (hs *HTTPServer) registerRoutes() {
 	r.Get("/import/dashboard", reqSignedIn, hs.Index)
 	r.Get("/dashboards/", reqSignedIn, hs.Index)
 	r.Get("/dashboards/*", reqSignedIn, hs.Index)
-	r.Get("/goto/:uid", reqSignedIn, hs.redirectFromShortURL, hs.Index)
 
 	if hs.Cfg.PublicDashboardsEnabled {
 		// list public dashboards
@@ -264,6 +263,9 @@ func (hs *HTTPServer) registerRoutes() {
 	providerParam := ac.Parameter(":provider")
 	r.Get("/admin/authentication/:provider", authorize(ac.EvalPermission(ac.ActionSettingsRead, ac.ScopeSettingsOAuth(providerParam))), hs.Index)
 
+	// ShortURL API
+	hs.registerShortURLAPI(r)
+
 	// authed api
 	r.Group("/api", func(apiRoute routing.RouteRegister) {
 		// user (signed in)
@@ -549,9 +551,6 @@ func (hs *HTTPServer) registerRoutes() {
 			// Some channels may have info
 			liveRoute.Get("/info/*", routing.Wrap(hs.Live.HandleInfoHTTP))
 		}, requestmeta.SetSLOGroup(requestmeta.SLOGroupNone))
-
-		// short urls
-		apiRoute.Post("/short-urls", routing.Wrap(hs.createShortURL))
 	}, reqSignedIn)
 
 	// admin api

pkg/api/short_url.go

@@ -7,6 +7,8 @@ import (
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
+	"github.com/grafana/grafana/pkg/api/routing"
+	"github.com/grafana/grafana/pkg/middleware"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/shorturls"
 	"github.com/grafana/grafana/pkg/setting"
@@ -14,6 +16,12 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
+func (hs *HTTPServer) registerShortURLAPI(apiRoute routing.RouteRegister) {
+	reqSignedIn := middleware.ReqSignedIn
+	apiRoute.Post("/api/short-urls", reqSignedIn, hs.createShortURL)
+	apiRoute.Get("/goto/:uid", reqSignedIn, hs.redirectFromShortURL, hs.Index)
+}
+
 // createShortURL handles requests to create short URLs.
 func (hs *HTTPServer) createShortURL(c *contextmodel.ReqContext) response.Response {
 	cmd := dtos.CreateShortURLCmd{}

pkg/tests/api/shorturl/short_url_test.go

@@ -7,8 +7,10 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/db"
@@ -31,10 +33,12 @@ func TestMain(m *testing.M) {
 func TestShortURL(t *testing.T) {
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		AppModeProduction: true,
+		DisableAnonymous:  true,
 	})
 
 	grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path)
 
+	// Test that the endpoint is accessible with authentication.
 	username, password := "viewer", "viewer"
 	createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{
 		DefaultOrgRole: string(org.RoleEditor),
@@ -50,7 +54,7 @@ func TestShortURL(t *testing.T) {
 	defer func() {
 		_ = res.Body.Close()
 	}()
-	require.Equal(t, http.StatusOK, res.StatusCode)
+	assert.Equal(t, http.StatusOK, res.StatusCode)
 
 	bodyRaw, err := io.ReadAll(res.Body)
 	require.NoError(t, err)
@@ -67,8 +71,8 @@ func TestShortURL(t *testing.T) {
 	defer func() {
 		_ = res.Body.Close()
 	}()
-	require.Equal(t, "http://localhost:3000/explore", res.Header.Get("Location"))
-	require.Equal(t, http.StatusFound, res.StatusCode)
+	assert.Equal(t, "http://localhost:3000/explore", res.Header.Get("Location"))
+	assert.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")
@@ -76,8 +80,28 @@ func TestShortURL(t *testing.T) {
 	defer func() {
 		_ = res.Body.Close()
 	}()
-	require.Equal(t, "http://localhost:3000/", res.Header.Get("Location"))
-	require.Equal(t, http.StatusPermanentRedirect, res.StatusCode)
+	assert.Equal(t, "http://localhost:3000/", res.Header.Get("Location"))
+	assert.Equal(t, http.StatusPermanentRedirect, res.StatusCode)
+
+	// Create a client that does not have authentication.
+	notLoggedInClient := client(grafanaListedAddr, "", "")
+	// Test that the short-urls endpoint is not accessible without authentication.
+	res, err = notLoggedInClient.post("/api/short-urls", bytes.NewReader([]byte(`{"path":"explore"}`)))
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusUnauthorized, res.StatusCode)
+	defer func() {
+		_ = res.Body.Close()
+	}()
+
+	// If the user is not logged in, it should redirect to the login page and return 302.
+	res, err = notLoggedInClient.get(fmt.Sprintf("/goto/%s", resParsed.UID))
+	require.NoError(t, err)
+	defer func() {
+		_ = res.Body.Close()
+	}()
+	expectedRedirect := "/login?redirectTo=" + url.QueryEscape("/goto/"+resParsed.UID)
+	assert.Equal(t, expectedRedirect, res.Header.Get("Location"))
+	assert.Equal(t, http.StatusFound, res.StatusCode)
 }
 
 func createUser(t *testing.T, db db.DB, cfg *setting.Cfg, cmd user.CreateUserCommand) int64 {