Repository: jkroepke/openvpn-auth-oauth2
Pull request #20 (2023-09-20 05:05:06)

add client connect tests

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) Report
Patch coverage: **`83.33%`** and project coverage change: **`-9.17%`** :warning:
> Comparison is base [(`b464c8e`)](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/commit/b464c8e7aa8d26bdd95346fb74f925a03fbb89d0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) 42.61% compared to head [(`0ee73f2`)](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) 33.44%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   42.61%   33.44%   -9.17%     
==========================================
  Files           5        7       +2     
  Lines         352      580     +228     
==========================================
+ Hits          150      194      +44     
- Misses        196      379     +183     
- Partials        6        7       +1     
```


| [Files Changed](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke) | Coverage Δ | |
|---|---|---|
| [internal/openvpn/client\_connection.go](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke#diff-aW50ZXJuYWwvb3BlbnZwbi9jbGllbnRfY29ubmVjdGlvbi5nbw==) | `93.61% <83.33%> (ø)` | |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/jkroepke/openvpn-auth-oauth2/pull/20?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jan-Otto+Kr%C3%B6pke).


Patch:
diff --git a/internal/openvpn/client_connection.go b/internal/openvpn/client_connection.go
index 725d004..d18298b 100644
--- a/internal/openvpn/client_connection.go
+++ b/internal/openvpn/client_connection.go
@@ -1,6 +1,8 @@
 package openvpn
 
 import (
+	"errors"
+	"fmt"
 	"strconv"
 	"strings"
 )
@@ -14,15 +16,23 @@ type ClientConnection struct {
 
 func NewClientConnection(message string) (*ClientConnection, error) {
 	clientConnection := &ClientConnection{
-		Env: map[string]string{},
+		Kid:    0,
+		Cid:    0,
+		Reason: "",
+		Env:    map[string]string{},
 	}
 
 	for _, line := range strings.Split(strings.TrimSpace(message), "\r\n") {
 		if strings.HasPrefix(line, ">CLIENT:CONNECT") ||
 			strings.HasPrefix(line, ">CLIENT:REAUTH") ||
 			strings.HasPrefix(line, ">CLIENT:DISCONNECT") ||
-			strings.HasPrefix(line, ">CLIENT:ESTABLISHED") {
+			strings.HasPrefix(line, ">CLIENT:ESTABLISHED") ||
+			strings.HasPrefix(line, ">CLIENT:CR_RESPONSE") {
 			clientInfo := strings.Split(strings.TrimSpace(line), ",")
+			if len(clientInfo) == 1 {
+				return nil, fmt.Errorf("unable to parse line %s", line)
+			}
+
 			clientConnection.Reason = strings.Replace(clientInfo[0], ">CLIENT:", "", 1)
 
 			if cid, err := strconv.Atoi(clientInfo[1]); err != nil {
@@ -40,7 +50,9 @@ func NewClientConnection(message string) (*ClientConnection, error) {
 			}
 		} else if strings.HasPrefix(line, ">CLIENT:ENV,") {
 			clientEnv := strings.SplitN(strings.SplitN(line, ",", 2)[1], "=", 2)
-			if len(clientEnv) == 2 {
+			if clientEnv[0] == "END" {
+				break
+			} else if len(clientEnv) == 2 {
 				clientConnection.Env[clientEnv[0]] = clientEnv[1]
 			} else {
 				clientConnection.Env[clientEnv[0]] = ""
@@ -48,5 +60,13 @@ func NewClientConnection(message string) (*ClientConnection, error) {
 		}
 	}
 
+	if clientConnection.Reason == "" {
+		return nil, errors.New("unable to parse client reason")
+	}
+
+	if clientConnection.Cid == 0 {
+		return nil, errors.New("unable to parse CID")
+	}
+
 	return clientConnection, nil
 }


Test patch:
diff --git a/internal/openvpn/client_connection_test.go b/internal/openvpn/client_connection_test.go
new file mode 100644
index 0000000..4fca329
--- /dev/null
+++ b/internal/openvpn/client_connection_test.go
@@ -0,0 +1,40 @@
+package openvpn
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewClientConnection(t *testing.T) {
+	for _, tt := range []struct {
+		name             string
+		lines            []string
+		clientConnection *ClientConnection
+		err              string
+	}{
+		{"client CONNECT", []string{">CLIENT:CONNECT,1,2", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 2, Reason: "CONNECT", Env: map[string]string{"name1": "val1", "name2": ""}}, ""},
+		{"client CONNECT invalid cid", []string{">CLIENT:CONNECT,k,2", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 2, Reason: "CONNECT", Env: map[string]string{"name1": "val1", "name2": ""}}, "strconv.Atoi: parsing \"k\": invalid syntax"},
+		{"client CONNECT invalid kid", []string{">CLIENT:CONNECT,1,k", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 2, Reason: "CONNECT", Env: map[string]string{"name1": "val1", "name2": ""}}, "strconv.Atoi: parsing \"k\": invalid syntax"},
+		{"client REAUTH", []string{">CLIENT:REAUTH,1,2", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 2, Reason: "REAUTH", Env: map[string]string{"name1": "val1", "name2": ""}}, ""},
+		{"client ESTABLISHED", []string{">CLIENT:ESTABLISHED,1", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 0, Reason: "ESTABLISHED", Env: map[string]string{"name1": "val1", "name2": ""}}, ""},
+		{"client DISCONNECT", []string{">CLIENT:DISCONNECT,1", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 0, Reason: "DISCONNECT", Env: map[string]string{"name1": "val1", "name2": ""}}, ""},
+		{"client CR_RESPONSE", []string{">CLIENT:CR_RESPONSE,1,2,YmFzZTY0", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2=", ">CLIENT:ENV,END"}, &ClientConnection{Cid: 1, Kid: 2, Reason: "CR_RESPONSE", Env: map[string]string{"name1": "val1", "name2": ""}}, ""},
+		{"client invalid reason", []string{">CLIENT:unknown", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2", ">CLIENT:ENV,END"}, nil, "unable to parse client reason"},
+		{"client invalid reason", []string{">CLIENT:CONNECT", ">CLIENT:ENV,name1=val1", ">CLIENT:ENV,name2", ">CLIENT:ENV,END"}, nil, "unable to parse line >CLIENT:CONNECT"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			message := strings.Join(tt.lines, "\r\n")
+
+			clientConnection, err := NewClientConnection(message)
+			if tt.err == "" {
+				assert.NoError(t, err)
+				assert.Equal(t, clientConnection, tt.clientConnection)
+			} else {
+				assert.Error(t, err)
+				assert.Equal(t, err.Error(), tt.err)
+			}
+		})
+	}
+}