Go cookie authentication system

Question

I'm working on a login system in Go (Golang). Previously, I was using Gorilla Sessions for this, but I wanted to see if I could reinvent the wheel and make it simpler.

Also, I don't need to store many user values—just one: whether a user is logged in or not. For this, I decided to use a map.

sessions := make(map[string]bool)

When a user tries to login, this function gets called.

func tryLogin(username, password string) (http.Cookie, error) {
    if exists := db.UserExists(username, password); !exists {
        return http.Cookie{},
            errors.New("The username or password you entered isn't correct.")
    }

    sid, err := randString(32)
    if err != nil {
         return http.Cookie{}, err
    }

    sessions[sid] = true

    loginCookie := http.Cookie{
        Name:     "id",
        Value:    sid,
        MaxAge:   int((time.Hour * 12).Seconds()),
        HttpOnly: true,
        Domain:   "mydomain.com",
        Path:     "/admin/",
    }

    return loginCookie, nil
}

My randString function, which I use to generate a session ID, just reads random bytes and base64 encodes them.

func randString(size int) (string, error) {
    buf := make([]byte, size)

    if _, err := rand.Read(buf); err != nil {
        log.Println(err)
        return "", errors.New("Couldn't generate random string")
    }

    return base64.URLEncoding.EncodeToString(buf)[:size], nil
}

If a user exists in my database, then they get redirected to an admin area. One of the first things I do on every admin page is call sessionExists to make sure they're still logged in.

func sessionExists(req *http.Request) bool {
    cookie, err := req.Cookie("id")
    if err == http.ErrNoCookie {
        return false
    } else if err != nil {
        log.Println(err)
        return false
    }

    if _, exists := sessions[cookie.Value]; !exists {
        return false
    }

    return true
}

If sessionExists returns false, then I redirect the user to the login page.

I don't currently have HTTPS working, so I can't turn on the secure flag on my login cookie. But besides that, would this design provide adequate protection against intruders?

Answer 1

Concurrent use of map

Quote from Golang FAQ:

Map access is unsafe only when updates are occurring. As long as all goroutines are only reading—looking up elements in the map, including iterating through it using a for range loop—and not changing the map by assigning to elements or doing deletions, it is safe for them to access the map concurrently without synchronization.

Since your functions modify sessions and are running in different routines you need to introduce some locking. Take look at sync package.

Also see question at StackOverflow.

sessionExists function

if _, exists := sessions[cookie.Value]; !exists {
    return false
}

return true

exists already holds the desired bool. Let's simply this part and return exists directly.

_, exists := sessions[cookie.Value]
return exixts

Also you may take advantage of named return values and simplify your function:

func sessionExists(req *http.Request) (exists bool) {
    cookie, err := req.Cookie("id")
    if err == nil {
        _, exists = sessions[cookie.Value]
    } else if err != http.ErrNoCookie {
        log.Println(err)
    }

    return
}

Now let's take a look at func (*Request) Cookie source code:

func (r *Request) Cookie(name string) (*Cookie, error) {
    for _, c := range readCookies(r.Header, name) {
        return c, nil
    }
    return nil, ErrNoCookie
}

The only possible results are c, nil and nil, ErrNoCookie so there is no point in logging errors:

func sessionExists(req *http.Request) (exists bool) {
    cookie, err := req.Cookie("id")
    if err == nil {
        _, exists = sessions[cookie.Value]
    }

    return
}