Slack App OAuth flow in golang

Question

I'm working on a Slack app that uses Slash Commands. I've been looking for an excuse to learn Go, but the online tour was a little slow paced so I tried jumping in with this project.

I'm looking for advice on everything I can do to make this code as idiomatic as possible (that includes using libraries that might make some of what I'm doing simpler).

I'm also having some trouble figuring out how to split this into separate source files "the go way" since the project is going to get a bit larger than this.

There are 3 endpoints exposed:

"/": Returns "Hello World"

"/setColor": Called by Slack when a user uses the command

"/setColor", currently just returns "Hello World"

"/authenticateSlackTeam": Callback given to Slack to kickoff the OAuth flow, generates token and stores in database

package main

import (
    "encoding/json"
    "fmt"
    "log"
    http "net/http"
    "os"
    "time"

    "github.com/dghubble/sling"
    "github.com/jinzhu/gorm"
    _ "github.com/jinzhu/gorm/dialects/postgres"
)

//Team exported for Gorm?
type team struct {
    ID    string `gorm:"primary_key"`
    Name  string
    Scope string `json:"scope"`
    Token string
}

type slackOauthRequestParams struct {
    ClientID     string `url:"client_id,omitempty"`
    ClientSecret string `url:"client_secret,omitempty"`
    Code         string `url:"code,omitempty"`
}
type slackOauthRequestResponse struct {
    AccessToken string `json:"access_token"`
    Scope       string `json:"scope"`
    TeamName    string `json:"team_name"`
    TeamID      string `json:"team_id"`
}

func sayHelloWorld(writer http.ResponseWriter, request *http.Request) {
    log.Print("Request Received: ", request)
    fmt.Fprintf(writer, "Hello World!")
}
func setColor(writer http.ResponseWriter, request *http.Request) {
    log.Print("Set Color Request Received: ", request)
    fmt.Fprintf(writer, "Hello World!")
}

func generateOAuthRequest(code string) (request *http.Request, err error) {
    params := slackOauthRequestParams{
        ClientID:     os.Getenv("SLACK_CLIENT_ID"),
        ClientSecret: os.Getenv("SLACK_CLIENT_SECRET"),
        Code:         code}

    request, err = sling.New().
        Get("https://slack.com/api/oauth.access").
        QueryStruct(params).
        Request()

    return
}
func commitTeam(team *team, database *gorm.DB) {

    transaction := database.Begin()

    teamExists := transaction.First(&team).RowsAffected > 0

    if teamExists {
        log.Print("Updating Token For Authenticated Team: ", team)
        transaction.Save(team)
    } else {
        log.Print("Creating Authenticated Team: ", team)
        transaction.Create(team)
    }

    transaction.Commit()
}

func oAuthResponseAsTeam(response *http.Response) (responseTeam team, err error) {
    decoder := json.NewDecoder(response.Body)

    var oAuthResponse slackOauthRequestResponse

    err = decoder.Decode(&oAuthResponse)
    if err != nil {
        return
    }
    responseTeam = team{Name: oAuthResponse.TeamName,
        ID:    oAuthResponse.TeamID,
        Token: oAuthResponse.AccessToken,
        Scope: oAuthResponse.Scope}
    return
}

func authorizeSlackTeam(db *gorm.DB) (handler func(writer http.ResponseWriter, request *http.Request)) {
    handler = func(writer http.ResponseWriter, request *http.Request) {
        body := request.Body
        defer body.Close()

        code := request.URL.Query().Get("code")
        oAuthRequest, err := generateOAuthRequest(code)
        if err != nil {
            http.Error(writer, fmt.Sprintf("Failed to create OAuth Token request: %v", err), 501)

            errorMessage := fmt.Sprintf("Failed to create OAuth Token request, parameters: %v", request)
            log.Fatal(errorMessage)
            http.Error(writer, errorMessage, 501)
            return
        }
        var client = &http.Client{
            Timeout: time.Second * 10,
        }

        oAuthRequestResponse, err := client.Do(oAuthRequest)

        if err != nil {
            http.Error(writer, fmt.Sprintf("Failed to execute OAuth request, request: %v, error: %v", oAuthRequest, err), 501)
            return
        }
        team, err := oAuthResponseAsTeam(oAuthRequestResponse)
        if err != nil {
            http.Error(writer, fmt.Sprintf("Failed to convert OAuth response to valid team, response: %v, error: %v", oAuthRequestResponse, err), 501)
            return
        }
        commitTeam(&team, db)
    }
    return
}

func openDb() (database *gorm.DB) {
    databaseURL := os.Getenv("DATABASE_URL")
    log.Print("Database URL: " + databaseURL)

    database, err := gorm.Open("postgres", databaseURL)
    if err != nil {
        log.Fatal("Failed to open database: ", err)
    }
    err = database.DB().Ping()
    if err != nil {
        log.Fatal("Failed to ping database after opening: ", err)
    }
    return
}

func main() {

    database, err := gorm.Open("postgres", os.Getenv("DATABASE_URL"))
    defer database.Close()

    database.CreateTable(&team{})
    database.AutoMigrate(&team{})

    port := os.Getenv("PORT")

    if port == "" {
        log.Fatal("$PORT must be set")
        return
    }

    http.HandleFunc("/", sayHelloWorld)
    http.HandleFunc("/setColor", setColor)
    http.HandleFunc("/authorizeSlackTeam", authorizeSlackTeam(database))
    err = http.ListenAndServe(":"+port, nil)
    if err != nil {
        log.Fatal("Server Error", err)
    }

}

Answer 1

Well, when you decide to something as a learning exercise it's normal to tackle just one problem at a time. In your program you have databases, object-relational mapping, JSON encoding, OAuth protocols, closures, REST, and more. It almost makes me a bit sad not to see slices, channels, and go-routines in there. What happened to the good-old FizzBuzz?

Your ambition here is admirable, and I understand the motivation to accomplish something meaningful (a cool slack-bot is awesome), but I worry that you'll maybe miss some details in your haste.

General

In general, your code looks like it has been through the wringer a few times, and it has wrinkles, and a few worn spots. While it looks like you are already in the habit of running your code through the go fmt routines, you should also add go vet and golint to your tools. You have a few issues I can see off-hand:

• there's no need to rename the import net/http to http... that's what it's called already.

• the comments on team are out of date. It's no longer exported (no longer capital-T Team.

• you have mis-uses of log.Fatal(...). That call really is fatal, it calls os.Exit(1) when done. No code can run after the log.Fatal() so the following lines are redundant:

      errorMessage := fmt.Sprintf("Failed to create OAuth Token request, parameters: %v", request)
      log.Fatal(errorMessage)
      http.Error(writer, errorMessage, 501)
      return
  

The httpError(...) is never sent.

There are some places where error checking is incomplete:

database, err := gorm.Open("postgres", os.Getenv("DATABASE_URL"))
defer database.Close()

The authorizeSlackTeam method's handler function captures the body of the request, but does not use it. If you are simply trying to force the close of the body at the function end (and not the beginning), you can replace:

    body := request.Body
    defer body.Close()

with:

    defer request.Body.Close()

In that same area, you have the function declaration:

 func authorizeSlackTeam(db *gorm.DB) (handler func(writer http.ResponseWriter, request *http.Request)) {

but there's no need to name the writer and `request. It can be just:

 func authorizeSlackTeam(db *gorm.DB) (handler func(http.ResponseWriter,  *http.Request)) {

but even better, you can reuse the http.HandlerFunc type to simplify it to:

func authorizeSlackTeam(db *gorm.DB) (handler http.HandlerFunc) {

Despite the above issues, in general, the code is good. The variable names are descriptive, the function names too. The code does flow quite well, and I don't feel like the functions are doing too much.

http errors

It's common in http-handling code to have a helper function to allow child methods to return an error, and then you catch that error and return a consistent, reused, error message system.

Consider a function like:

type UncheckedHandler func(http.ResponseWriter, *http.Request) error

func checkedHandler(handler UncheckedHandler) http.HandlerFunc {

    return func(writer http.ResponseWriter, request *http.Request) {
        if err := handler(writer, request) err != nil {
            log.Printf(err)
            http.Error(writer, err.Error(), 501)
        }
    }

}

Now you can have your own set of handlers that have an error return value, and you can simplify the error-handling code that you have scattered over the place. The setup routine changes from:

http.HandleFunc("/", sayHelloWorld)
http.HandleFunc("/setColor", setColor)
http.HandleFunc("/authorizeSlackTeam", authorizeSlackTeam(database))

to

http.HandleFunc("/", checkedHandler(sayHelloWorld))
http.HandleFunc("/setColor", checkedHandler(setColor))
http.HandleFunc("/authorizeSlackTeam", checkedHandler(authorizeSlackTeam(database)))