Fix IDP deadlock due to recursive locking (crewjam#553)

While using the IDP server for integration tests, I've experienced a
deadlock after issuing many concurrent requests to the following
endpoints:

- PUT /services/:id
- GET /sso

Apparently, the issue is that the IDP code attempts to acquires locks on
the Server.idpConfigMu recursively:

1. The /sso HTTP handler calls Server.idpConfigMu.RLock()
2. Before releasing the above lock, this same handler eventually calls
   s.GetServiceProvider, which in turn attempts to acquire the same lock
   by calling Server.idpConfigMu.RLock()
3. Concurrent requests to `PUT /services/:id` acquire a write lock by
   calling Server.idpConfigMu.Lock()

Step (2) is a recursive lock, which won't work, as stated by the
documentation of sync.RWLock[1]:

    If a goroutine holds a RWMutex for reading and another goroutine
    might call Lock, no goroutine should expect to be able to acquire a
    read lock until the initial read lock is released. In particular,
    this prohibits recursive read locking. This is to ensure that the
    lock eventually becomes available; a blocked Lock call excludes new
    readers from acquiring the lock.

This patch fixes the issue by removing the unnecessary lock acquired in
step (1). This was redundant since GetServiceProvider() already takes
care of serializing access to the s.serviceProviders map, which is the
resource in question.

[1] https://golang.org/pkg/sync/#RWMutex

Author: agis

samlidp/samlidp.go

@@ -93,8 +93,6 @@ func (s *Server) InitializeHTTP() {
 		s.IDP.ServeMetadata(w, r)
 	})
 	mux.Handle("/sso", func(w http.ResponseWriter, r *http.Request) {
-		s.idpConfigMu.RLock()
-		defer s.idpConfigMu.RUnlock()
 		s.IDP.ServeSSO(w, r)
 	})