Add descriptive comments, fix bugs.

Signed-off-by: Anant Prakash <anantprakashjsr@gmail.com>

src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/loginrequest.go

@@ -1,3 +1,5 @@
+// Copyright 2018 New Vector Ltd
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at

src/github.com/matrix-org/dendrite/clientapi/auth/authtypes/loginresponse.go

@@ -1,3 +1,5 @@
+// Copyright 2018 New Vector Ltd
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at

src/github.com/matrix-org/dendrite/clientapi/auth/tokens/tokens.go

@@ -1,4 +1,4 @@
-// Copyright 2018 Vector Creations Ltd
+// Copyright 2018 New Vector Ltd
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -26,11 +26,11 @@ import (
 const (
 	macaroonVersion = macaroon.V2
 	defaultDuration = 2 * 60
-	// UserPrefix for user_id caveat
+	// UserPrefix is common prefix for every user_id caveat
 	UserPrefix = "user_id = "
-	// TimePrefix for expiry caveat
+	// TimePrefix is common prefix for every expiry caveat
 	TimePrefix = "time < "
-	// Gen represents an unique identifier used as a caveat
+	// Gen is a common caveat for every token
 	Gen = "gen = 1"
 )
 
@@ -61,14 +61,17 @@ func GenerateLoginToken(op TokenOptions) (string, error) {
 	if err != nil {
 		return "", macaroonError(err)
 	}
+	urlSafeEncode, err := SerializeMacaroon(*mac)
 
-	urlSafeEncode, err := serializeMacaroon(*mac)
-	return urlSafeEncode, macaroonError(err)
+	if err != nil {
+		return "", macaroonError(err)
+	}
+	return urlSafeEncode, nil
 }
 
-// generateBaseMacaroon generates a base macaroon common for accessToken & loginToken
+// generateBaseMacaroon generates a base macaroon common for accessToken & loginToken.
 // Returns a macaroon tied with userID,
-// returns formated error if something goes wrong.
+// returns an error if something goes wrong.
 func generateBaseMacaroon(
 	secret []byte, ServerName string, userID string,
 ) (*macaroon.Macaroon, error) {
@@ -94,9 +97,9 @@ func macaroonError(err error) error {
 	return fmt.Errorf("Macaroon creation failed: %s", err.Error())
 }
 
-// serializeMacaroon takes a macaroon to be serialized.
-// returns a base64 encoded string, URL safe, can be sent via web, email, etc.
-func serializeMacaroon(m macaroon.Macaroon) (string, error) {
+// SerializeMacaroon takes a macaroon to be serialized.
+// returns it's base64 encoded string, URL safe, which can be sent via web, email, etc.
+func SerializeMacaroon(m macaroon.Macaroon) (string, error) {
 	bin, err := m.MarshalBinary()
 
 	if err != nil {
@@ -107,19 +110,16 @@ func serializeMacaroon(m macaroon.Macaroon) (string, error) {
 	return urlSafeEncode, nil
 }
 
-// deserializeMacaroon takes a base64 encoded string to deserialized.
+// DeSerializeMacaroon takes a base64 encoded string (of a macaroon, hopefully) to de-serialized.
 // Returns a macaroon. On failure returns error with description.
-func deserializeMacaroon(urlSafeEncode string) (*macaroon.Macaroon, error) {
+func DeSerializeMacaroon(urlSafeEncode string) (*macaroon.Macaroon, error) {
 	bin, err := base64.RawURLEncoding.DecodeString(urlSafeEncode)
 
 	if err != nil {
 		return nil, err
 	}
-	var mac *macaroon.Macaroon
+	var mac macaroon.Macaroon
 	err = mac.UnmarshalBinary(bin)
 
-	if err != nil {
-		return nil, err
-	}
-	return mac, nil
+	return &mac, err
 }

src/github.com/matrix-org/dendrite/clientapi/auth/tokens/tokens_handler.go

@@ -1,4 +1,4 @@
-// Copyright 2018 Vector Creations Ltd
+// Copyright 2018 New Vector Ltd
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -19,95 +19,93 @@ import (
 	"strconv"
 	"strings"
 	"time"
-
-	"github.com/matrix-org/dendrite/clientapi/jsonerror"
-	"github.com/matrix-org/util"
 )
 
 // GetUserFromToken returns the user associated with the token
 // Returns the error if something goes wrong.
 // Warning: Does not validate the token. Use ValidateToken for that.
 func GetUserFromToken(token string) (user string, err error) {
-	mac, err := deserializeMacaroon(token)
+	mac, err := DeSerializeMacaroon(token)
 
 	if err != nil {
 		return
 	}
+
+	// ID() returns a []byte, so convert it to string
 	user = string(mac.Id()[:])
 	return
 }
 
 // ValidateToken validates that the Token is understood and was signed by this server.
-// Returns nil if token is valid, otherwise returns a error response which can be sent to client.
-func ValidateToken(op TokenOptions, token string) *util.JSONResponse {
-	macaroon, err := deserializeMacaroon(token)
+// Returns nil if token is valid, otherwise returns an error/
+func ValidateToken(op TokenOptions, token string) error {
+	macaroon, err := DeSerializeMacaroon(token)
 
 	if err != nil {
-		return &util.JSONResponse{
-			Code: 401,
-			JSON: jsonerror.UnknownToken("Token does not represent a valid macaroon"),
-		}
+		return errors.New("Token does not represent a valid macaroon")
 	}
 
+	// VerifySignature returns all caveats in the macaroon
 	caveats, err := macaroon.VerifySignature(op.ServerMacaroonSecret, nil)
 
 	if err != nil {
-		return &util.JSONResponse{
-			Code: 401,
-			JSON: jsonerror.UnknownToken("Provided token was not issued by this server"),
-		}
+		return errors.New("Provided token was not issued by this server")
 	}
 
 	err = verifyCaveats(caveats, op.UserID)
 
 	if err != nil {
-		return &util.JSONResponse{
-			Code: 403,
-			JSON: jsonerror.Forbidden("Provided token not authorized"),
-		}
+		return errors.New("Provided token not authorized")
 	}
 	return nil
 }
 
-// verifyCaveats verifies caveats associated with a login token macaroon.
-// which are "gen = 1", "user_id = ...", "time < ..."
-// Returns nil on successful verification, else returns the error.
+// verifyCaveats verifies caveats associated with a login macaroon token.
+// Which are "gen = 1", "user_id = ...", "time < ..."
+// Returns nil on successful verification, else returns an error.
 func verifyCaveats(caveats []string, userID string) error {
-	// let last 4 bit be Uabc where,
-	// U: unknownCaveat
-	// a, b, c: 1st, 2nd & 3rd caveat to be verified respectively.
+	// verified is a bitmask
+	// let last 4 bit be uabc where,
+	// u denotes any unknownCaveat
+	// a, b, c denotes the three caveats to be verified respectively.
 	var verified uint8
 	now := time.Now().Second()
 
 	for _, caveat := range caveats {
 		switch {
 		case caveat == Gen:
+			// "gen = 1"
 			verified |= 1
 		case strings.HasPrefix(caveat, UserPrefix):
+			// "user_id = ..."
 			if caveat[len(UserPrefix):] == userID {
 				verified |= 2
 			}
 		case strings.HasPrefix(caveat, TimePrefix):
+			// "time < ..."
 			if verifyExpiry(caveat[len(TimePrefix):], now) {
 				verified |= 4
 			}
 		default:
+			// Unknown caveat
 			verified |= 8
-			break
 		}
 	}
 
-	// Check that all three caveats are verified and no extra caveats
-	// i.e. Uabc == 0111
-	if verified == 7 {
-		return nil
-	} else if verified >= 8 {
-		return errors.New("Unknown caveat present")
+	// Check that all three caveats are verified and no extra caveats are present
+	// i.e. uabc == 0111, which implies verified == 7
+	if verified != 7 {
+		if verified >= 8 {
+			return errors.New("Unknown caveat present")
+		}
+
+		return errors.New("Required caveats not present")
 	}
 
-	return errors.New("Required caveats not present")
+	return nil
 }
 
+// verifyExpiry verifies an expiry caveat
 func verifyExpiry(t string, now int) bool {
 	expiry, err := strconv.Atoi(t)
 

src/github.com/matrix-org/dendrite/clientapi/routing/login.go

@@ -141,22 +141,6 @@ func handlePasswordLogin(
 		}
 	}
 
-	var token string
-	token, err = tokens.GenerateLoginToken(tokens.TokenOptions{
-		ServerMacaroonSecret: []byte(string(cfg.Matrix.PrivateKey)),
-		ServerName:           string(cfg.Matrix.ServerName),
-		UserID:               r.User,
-	})
-
-	if err != nil {
-		return util.JSONResponse{
-			Code: http.StatusNotImplemented,
-			JSON: jsonerror.Unknown("Token generation failed"),
-		}
-	}
-
-	util.GetLogger(req.Context()).WithField("user", r.User).Info(token)
-
 	return completeLogin(r, localpart, deviceDB, req, cfg)
 }
 
@@ -206,15 +190,19 @@ func handleTokenLogin(
 	}
 
 	tokenOptions := tokens.TokenOptions{
-		ServerMacaroonSecret: []byte(string(cfg.Matrix.PrivateKey)),
+		// ServerMacaroonSecret should be a []byte
+		ServerMacaroonSecret: []byte(cfg.Matrix.PrivateKey),
 		ServerName:           string(cfg.Matrix.ServerName),
 		UserID:               userID,
 	}
 
-	resErr := tokens.ValidateToken(tokenOptions, r.Token)
+	err := tokens.ValidateToken(tokenOptions, r.Token)
 
-	if resErr != nil {
-		return *resErr
+	if err != nil {
+		return util.JSONResponse{
+			Code: http.StatusUnauthorized,
+			JSON: jsonerror.UnknownToken(err.Error()),
+		}
 	}
 
 	return completeLogin(r, localpart, deviceDB, req, cfg)