Skip to content

Commit b75e651

Browse files
committed
add integration tests for auth
1 parent b3f6999 commit b75e651

File tree

5 files changed

+383
-4
lines changed

5 files changed

+383
-4
lines changed

pkg/api/logs_handler.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strconv"
99
"time"
1010

11+
"github.com/nebari-dev/jhub-app-proxy/pkg/auth"
1112
"github.com/nebari-dev/jhub-app-proxy/pkg/logger"
1213
"github.com/nebari-dev/jhub-app-proxy/pkg/process"
1314
"github.com/nebari-dev/jhub-app-proxy/pkg/ui"
@@ -275,6 +276,9 @@ func (h *LogsHandler) RegisterRoutesWithPrefix(mux *http.ServeMux, prefix string
275276
// These routes are at /_temp/jhub-app-proxy/api/* (or with service prefix)
276277
// and are used by the interim log viewer page.
277278
//
279+
// SECURITY: These routes are NOT automatically protected by authentication.
280+
// The caller MUST wrap them with OAuth middleware if authentication is required.
281+
//
278282
// Grace Period Behavior:
279283
// These routes remain accessible for a grace period after the app deploys,
280284
// allowing the interim page to fetch final logs before redirecting.
@@ -301,3 +305,31 @@ func (h *LogsHandler) RegisterInterimRoutes(mux *http.ServeMux, basePath string)
301305
"GET " + basePath + "/api/logo",
302306
})
303307
}
308+
309+
// RegisterInterimRoutesWithAuth registers all log API routes under the interim path with OAuth authentication
310+
// CRITICAL SECURITY: Use this method instead of RegisterInterimRoutes when OAuth is enabled!
311+
//
312+
// Parameters:
313+
// - mux: The HTTP request multiplexer
314+
// - basePath: The base interim path
315+
// - oauthMW: OAuth middleware for authentication
316+
func (h *LogsHandler) RegisterInterimRoutesWithAuth(mux *http.ServeMux, basePath string, oauthMW *auth.OAuthMiddleware) {
317+
// Wrap each handler with OAuth middleware
318+
mux.Handle(basePath+"/api/logs", oauthMW.Wrap(http.HandlerFunc(h.HandleGetLogs)))
319+
mux.Handle(basePath+"/api/logs/all", oauthMW.Wrap(http.HandlerFunc(h.HandleGetAllLogs)))
320+
mux.Handle(basePath+"/api/logs/since", oauthMW.Wrap(http.HandlerFunc(h.HandleGetLogsSince)))
321+
mux.Handle(basePath+"/api/logs/stats", oauthMW.Wrap(http.HandlerFunc(h.HandleGetStats)))
322+
mux.Handle(basePath+"/api/logs/clear", oauthMW.Wrap(http.HandlerFunc(h.HandleClearLogs)))
323+
mux.Handle(basePath+"/api/logo", oauthMW.Wrap(http.HandlerFunc(h.HandleGetLogo)))
324+
325+
h.logger.Info("interim log API routes registered WITH OAUTH PROTECTION",
326+
"base_path", basePath,
327+
"endpoints", []string{
328+
"GET " + basePath + "/api/logs",
329+
"GET " + basePath + "/api/logs/all",
330+
"GET " + basePath + "/api/logs/since",
331+
"GET " + basePath + "/api/logs/stats",
332+
"DELETE " + basePath + "/api/logs/clear",
333+
"GET " + basePath + "/api/logo",
334+
})
335+
}

pkg/config/config.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import (
1111
// Config holds application configuration
1212
type Config struct {
1313
// Authentication
14-
AuthType string // "oauth", "none"
14+
AuthType string // "oauth", "none"
15+
InterimPageAuth bool // If true, protect interim pages/logs API even when AuthType is "none"
1516

1617
// Process
1718
Command []string
@@ -71,6 +72,8 @@ Framework-agnostic - works with any web application (Streamlit, Voila, Panel, et
7172
// Core flags
7273
rootCmd.Flags().StringVar(&cfg.AuthType, "authtype", "oauth",
7374
"Authentication type (oauth, none)")
75+
rootCmd.Flags().BoolVar(&cfg.InterimPageAuth, "interim-page-auth", false,
76+
"Protect interim pages and logs API with OAuth even when --authtype=none (allows public app with protected logs)")
7477
rootCmd.Flags().IntVar(&cfg.Port, "port", 0,
7578
"Port for proxy server to listen on (what JupyterHub expects)")
7679
rootCmd.Flags().IntVar(&cfg.ListenPort, "listen-port", 0,

pkg/server/server.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"time"
1313

1414
"github.com/nebari-dev/jhub-app-proxy/pkg/api"
15+
"github.com/nebari-dev/jhub-app-proxy/pkg/auth"
1516
"github.com/nebari-dev/jhub-app-proxy/pkg/config"
1617
"github.com/nebari-dev/jhub-app-proxy/pkg/hub"
1718
"github.com/nebari-dev/jhub-app-proxy/pkg/interim"
@@ -58,17 +59,55 @@ func New(cfg Config) (*Server, error) {
5859
mux := http.NewServeMux()
5960
api.Version = cfg.Version
6061

61-
// Create and register logs API handler
62+
// Create OAuth middleware if authentication is enabled for main app OR interim pages
63+
var oauthMW *auth.OAuthMiddleware
64+
needsOAuth := cfg.AppConfig.AuthType == "oauth" || cfg.AppConfig.InterimPageAuth
65+
66+
if needsOAuth {
67+
var err error
68+
oauthMW, err = auth.NewOAuthMiddleware(log)
69+
if err != nil {
70+
return nil, fmt.Errorf("failed to create OAuth middleware: %w", err)
71+
}
72+
73+
if cfg.AppConfig.AuthType == "oauth" {
74+
log.Info("OAuth authentication enabled for ALL routes (app + interim pages)")
75+
} else if cfg.AppConfig.InterimPageAuth {
76+
log.Info("OAuth authentication enabled for INTERIM PAGES ONLY (app is public)")
77+
}
78+
}
79+
80+
// CRITICAL SECURITY: Determine if interim pages need authentication
81+
// If --authtype=oauth: protect everything (app + interim pages)
82+
// If --interim-page-auth: protect only interim pages (app is public)
83+
// Otherwise: protect nothing
84+
protectInterim := cfg.AppConfig.AuthType == "oauth" || cfg.AppConfig.InterimPageAuth
85+
86+
// CRITICAL SECURITY: Register logs API handler with or without authentication
6287
logsHandler := api.NewLogsHandler(cfg.Manager, log)
63-
logsHandler.RegisterInterimRoutes(mux, interimBasePath)
88+
if protectInterim && oauthMW != nil {
89+
logsHandler.RegisterInterimRoutesWithAuth(mux, interimBasePath, oauthMW)
90+
} else {
91+
logsHandler.RegisterInterimRoutes(mux, interimBasePath)
92+
log.Warn("logs API NOT protected - sensitive logs exposed!", "path", interimBasePath+"/api/*")
93+
}
6494

6595
// Create interim page handler
6696
interimHandler := interim.NewHandler(interim.Config{
6797
Manager: cfg.Manager,
6898
Logger: log,
6999
AppURLPath: appRootPath,
70100
})
71-
mux.Handle(interimBasePath, interimHandler)
101+
102+
// CRITICAL SECURITY: Wrap interim handler with OAuth authentication if needed
103+
// Interim pages can expose sensitive subprocess logs!
104+
if protectInterim && oauthMW != nil {
105+
mux.Handle(interimBasePath, oauthMW.Wrap(interimHandler))
106+
log.Info("interim page protected with OAuth authentication", "path", interimBasePath)
107+
} else {
108+
mux.Handle(interimBasePath, interimHandler)
109+
log.Warn("interim page NOT protected - sensitive logs exposed!", "path", interimBasePath)
110+
}
72111

73112
// Create backend proxy handler
74113
proxyHandler, err := proxy.NewHandler(
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// security_interim_only_test.go - Tests --interim-page-auth flag
2+
//
3+
// Verifies the --interim-page-auth flag behavior:
4+
// - Main application: accessible without authentication (200 OK)
5+
// - Interim pages & logs API: requires authentication (302 redirect)
6+
7+
package integration
8+
9+
import (
10+
"context"
11+
"fmt"
12+
"net/http"
13+
"os"
14+
"os/exec"
15+
"testing"
16+
"time"
17+
)
18+
19+
// TestInterimPageAuthFlag tests the --interim-page-auth flag
20+
// This flag allows protecting interim pages and logs API while keeping the main app public
21+
func TestInterimPageAuthFlag(t *testing.T) {
22+
// Get free ports for proxy and subprocess
23+
proxyPort := getFreePort(t)
24+
destPort := getFreePort(t)
25+
26+
// Build the binary first
27+
binaryPath := buildBinary(t)
28+
29+
// Start jhub-app-proxy with --authtype=none but --interim-page-auth=true
30+
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
31+
defer cancel()
32+
33+
cmd := exec.CommandContext(ctx, binaryPath,
34+
"--port", fmt.Sprintf("%d", proxyPort),
35+
"--destport", fmt.Sprintf("%d", destPort),
36+
"--authtype", "none", // Main app is PUBLIC
37+
"--interim-page-auth", // But interim pages are PROTECTED
38+
"--log-format", "pretty",
39+
"--log-level", "info",
40+
"--",
41+
"python3", "-m", "http.server", "{port}",
42+
)
43+
44+
// Set minimal JupyterHub environment variables (required for OAuth)
45+
cmd.Env = append(os.Environ(),
46+
"JUPYTERHUB_API_TOKEN=test-token-12345",
47+
"JUPYTERHUB_API_URL=http://localhost:8081/hub/api",
48+
"JUPYTERHUB_USER=testuser",
49+
"JUPYTERHUB_SERVICE_PREFIX=/user/testuser/",
50+
)
51+
52+
cmd.Stdout = os.Stdout
53+
cmd.Stderr = os.Stderr
54+
55+
if err := cmd.Start(); err != nil {
56+
t.Fatalf("Failed to start jhub-app-proxy: %v", err)
57+
}
58+
defer func() {
59+
if cmd.Process != nil {
60+
cmd.Process.Kill()
61+
}
62+
}()
63+
64+
proxyURL := fmt.Sprintf("http://127.0.0.1:%d", proxyPort)
65+
servicePrefix := "/user/testuser"
66+
interimPath := servicePrefix + "/_temp/jhub-app-proxy"
67+
68+
// Wait for proxy to be ready (use main app since interim is protected)
69+
if err := waitForHTTP(proxyURL+servicePrefix+"/", 5*time.Second); err != nil {
70+
t.Fatalf("Proxy did not become ready: %v", err)
71+
}
72+
73+
// Give the subprocess time to fully start (we can't use stats API since it's protected)
74+
time.Sleep(3 * time.Second)
75+
76+
// Test 1: Main app should be PUBLIC (no auth required)
77+
t.Run("MainAppIsPublic", func(t *testing.T) {
78+
resp, err := http.Get(proxyURL + servicePrefix + "/")
79+
if err != nil {
80+
t.Fatalf("Failed to request main app: %v", err)
81+
}
82+
defer resp.Body.Close()
83+
84+
// Should return 200 OK - app is public!
85+
if resp.StatusCode != 200 {
86+
t.Errorf("Expected 200 for public app, got %d", resp.StatusCode)
87+
}
88+
})
89+
90+
// Test 2: Interim page should be PROTECTED (auth required)
91+
t.Run("InterimPageIsProtected", func(t *testing.T) {
92+
client := &http.Client{
93+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
94+
return http.ErrUseLastResponse
95+
},
96+
}
97+
resp, err := client.Get(proxyURL + interimPath)
98+
if err != nil {
99+
t.Fatalf("Failed to request interim page: %v", err)
100+
}
101+
defer resp.Body.Close()
102+
103+
// Should NOT return 200 - interim page is protected!
104+
if resp.StatusCode == 200 {
105+
t.Errorf("SECURITY ISSUE: Interim page should be protected but got 200")
106+
}
107+
108+
// Should redirect to OAuth
109+
if resp.StatusCode != 302 {
110+
t.Errorf("Expected 302 redirect for protected interim page, got %d", resp.StatusCode)
111+
}
112+
})
113+
114+
// Test 3: Logs API should be PROTECTED (auth required)
115+
t.Run("LogsAPIIsProtected", func(t *testing.T) {
116+
client := &http.Client{
117+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
118+
return http.ErrUseLastResponse
119+
},
120+
}
121+
resp, err := client.Get(proxyURL + interimPath + "/api/logs/all")
122+
if err != nil {
123+
t.Fatalf("Failed to request logs API: %v", err)
124+
}
125+
defer resp.Body.Close()
126+
127+
// Should NOT return 200 - logs API is protected!
128+
if resp.StatusCode == 200 {
129+
t.Errorf("SECURITY ISSUE: Logs API should be protected but got 200")
130+
}
131+
132+
// Should redirect to OAuth
133+
if resp.StatusCode != 302 {
134+
t.Errorf("Expected 302 redirect for protected logs API, got %d", resp.StatusCode)
135+
}
136+
})
137+
}

0 commit comments

Comments
 (0)