Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] update session expire date use *Session not only *Sessions #1485

Closed
huyinghuan opened this issue Apr 11, 2020 · 8 comments
Closed

Comments

@huyinghuan
Copy link

Is your feature request related to a problem? Please describe.

here is "remember me" checkbox in my sign in page , at the most time, i use config
set the cookie expire date:

session := sessions.New(sessions.Config{
		Cookie: "xxxx",
		Expires: 45 * time.Minute,
})

app.Use(session.Handler())

if user checked "remember me", i hope I can set cookie expire date use other date.

Describe the solution you'd like

sess = sessions.Get(ctx)
sess.LifeTime.Add(24 * time.Hour)
sess.ShiftExpiration()
@huyinghuan huyinghuan changed the title [FEATURE REQUEST] update session expire date use *Session not *Sessions [FEATURE REQUEST] update session expire date use *Session not only *Sessions Apr 11, 2020
@kataras
Copy link
Owner

kataras commented Apr 11, 2020

Hello @huyinghuan,

The Session.LifeTime.Shift(time.Duration) does not work for you? It should do exactly that you want to do, is there any particular reason for asking a new method to be added?

Thanks,
Gerasimos Maropoulos

@huyinghuan
Copy link
Author

@kataras I use LiteTime.Shift , but expire date in chrome browser cookie no change.

my code is

...
	session                = sessions.New(sessions.Config{
		Cookie: cookieNameForSessionID,
		Expires: 30 * time.Minute, // <=0 means unlimited life
	})
....

 app.Party("/api", session.Handler(), middleware.UserAuth)

.....

ctx.StatusCode(200)
		if account.Remember {
			//d := sess.Lifetime.Add()
			sess.Lifetime.Shift(24 * time.Hour)
			ctx.WriteString("expire date should add 24 hours")
		}else{
			ctx.WriteString("expire date should  be 30 minutes")
		}



result:

image

@kataras
Copy link
Owner

kataras commented Apr 12, 2020

Ah I see, it applies the new time for expiration at the database(or memory) storage but not in the cookie, let me see what I can do. I am sorry, it's not just documented but you can do it by ctx.UpdateExpiration I must add it to the documentation.

Did you try the following?

// sess.Lifetime.Shift(24 * time.Hour)
// session.UpdateExpiration(ctx,  sess.Lifetime.DurationUntilExpiration())

// OR just
session.UpdateExpiration(ctx, 24* time.hour)

The following test passes:

func TestSessionsUpdateExpiration(t *testing.T) {
	app := iris.New()

	cookieName := "mycustomsessionid"

	sess := sessions.New(sessions.Config{
		Cookie:  cookieName,
		Expires: 30 * time.Minute,
	})

	app.Use(sess.Handler())

	type response struct {
		SessionID string `json:"sessionID"`
		Logged    bool   `json:"logged"`
	}

	var writeResponse = func(ctx context.Context) {
		session := sessions.Get(ctx)
		ctx.JSON(response{
			SessionID: session.ID(),
			Logged:    session.GetBooleanDefault("logged", false),
		})
	}

	app.Get("/get", func(ctx context.Context) {
		writeResponse(ctx)
	})

	app.Get("/set", func(ctx iris.Context) {
		sessions.Get(ctx).Set("logged", true)
		writeResponse(ctx)
	})

	app.Post("/remember_me", func(ctx iris.Context) {
		// re-sends the cookie with the new Expires and MaxAge fields,
		// test checks that on same session id too.
		sess.UpdateExpiration(ctx, 24*time.Hour)
		writeResponse(ctx)
	})

	e := httptest.New(t, app, httptest.URL("http://example.com"))

	tt := e.GET("/set").Expect().Status(httptest.StatusOK)
	tt.Cookie(cookieName).MaxAge().Equal(30 * time.Minute)
	sessionID := tt.JSON().Object().Raw()["sessionID"].(string)

	expectedResponse := response{SessionID: sessionID, Logged: true}
	e.GET("/get").Expect().Status(httptest.StatusOK).
		JSON().Equal(expectedResponse)

	tt = e.POST("/remember_me").Expect().Status(httptest.StatusOK)
	tt.Cookie(cookieName).MaxAge().Equal(24 * time.Hour)
	tt.JSON().Equal(expectedResponse)
}

@huyinghuan
Copy link
Author

huyinghuan commented Apr 13, 2020

TestFirstPostWillFail canot pass test

import (
  "os"
  "testing"
  "time"

  "github.com/kataras/iris/v12"
  "github.com/kataras/iris/v12/httptest"
  "github.com/kataras/iris/v12/sessions"
)

var app *iris.Application
var cookieName = "mycustomsessionid"


func TestMain(m *testing.M){
  sess := sessions.New(sessions.Config{
    Cookie:  cookieName,
    Expires: 30 * time.Minute,
  })

  app = iris.New()

  app.Use(sess.Handler())

  app.Get("/get", func(ctx iris.Context) {
    ctx.Writef("%v", sessions.Get(ctx).GetBooleanDefault("logged", false))
  })

  app.Get("/normal", func(ctx iris.Context) {
    sessions.Get(ctx).Set("logged", true)
    ctx.WriteString("OK")
  })


  app.Get("/cleanCookie", func(ctx iris.Context) {
    sessions.Get(ctx).Clear()
    sessions.Get(ctx).ClearFlashes()
    sessions.Get(ctx).Destroy()
    ctx.WriteString("OK")
  })

  app.Get("/remember_me", func(ctx iris.Context) {
    sessions.Get(ctx).Set("logged", true)
    sess.UpdateExpiration(ctx, 24*time.Hour)
    ctx.WriteString("OK")
  })

  app.Post("/post/remember_me", func(ctx iris.Context) {
    sessions.Get(ctx).Set("logged", true)
    sess.UpdateExpiration(ctx, 48*time.Hour)
    ctx.WriteString("OK")
  })
  exit := m.Run()
  os.Exit(exit)
}

func TestFirstPostWillFail(t *testing.T){
  e := httptest.New(t, app, httptest.URL("http://example.com"))
  t.Run("Post Set expire", func(t *testing.T){
    tt := e.POST("/post/remember_me").Expect().Status(httptest.StatusOK)
    tt.Cookie(cookieName).MaxAge().Equal(48 * time.Hour - time.Second)
    tt.Body().Equal("OK")
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("true")
  })
}

func TestSessionsUpdateExpiration(t *testing.T) {
  e := httptest.New(t, app, httptest.URL("http://example.com"))
  t.Run("normal", func(t *testing.T) {
    tt := e.GET("/normal").Expect().Status(httptest.StatusOK)
    tt.Cookie(cookieName).MaxAge().Equal(30 * time.Minute - time.Second)
    tt.Body().Equal("OK")
    // e.GET("/set").Expect().Status(httptest.StatusOK).Body().Equal("OK")
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("true")
  })
  t.Run("clean", func(t *testing.T) {
    // clean
    e.GET("/cleanCookie").Expect().Status(httptest.StatusOK)
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("false")
  })

  t.Run("Get Set expire", func(t *testing.T) {
    tt := e.GET("/remember_me").Expect().Status(httptest.StatusOK)
    tt.Cookie(cookieName).MaxAge().Equal(24 * time.Hour - time.Second)
    tt.Body().Equal("OK")
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("true")
  })
  t.Run("clean", func(t *testing.T) {
    // clean
    e.GET("/cleanCookie").Expect().Status(httptest.StatusOK)
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("false")
  })

  t.Run("Post Set expire", func(t *testing.T){
    tt := e.POST("/post/remember_me").Expect().Status(httptest.StatusOK)
    tt.Cookie(cookieName).MaxAge().Equal(48 * time.Hour - time.Second)
    tt.Body().Equal("OK")
    e.GET("/get").Expect().Status(httptest.StatusOK).Body().Equal("true")
  })
}

image

@kataras
Copy link
Owner

kataras commented Apr 13, 2020

Hello @huyinghuan, let me explain the issue here.

The TestFirstPostWillFail test is failing but the /Post_Set_expire subtest passes, the problem is not the POST HTTP verb, even if you change it to GET you will get the same result. The issue here is that: when the session does not exist, the session manager will allocate a new session and send a cookie based on the configured Expires field to the client, so the UpdateExpiration with the new expiration value will not be respected because:

  1. the UpdateExpiration will try to see if a cookie exists on the ctx.Request() and it sees nothing - this can be solved by setting the sessions.AllowReclaim field to true.
  2. Even if 1 solved by AllowReclaim: true it will call the ctx.SetCookie and this will call the http.SetCookie which internally does: w.Header().Add("Set-Cookie", v) which means that it adds a cookie to the "Set-Cookie" response header.

The 1. can be fixed by:

  sess := sessions.New(sessions.Config{
    Cookie:  cookieName,
    Expires: 30 * time.Minute,
    AllowReclaim: true, // <---HERE
  })

The 2. can be fixed by modifying the following code:

iris/sessions/cookie.go

Lines 35 to 40 in b6ac394

func AddCookie(ctx context.Context, cookie *http.Cookie, reclaim bool) {
if reclaim {
ctx.Request().AddCookie(cookie)
}
ctx.SetCookie(cookie)
}

to:

// UpsertCookie adds a cookie to the response like `SetCookie` does
// but it will also perform a replacement of the cookie
// if already set by a previous `SetCookie` call.
// It reports whether the cookie is new (true) or an existing one was updated (false).
func (ctx *context) UpsertCookie(cookie *http.Cookie, options ...CookieOption) bool {
	for _, opt := range options {
		opt(cookie)
	}

	header := ctx.ResponseWriter().Header()

	if cookies := header["Set-Cookie"]; len(cookies) > 0 {
		s := cookie.Name + "=" // name=?value
		for i, c := range cookies {
			if strings.HasPrefix(c, s) {
				// We need to update the Set-Cookie (to update the expiration or any other cookie's properties).
				// Probably the cookie is set and then updated in the first session creation
				// (e.g. UpdateExpiration, see https://github.com/kataras/iris/issues/1485).
				cookies[i] = cookie.String()
				header["Set-Cookie"] = cookies
				return false
			}
		}
	}

	header.Add("Set-Cookie", cookie.String())
	return true
}
func AddCookie(ctx context.Context, cookie *http.Cookie, reclaim bool) {
	if reclaim {
		ctx.Request().AddCookie(cookie)
	}

	ctx.UpsertCookie(cookie)
}

I think this is the problem you are facing in your application?

@kataras
Copy link
Owner

kataras commented Apr 13, 2020

Here is the commit which fixes this issue: c61d047. It will be available on the upcoming v12.2.0 release.

@huyinghuan
Copy link
Author

thanks for your suggestion ! It's solved by use config AllowReclaim: true

@kataras
Copy link
Owner

kataras commented Apr 13, 2020

Good @huyinghuan! I've also added a new Session.Man field to have access to the sessions manager itself through, e.g. sessions.Get(ctx).Man.Destroy().

kataras added a commit that referenced this issue Jul 26, 2020
…of 'context.SetCookie'

Former-commit-id: 31a50e580929616504b9bbbb1d602b0e9274a568
kataras added a commit that referenced this issue Jul 26, 2020
relative to: #1485


Former-commit-id: c4ced38b74af42bfcd17abe6b439b35db6837bbf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants