From 65940c77262780548df9e5fc20f11cd4c926feff Mon Sep 17 00:00:00 2001 From: Pete Davids <5599894+MisterSquishy@users.noreply.github.com> Date: Wed, 14 Oct 2020 06:48:48 -0400 Subject: [PATCH] Dashboard: Allow shortlink generation (#27409) * intial frontend resolution/redirection logic * backend scaffolding * enough of the frontend to actually test end to end * bugfixes * add tests * cleanup * explore too hard for now * fix build * Docs: add docs * FE test * redirect directly from backend * validate incoming uids * add last_seen_at * format documentation * more documentation feedback * very shaky migration of get route to middleware * persist unix timestamps * add id, orgId to table * fixes for orgId scoping * whoops forgot the middleware * only redirect to absolute URLs under the AppUrl domain * move lookup route to /goto/:uid, stop manually setting 404 response code * renaming things according to PR feedback * tricky deletion * sneaky readd * fix test * more BE renaming * FE updates -- no more @ts-ignore hacking :) and accounting for subpath * Simplify code Signed-off-by: Arve Knudsen * Short URLs: Drop usage of bus Signed-off-by: Arve Knudsen * ShortURLService: Make injectable Signed-off-by: Arve Knudsen * Rename file Signed-off-by: Arve Knudsen * Add handling of url parsing and creating of full shortURL to backend * Update test, remove unused imports * Update pkg/api/short_urls.go Co-authored-by: Arve Knudsen * Add correct import * Pass context to short url service * Remove not needed error log * Rename dto and field to denote URL rather than path * Update api docs based on feedback/suggestion * Rename files to singular * Revert to send relative path to backend * Fixes after review * Return dto when creating short URL that includes the full url Use full url to provide shorten URL to the user * Fix after review * Fix relative url path when creating new short url Co-authored-by: Arve Knudsen Co-authored-by: Ivana Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Co-authored-by: Marcus Efraimsson --- docs/sources/http_api/short_url.md | 55 ++++++++++++++++ pkg/api/api.go | 4 ++ pkg/api/dtos/short_url.go | 10 +++ pkg/api/http_server.go | 2 + pkg/api/short_url.go | 61 ++++++++++++++++++ pkg/models/shorturl.go | 19 ++++++ pkg/services/shorturls/short_url_service.go | 64 +++++++++++++++++++ .../shorturls/short_url_service_test.go | 40 ++++++++++++ .../sqlstore/migrations/migrations.go | 1 + .../sqlstore/migrations/short_url_mig.go | 28 ++++++++ .../components/ShareModal/ShareLink.test.tsx | 29 +++++++++ .../components/ShareModal/ShareLink.tsx | 30 +++++++-- .../dashboard/components/ShareModal/utils.ts | 14 ++++ 13 files changed, 351 insertions(+), 6 deletions(-) create mode 100644 docs/sources/http_api/short_url.md create mode 100644 pkg/api/dtos/short_url.go create mode 100644 pkg/api/short_url.go create mode 100644 pkg/models/shorturl.go create mode 100644 pkg/services/shorturls/short_url_service.go create mode 100644 pkg/services/shorturls/short_url_service_test.go create mode 100755 pkg/services/sqlstore/migrations/short_url_mig.go diff --git a/docs/sources/http_api/short_url.md b/docs/sources/http_api/short_url.md new file mode 100644 index 00000000000..b1b68c51dc1 --- /dev/null +++ b/docs/sources/http_api/short_url.md @@ -0,0 +1,55 @@ ++++ +title = "Short URL HTTP API " +description = "Grafana Short URL HTTP API" +keywords = ["grafana", "http", "documentation", "api", "shortUrl"] +aliases = ["/docs/grafana/latest/http_api/short_url/"] +type = "docs" +[menu.docs] +name = "Short URL" +parent = "http_api" ++++ + +# Short URL API + +Use this API to create shortened URLs. A short URL represents a longer URL containing complex query parameters in a smaller and simpler format. + +## Create short URL + +`POST /api/short-urls` + +Creates a short URL. + +**Example request:** + +```http +POST /api/short-urls HTTP/1.1 +Accept: application/json +Content-Type: application/json +Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk + +{ + "path": "d/TxKARsmGz/new-dashboard?orgId=1&from=1599389322894&to=1599410922894" +} +``` + +JSON body schema: + +- **path** – The path to shorten, relative to the Grafana [root_url]({{< relref "../administration/configuration.md#root_url" >}}). + +**Example response:** + +```http +HTTP/1.1 200 +Content-Type: application/json + +{ + "uid": AT76wBvGk, + "url": http://localhost:3000/goto/AT76wBvGk +} + +``` + +Status codes: + +- **200** – Created +- **400** – Errors (invalid JSON, missing or invalid fields) diff --git a/pkg/api/api.go b/pkg/api/api.go index 24957d75201..e9aa060d677 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -78,6 +78,7 @@ 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) r.Get("/explore", reqSignedIn, middleware.EnsureEditorOrViewerCanEdit, hs.Index) @@ -390,6 +391,9 @@ func (hs *HTTPServer) registerRoutes() { // error test r.Get("/metrics/error", Wrap(GenerateError)) + + // short urls + apiRoute.Post("/short-urls", bind(dtos.CreateShortURLCmd{}), Wrap(hs.createShortURL)) }, reqSignedIn) // admin api diff --git a/pkg/api/dtos/short_url.go b/pkg/api/dtos/short_url.go new file mode 100644 index 00000000000..ca185c491e0 --- /dev/null +++ b/pkg/api/dtos/short_url.go @@ -0,0 +1,10 @@ +package dtos + +type ShortURL struct { + UID string `json:"uid"` + URL string `json:"url"` +} + +type CreateShortURLCmd struct { + Path string `json:"path"` +} diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 385d0df697a..b89939989ab 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/live" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/plugins/backendplugin" @@ -72,6 +73,7 @@ type HTTPServer struct { PluginManager *plugins.PluginManager `inject:""` SearchService *search.SearchService `inject:""` AlertNG *eval.AlertNG `inject:""` + ShortURLService *shorturls.ShortURLService `inject:""` Live *live.GrafanaLive Listener net.Listener } diff --git a/pkg/api/short_url.go b/pkg/api/short_url.go new file mode 100644 index 00000000000..2108b945d8f --- /dev/null +++ b/pkg/api/short_url.go @@ -0,0 +1,61 @@ +package api + +import ( + "errors" + "path" + "strings" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" +) + +// createShortURL handles requests to create short URLs. +func (hs *HTTPServer) createShortURL(c *models.ReqContext, cmd dtos.CreateShortURLCmd) Response { + hs.log.Debug("Received request to create short URL", "path", cmd.Path) + + cmd.Path = strings.TrimSpace(cmd.Path) + + if path.IsAbs(cmd.Path) { + hs.log.Error("Invalid short URL path", "path", cmd.Path) + return Error(400, "Path should be relative", nil) + } + + shortURL, err := hs.ShortURLService.CreateShortURL(c.Req.Context(), c.SignedInUser, cmd.Path) + if err != nil { + return Error(500, "Failed to create short URL", err) + } + + url := path.Join(setting.AppUrl, "goto", shortURL.Uid) + c.Logger.Debug("Created short URL", "url", url) + + dto := dtos.ShortURL{ + UID: shortURL.Uid, + URL: url, + } + + return JSON(200, dto) +} + +func (hs *HTTPServer) redirectFromShortURL(c *models.ReqContext) { + shortURLUID := c.Params(":uid") + + if !util.IsValidShortUID(shortURLUID) { + return + } + + shortURL, err := hs.ShortURLService.GetShortURLByUID(c.Req.Context(), c.SignedInUser, shortURLUID) + if err != nil { + if errors.Is(err, models.ErrShortURLNotFound) { + hs.log.Debug("Not redirecting short URL since not found") + return + } + + hs.log.Error("Short URL redirection error", "err", err) + return + } + + hs.log.Debug("Redirecting short URL", "path", shortURL.Path) + c.Redirect(setting.ToAbsUrl(shortURL.Path), 302) +} diff --git a/pkg/models/shorturl.go b/pkg/models/shorturl.go new file mode 100644 index 00000000000..e3965a3213e --- /dev/null +++ b/pkg/models/shorturl.go @@ -0,0 +1,19 @@ +package models + +import ( + "errors" +) + +var ( + ErrShortURLNotFound = errors.New("short URL not found") +) + +type ShortUrl struct { + Id int64 + OrgId int64 + Uid string + Path string + CreatedBy int64 + CreatedAt int64 + LastSeenAt int64 +} diff --git a/pkg/services/shorturls/short_url_service.go b/pkg/services/shorturls/short_url_service.go new file mode 100644 index 00000000000..b2694bc3f4f --- /dev/null +++ b/pkg/services/shorturls/short_url_service.go @@ -0,0 +1,64 @@ +package shorturls + +import ( + "context" + "time" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/registry" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/util" +) + +func init() { + registry.RegisterService(&ShortURLService{}) +} + +type ShortURLService struct { + SQLStore *sqlstore.SqlStore `inject:""` +} + +func (s *ShortURLService) Init() error { + return nil +} + +func (s ShortURLService) GetShortURLByUID(ctx context.Context, user *models.SignedInUser, uid string) (*models.ShortUrl, error) { + var shortURL models.ShortUrl + err := s.SQLStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { + exists, err := dbSession.Where("org_id=? AND uid=?", user.OrgId, uid).Get(&shortURL) + if err != nil { + return err + } + if !exists { + return models.ErrShortURLNotFound + } + + return nil + }) + if err != nil { + return nil, err + } + + return &shortURL, nil +} + +func (s ShortURLService) CreateShortURL(ctx context.Context, user *models.SignedInUser, path string) (*models.ShortUrl, error) { + now := time.Now().Unix() + shortURL := models.ShortUrl{ + OrgId: user.OrgId, + Uid: util.GenerateShortUID(), + Path: path, + CreatedBy: user.UserId, + CreatedAt: now, + } + + err := s.SQLStore.WithDbSession(ctx, func(session *sqlstore.DBSession) error { + _, err := session.Insert(&shortURL) + return err + }) + if err != nil { + return nil, err + } + + return &shortURL, nil +} diff --git a/pkg/services/shorturls/short_url_service_test.go b/pkg/services/shorturls/short_url_service_test.go new file mode 100644 index 00000000000..64c93e37305 --- /dev/null +++ b/pkg/services/shorturls/short_url_service_test.go @@ -0,0 +1,40 @@ +package shorturls + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/stretchr/testify/require" +) + +func TestShortURLService(t *testing.T) { + user := &models.SignedInUser{UserId: 1} + sqlStore := sqlstore.InitTestDB(t) + + t.Run("User can create and read short URLs", func(t *testing.T) { + const refPath = "mock/path?test=true" + + service := ShortURLService{SQLStore: sqlStore} + + newShortURL, err := service.CreateShortURL(context.Background(), user, refPath) + require.NoError(t, err) + require.NotNil(t, newShortURL) + require.NotEmpty(t, newShortURL.Uid) + + existingShortURL, err := service.GetShortURLByUID(context.Background(), user, newShortURL.Uid) + require.NoError(t, err) + require.NotNil(t, existingShortURL) + require.Equal(t, refPath, existingShortURL.Path) + }) + + t.Run("User cannot look up nonexistent short URLs", func(t *testing.T) { + service := ShortURLService{SQLStore: sqlStore} + + shortURL, err := service.GetShortURLByUID(context.Background(), user, "testnotfounduid") + require.Error(t, err) + require.Equal(t, models.ErrShortURLNotFound, err) + require.Nil(t, shortURL) + }) +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 3e40c749f37..c94e6f5b391 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -34,6 +34,7 @@ func AddMigrations(mg *Migrator) { addServerlockMigrations(mg) addUserAuthTokenMigrations(mg) addCacheMigration(mg) + addShortURLMigrations(mg) } func addMigrationLogMigrations(mg *Migrator) { diff --git a/pkg/services/sqlstore/migrations/short_url_mig.go b/pkg/services/sqlstore/migrations/short_url_mig.go new file mode 100755 index 00000000000..163e9f8ae7d --- /dev/null +++ b/pkg/services/sqlstore/migrations/short_url_mig.go @@ -0,0 +1,28 @@ +package migrations + +import ( + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +) + +func addShortURLMigrations(mg *Migrator) { + shortURLV1 := Table{ + Name: "short_url", + Columns: []*Column{ + {Name: "id", Type: DB_BigInt, Nullable: false, IsPrimaryKey: true, IsAutoIncrement: true}, + {Name: "org_id", Type: DB_BigInt, Nullable: false}, + {Name: "uid", Type: DB_NVarchar, Length: 40, Nullable: false}, + {Name: "path", Type: DB_Text, Nullable: false}, + {Name: "created_by", Type: DB_Int, Nullable: false}, + {Name: "created_at", Type: DB_Int, Nullable: false}, + {Name: "last_seen_at", Type: DB_Int, Nullable: true}, + }, + Indices: []*migrator.Index{ + {Cols: []string{"org_id", "uid"}, Type: migrator.UniqueIndex}, + }, + } + + mg.AddMigration("create short_url table v1", NewAddTableMigration(shortURLV1)) + + mg.AddMigration("add index short_url.org_id-uid", migrator.NewAddIndexMigration(shortURLV1, shortURLV1.Indices[0])) +} diff --git a/public/app/features/dashboard/components/ShareModal/ShareLink.test.tsx b/public/app/features/dashboard/components/ShareModal/ShareLink.test.tsx index 20847f65b48..c43b86a3035 100644 --- a/public/app/features/dashboard/components/ShareModal/ShareLink.test.tsx +++ b/public/app/features/dashboard/components/ShareModal/ShareLink.test.tsx @@ -49,6 +49,21 @@ function setUTCTimeZone() { }; } +const mockUid = 'abc123'; +jest.mock('@grafana/runtime', () => { + const original = jest.requireActual('@grafana/runtime'); + + return { + ...original, + getBackendSrv: () => ({ + post: jest.fn().mockResolvedValue({ + uid: mockUid, + url: `http://localhost:3000/goto/${mockUid}`, + }), + }), + }; +}); + interface ScenarioContext { wrapper?: ShallowWrapper; mount: (propOverrides?: Partial) => void; @@ -167,5 +182,19 @@ describe('ShareModal', () => { 'http://server/#!/test?from=1000&to=2000&orgId=1&var-app=mupp&var-server=srv-01' ); }); + + it('should shorten url', () => { + mockLocationHref('http://server/#!/test'); + fillVariableValuesForUrlMock = (params: any) => { + params['var-app'] = 'mupp'; + params['var-server'] = 'srv-01'; + }; + ctx.mount(); + ctx.wrapper?.setState({ includeTemplateVars: true, useShortUrl: true }, async () => { + await ctx.wrapper?.instance().buildUrl(); + const state = ctx.wrapper?.state(); + expect(state?.shareUrl).toContain(`/goto/${mockUid}`); + }); + }); }); }); diff --git a/public/app/features/dashboard/components/ShareModal/ShareLink.tsx b/public/app/features/dashboard/components/ShareModal/ShareLink.tsx index c8d9fc6f8db..39abba1aab1 100644 --- a/public/app/features/dashboard/components/ShareModal/ShareLink.tsx +++ b/public/app/features/dashboard/components/ShareModal/ShareLink.tsx @@ -3,8 +3,9 @@ import { selectors as e2eSelectors } from '@grafana/e2e-selectors'; import { LegacyForms, ClipboardButton, Icon, InfoBox } from '@grafana/ui'; const { Select, Switch } = LegacyForms; import { SelectableValue, PanelModel, AppEvents } from '@grafana/data'; +import { getBackendSrv } from '@grafana/runtime'; import { DashboardModel } from 'app/features/dashboard/state'; -import { buildImageUrl, buildShareUrl } from './utils'; +import { buildImageUrl, buildShareUrl, getRelativeURLPath } from './utils'; import { appEvents } from 'app/core/core'; import config from 'app/core/config'; @@ -22,6 +23,7 @@ export interface Props { export interface State { useCurrentTimeRange: boolean; includeTemplateVars: boolean; + useShortUrl: boolean; selectedTheme: SelectableValue; shareUrl: string; imageUrl: string; @@ -33,6 +35,7 @@ export class ShareLink extends PureComponent { this.state = { useCurrentTimeRange: true, includeTemplateVars: true, + useShortUrl: false, selectedTheme: themeOptions[0], shareUrl: '', imageUrl: '', @@ -44,11 +47,12 @@ export class ShareLink extends PureComponent { } componentDidUpdate(prevProps: Props, prevState: State) { - const { useCurrentTimeRange, includeTemplateVars, selectedTheme } = this.state; + const { useCurrentTimeRange, includeTemplateVars, useShortUrl, selectedTheme } = this.state; if ( prevState.useCurrentTimeRange !== useCurrentTimeRange || prevState.includeTemplateVars !== includeTemplateVars || - prevState.selectedTheme.value !== selectedTheme.value + prevState.selectedTheme.value !== selectedTheme.value || + prevState.useShortUrl !== useShortUrl ) { this.buildUrl(); } @@ -56,11 +60,20 @@ export class ShareLink extends PureComponent { buildUrl = () => { const { panel } = this.props; - const { useCurrentTimeRange, includeTemplateVars, selectedTheme } = this.state; + const { useCurrentTimeRange, includeTemplateVars, useShortUrl, selectedTheme } = this.state; const shareUrl = buildShareUrl(useCurrentTimeRange, includeTemplateVars, selectedTheme.value, panel); const imageUrl = buildImageUrl(useCurrentTimeRange, includeTemplateVars, selectedTheme.value, panel); - this.setState({ shareUrl, imageUrl }); + + if (useShortUrl) { + getBackendSrv() + .post(`/api/short-urls`, { + path: getRelativeURLPath(shareUrl), + }) + .then(res => this.setState({ shareUrl: res.url, imageUrl })); + } else { + this.setState({ shareUrl, imageUrl }); + } }; onUseCurrentTimeRangeChange = () => { @@ -71,6 +84,10 @@ export class ShareLink extends PureComponent { this.setState({ includeTemplateVars: !this.state.includeTemplateVars }); }; + onUrlShorten = () => { + this.setState({ useShortUrl: !this.state.useShortUrl }); + }; + onThemeChange = (value: SelectableValue) => { this.setState({ selectedTheme: value }); }; @@ -85,7 +102,7 @@ export class ShareLink extends PureComponent { render() { const { panel } = this.props; - const { useCurrentTimeRange, includeTemplateVars, selectedTheme, shareUrl, imageUrl } = this.state; + const { useCurrentTimeRange, includeTemplateVars, useShortUrl, selectedTheme, shareUrl, imageUrl } = this.state; const selectors = e2eSelectors.pages.SharePanelModal; return ( @@ -109,6 +126,7 @@ export class ShareLink extends PureComponent { checked={includeTemplateVars} onChange={this.onIncludeTemplateVarsChange} /> +