From 78772cc8303eb7c4c0795a75636ef5263613fa97 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Wed, 8 Nov 2017 22:26:09 +0000
Subject: [PATCH] apply review feedback

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
 .../dendrite/clientapi/routing/voip.go        | 28 +++++++++----------
 .../dendrite/common/config/config.go          | 13 ++++++++-
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/voip.go b/src/github.com/matrix-org/dendrite/clientapi/routing/voip.go
index c718a252c..d3212c4ed 100644
--- a/src/github.com/matrix-org/dendrite/clientapi/routing/voip.go
+++ b/src/github.com/matrix-org/dendrite/clientapi/routing/voip.go
@@ -26,6 +26,7 @@ import (
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/common/config"
 	"github.com/matrix-org/util"
+	"github.com/matrix-org/dendrite/clientapi/httputil"
 )
 
 type turnServerResponse struct {
@@ -39,38 +40,35 @@ func RequestTurnServer(req *http.Request, device *authtypes.Device, cfg config.D
 	turnConfig := cfg.TURN
 
 	// TODO Guest Support
-	if len(turnConfig.URIs) == 0 /* || (isGuest && !turnConfig.AllowGuests) */ {
+	if len(turnConfig.URIs) == 0 || turnConfig.UserLifetime == "" {
 		return util.JSONResponse{
 			Code: 200,
 			JSON: struct{}{},
 		}
 	}
 
-	duration, err := time.ParseDuration(turnConfig.UserLifetime)
-	if err != nil {
-		util.GetLogger(req.Context()).WithError(err).Warn("Invalid configuration value turn.turn_user_lifetime")
-		return util.JSONResponse{
-			Code: 200,
-			JSON: struct{}{},
-		}
-	}
+	// Duration checked at startup, err not possible
+	duration, _ := time.ParseDuration(turnConfig.UserLifetime)
 
 	resp := turnServerResponse{
-		Username: turnConfig.Username,
-		Password: turnConfig.Password,
 		URIs: turnConfig.URIs,
 		TTL: int(duration.Seconds()),
 	}
 
 	if turnConfig.SharedSecret != "" {
 		expiry := time.Now().Add(duration).Unix()
-		resp.Username = fmt.Sprintf("%d:%s", expiry, device.UserID)
-
 		mac := hmac.New(sha1.New, []byte(turnConfig.SharedSecret))
-		mac.Write([]byte(resp.Username))
+		_, err := mac.Write([]byte(resp.Username))
+
+		if err != nil {
+			return httputil.LogThenError(req, err)
+		}
+
+		resp.Username = fmt.Sprintf("%d:%s", expiry, device.UserID)
 		resp.Password = base64.StdEncoding.EncodeToString(mac.Sum(nil))
 	} else if turnConfig.Username != "" && turnConfig.Password != "" {
-		// Already have turnConfig.Username and turnConfig.Password in resp
+		resp.Username = turnConfig.Username
+		resp.Password = turnConfig.Password
 	} else {
 		return util.JSONResponse{
 			Code: 200,
diff --git a/src/github.com/matrix-org/dendrite/common/config/config.go b/src/github.com/matrix-org/dendrite/common/config/config.go
index 1723848e7..82bdc3dca 100644
--- a/src/github.com/matrix-org/dendrite/common/config/config.go
+++ b/src/github.com/matrix-org/dendrite/common/config/config.go
@@ -152,8 +152,9 @@ type Dendrite struct {
 
 	// TURN Server Config
 	TURN struct {
+		// TODO Guest Support
 		// Whether or not guests can request TURN credentials
-		AllowGuests bool `yaml:"turn_allow_guests"`
+		//AllowGuests bool `yaml:"turn_allow_guests"`
 		// How long the authorization should last
 		UserLifetime string `yaml:"turn_user_lifetime"`
 		// The list of TURN URIs to pass to clients
@@ -360,10 +361,20 @@ func (config *Dendrite) check(monolithic bool) error {
 		}
 	}
 
+	checkValidDuration := func(key, value string) {
+		if _, err := time.ParseDuration(config.TURN.UserLifetime); err != nil {
+			problems = append(problems, fmt.Sprintf("invalid duration for config key %q: %s", key, value))
+		}
+	}
+
 	checkNotEmpty("matrix.server_name", string(config.Matrix.ServerName))
 	checkNotEmpty("matrix.private_key", string(config.Matrix.PrivateKeyPath))
 	checkNotZero("matrix.federation_certificates", int64(len(config.Matrix.FederationCertificatePaths)))
 
+	if config.TURN.UserLifetime != "" {
+		checkValidDuration("turn.turn_user_lifetime", config.TURN.UserLifetime)
+	}
+
 	checkNotEmpty("media.base_path", string(config.Media.BasePath))
 	checkPositive("media.max_file_size_bytes", int64(*config.Media.MaxFileSizeBytes))
 	checkPositive("media.max_thumbnail_generators", int64(config.Media.MaxThumbnailGenerators))