Skip to content

Commit

Permalink
pkg/linksharing: remove tier querying for https redirects
Browse files Browse the repository at this point in the history
Change-Id: Ia712d89c5ed4c911ec803fbb8e8f6919dcb91a69
  • Loading branch information
pwilloughby authored and Storj Robot committed Dec 3, 2024
1 parent 8f996ca commit 1136390
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 35 deletions.
2 changes: 1 addition & 1 deletion pkg/httpserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestServer(t *testing.T) {
URLBases: []string{"https://localhost:15001"},
}
mapper := objectmap.NewIPDB(&objectmap.MockReader{})
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, nil, handlerConfig)
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, handlerConfig)
require.NoError(t, err)

tempdir := t.TempDir()
Expand Down
2 changes: 1 addition & 1 deletion pkg/linksharing/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func New(log *zap.Logger, config Config) (_ *Peer, err error) {
}
}

handle, err := sharing.NewHandler(log, peer.Mapper, txtRecords, authClient, tqs, &peer.inShutdown, config.Handler)
handle, err := sharing.NewHandler(log, peer.Mapper, txtRecords, authClient, &peer.inShutdown, config.Handler)
if err != nil {
return nil, errs.New("unable to create handler: %w", err)
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/linksharing/sharing/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ type Handler struct {
mapper *objectmap.IPDB
txtRecords *TXTRecords
authClient *authclient.AuthClient
tierQuerying *TierQueryingService
static http.Handler
redirectHTTPS bool
landingRedirect string
Expand All @@ -198,7 +197,7 @@ type Handler struct {
}

// NewHandler creates a new link sharing HTTP handler.
func NewHandler(log *zap.Logger, mapper *objectmap.IPDB, txtRecords *TXTRecords, authClient *authclient.AuthClient, tqs *TierQueryingService, inShutdown *int32, config Config) (*Handler, error) {
func NewHandler(log *zap.Logger, mapper *objectmap.IPDB, txtRecords *TXTRecords, authClient *authclient.AuthClient, inShutdown *int32, config Config) (*Handler, error) {
if config.ListPageLimit <= 0 {
return nil, ErrInvalidListPageLimit
}
Expand Down Expand Up @@ -328,7 +327,6 @@ func NewHandler(log *zap.Logger, mapper *objectmap.IPDB, txtRecords *TXTRecords,
mapper: mapper,
txtRecords: txtRecords,
authClient: authClient,
tierQuerying: tqs,
static: static,
landingRedirect: config.LandingRedirectTarget,
redirectHTTPS: config.RedirectHTTPS,
Expand Down
4 changes: 2 additions & 2 deletions pkg/linksharing/sharing/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestHandler_CORS(t *testing.T) {
URLBases: []string{"http://test.test"},
}

handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, nil, cfg)
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, cfg)
require.NoError(t, err)
_ = handler.serveHTTP(testcontext.New(t), rec, req)

Expand Down Expand Up @@ -93,7 +93,7 @@ func TestHandler_Shutdown(t *testing.T) {
URLBases: []string{"http://test.test"},
}

handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, inShutdown, cfg)
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, inShutdown, cfg)
require.NoError(t, err)
err = handler.serveHTTP(testcontext.New(t), rec, req)
require.NoError(t, err)
Expand Down
17 changes: 5 additions & 12 deletions pkg/linksharing/sharing/hosting.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,11 @@ func (handler *Handler) handleHostingService(ctx context.Context, w http.Respons
return creds.err
}

// Redirect to HTTPS only custom domains that are paid-tier and with `storj-tls:true` TXT record
if handler.redirectHTTPS && r.TLS == nil && creds.hostingTLS && handler.tierQuerying != nil {
paidTier, err := handler.tierQuerying.Do(ctx, creds.access, creds.hostingHost)
if err != nil {
return errdata.WithAction(err, "query user tier")
}

if paidTier {
target := url.URL{Scheme: "https", Host: r.Host, Path: r.URL.Path, RawPath: r.URL.RawPath, RawQuery: r.URL.RawQuery}
http.Redirect(w, r, target.String(), http.StatusPermanentRedirect)
return nil
}
// Redirect to HTTPS only custom domains with `storj-tls:true` TXT record
if handler.redirectHTTPS && r.TLS == nil && creds.hostingTLS {
target := url.URL{Scheme: "https", Host: r.Host, Path: r.URL.Path, RawPath: r.URL.RawPath, RawQuery: r.URL.RawQuery}
http.Redirect(w, r, target.String(), http.StatusPermanentRedirect)
return nil
}

bucket, key := determineBucketAndObjectKey(creds.hostingRoot, r.URL.Path)
Expand Down
8 changes: 4 additions & 4 deletions pkg/linksharing/sharing/present_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestDownloadMetadataHeaders(t *testing.T) {
URLBases: []string{"http://test.test"},
}

handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, nil, cfg)
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, cfg)
require.NoError(t, err)

ctx := testcontext.New(t)
Expand Down Expand Up @@ -220,7 +220,7 @@ func TestContentDisposition(t *testing.T) {
StandardRendersContent: tc.standardRendersContent,
}

handler, err := NewHandler(&zap.Logger{}, nil, nil, nil, nil, nil, cfg)
handler, err := NewHandler(&zap.Logger{}, nil, nil, nil, nil, cfg)
require.NoError(t, err)

ctx := testcontext.New(t)
Expand Down Expand Up @@ -387,7 +387,7 @@ func TestZipArchiveContentType(t *testing.T) {
ListPageLimit: 1,
URLBases: []string{"http://test.test"},
}
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, nil, cfg)
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, cfg)
require.NoError(t, err)
handler.archiveRanger = func(_ context.Context, _ *uplink.Project, _, _, _ string, _ bool) (ranger.Ranger, bool, error) {
return SimpleRanger(nil, 0), false, nil
Expand Down Expand Up @@ -503,7 +503,7 @@ func TestImagePreviewPath(t *testing.T) {
}

func TestIsDownloadAllowed(t *testing.T) {
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, nil, Config{
handler, err := NewHandler(&zap.Logger{}, &objectmap.IPDB{}, nil, nil, nil, Config{
ListPageLimit: 1,
URLBases: []string{"http://test.test"},
})
Expand Down
4 changes: 2 additions & 2 deletions testsuite/linksharing/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestNewHandler(t *testing.T) {
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, nil, testCase.config)
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, testCase.config)
if testCase.err != "" {
require.EqualError(t, err, testCase.err)
return
Expand Down Expand Up @@ -985,7 +985,7 @@ func testHandlerRequests(t *testing.T, ctx *testcontext.Context, planet *testpla
listPageLimit = testCase.listPageLimit.v
}

handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, nil, sharing.Config{
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, sharing.Config{
Assets: assets.FS(),
URLBases: []string{"http://localhost"},
AuthServiceConfig: authConfig,
Expand Down
9 changes: 0 additions & 9 deletions testsuite/linksharing/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,6 @@ func TestIntegration(t *testing.T) {
redirectHTTPS: true,
wantRedirectResp: false,
},
{
name: "Custom domain insecure not paid tier redirect",
tlsRecord: true,
url: func(t *testing.T, peer *linksharing.Peer, _, _, _, customDomain string) string {
return fmt.Sprintf("http://%s:%d", customDomain, lookupPort(t, peer.Server.Addr()))
},
redirectHTTPS: true,
wantRedirectResp: false,
},
{
name: "Custom domain TLS not paid tier",
tlsRecord: true,
Expand Down
2 changes: 1 addition & 1 deletion testsuite/linksharing/zip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func testZipRequests(t *testing.T, ctx *testcontext.Context, planet *testplanet.
require.NoError(t, err)
}

handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, nil, sharing.Config{
handler, err := sharing.NewHandler(zaptest.NewLogger(t), mapper, nil, nil, nil, sharing.Config{
Assets: assets.FS(),
ListPageLimit: 1,
URLBases: []string{"http://localhost"},
Expand Down

0 comments on commit 1136390

Please sign in to comment.