gorilla / sessions

Package gorilla/sessions provides cookie and filesystem sessions and infrastructure for custom session backends.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
2.89k stars 370 forks source link

Can two different controllers in two different files reference the same session object successfully? #239

Closed darienmiller88 closed 3 years ago

darienmiller88 commented 3 years ago

Describe the problem you're having I am trying to create a sign-in page where I split the functionality between two controllers, a "views controller" that renders the html pages and a "users controller" that acts as the sign-in and sign-up API that responds with JSON to the Javascript acting on the front end. Both controllers reference a global "Store" object of type *sessions.CookieStore in my "session" package, with the user controller adding a new session after the user signs in, and the views controller protecting the html pages from unauthorized users by accessing that same store object to see if the user was found. Unfortunately, after the user is added to the session store in the sign-in route in the user controller, the user is no longer stored in the "Values" map when the views controller tries to access the same session the user controller stored the fuser in. Why does this happen? Adding the same auth code in the user controller that I have in the views controller results in it finding both the session and user, but in the views controller, it finds neither. …

Versions

Go version: go version go 1.14

"Show me the code!" This is my "session" package that initiates the Store object.

package session

var Store *sessions.CookieStore

func init(){
    authKeyOne := securecookie.GenerateRandomKey(32)

    Store = sessions.NewCookieStore([]byte(authKeyOne))
    Store.Options = &sessions.Options{
        MaxAge:   0,
        HttpOnly: true,
    }

    fmt.Println("initialized")
    gob.Register(models.User{})
}

User model.

type User struct{
    gorm.Model
    Username string `json:"username" gorm:"type:string; unique; not null"`
    Password string `json:"password" gorm:"type:string"`
    Authenticated bool `gorm:"-"`//Exclude column from users table
}

//Example route from my "ViewsController" struct in the "ViewsController.go" file.

func (v *ViewsController) home(res http.ResponseWriter, req *http.Request) {
    newSession, _ := session.Store.Get(req, "cookie-name")
    user, ok := newSession.Values["user"].(models.User)

    if !newSession.IsNew{
        fmt.Println("Existing session")
    }else{
        fmt.Println("new session") //This one gets printed.
    }

    if !user.Authenticated || !ok{
        v.render.JSON(res, http.StatusUnauthorized, user)
        return
    }

    v.render.HTML(res, http.StatusOK, "home", user)

}

//Example route from my "UserController" struct in the "UserController.go" file.

func (u *UserController) signin(res http.ResponseWriter, req *http.Request){
    newSession, _ := session.Store.Get(req, "cookie-name")
    user := models.User{}
    err  := render.DecodeJSON(req.Body, &user)
    password := user.Password

    if err != nil{
        res.WriteHeader(http.StatusBadRequest)
        return
    }

    result := u.db.Where("username = ?", user.Username).First(&user).RowsAffected
    err     = bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(password))

    //If the username or password is incorrect, return an error message.
    if result == 0 || err != nil{
        res.WriteHeader(http.StatusUnauthorized)
        render.JSON(res, req, m{"error_message": "Username or password incorrect. Please try again."})
        return
    }

    user.Authenticated = true
    newSession.Values["user"] = user
        err = newSession.Save(req, res)

    if err != nil{
        http.Error(res, err.Error(), http.StatusInternalServerError)
        return
    }

    render.JSON(res, req, m{})
}

Here is the glue code in main.go. For reference, I am using the chi router.

router.Mount("/", viewsController.Router)
router.Mount("/api/users", userController.Router)

Front end sign in code just in case.

const API_URL = `http://localhost:7000/api/users/signin`
const response = await fetch(API_URL, {
        method: "POST",
        body: JSON.stringify(userData),
        headers: {
            "Content-type": "application/json"
        }
    })

    const result = await response.json()

    if(result["error_message"]){
        errorMessageBodyDiv.style.display = ''
        errorMessageDiv.innerHTML = result["error_message"]
        return
    }

    console.log(result)
    signInForm.reset()
    window.location.href = "/"

When the user signs in on the front end, the information is validated on the "signin" method, it returns back to the front end, and the Javascript then reroutes from the signin page to the home page, which then triggers the "home" method. However, the user (and any other key-value pair for that matter) that was added to the "Values" map is no longer there, as the session retrieved is a new one, not the existing one created when the user signed. Why does this happen, and is there any way to preserve sessions across different files?

elithrar commented 3 years ago

Two quick observations:

• you don’t appear to save the session in the first controller • you generate a random key at startup, which means all previously issued cookies cannot be decoded when you restart your app during development

darienmiller88 commented 3 years ago

Ahh, I forgot to add the first part to my original question. And I didn't realize that the random key was problematic in that way, thanks. Changing this results in the ViewsController referencing an existing session created in the UserController instead of a new one previously, but all of the values added to the "Values" map by that same Controller are still not there for whatever reason. I have added an "isAuth" method to both the ViewsController and UserController to check to see the users that were added the "Values" map :

func  isAuth(res http.ResponseWriter, req *http.Request){
    newSession, _ := session.Store.Get(req, "cookie-name")
     user, ok := newSession.Values["user"].(models.User)

    if !user.Authenticated || !ok{
        newSession.Save(req, res)
        res.WriteHeader(http.StatusUnauthorized)
        render.JSON(res, req,  m{"error": "not authorized})
        return
    }

    fmt.Println("values:", newSession.Values, "and user", user)//Prints expected values in the UserController, but not for the ViewsController
    render.JSON(res, req, user)
}

When this method is hit in the UserController, it returns session values as expected, but when the same method is hit in the ViewsController, the session values map is still empty.

darienmiller88 commented 3 years ago

Okay, so I noticed that when session values are saved on a particular path prefix, they can ONLY be accessed on those path prefixes, and not anywhere else. I'm not sure if that's by design, or if there's something I have to change in my code to give every path access to the users in the session, but I was considering wrapping the *sessions.CookieStore object in a struct with a map[interface{}]interface{} field as an accompanying field to store the session values to allow for persistence.

elithrar commented 3 years ago

That's how cookies work: they are only sent when the (domain, path, secure) tuple match - MDN - https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#define_where_cookies_are_sent

If you wrap the CookieStore in a global map, it won't be concurrency safe. Have you considered using a request context to share values during a single request's lifecycle?

darienmiller88 commented 3 years ago

Sorry for the absence, I ran into a few hurdles along the way. So in my "session" package where I have my "Store" object of type *sessions.CookieStore, I also have a "Ctx" object of type context.Context which both my ViewsController and UserController access, with the UserController sending user data into the context, and the ViewsController accessing them to render the pages using that data. Is this way concurrency safe?

elithrar commented 3 years ago

If the context is only on the same request, then the context is safe within that request. If the context is NOT request scoped and you're passing the same data around across requests, it will not be concurrency safe.

On Mon, May 10, 2021 at 11:21 AM darienmiller88 @.***> wrote:

Sorry for the absence, I ran into a few hurdles along the way. So in my "session" package where I have my "Store" object of type *sessions.CookieStore, I also have a "Ctx" object of type context.Context which both my ViewsController and UserController access, with the UserController sending user data into the context, and the ViewsController accessing them to render the pages using that data. Is this way concurrency safe?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/gorilla/sessions/issues/239#issuecomment-837091089, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEQ4FRHFUIHW64FWFILP3TNAP4DANCNFSM44DKMNFQ .

darienmiller88 commented 3 years ago

I'm storing values in a global context object in my session package, and I have both my Views and User controllers store and retrieve values from it, so I appear to be doing the latter. The challenge I'm having then is how to I send a request scope context from one controller to another? On the front end, after the user signs in, Javascript sends the data to my UserController via fetch request, which is then intercepted by a middleware that parses the data into a User object, which is then put into a request scoped context and sent to the /api/users/signin route. That route then stores the user in a session, and sends JSON back to the frontend, which then reroutes to the / route. I'm not sure how to restructure my code to allow requests scoped contexts to be sent from one controller serving one path prefix to another controller serving another. Is there a particular model or pattern I have to follow?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.