fixes after review

11 files changed, 154 insertions, 147 deletions

diff --git a/Makefile b/Makefile
index 35877d5..cc1ca07 100644
--- a/Makefile
+++ b/Makefile
@@ -7,9 +7,9 @@ PROVIDER=deploy/public/provider.json
 build:
 	go build cmd/vpnweb/vpnweb.go
 demo-sip:
-	. config/CONFIG && ./vpnweb -notls -auth=sip
+	. config/CONFIG && ./vpnweb -auth=sip
 demo-anon:
-	. config/CONFIG && ./vpnweb -notls -auth=anon
+	. config/CONFIG && ./vpnweb -auth=anon
 clean:
 	rm -f public/1/*
 	rm public/ca.crt
diff --git a/chaperone.d/vpnweb.conf b/chaperone.d/vpnweb.conf
index 1c10f09..7b7bd4a 100644
--- a/chaperone.d/vpnweb.conf
+++ b/chaperone.d/vpnweb.conf
@@ -1,4 +1,4 @@
 vpnweb-server.service: {
-     command: "/bin/sh -c 'exec /usr/local/bin/vpnweb -caCrt ${CLIENT_CA.CRT:-/etc/leap/ca/client_ca.crt} -caKey ${CLIENT_CA.KEY:-/etc/leap/ca/client_ca.key} -notls }'",
+     command: "/bin/sh -c 'exec /usr/local/bin/vpnweb -caCrt ${CLIENT_CA.CRT:-/etc/leap/ca/client_ca.crt} -caKey ${CLIENT_CA.KEY:-/etc/leap/ca/client_ca.key} }'",
      exit_kills: false
 }
diff --git a/cmd/vpnweb/vpnweb.go b/main.go
index ddd919c..d719149 100644
--- a/cmd/vpnweb/vpnweb.go
+++ b/main.go
@@ -4,23 +4,18 @@ import (
 	"log"
 	"net/http"
 
-	//"0xacab.org/leap/pkg/auth"
 	"0xacab.org/leap/vpnweb/pkg/auth"
 	"0xacab.org/leap/vpnweb/pkg/config"
 	"0xacab.org/leap/vpnweb/pkg/web"
 )
 
 func main() {
-	opts := new(config.Opts)
-	config.InitializeFlags(opts)
-	config.CheckConfigurationOptions(opts)
-
-	ci := web.NewCaInfo(opts.CaCrt, opts.CaKey)
-	ch := web.CertHandler{ci}
+	opts := config.NewOpts()
+	ch := web.NewCertHandler(opts.CaCrt, opts.CaKey)
 
 	/* protected routes */
 
-	/* TODO ----
+	/* TODO https://0xacab.org/leap/vpnweb/issues/4
 	http.HandleFunc("/3/refresh-token", auth.RefreshAuthMiddleware(opts.Auth))
 	*/
 
@@ -41,10 +36,10 @@ func main() {
 	pstr := ":" + opts.Port
 	log.Println("Listening in port", opts.Port)
 
-	if opts.Notls == true {
-		log.Fatal(http.ListenAndServe(pstr, nil))
-	} else {
+	if opts.tls == true {
 		log.Fatal(http.ListenAndServeTLS(pstr, opts.TlsCrt, opts.TlsKey, nil))
+	} else {
+		log.Fatal(http.ListenAndServe(pstr, nil))
 
 	}
 }
diff --git a/pkg/auth/middleware.go b/pkg/auth/middleware.go
index dfd4da3..280ceeb 100644
--- a/pkg/auth/middleware.go
+++ b/pkg/auth/middleware.go
@@ -5,13 +5,15 @@ import (
 	"0xacab.org/leap/vpnweb/pkg/config"
 	"0xacab.org/leap/vpnweb/pkg/web"
 	"github.com/auth0/go-jwt-middleware"
-	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/dgrijalva/jwt-go"
 	"log"
 	"net/http"
 )
 
-const anonAuth string = "anon"
-const sip2Auth string = "sip"
+const (
+	anonAuth = "anon"
+	sip2Auth = "sip"
+)
 
 func bailOnBadAuthModule(module string) {
 	log.Fatal("Unknown auth module: '", module, "'. Should be one of: ", anonAuth, ", ", sip2Auth, ".")
diff --git a/pkg/auth/sip2/auth.go b/pkg/auth/sip2/auth.go
index 58441e4..9c01c28 100644
--- a/pkg/auth/sip2/auth.go
+++ b/pkg/auth/sip2/auth.go
@@ -2,7 +2,7 @@ package sip2
 
 import (
 	"encoding/json"
-	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/dgrijalva/jwt-go"
 	"log"
 	"net/http"
 	"os"
@@ -11,14 +11,13 @@ import (
 	"0xacab.org/leap/vpnweb/pkg/config"
 )
 
-const SipUserVar string = "VPNWEB_SIP_USER"
-const SipPassVar string = "VPNWEB_SIP_PASS"
-const SipPortVar string = "VPNWEB_SIP_PORT"
-const SipHostVar string = "VPNWEB_SIP_HOST"
-const SipLibrLocVar string = "VPNWEB_SIP_LIBR_LOCATION"
-const SipTerminatorVar string = "VPNWEB_SIP_TERMINATOR"
-
-const SipDefaultTerminator string = "\r\n"
+const sipUserVar string = "VPNWEB_SIP_USER"
+const sipPassVar string = "VPNWEB_SIP_PASS"
+const sipPortVar string = "VPNWEB_SIP_PORT"
+const sipHostVar string = "VPNWEB_SIP_HOST"
+const sipLibrLocVar string = "VPNWEB_SIP_LIBR_LOCATION"
+const sipTerminatorVar string = "VPNWEB_SIP_TERMINATOR"
+const sipDefaultTerminator string = "\r\n"
 
 type Credentials struct {
 	User     string
@@ -34,11 +33,11 @@ func getConfigFromEnv(envVar string) string {
 }
 
 func setupTerminatorFromEnv() {
-	config.FallbackToEnv(&TelnetTerminator, SipTerminatorVar, SipDefaultTerminator)
-	if TelnetTerminator == "\\r" {
-		TelnetTerminator = "\r"
-	} else if TelnetTerminator == "\\r\\n" {
-		TelnetTerminator = "\r\n"
+	config.FallbackToEnv(&telnetTerminator, sipTerminatorVar, sipDefaultTerminator)
+	if telnetTerminator == "\\r" {
+		telnetTerminator = "\r"
+	} else if telnetTerminator == "\\r\\n" {
+		telnetTerminator = "\r\n"
 	}
 }
 
@@ -46,11 +45,11 @@ func SipAuthenticator(opts *config.Opts) http.HandlerFunc {
 
 	log.Println("Initializing SIP2 authenticator")
 
-	SipUser := getConfigFromEnv(SipUserVar)
-	SipPass := getConfigFromEnv(SipPassVar)
-	SipHost := getConfigFromEnv(SipHostVar)
-	SipPort := getConfigFromEnv(SipPortVar)
-	SipLibrLoc := getConfigFromEnv(SipLibrLocVar)
+	SipUser := getConfigFromEnv(sipUserVar)
+	SipPass := getConfigFromEnv(sipPassVar)
+	SipHost := getConfigFromEnv(sipHostVar)
+	SipPort := getConfigFromEnv(sipPortVar)
+	SipLibrLoc := getConfigFromEnv(sipLibrLocVar)
 
 	setupTerminatorFromEnv()
 
diff --git a/pkg/auth/sip2/spec.go b/pkg/auth/sip2/spec.go
index ba7c356..af65b33 100644
--- a/pkg/auth/sip2/spec.go
+++ b/pkg/auth/sip2/spec.go
@@ -6,136 +6,136 @@ import (
 	"strings"
 )
 
-type FixedFieldSpec struct {
+const (
+	yes                  string = "Y"
+	trueVal              string = "1"
+	okVal                string = "ok"
+	language             string = "language"
+	patronStatus         string = "patron status"
+	date                 string = "transaction date"
+	patronIdentifier     string = "patron identifier"
+	patronPassword       string = "patron password"
+	personalName         string = "personal name"
+	screenMessage        string = "screen message"
+	institutionID        string = "institution id"
+	validPatron          string = "valid patron"
+	validPatronPassword  string = "valid patron password"
+	loginResponse        string = "Login Response"
+	patronStatusResponse string = "Patron Status Response"
+)
+
+type fixedFieldSpec struct {
 	length int
 	label  string
 }
 
-type FixedField struct {
-	spec  FixedFieldSpec
+type fixedField struct {
+	spec  fixedFieldSpec
 	value string
 }
 
-type VariableFieldSpec struct {
+type variableFieldSpec struct {
 	id    string
 	label string
 }
 
-type VariableField struct {
-	spec  VariableFieldSpec
+type variableField struct {
+	spec  variableFieldSpec
 	value string
 }
 
-type MessageSpec struct {
+type messageSpec struct {
 	id     int
 	label  string
-	fields []FixedFieldSpec
+	fields []fixedFieldSpec
 }
 
-type Message struct {
-	fields       []VariableField
-	fixed_fields []FixedField
-	msg_txt      string
+type message struct {
+	fields      []variableField
+	fixedFields []fixedField
+	msgTxt      string
 }
 
 type Parser struct {
-	getMessageSpecByCode   func(int) MessageSpec
-	getVariableFieldByCode func(string) VariableFieldSpec
-	getFixedFieldValue     func(*Message, string) (string, bool)
-	getFieldValue          func(*Message, string) (string, bool)
-	parseMessage           func(string) *Message
+	msgByCodeMap           map[int]messageSpec
+	variableFieldByCodeMap map[string]variableFieldSpec
 }
 
-const (
-	YES                  string = "Y"
-	TRUE                 string = "1"
-	Ok                   string = "ok"
-	Language             string = "language"
-	PatronStatus         string = "patron status"
-	Date                 string = "transaction date"
-	PatronIdentifier     string = "patron identifier"
-	PatronPassword       string = "patron password"
-	PersonalName         string = "personal name"
-	ScreenMessage        string = "screen message"
-	InstitutionId        string = "institution id"
-	ValidPatron          string = "valid patron"
-	ValidPatronPassword  string = "valid patron password"
-	LoginResponse        string = "Login Response"
-	PatronStatusResponse string = "Patron Status Response"
-)
-
 func getParser() *Parser {
 
-	LanguageSpec := FixedFieldSpec{3, Language}
-	PatronStatusSpec := FixedFieldSpec{14, PatronStatus}
-	DateSpec := FixedFieldSpec{18, Date}
-	OkSpec := FixedFieldSpec{1, Ok}
+	languageSpec := fixedFieldSpec{3, language}
+	patronStatusSpec := fixedFieldSpec{14, patronStatus}
+	dateSpec := fixedFieldSpec{18, date}
+	okSpec := fixedFieldSpec{1, okVal}
 
-	msgByCodeMap := map[int]MessageSpec{
-		94: MessageSpec{94, LoginResponse, []FixedFieldSpec{OkSpec}},
-		24: MessageSpec{24, PatronStatusResponse, []FixedFieldSpec{PatronStatusSpec, LanguageSpec, DateSpec}},
+	msgByCodeMap := map[int]messageSpec{
+		94: messageSpec{94, loginResponse, []fixedFieldSpec{okSpec}},
+		24: messageSpec{24, patronStatusResponse, []fixedFieldSpec{patronStatusSpec, languageSpec, dateSpec}},
 	}
 
-	variableFieldByCodeMap := map[string]VariableFieldSpec{
-		"AA": VariableFieldSpec{"AA", PatronIdentifier},
-		"AD": VariableFieldSpec{"AD", PatronPassword},
-		"AE": VariableFieldSpec{"AE", PersonalName},
-		"AF": VariableFieldSpec{"AF", ScreenMessage},
-		"AO": VariableFieldSpec{"AO", InstitutionId},
-		"BL": VariableFieldSpec{"BL", ValidPatron},
-		"CQ": VariableFieldSpec{"CQ", ValidPatronPassword},
+	variableFieldByCodeMap := map[string]variableFieldSpec{
+		"AA": variableFieldSpec{"AA", patronIdentifier},
+		"AD": variableFieldSpec{"AD", patronPassword},
+		"AE": variableFieldSpec{"AE", personalName},
+		"AF": variableFieldSpec{"AF", screenMessage},
+		"AO": variableFieldSpec{"AO", institutionID},
+		"BL": variableFieldSpec{"BL", validPatron},
+		"CQ": variableFieldSpec{"CQ", validPatronPassword},
 	}
 
-	parser := new(Parser)
-	parser.getMessageSpecByCode = func(code int) MessageSpec {
-		return msgByCodeMap[code]
-	}
-	parser.getVariableFieldByCode = func(code string) VariableFieldSpec {
-		return variableFieldByCodeMap[code]
-	}
-	parser.getFixedFieldValue = func(msg *Message, field string) (string, bool) {
-		for _, v := range msg.fixed_fields {
-			if v.spec.label == field {
-				return v.value, true
-			}
+	return &Parser{msgByCodeMap, variableFieldByCodeMap}
+}
+
+func (p *Parser) getMessageSpecByCode(code int) messageSpec {
+	return p.msgByCodeMap[code]
+}
+
+func (p *Parser) getVariableFieldByCode(code string) variableFieldSpec {
+	return p.variableFieldByCodeMap[code]
+}
+
+func (p *Parser) getFixedFieldValue(msg *message, field string) (string, bool) {
+	for _, v := range msg.fixedFields {
+		if v.spec.label == field {
+			return v.value, true
 		}
-		return "", false
 	}
-	parser.getFieldValue = func(msg *Message, field string) (string, bool) {
-		for _, v := range msg.fields {
-			if v.spec.label == field {
-				return v.value, true
-			}
+	return "", false
+}
+
+func (p *Parser) getFieldValue(msg *message, field string) (string, bool) {
+	for _, v := range msg.fields {
+		if v.spec.label == field {
+			return v.value, true
 		}
-		return "", false
 	}
+	return "", false
+}
 
-	parser.parseMessage = func(msg string) *Message {
-		txt := msg[:len(msg)-len(TelnetTerminator)]
-		code, err := strconv.Atoi(txt[:2])
-		if nil != err {
-			log.Printf("Error parsing integer: %s\n", txt[:2])
-		}
-		spec := parser.getMessageSpecByCode(code)
-		txt = txt[2:]
-
-		message := new(Message)
-		for _, sp := range spec.fields {
-			value := txt[:sp.length]
-			txt = txt[sp.length:]
-			message.fixed_fields = append(message.fixed_fields, FixedField{sp, value})
-		}
-		if len(txt) == 0 {
-			return message
-		}
-		for _, part := range strings.Split(txt, "|") {
-			if len(part) > 0 {
-				part_spec := parser.getVariableFieldByCode(part[:2])
-				value := part[2:]
-				message.fields = append(message.fields, VariableField{part_spec, value})
-			}
-		}
+func (p *Parser) parseMessage(msg string) *message {
+	txt := msg[:len(msg)-len(telnetTerminator)]
+	code, err := strconv.Atoi(txt[:2])
+	if nil != err {
+		log.Printf("Error parsing integer: %s\n", txt[:2])
+	}
+	spec := p.getMessageSpecByCode(code)
+	txt = txt[2:]
+
+	message := new(message)
+	for _, sp := range spec.fields {
+		value := txt[:sp.length]
+		txt = txt[sp.length:]
+		message.fixedFields = append(message.fixedFields, fixedField{sp, value})
+	}
+	if len(txt) == 0 {
 		return message
 	}
-	return parser
+	for _, part := range strings.Split(txt, "|") {
+		if len(part) > 0 {
+			partSpec := p.getVariableFieldByCode(part[:2])
+			value := part[2:]
+			message.fields = append(message.fields, variableField{partSpec, value})
+		}
+	}
+	return message
 }
diff --git a/pkg/auth/sip2/telnet.go b/pkg/auth/sip2/telnet.go
index faa72ff..ae5004e 100644
--- a/pkg/auth/sip2/telnet.go
+++ b/pkg/auth/sip2/telnet.go
@@ -7,7 +7,7 @@ import (
 // The terminator can be configured differently for different SIP endpoints.
 // This gets set in sip2.auth according to an environment variable
 
-var TelnetTerminator string
+var telnetTerminator string
 
 func telnetRead(conn *telnet.Conn) (out string) {
 	var buffer [1]byte
@@ -22,7 +22,7 @@ func telnetRead(conn *telnet.Conn) (out string) {
 		} else {
 			out += string(recvData)
 		}
-		if len(out) > 1 && out[len(out)-len(TelnetTerminator):] == TelnetTerminator {
+		if len(out) > 1 && out[len(out)-len(telnetTerminator):] == telnetTerminator {
 			break
 		}
 	}
diff --git a/pkg/config/main.go b/pkg/config/main.go
index f5c0c35..c5b687e 100644
--- a/pkg/config/main.go
+++ b/pkg/config/main.go
@@ -9,7 +9,7 @@ import (
 const DefaultAuthenticationModule string = "anon"
 
 type Opts struct {
-	Notls      bool
+	Tls        bool
 	CaCrt      string
 	CaKey      string
 	TlsCrt     string
@@ -19,8 +19,6 @@ type Opts struct {
 	AuthSecret string
 }
 
-var SIPTelnetTerminator string = ""
-
 func FallbackToEnv(variable *string, envVar, defaultVar string) {
 
 	if *variable == "" {
@@ -51,10 +49,17 @@ func doTlsFilesSanityCheck(tlsCrt string, tlsKey string) {
 	}
 }
 
-func InitializeFlags(opts *Opts) {
-	flag.BoolVar(&opts.Notls, "notls", false, "Disable TLS on the service")
-	flag.StringVar(&opts.CaCrt, "caCrt", "", "Path to the CA public key")
-	flag.StringVar(&opts.CaKey, "caKey", "", "Path to the CA private key")
+func NewOpts() *Opts {
+	opts := new(Opts)
+	initializeFlags(opts)
+	checkConfigurationOptions(opts)
+	return opts
+}
+
+func initializeFlags(opts *Opts) {
+	flag.StringVar(&opts.CaCrt, "caCrt", "", "Path to the CA public key used for VPN certificates")
+	flag.StringVar(&opts.CaKey, "caKey", "", "Path to the CA private key used for VPN certificates")
+	flag.BoolVar(&opts.Tls, "tls", false, "Enable TLS on the service")
 	flag.StringVar(&opts.TlsCrt, "tlsCrt", "", "Path to the cert file for TLS")
 	flag.StringVar(&opts.TlsKey, "tlsKey", "", "Path to the key file for TLS")
 	flag.StringVar(&opts.Port, "port", "", "Port where the server will listen (default: 8000)")
@@ -71,7 +76,7 @@ func InitializeFlags(opts *Opts) {
 	FallbackToEnv(&opts.AuthSecret, "VPNWEB_AUTH_SECRET", "")
 }
 
-func CheckConfigurationOptions(opts *Opts) {
+func checkConfigurationOptions(opts *Opts) {
 	if opts.CaCrt == "" {
 		log.Fatal("missing caCrt parameter")
 	}
@@ -79,17 +84,17 @@ func CheckConfigurationOptions(opts *Opts) {
 		log.Fatal("missing caKey parameter")
 	}
 
-	if opts.Notls == false {
+	if opts.Tls == true {
 		if opts.TlsCrt == "" {
-			log.Fatal("missing tls_crt parameter. maybe use -notls?")
+			log.Fatal("missing tls_crt parameter")
 		}
 		if opts.TlsKey == "" {
-			log.Fatal("missing tls_key parameter. maybe use -notls?")
+			log.Fatal("missing tls_key parameter")
 		}
 	}
 
 	doCaFilesSanityCheck(opts.CaCrt, opts.CaKey)
-	if opts.Notls == false {
+	if opts.Tls == true {
 		doTlsFilesSanityCheck(opts.TlsCrt, opts.TlsKey)
 	}
 
diff --git a/pkg/web/certs.go b/pkg/web/certs.go
index 8c5d423..9cccc65 100644
--- a/pkg/web/certs.go
+++ b/pkg/web/certs.go
@@ -31,7 +31,7 @@ type caInfo struct {
 	cacrt, cakey string
 }
 
-func NewCaInfo(cacrt string, cakey string) caInfo {
+func newCaInfo(cacrt string, cakey string) caInfo {
 	return caInfo{cacrt, cakey}
 }
 
diff --git a/pkg/web/handlers.go b/pkg/web/handlers.go
index c4f2e9a..b7675f5 100644
--- a/pkg/web/handlers.go
+++ b/pkg/web/handlers.go
@@ -8,6 +8,12 @@ type CertHandler struct {
 	Cainfo caInfo
 }
 
+func NewCertHandler(caCrt, caKey string) CertHandler {
+	ci := newCaInfo(caCrt, caKey)
+	ch := CertHandler{ci}
+	return ch
+}
+
 func (ch *CertHandler) CertResponder(w http.ResponseWriter, r *http.Request) {
 	ch.Cainfo.CertWriter(w)
 }
diff --git a/test/integration/sipcli/main.go b/test/integration/sipcli/main.go
index 163d21d..e211169 100644
--- a/test/integration/sipcli/main.go
+++ b/test/integration/sipcli/main.go
@@ -15,7 +15,7 @@ const authURI string = "http://%s:%s/3/auth"
 const certURI string = "http://%s:%s/3/cert"
 
 func formatCredentials(user, pass string) (string, error) {
-	c := sip2.Credentials{user, pass}
+	c := sip2.Credentials{User: user, Password: pass}
 	credJson, err := json.Marshal(c)
 	if err != nil {
 		return "", err