HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

Go cookie authentication system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
systemcookieauthentication

Problem

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 th

Solution

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
}

Code Snippets

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

return true
_, exists := sessions[cookie.Value]
return exixts
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
}
func (r *Request) Cookie(name string) (*Cookie, error) {
    for _, c := range readCookies(r.Header, name) {
        return c, nil
    }
    return nil, ErrNoCookie
}
func sessionExists(req *http.Request) (exists bool) {
    cookie, err := req.Cookie("id")
    if err == nil {
        _, exists = sessions[cookie.Value]
    }

    return
}

Context

StackExchange Code Review Q#74446, answer score: 2

Revisions (0)

No revisions yet.