From 4262275de514f34bfc9e0e2681e43b0b904fe5f3 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 31 Jan 2022 22:30:27 +0100 Subject: [PATCH 1/3] Test: remove part of the hardcoded network ports --- internal/sessiontest/helper_servers_test.go | 67 ++++++++------------- internal/sessiontest/redis_test.go | 10 +-- internal/sessiontest/session_test.go | 37 ++++++++---- internal/test/testdata.go | 41 +++++++------ irmago_test.go | 6 +- server/api_test.go | 29 +++------ 6 files changed, 88 insertions(+), 102 deletions(-) diff --git a/internal/sessiontest/helper_servers_test.go b/internal/sessiontest/helper_servers_test.go index 0e88eba1e..a0ffef7ad 100644 --- a/internal/sessiontest/helper_servers_test.go +++ b/internal/sessiontest/helper_servers_test.go @@ -5,8 +5,11 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "net/http" + "net/http/httptest" "path/filepath" + "strings" "testing" "time" @@ -37,22 +40,18 @@ var ( jwtPrivkeyPath = filepath.Join(testdata, "jwtkeys", "sk.pem") ) +// Some urls are hardcoded in the test configuration, so we have to hardcode them here too. const ( - irmaServerPort = 48680 - schemeServerURL = "http://localhost:48681" - requestorServerPort = 48682 - requestorServerURL = "http://localhost:48682" - revocationServerPort = 48683 revocationServerURL = "http://localhost:48683" +) - staticSessionServerPort = 48685 - staticSessionServerURL = "http://localhost:48685" - - nextSessionServerPort = 48686 - nextSessionServerURL = "http://localhost:48686" +// The doSession helper expects the requestor server URL to be globally defined, to support the optionReuseServer. +var ( + requestorServerPort = findFreePort() + requestorServerURL = fmt.Sprintf("http://localhost:%d", requestorServerPort) ) type IrmaServer struct { @@ -87,6 +86,12 @@ func apply( } } +func findFreePort() int { + s := httptest.NewUnstartedServer(http.NotFoundHandler()) + defer s.Close() + return s.Listener.Addr().(*net.TCPAddr).Port +} + func StartRequestorServer(t *testing.T, configuration *requestorserver.Configuration) *requestorserver.Server { requestorServer, err := requestorserver.New(configuration) require.NoError(t, err) @@ -103,19 +108,19 @@ func StartIrmaServer(t *testing.T, conf *server.Configuration) *IrmaServer { conf = IrmaServerConfiguration() } + mux := http.NewServeMux() + httpServer := httptest.NewServer(mux) + + // Make sure domain is used instead of IP address. + conf.URL = strings.Replace(httpServer.URL, "127.0.0.1", "localhost", 1) irmaServer, err := irmaserver.New(conf) require.NoError(t, err) - mux := http.NewServeMux() mux.HandleFunc("/", irmaServer.HandlerFunc()) - httpServer := &http.Server{Addr: fmt.Sprintf("localhost:%d", irmaServerPort), Handler: mux} - go func() { - _ = httpServer.ListenAndServe() - }() return &IrmaServer{ irma: irmaServer, conf: conf, - http: httpServer, + http: httpServer.Config, } } @@ -137,7 +142,7 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler { request := &irma.ServiceProviderRequest{ Request: getDisclosureRequest(id), RequestorBaseRequest: irma.RequestorBaseRequest{ - NextSession: &irma.NextSessionData{URL: nextSessionServerURL + "/2"}, + NextSession: &irma.NextSessionData{URL: fmt.Sprintf("http://%s/2", r.Host)}, }, } bts, err := json.Marshal(request) @@ -180,7 +185,7 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler { bts, err = json.Marshal(irma.IdentityProviderRequest{ Request: irma.NewIssuanceRequest([]*irma.CredentialRequest{cred}), RequestorBaseRequest: irma.RequestorBaseRequest{ - NextSession: &irma.NextSessionData{URL: nextSessionServerURL + "/3"}, + NextSession: &irma.NextSessionData{URL: fmt.Sprintf("http://%s/3", r.Host)}, }, }) require.NoError(t, err) @@ -206,20 +211,8 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler { return mux } -func StartNextRequestServer(t *testing.T, jwtPubKey *rsa.PublicKey) *http.Server { - s := &http.Server{ - Addr: fmt.Sprintf("localhost:%d", nextSessionServerPort), - Handler: chainedServerHandler(t, jwtPubKey), - } - go func() { - _ = s.ListenAndServe() - }() - return s -} - func IrmaServerConfiguration() *server.Configuration { return &server.Configuration{ - URL: fmt.Sprintf("http://localhost:%d", irmaServerPort), Logger: logger, DisableSchemesUpdate: true, SchemesPath: filepath.Join(testdata, "irma_configuration"), @@ -229,19 +222,7 @@ func IrmaServerConfiguration() *server.Configuration { revKeyshareTestCred: {RevocationServerURL: revocationServerURL}, }, JwtPrivateKeyFile: jwtPrivkeyPath, - StaticSessions: map[string]interface{}{ - "staticsession": irma.ServiceProviderRequest{ - RequestorBaseRequest: irma.RequestorBaseRequest{ - CallbackURL: staticSessionServerURL, - }, - Request: &irma.DisclosureRequest{ - BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest}, - Disclose: irma.AttributeConDisCon{ - {{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}}, - }, - }, - }, - }, + StaticSessions: map[string]interface{}{}, } } diff --git a/internal/sessiontest/redis_test.go b/internal/sessiontest/redis_test.go index 92fbd75b7..e19ba48d2 100644 --- a/internal/sessiontest/redis_test.go +++ b/internal/sessiontest/redis_test.go @@ -240,14 +240,16 @@ func TestRedisRedundancy(t *testing.T) { mr, cert := startRedis(t, true) defer mr.Close() - ports := []int{48690, 48691, 48692} + ports := make([]int, 3) servers := make([]*requestorserver.Server, len(ports)) - - for i, port := range ports { + for i := range ports { + port := findFreePort() c := redisRequestorConfigDecorator(mr, cert, "", RequestorServerAuthConfiguration)() - c.Configuration.URL = fmt.Sprintf("http://localhost:%d/irma", port) + // Make sure URL of load balancer is being used in QRs. + c.URL = requestorServerURL c.Port = port rs := StartRequestorServer(t, c) + ports[i] = port servers[i] = rs } lb := startLoadBalancer(t, ports) diff --git a/internal/sessiontest/session_test.go b/internal/sessiontest/session_test.go index 8aa38d455..7d4e130c2 100644 --- a/internal/sessiontest/session_test.go +++ b/internal/sessiontest/session_test.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/http/httptest" "os" "path/filepath" "reflect" @@ -380,8 +381,6 @@ func testDisclosureNewAttributeUpdateSchemeManager(t *testing.T, conf interface{ func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) { client, handler := parseStorage(t, opts...) defer test.ClearTestStorage(t, handler.storage) - rs := StartRequestorServer(t, RequestorServerAuthConfiguration()) - defer rs.Stop() // start server to receive session result callback after the session var received bool @@ -389,8 +388,24 @@ func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) { mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { received = true }) - s := &http.Server{Addr: fmt.Sprintf("localhost:%d", staticSessionServerPort), Handler: mux} - go func() { _ = s.ListenAndServe() }() + staticSessionServer := httptest.NewServer(mux) + defer staticSessionServer.Close() + + config := RequestorServerAuthConfiguration() + config.StaticSessions["staticsession"] = irma.ServiceProviderRequest{ + RequestorBaseRequest: irma.RequestorBaseRequest{ + CallbackURL: staticSessionServer.URL, + }, + Request: &irma.DisclosureRequest{ + BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest}, + Disclose: irma.AttributeConDisCon{ + {{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}}, + }, + }, + } + + rs := StartRequestorServer(t, config) + defer rs.Stop() // setup static QR and other variables qr := &irma.Qr{ @@ -410,7 +425,6 @@ func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) { // give irma server time to post session result to the server started above, and check the call was received time.Sleep(200 * time.Millisecond) - require.NoError(t, s.Shutdown(context.Background())) require.True(t, received) } @@ -511,13 +525,11 @@ func testChainedSessions(t *testing.T, conf interface{}, opts ...option) { require.IsType(t, IrmaServerConfiguration, conf) irmaServer := StartIrmaServer(t, conf.(func() *server.Configuration)()) defer irmaServer.Stop() - nextServer := StartNextRequestServer(t, &irmaServer.conf.JwtRSAPrivateKey.PublicKey) - defer func() { - _ = nextServer.Close() - }() + nextServer := httptest.NewServer(chainedServerHandler(t, &irmaServer.conf.JwtRSAPrivateKey.PublicKey)) + defer nextServer.Close() var request irma.ServiceProviderRequest - require.NoError(t, irma.NewHTTPTransport(nextSessionServerURL, false).Get("1", &request)) + require.NoError(t, irma.NewHTTPTransport(nextServer.URL, false).Get("1", &request)) doSession(t, &request, client, irmaServer, nil, nil, nil) // check that our credential instance is new @@ -947,7 +959,8 @@ func TestDisclosureNonexistingCredTypeUpdateSchemeManager(t *testing.T) { } func TestPOSTSizeLimit(t *testing.T) { - rs := StartRequestorServer(t, RequestorServerConfiguration()) + config := RequestorServerConfiguration() + rs := StartRequestorServer(t, config) defer rs.Stop() server.PostSizeLimit = 1 << 10 @@ -957,7 +970,7 @@ func TestPOSTSizeLimit(t *testing.T) { req, err := http.NewRequest( http.MethodPost, - requestorServerURL+"/session/", + fmt.Sprintf("http://localhost:%d/session/", config.Port), bytes.NewReader(make([]byte, server.PostSizeLimit+1, server.PostSizeLimit+1)), ) require.NoError(t, err) diff --git a/internal/test/testdata.go b/internal/test/testdata.go index 83faa26e4..4dc822b76 100644 --- a/internal/test/testdata.go +++ b/internal/test/testdata.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -28,8 +29,6 @@ func checkError(t *testing.T, err error) { } var schemeServer *http.Server -var badServer *http.Server -var badServerCount int var testStorageDir = "client" func StartSchemeManagerHttpServer() { @@ -45,26 +44,30 @@ func StopSchemeManagerHttpServer() { _ = schemeServer.Close() } -// StartBadHttpServer starts an HTTP server that times out and returns 500 on the first few times. -func StartBadHttpServer(count int, timeout time.Duration, success string) { - badServer = &http.Server{Addr: "localhost:48682", Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if badServerCount >= count { - _, _ = fmt.Fprintln(w, success) - return - } else { - badServerCount++ - time.Sleep(timeout) - } - })} +type BadServer struct { + count int + success string + timeout time.Duration +} - go func() { - _ = badServer.ListenAndServe() - }() - time.Sleep(100 * time.Millisecond) // Give server time to start +func (s *BadServer) ServeHTTP(w http.ResponseWriter, _ *http.Request) { + if s.count > 0 { + _, _ = fmt.Fprintln(w, s.success) + return + } else { + s.count-- + time.Sleep(s.timeout) + } } -func StopBadHttpServer() { - _ = badServer.Close() +// StartBadHttpServer starts an HTTP server that times out and returns 500 on the first few times. +func StartBadHttpServer(count int, timeout time.Duration, success string) *httptest.Server { + s := &BadServer{ + count: count, + timeout: timeout, + success: success, + } + return httptest.NewServer(s) } // FindTestdataFolder finds the "testdata" folder which is in . or .. diff --git a/irmago_test.go b/irmago_test.go index 601d66bf2..b05ce30f0 100644 --- a/irmago_test.go +++ b/irmago_test.go @@ -117,10 +117,10 @@ func TestParseInvalidIrmaConfiguration(t *testing.T) { } func TestRetryHTTPRequest(t *testing.T) { - test.StartBadHttpServer(2, 1*time.Second, "42") - defer test.StopBadHttpServer() + badServer := test.StartBadHttpServer(2, 1*time.Second, "42") + defer badServer.Close() - transport := NewHTTPTransport("http://localhost:48682", false) + transport := NewHTTPTransport(badServer.URL, false) transport.client.HTTPClient.Timeout = 500 * time.Millisecond bts, err := transport.GetBytes("") require.NoError(t, err) diff --git a/server/api_test.go b/server/api_test.go index 3a1ac09fa..3c1543608 100644 --- a/server/api_test.go +++ b/server/api_test.go @@ -1,12 +1,12 @@ package server import ( - "context" "encoding/json" "fmt" "io" "io/ioutil" "net/http" + "net/http/httptest" "testing" "time" @@ -130,12 +130,12 @@ func TestServerTimeouts(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // start server - s := startServer(t, test.handler, test.readTimeout) - defer stopServer(t, s) + s := startServer(test.handler, test.readTimeout) + defer s.Close() // do request called = false - req, err := http.NewRequest(http.MethodPost, "http://localhost:34534", test.body) + req, err := http.NewRequest(http.MethodPost, s.URL, test.body) require.NoError(t, err) start := time.Now() res, err := http.DefaultClient.Do(req) @@ -149,22 +149,9 @@ func TestServerTimeouts(t *testing.T) { } } -func startServer(t *testing.T, handler http.Handler, timeout time.Duration) *http.Server { - s := &http.Server{ - Addr: "localhost:34534", - Handler: handler, - ReadTimeout: timeout, - } - go func() { - err := s.ListenAndServe() - require.Equal(t, http.ErrServerClosed, err) - }() - time.Sleep(50 * time.Millisecond) // give server time to start +func startServer(handler http.Handler, timeout time.Duration) *httptest.Server { + s := httptest.NewUnstartedServer(handler) + s.Config.ReadTimeout = timeout + s.Start() return s } - -func stopServer(t *testing.T, server *http.Server) { - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - require.NoError(t, server.Shutdown(ctx)) - cancel() -} From f1be26a6ba2e2a76b74a1c389271c21c3ee30229 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 31 Jan 2022 23:07:56 +0100 Subject: [PATCH 2/3] Fix: TestRetryHTTPRequest should actually check using a bad server --- internal/test/testdata.go | 2 +- irmago_test.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/test/testdata.go b/internal/test/testdata.go index 4dc822b76..4752bba8f 100644 --- a/internal/test/testdata.go +++ b/internal/test/testdata.go @@ -51,7 +51,7 @@ type BadServer struct { } func (s *BadServer) ServeHTTP(w http.ResponseWriter, _ *http.Request) { - if s.count > 0 { + if s.count <= 0 { _, _ = fmt.Fprintln(w, s.success) return } else { diff --git a/irmago_test.go b/irmago_test.go index b05ce30f0..853d60403 100644 --- a/irmago_test.go +++ b/irmago_test.go @@ -117,11 +117,18 @@ func TestParseInvalidIrmaConfiguration(t *testing.T) { } func TestRetryHTTPRequest(t *testing.T) { - badServer := test.StartBadHttpServer(2, 1*time.Second, "42") + // Make sure that first 5 requests fail. + badServer := test.StartBadHttpServer(5, 1*time.Second, "42") defer badServer.Close() transport := NewHTTPTransport(badServer.URL, false) transport.client.HTTPClient.Timeout = 500 * time.Millisecond + + // Retryable HTTP tries 3 times, so this attempt should fail. + _, err := transport.GetBytes("") + require.Error(t, err) + + // The 6th request should succeed. bts, err := transport.GetBytes("") require.NoError(t, err) require.Equal(t, "42\n", string(bts)) From 15e1d2e4b6ef7f001493bc3c450c805cb5dd200e Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 2 Feb 2022 17:46:16 +0100 Subject: [PATCH 3/3] Refactor: remove empty StaticSessions option from the unit test IrmaConfiguration --- internal/sessiontest/helper_servers_test.go | 1 - internal/sessiontest/session_test.go | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/sessiontest/helper_servers_test.go b/internal/sessiontest/helper_servers_test.go index a0ffef7ad..a086a2a1c 100644 --- a/internal/sessiontest/helper_servers_test.go +++ b/internal/sessiontest/helper_servers_test.go @@ -222,7 +222,6 @@ func IrmaServerConfiguration() *server.Configuration { revKeyshareTestCred: {RevocationServerURL: revocationServerURL}, }, JwtPrivateKeyFile: jwtPrivkeyPath, - StaticSessions: map[string]interface{}{}, } } diff --git a/internal/sessiontest/session_test.go b/internal/sessiontest/session_test.go index 7d4e130c2..21b04f233 100644 --- a/internal/sessiontest/session_test.go +++ b/internal/sessiontest/session_test.go @@ -392,14 +392,16 @@ func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) { defer staticSessionServer.Close() config := RequestorServerAuthConfiguration() - config.StaticSessions["staticsession"] = irma.ServiceProviderRequest{ - RequestorBaseRequest: irma.RequestorBaseRequest{ - CallbackURL: staticSessionServer.URL, - }, - Request: &irma.DisclosureRequest{ - BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest}, - Disclose: irma.AttributeConDisCon{ - {{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}}, + config.StaticSessions = map[string]interface{}{ + "staticsession": irma.ServiceProviderRequest{ + RequestorBaseRequest: irma.RequestorBaseRequest{ + CallbackURL: staticSessionServer.URL, + }, + Request: &irma.DisclosureRequest{ + BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest}, + Disclose: irma.AttributeConDisCon{ + {{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}}, + }, }, }, }