1
\$\begingroup\$

I'm developing a Go REST service that uses JWT (JSON Web Tokens) for authentication.

I've written a JWTHandler which validates the token using a Validation Handler. If validation succeeds, the request is passed on to a Token Handler. If the request fails, it is passed to an Error handler. All three of these handlers are public for the sake of flexibility but I've provided default implementations for Validation and Error handlers.

jwthandler.go

package handlers

import(
    "net/http"
    "github.com/dgrijalva/jwt-go"
    "fmt"
)

type JWTTokenFunc func(http.ResponseWriter,*http.Request, *jwt.Token) error
type JWTErrorFunc func(http.ResponseWriter,error)
type JWTValidationFunc func(*jwt.Token) error


type JWTRequestHandler struct{
    Secret []byte
    Param string
    ErrorHandler JWTErrorFunc
    TokenHandler JWTTokenFunc
    ValidationHandler JWTValidationFunc
}

func QuickJWT(param string,secret []byte,tokenHandler JWTTokenFunc) *JWTRequestHandler{

    return &JWTRequestHandler{
        Secret: secret,
        Param: param,
        ErrorHandler: DefaultErrorHandler,
        TokenHandler: tokenHandler,
        ValidationHandler: HMACValidationHandler,
    }
}

func (handler * JWTRequestHandler) ServeHTTP(w http.ResponseWriter, req *http.Request){

    tokenString := req.FormValue(handler.Param)

    if len(tokenString) == 0{
        fmt.Fprintf(w,fmt.Sprintf("The request does not contain a parameter named '%s'. This parameter is expected to contain the JWT.",http.StatusInternalServerError))
    }

    decodeToken,err := jwt.Parse(tokenString,func(token *jwt.Token)(interface{},error){

        result := handler.Secret
        //How to ensure that validation function has been provided.
        validError := handler.ValidationHandler(token)

        return result,validError
    })


    if err == nil{
        err = handler.TokenHandler(w,req,decodeToken)
    }

    if err != nil{
        handler.ErrorHandler(w,err)
    }
}

var DefaultErrorHandler = func(w http.ResponseWriter, err error){

    if ve,ok := err.(*jwt.ValidationError); ok{

        if ve.Errors & jwt.ValidationErrorMalformed != 0 {

            fmt.Fprintf(w,"%d - Malformed Token",http.StatusBadRequest)

        } else if ve.Errors & jwt.ValidationErrorExpired != 0 {

            fmt.Fprintf(w,"%d - Token Expired",http.StatusBadRequest)

        } else if ve.Errors & jwt.ValidationErrorNotValidYet != 0{

            fmt.Fprintf(w,"%d - Token used before valid time",http.StatusBadRequest)

        }else{

            fmt.Fprintf(w,"%d - %s",http.StatusInternalServerError,ve.Error())

        }

    }else{

        fmt.Fprintf(w,err.Error(),http.StatusInternalServerError)

    }


}

var HMACValidationHandler = func(token *jwt.Token) error{

    if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
            return fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])
    }

    if token.Claims["exp"] == nil{
        return fmt.Errorf("Token does not contain 'exp' claim. Required for security.")
    }

    if token.Claims["jti"] == nil{
        return fmt.Errorf("Token does not contain 'jti' claim. Required for security.")
    }

    return nil
}

Here's how I'm using the handler

services.go

package api

import (
    //...
    "bitbucket.org/wks/toolkit/handlers"
)

func jwtHandlerFactory(handler func (http.ResponseWriter,*http.Request,*jwt.Token) error) *handlers.JWTRequestHandler{

    return handlers.QuickJWT(
        "token",//parameter name
        []byte("secret"),//secret
        handler,
    )
}

func GetJWTCategories(dm *datastore.Manager, w http.ResponseWriter, req *http.Request, r render.Render){

    jwtHandlerFactory(func (w http.ResponseWriter,req *http.Request,token *jwt.Token) error{

        format := token.Claims["format"]
        limit, err := strconv.Atoi(token.Claims["limit"].(string))

        if err != nil { return err }

        categories, err := dm.CategoryStore.GetAll(req, limit)

        if err != nil { return err }

        if format == FormatJSON {
            r.JSON(200, categories)
        } else {
            r.XML(200, categories)
        }

        return nil


    }).ServeHTTP(w,req)
}

index.go

package main

func init() {

    m := martini.Classic()
    m.Use(render.Renderer(render.Options{
        IndentJSON: true,
        IndentXML:  true,
        Charset:    "UTF-8",
    }))

    m.Get("/jwt/categories", api.GetJWTCategories)


    http.Handle("/", m)
}

I feel that the code can be written in a far more elegant way. Specifically:

  1. In GetJWTCategories, I don't like how I'm using a factory to create a dynamic handler for every request. Is there a way that the same thing can be written without the factory method.

  2. There are just way too many if-else conditions for the sake of error handling and I just can't figure out how to clean it up. The design of JWT Handler was inspired by the Go blogpost on 'Effective Error Handling in Go', but I just can't get my head to think that way.

\$\endgroup\$
  • \$\begingroup\$ ` //...` - don't exclude code! Best is if the code is actually runnable as is :) \$\endgroup\$ – jacwah Oct 8 '15 at 12:40
1
\$\begingroup\$

Formatting

First of all, Go has an official coding style. Use go fmt or IDE tools to format your code! If you happen to be using Vim I recommend the vim-go plugin.

I would additionally consider it more readable if the code uses newlines more sparingly, only to separate logically disparate sets of lines.

type JWTRequestHandler struct {
        Secret            []byte
        Param             string
        ErrorHandler      JWTErrorFunc
        TokenHandler      JWTTokenFunc
        ValidationHandler JWTValidationFunc
}

} else {
    fmt.Fprintf(w, "%d - %s", http.StatusInternalServerError, ve.Error())
}

Etc...

Factory

I guess you don't want to create a new instance for each request because you don't want to strain the garbage collector.

The reason you have to use the factory pattern in your current design is that you're forcing JWTRequestHandler to conform to the http.Handler interface. There's no need to conform to this interface as you're just calling it yourself.

Instead of your very object-centric approach, you could transform the handler function from and instance variable to a function argument. Keep the non-variable fields as is.

func (handler * JWTRequestHandler) HandleRequest(w http.ResponseWriter,
    req *http.Request,
    handler func (http.ResponseWriter, *http.Request, *jwt.Token))

You can collapse other pieces, e.g. the handler closure, in a similar way.

Magic numbers and strings

"token",//parameter name
[]byte("secret"),//secret

I guess you've hardcoded the real values here in your source code. Don't do this. This SO question has some discussion of good ways to handle secrets on App Engine. The basic idea is to store it in a separate file not checked into source control (you're using source control, right?).

r.JSON(200, categories)

Just use http.StatusOK. It's much easier for someone skimming the code to understand.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.