diff --git a/app/rest/api/middleware_test.go b/app/rest/api/middleware_test.go index 95420f0b..5554b768 100644 --- a/app/rest/api/middleware_test.go +++ b/app/rest/api/middleware_test.go @@ -46,7 +46,7 @@ func TestMiddleware_GetBodyAndUser(t *testing.T) { assert.Equal(t, "", user, "no user") req = rest.SetUserInfo(req, store.User{ID: "id1", Name: "user1"}) - body, user = getBodyAndUser(req, []LoggerFlag{LogAll}) + _, user = getBodyAndUser(req, []LoggerFlag{LogAll}) assert.Equal(t, ` - id1 "user1"`, user, "no user") body, user = getBodyAndUser(req, nil) diff --git a/app/rest/api/rest.go b/app/rest/api/rest.go index b09a8a06..bb88c64a 100644 --- a/app/rest/api/rest.go +++ b/app/rest/api/rest.go @@ -105,7 +105,7 @@ func (s *Rest) routes() chi.Router { cache: s.Cache, } - ipFn := func(ip string) string { return store.HashValue(ip, s.DataService.Secret)[:12] } + ipFn := func(ip string) string { return store.HashValue(ip, s.DataService.Secret)[:12] } // logger uses it for anonymization // auth routes for all providers router.Route("/auth", func(r chi.Router) { diff --git a/app/rest/auth/jwt.go b/app/rest/auth/jwt.go index b11a3c0a..a4b8664f 100644 --- a/app/rest/auth/jwt.go +++ b/app/rest/auth/jwt.go @@ -24,9 +24,10 @@ type CustomClaims struct { User *store.User `json:"user,omitempty"` // state and from used for oauth handshake - State string `json:"state,omitempty"` - From string `json:"from,omitempty"` - SiteID string `json:"site_id,omitempty"` + State string `json:"state,omitempty"` + From string `json:"from,omitempty"` + SiteID string `json:"site_id,omitempty"` + SessionOnly bool `json:"sess_only,omitempty"` } const jwtCookieName = "JWT" @@ -45,8 +46,8 @@ func NewJWT(secret string, secureCookies bool, exp time.Duration) *JWT { } // Set creates jwt cookie with xsrf cookie and put it to ResponseWriter -// accepts claims and sets expiration if none defined -func (j *JWT) Set(w http.ResponseWriter, claims *CustomClaims) error { +// accepts claims and sets expiration if none defined. permanent flag means long-living cookie, false makes it session only. +func (j *JWT) Set(w http.ResponseWriter, claims *CustomClaims, sessionOnly bool) error { if claims.ExpiresAt == 0 { claims.ExpiresAt = time.Now().Add(j.exp).Unix() } @@ -56,7 +57,10 @@ func (j *JWT) Set(w http.ResponseWriter, claims *CustomClaims) error { return errors.Wrap(err, "can't sign jwt token") } - cookieExpiration := 365 * 24 * 3600 // 1 year + cookieExpiration := 0 // session cookie + if !sessionOnly { + cookieExpiration = 365 * 24 * 3600 // 1 year + } jwtCookie := http.Cookie{Name: jwtCookieName, Value: tokenString, HttpOnly: true, Path: "/", MaxAge: cookieExpiration, Secure: j.secureCookies} @@ -123,7 +127,7 @@ func (j *JWT) Refresh(w http.ResponseWriter, r *http.Request) (*CustomClaims, er untilExp := claims.ExpiresAt - time.Now().Unix() if untilExp <= int64(j.exp.Seconds()/2) { claims.ExpiresAt = time.Now().Add(j.exp).Unix() - e := j.Set(w, claims) + e := j.Set(w, claims, claims.SessionOnly) return claims, e } return claims, nil diff --git a/app/rest/auth/jwt_test.go b/app/rest/auth/jwt_test.go index 995fbc1e..d057c05d 100644 --- a/app/rest/auth/jwt_test.go +++ b/app/rest/auth/jwt_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - jwt "github.com/dgrijalva/jwt-go" + "github.com/dgrijalva/jwt-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,7 +18,12 @@ var testJwtValid = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjI3ODkxOTE4MjI "sImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZS" + "I6IiIsImFkbWluIjpmYWxzZX0sInN0YXRlIjoiMTIzNDU2IiwiZnJvbSI6ImZyb20ifQ._loFgh3g45gr9TtGqvM3N584I_6EHEOJnYb6Py84stQ" -var testJwtExpired = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1MjY4ODc4MjIsImp0aSI6InJhbmRvbSBpZCIs" + "ImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZSI6IiI" + +var testJwtValidSess = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjI3ODkxOTE4MjIsImp0aSI6InJhbmRvbSBpZCIsImlzcyI6In" + + "JlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZSI6IiIsIm" + + "FkbWluIjpmYWxzZX0sInN0YXRlIjoiMTIzNDU2IiwiZnJvbSI6ImZyb20iLCJzZXNzX29ubHkiOnRydWV9.p6w0sM_NYaRuyhyA9jqfWlB5cx1vZPGhXGC5geSX7nA" + +var testJwtExpired = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1MjY4ODc4MjIsImp0aSI6InJhbmRvbSBpZCIs" + + "ImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZSI6IiI" + "sImFkbWluIjpmYWxzZX0sInN0YXRlIjoiMTIzNDU2IiwiZnJvbSI6ImZyb20ifQ.4_dCrY9ihyfZIedz-kZwBTxmxU1a52V7IqeJrOqTzE4" func TestJWT_Set(t *testing.T) { @@ -39,15 +44,29 @@ func TestJWT_Set(t *testing.T) { }, } + claims.SessionOnly = false rr := httptest.NewRecorder() - err := j.Set(rr, claims) + err := j.Set(rr, claims, claims.SessionOnly) assert.Nil(t, err) cookies := rr.Result().Cookies() t.Log(cookies) require.Equal(t, 2, len(cookies)) assert.Equal(t, "JWT", cookies[0].Name) assert.Equal(t, testJwtValid, cookies[0].Value) + assert.Equal(t, 31536000, cookies[0].MaxAge) + assert.Equal(t, "XSRF-TOKEN", cookies[1].Name) + assert.Equal(t, "random id", cookies[1].Value) + claims.SessionOnly = true + rr = httptest.NewRecorder() + err = j.Set(rr, claims, claims.SessionOnly) + assert.Nil(t, err) + cookies = rr.Result().Cookies() + t.Log(cookies) + require.Equal(t, 2, len(cookies)) + assert.Equal(t, "JWT", cookies[0].Name) + assert.Equal(t, testJwtValidSess, cookies[0].Value) + assert.Equal(t, 0, cookies[0].MaxAge) assert.Equal(t, "XSRF-TOKEN", cookies[1].Name) assert.Equal(t, "random id", cookies[1].Value) } @@ -80,8 +99,9 @@ func TestJWT_SetAndGetWithCookies(t *testing.T) { j := NewJWT("xyz 12345", false, time.Hour) claims := &CustomClaims{ - State: "123456", - From: "from", + State: "123456", + From: "from", + SessionOnly: true, User: &store.User{ ID: "id1", Name: "name1", @@ -96,7 +116,7 @@ func TestJWT_SetAndGetWithCookies(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/valid" { - j.Set(w, claims) + j.Set(w, claims, true) w.WriteHeader(200) } })) @@ -113,6 +133,8 @@ func TestJWT_SetAndGetWithCookies(t *testing.T) { assert.Nil(t, err) assert.Equal(t, &store.User{Name: "name1", ID: "id1", Picture: "", Admin: false, Blocked: false, IP: ""}, claims.User) assert.Equal(t, "remark42", claims.Issuer) + assert.Equal(t, true, claims.SessionOnly) + t.Log(resp.Cookies()) } func TestJWT_SetAndGetWithXsrfMismatch(t *testing.T) { @@ -135,7 +157,7 @@ func TestJWT_SetAndGetWithXsrfMismatch(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/valid" { - j.Set(w, claims) + j.Set(w, claims, true) w.WriteHeader(200) } })) @@ -172,7 +194,7 @@ func TestJWT_SetAndGetWithCookiesExpired(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/expired" { - j.Set(w, claims) + j.Set(w, claims, true) w.WriteHeader(200) } })) @@ -207,7 +229,7 @@ func TestJWT_Refresh(t *testing.T) { } // set token rr := httptest.NewRecorder() - err := j.Set(rr, claims) + err := j.Set(rr, claims, true) assert.Nil(t, err) cookies := rr.Result().Cookies() require.Equal(t, 2, len(cookies)) diff --git a/app/rest/auth/provider.go b/app/rest/auth/provider.go index f3532967..b6399832 100644 --- a/app/rest/auth/provider.go +++ b/app/rest/auth/provider.go @@ -80,7 +80,7 @@ func (p Provider) Routes() chi.Router { return router } -// loginHandler - GET /login?from=redirect-back-url&site=siteID +// loginHandler - GET /login?from=redirect-back-url&site=siteID&session=1 func (p Provider) loginHandler(w http.ResponseWriter, r *http.Request) { log.Printf("[DEBUG] login with %s", p.Name) @@ -88,9 +88,10 @@ func (p Provider) loginHandler(w http.ResponseWriter, r *http.Request) { state := p.randToken() claims := CustomClaims{ - State: state, - From: r.URL.Query().Get("from"), - SiteID: r.URL.Query().Get("site"), + State: state, + From: r.URL.Query().Get("from"), + SiteID: r.URL.Query().Get("site"), + SessionOnly: r.URL.Query().Get("session") != "" && r.URL.Query().Get("session") != "0", StandardClaims: jwt.StandardClaims{ Id: p.randToken(), Issuer: "remark42", @@ -99,7 +100,7 @@ func (p Provider) loginHandler(w http.ResponseWriter, r *http.Request) { }, } - if err := p.JwtService.Set(w, &claims); err != nil { + if err := p.JwtService.Set(w, &claims, false); err != nil { rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "failed to set jwt") return } @@ -180,9 +181,10 @@ func (p Provider) authHandler(w http.ResponseWriter, r *http.Request) { Issuer: "remark42", Id: p.randToken(), }, + SessionOnly: oauthClaims.SessionOnly, } - if err = p.JwtService.Set(w, authClaims); err != nil { + if err = p.JwtService.Set(w, authClaims, oauthClaims.SessionOnly); err != nil { rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "failed to save user info") return } diff --git a/app/rest/auth/provider_test.go b/app/rest/auth/provider_test.go index 4f130a94..ae42865b 100644 --- a/app/rest/auth/provider_test.go +++ b/app/rest/auth/provider_test.go @@ -29,6 +29,8 @@ func TestLogin(t *testing.T) { jar, err := cookiejar.New(nil) require.Nil(t, err) client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + // check non-admin, permanent resp, err := client.Get("http://localhost:8981/login?site=remark") require.Nil(t, err) assert.Equal(t, 200, resp.StatusCode) @@ -40,6 +42,7 @@ func TestLogin(t *testing.T) { assert.Equal(t, 2, len(resp.Cookies())) assert.Equal(t, "JWT", resp.Cookies()[0].Name) assert.NotEqual(t, "", resp.Cookies()[0].Value, "jwt set") + assert.Equal(t, 31536000, resp.Cookies()[0].MaxAge) assert.Equal(t, "XSRF-TOKEN", resp.Cookies()[1].Name) assert.NotEqual(t, "", resp.Cookies()[1].Value, "xsrf cookie set") @@ -61,6 +64,42 @@ func TestLogin(t *testing.T) { Admin: true, Blocked: false, IP: "", Verified: true}, u) } +func TestLoginSessionOnly(t *testing.T) { + + ts, ots := mockProvider(t, 8981, 8982) + defer func() { + ts.Close() + ots.Close() + }() + + jar, err := cookiejar.New(nil) + require.Nil(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + + // check non-admin, session + resp, err := client.Get("http://localhost:8981/login?site=remark&session=1") + require.Nil(t, err) + assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, 2, len(resp.Cookies())) + assert.Equal(t, "JWT", resp.Cookies()[0].Name) + assert.NotEqual(t, "", resp.Cookies()[0].Value, "jwt set") + assert.Equal(t, 0, resp.Cookies()[0].MaxAge) + assert.Equal(t, "XSRF-TOKEN", resp.Cookies()[1].Name) + assert.NotEqual(t, "", resp.Cookies()[1].Value, "xsrf cookie set") + + req, err := http.NewRequest("GET", "http://example.com", nil) + require.Nil(t, err) + req.AddCookie(resp.Cookies()[0]) + req.AddCookie(resp.Cookies()[1]) + req.Header.Add("X-XSRF-TOKEN", resp.Cookies()[1].Value) + + jwtService := NewJWT("12345", false, time.Hour) + res, err := jwtService.Get(req) + require.Nil(t, err) + assert.Equal(t, true, res.SessionOnly) + t.Logf("%+v", res) +} + func TestLogout(t *testing.T) { ts, ots := mockProvider(t, 8691, 8692) @@ -170,6 +209,6 @@ func mockProvider(t *testing.T, loginPort, authPort int) (*http.Server, *http.Se go oauth.ListenAndServe() go ts.ListenAndServe() - time.Sleep(time.Millisecond * 100) // let the start + time.Sleep(time.Millisecond * 100) // let them start return ts, oauth } diff --git a/app/rest/cache/cache_test.go b/app/rest/cache/cache_test.go index 483b49a0..58cd599c 100644 --- a/app/rest/cache/cache_test.go +++ b/app/rest/cache/cache_test.go @@ -42,7 +42,7 @@ func TestLoadingCache_Get(t *testing.T) { time.Sleep(100 * time.Millisecond) // let postFn to do its thing assert.Equal(t, int32(1), atomic.LoadInt32(&postFnCall)) - res, err = lc.Get("key", func() ([]byte, error) { + _, err = lc.Get("key", func() ([]byte, error) { return nil, errors.New("err") }) assert.NotNil(t, err) @@ -199,6 +199,7 @@ func TestLoadingCache_Scopes(t *testing.T) { res, err = lc.Get(Key("key", "s1", "s2"), func() ([]byte, error) { return []byte("value-upd"), nil }) + assert.Nil(t, err) assert.Equal(t, "value-upd", string(res), "was deleted, update") }