Return 404 for unauthenticated requests to URLs that do not exist

This avoids browsers messing up the session state (given that /login
calls session.Reset) with requests to various kinds of well-known URLs
that might not exist.

Also add an integration test for a server with non-nil URL prefix.

server/http.go

@@ -140,9 +140,9 @@ func New(loginService *LoginService, authClient authclient.Client, config *Confi
 	// HTTP-based login workflow).
 	root.HandleFunc(h.urlFor("/exchange"), h.handleExchange)
 
-	// Build the main IDP application router, wrap it with a login
-	// handler, optional CSRF protection, custom HTTP headers,
-	// etc.
+	// Build the main application router (which only serves / and
+	// /logout), wrap it with a login handler, optional CSRF
+	// protection, custom HTTP headers, etc.
 	mainh := http.NewServeMux()
 	mainh.HandleFunc(h.urlFor("/logout"), h.handleLogout)
 	mainh.HandleFunc(h.urlFor("/"), h.handleGrantTicket)
@@ -167,7 +167,20 @@ func New(loginService *LoginService, authClient authclient.Client, config *Confi
 	})
 	apph = corsp.Handler(apph)
 
-	root.Handle(h.urlFor("/"), apph)
+	// Now we need to remap 'apph' onto 'root'. We do this by
+	// whitelisting certain methods only, which allows us to
+	// return 404s *before* authentication.
+	root.Handle(h.urlFor("/login"), apph)
+	root.Handle(h.urlFor("/login/"), apph)
+	root.Handle(h.urlFor("/logout"), apph)
+	root.HandleFunc(h.urlFor("/"), func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != h.urlFor("/") {
+			http.NotFound(w, r)
+			return
+		}
+		apph.ServeHTTP(w, r)
+	})
+
 	h.handler = root
 
 	return h, nil

server/http_test.go

@@ -158,6 +158,12 @@ func checkStatusOk(t testing.TB, resp *http.Response) {
 	}
 }
 
+func checkStatusNotFound(t testing.TB, resp *http.Response) {
+	if resp.StatusCode != 404 {
+		t.Fatalf("expected status 404, got %s", resp.Status)
+	}
+
+}
 func checkRedirectToTargetService(t testing.TB, resp *http.Response) {
 	if resp.StatusCode != 302 {
 		t.Fatalf("expected status 302, got %s", resp.Status)
@@ -196,10 +202,13 @@ func checkTargetSSOTicket(config *Config) func(testing.TB, *http.Response) {
 
 var usernameFieldRx = regexp.MustCompile(`<input[^>]*name="username"`)
 
-func checkLoginPasswordPage(t testing.TB, resp *http.Response) {
+func checkLoginPageURL(t testing.TB, resp *http.Response) {
 	if resp.Request.URL.Path != "/login" {
 		t.Errorf("request path is not /login (%s)", resp.Request.URL.String())
 	}
+}
+
+func checkLoginPasswordPage(t testing.TB, resp *http.Response) {
 	data, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		t.Fatalf("reading body: %v", err)
@@ -253,7 +262,7 @@ func TestHTTP_Login(t *testing.T) {
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
 	v.Set("g", "users")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We expect the
 	// result to be a 302 redirect to the target service.
@@ -276,13 +285,13 @@ func TestHTTP_LoginOnSecondAttempt(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login with wrong credentials.
 	v = make(url.Values)
 	v.Set("username", "testuser")
 	v.Set("password", "badpassword")
-	doPostForm(t, c, httpSrv.URL+"/login", v, checkStatusOk, checkLoginPasswordPage)
+	doPostForm(t, c, httpSrv.URL+"/login", v, checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We expect the
 	// result to be a 302 redirect to the target service.
@@ -305,7 +314,7 @@ func TestHTTP_LoginAndLogout(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We expect the
 	// result to be a 302 redirect to the target service.
@@ -322,7 +331,7 @@ func TestHTTP_LoginAndLogout(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 }
 
 func TestHTTP_LoginOTP(t *testing.T) {
@@ -338,7 +347,39 @@ func TestHTTP_LoginOTP(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
+
+	// Attempt to login by submitting the form. We should see the OTP page.
+	v = make(url.Values)
+	v.Set("username", "test2fa")
+	v.Set("password", "password")
+	doPostForm(t, c, httpSrv.URL+"/login", v, checkStatusOk, checkLoginOTPPage)
+
+	// Submit the correct OTP token. We expect the result to be a
+	// 302 redirect to the target service.
+	v = make(url.Values)
+	v.Set("otp", "123456")
+	doPostForm(t, c, httpSrv.URL+"/login/otp", v, checkRedirectToTargetService)
+}
+
+func TestHTTP_LoginOTP_Intermediate404(t *testing.T) {
+	// This test verifies that the session is not disrupted by a
+	// request for a URL that does not exist during a 2FA login
+	// workflow. The point is that the 404 should *not* Reset()
+	// the session.
+	tmpdir, httpSrv := startTestHTTPServer(t)
+	defer os.RemoveAll(tmpdir)
+	defer httpSrv.Close()
+
+	c := newTestHTTPClient()
+
+	// Simulate an authorization request from a service, expect to
+	// see the login page.
+	v := make(url.Values)
+	v.Set("s", "service.example.com/")
+	v.Set("d", "https://service.example.com/admin/")
+	v.Set("n", "averysecretnonce")
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We should see the OTP page.
 	v = make(url.Values)
@@ -346,6 +387,10 @@ func TestHTTP_LoginOTP(t *testing.T) {
 	v.Set("password", "password")
 	doPostForm(t, c, httpSrv.URL+"/login", v, checkStatusOk, checkLoginOTPPage)
 
+	// Make a request for a URL that does not exist, browsers might do this
+	// for a number of reasons.
+	doGet(t, c, httpSrv.URL+"/apple-iphone-special-icon.ico", checkStatusNotFound)
+
 	// Submit the correct OTP token. We expect the result to be a
 	// 302 redirect to the target service.
 	v = make(url.Values)
@@ -389,7 +434,7 @@ func TestHTTP_LoginWithKeyStore(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We expect the
 	// result to be a 302 redirect to the target service.
@@ -413,7 +458,7 @@ func TestHTTP_CORS(t *testing.T) {
 	v.Set("s", "service.example.com/")
 	v.Set("d", "https://service.example.com/admin/")
 	v.Set("n", "averysecretnonce")
-	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, httpSrv.URL+"/?"+v.Encode(), checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	// Attempt to login by submitting the form. We expect the
 	// result to be a 302 redirect to the target service.

server/integration_test.go

@@ -48,6 +48,21 @@ func startTestHTTPServerAndApp(t testing.TB) (string, *httptest.Server, *httptes
 	return tmpdir, srv, app
 }
 
+func startTestHTTPServerWithPrefixAndApp(t testing.TB) (string, *httptest.Server, *httptest.Server) {
+	tmpdir, _ := ioutil.TempDir("", "")
+	config := testConfig(t, tmpdir, "")
+	config.URLPrefix = "/sso"
+	srv := createTestHTTPServer(t, config)
+	app := createTestProtectedService(t, "https://login.example.com/sso", tmpdir)
+	return tmpdir, srv, app
+}
+
+func checkLoginPageURLWithPrefix(t testing.TB, resp *http.Response) {
+	if resp.Request.URL.Path != "/sso/login" {
+		t.Errorf("request path is not /sso/login (%s)", resp.Request.URL.String())
+	}
+}
+
 func checkIsProtectedService(t testing.TB, resp *http.Response) {
 	data, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
@@ -91,7 +106,7 @@ func TestIntegration(t *testing.T) {
 		"service.example.com:443": addrFromURL(app.URL),
 	}, true)
 
-	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
 
 	v := make(url.Values)
 	v.Set("username", "testuser")
@@ -101,5 +116,30 @@ func TestIntegration(t *testing.T) {
 	// Now attempt to logout, and verify that we can't access the service anymore.
 	doGet(t, c, "https://login.example.com/logout", checkStatusOk, checkLogoutPageHasLinks)
 	doGet(t, c, "https://service.example.com/sso_logout", checkStatusOk)
-	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPasswordPage)
+	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPageURL, checkLoginPasswordPage)
+}
+
+// Same test as above, but the server application has a URL prefix.
+func TestIntegration_WithURLPrefix(t *testing.T) {
+	tmpdir, srv, app := startTestHTTPServerWithPrefixAndApp(t)
+	defer os.RemoveAll(tmpdir)
+	defer srv.Close()
+	defer app.Close()
+
+	c := makeHTTPClient(map[string]string{
+		"login.example.com:443":   addrFromURL(srv.URL),
+		"service.example.com:443": addrFromURL(app.URL),
+	}, true)
+
+	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPageURLWithPrefix, checkLoginPasswordPage)
+
+	v := make(url.Values)
+	v.Set("username", "testuser")
+	v.Set("password", "password")
+	doPostForm(t, c, "https://login.example.com/sso/login", v, checkStatusOk, checkIsProtectedService)
+
+	// Now attempt to logout, and verify that we can't access the service anymore.
+	doGet(t, c, "https://login.example.com/sso/logout", checkStatusOk, checkLogoutPageHasLinks)
+	doGet(t, c, "https://service.example.com/sso_logout", checkStatusOk)
+	doGet(t, c, "https://service.example.com/", checkStatusOk, checkLoginPageURLWithPrefix, checkLoginPasswordPage)
 }