rename and move stuff, two new tests

pkg/apiserver/middlewares/v1/cache.go

@@ -36,14 +36,14 @@ func (rc *RevocationCache) Get(sn string, logger *log.Entry) (bool, bool) {
 	}
 
 	rc.mu.Lock()
+	defer rc.mu.Unlock()
+
 	if entry.timestamp.Add(rc.expiration).Before(time.Now()) {
 		logger.Debugf("TLSAuth: cached value for %s expired, removing from cache", sn)
 		delete(rc.cache, sn)
-		rc.mu.Unlock()
 
 		return false, false
 	}
-	rc.mu.Unlock()
 
 	logger.Debugf("TLSAuth: using cached value for cert %s: %t", sn, entry.revoked)
 
@@ -52,9 +52,10 @@ func (rc *RevocationCache) Get(sn string, logger *log.Entry) (bool, bool) {
 
 func (rc *RevocationCache) Set(sn string, revoked bool) {
 	rc.mu.Lock()
+	defer rc.mu.Unlock()
+
 	rc.cache[sn] = cacheEntry{
 		revoked:   revoked,
 		timestamp: time.Now(),
 	}
-	rc.mu.Unlock()
 }

pkg/apiserver/middlewares/v1/tls_auth.go

@@ -37,15 +37,15 @@ func (ta *TLSAuth) isExpired(cert *x509.Certificate) bool {
 // isRevoked checks a certificate against OCSP and CRL
 func (ta *TLSAuth) isRevoked(cert *x509.Certificate, issuer *x509.Certificate) (bool, error) {
 	sn := cert.SerialNumber.String()
-	if revoked, ok := ta.revocationCache.Get(sn, ta.logger); ok {
+	if revoked, cached := ta.revocationCache.Get(sn, ta.logger); cached {
 		return revoked, nil
 	}
 
-	revokedByOCSP, cacheOCSP := ta.ocspChecker.isRevoked(cert, issuer)
-	revokedByCRL, cacheCRL := ta.crlChecker.isRevoked(cert)
+	revokedByOCSP, checkedByOCSP := ta.ocspChecker.isRevoked(cert, issuer)
+	revokedByCRL, checkedByCRL := ta.crlChecker.isRevoked(cert)
 	revoked := revokedByOCSP || revokedByCRL
 
-	if cacheOCSP && cacheCRL {
+	if checkedByOCSP && checkedByCRL {
 		ta.revocationCache.Set(sn, revoked)
 	}
 

test/bats/11_bouncers_tls.bats

@@ -107,7 +107,7 @@ teardown() {
     assert_output "[]"
 }
 
-@test "simulate one bouncer request with a valid cert" {
+@test "simulate a bouncer request with a valid cert" {
     rune -0 curl -f -s \
         --cert "${tmpdir}/bouncer.pem" \
         --key "${tmpdir}/bouncer-key.pem" \
@@ -123,7 +123,7 @@ teardown() {
     rune cscli bouncers delete [email protected]
 }
 
-@test "simulate one bouncer request with an invalid cert" {
+@test "simulate a bouncer request with an invalid cert" {
     rune -77 curl -f -s \
         --cert "${tmpdir}/bouncer_invalid.pem" \
         --key "${tmpdir}/bouncer_invalid-key.pem" \
@@ -133,7 +133,7 @@ teardown() {
     assert_output "[]"
 }
 
-@test "simulate one bouncer request with an invalid OU" {
+@test "simulate a bouncer request with an invalid OU" {
     rune -22 curl -f -s \
         --cert "${tmpdir}/bouncer_bad_ou.pem" \
         --key "${tmpdir}/bouncer_bad_ou-key.pem" \
@@ -143,7 +143,7 @@ teardown() {
     assert_output "[]"
 }
 
-@test "simulate one bouncer request with a revoked certificate" {
+@test "simulate a bouncer request with a revoked certificate" {
     # we have two certificates revoked by different CRL blocks
     for cert_name in "revoked_1" "revoked_2"; do
         truncate_log
@@ -159,3 +159,17 @@ teardown() {
         assert_output "[]"
     done
 }
+
+# vvv this test must be last, or it can break the ones that follow
+
+@test "allowed_ou can't contain an empty string" {
+    ./instance-crowdsec stop
+    config_set '
+        .common.log_media="stdout" |
+        .api.server.tls.bouncers_allowed_ou=["bouncer-ou", ""]
+    '
+    rune -1 wait-for "$CROWDSEC"
+    assert_stderr --partial "allowed_ou configuration contains invalid empty string"
+}
+
+# ^^^ this test must be last, or it can break the ones that follow

test/bats/30_machines_tls.bats

@@ -243,3 +243,16 @@ teardown() {
         ./instance-crowdsec stop
     done
 }
+
+# vvv this test must be last, or it can break the ones that follow
+
+@test "allowed_ou can't contain an empty string" {
+    config_set '
+        .common.log_media="stdout" |
+        .api.server.tls.agents_allowed_ou=["agent-ou", ""]
+    '
+    rune -1 wait-for "$CROWDSEC"
+    assert_stderr --partial "allowed_ou configuration contains invalid empty string"
+}
+
+# ^^^ this test must be last, or it can break the ones that follow