From 3abe4146b13d899fefbd01cf5ee9bbb5864483d0 Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 18 Jul 2018 16:56:23 -0500 Subject: [PATCH] add deleteme flag to jwt token it should prevent misuse of laked token to request user's data removal --- backend/app/rest/api/admin.go | 5 +++++ backend/app/rest/api/admin_test.go | 17 +++++++++++++++++ backend/app/rest/api/rest_private.go | 4 +++- backend/app/rest/auth/auth.go | 6 ++++++ backend/app/rest/auth/auth_test.go | 22 ++++++++++++++++++++++ backend/app/rest/auth/jwt.go | 3 +++ 6 files changed, 56 insertions(+), 1 deletion(-) diff --git a/backend/app/rest/api/admin.go b/backend/app/rest/api/admin.go index 56deb942..c1f5e760 100644 --- a/backend/app/rest/api/admin.go +++ b/backend/app/rest/api/admin.go @@ -108,6 +108,11 @@ func (a *admin) deleteMeRequestCtrl(w http.ResponseWriter, r *http.Request) { log.Printf("[INFO] delete all user comments by request for %s, site %s", claims.User.ID, claims.SiteID) + if !claims.DeleteMe { // deletme set by deleteMeCtrl, this check just to make sure we not trying to delete with leaked token + rest.SendErrorJSON(w, r, http.StatusForbidden, errors.New("forbidden"), "can't use provided token") + return + } + if err := a.dataService.DeleteUser(claims.SiteID, claims.User.ID); err != nil { rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't delete user") return diff --git a/backend/app/rest/api/admin_test.go b/backend/app/rest/api/admin_test.go index 7193a120..1e0b5a93 100644 --- a/backend/app/rest/api/admin_test.go +++ b/backend/app/rest/api/admin_test.go @@ -497,6 +497,7 @@ func TestAdmin_DeleteMeRequest(t *testing.T) { User: &store.User{ ID: "user1", }, + DeleteMe: true, } token, err := srv.Authenticator.JWTService.Token(&claims) @@ -551,6 +552,7 @@ func TestAdmin_DeleteMeRequestFailed(t *testing.T) { User: &store.User{ ID: "user1", }, + DeleteMe: true, } token, err := srv.Authenticator.JWTService.Token(&claims) @@ -573,6 +575,21 @@ func TestAdmin_DeleteMeRequestFailed(t *testing.T) { resp, err = client.Do(req) assert.Nil(t, err) assert.Equal(t, 400, resp.StatusCode, resp.Status) + + // try without deleteme flag + badClaims2 := claims + badClaims2.DeleteMe = false + token, err = srv.Authenticator.JWTService.Token(&badClaims2) + assert.Nil(t, err) + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/admin/deleteme?token=%s", ts.URL, token), nil) + assert.Nil(t, err) + req.SetBasicAuth("dev", "password") + resp, err = client.Do(req) + assert.Nil(t, err) + assert.Equal(t, 403, resp.StatusCode) + b, err := ioutil.ReadAll(resp.Body) + assert.Nil(t, err) + assert.True(t, strings.Contains(string(b), "can't use provided token")) } func TestAdmin_GetUserInfo(t *testing.T) { diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 5a7159cd..8bbc0815 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -255,7 +255,8 @@ func (s *Rest) deleteMeCtrl(w http.ResponseWriter, r *http.Request) { ExpiresAt: time.Now().AddDate(0, 3, 0).Unix(), NotBefore: time.Now().Add(-1 * time.Minute).Unix(), }, - User: &user, + User: &user, + DeleteMe: true, // prevent this token from being used for login } tokenStr, err := s.Authenticator.JWTService.Token(&claims) @@ -263,6 +264,7 @@ func (s *Rest) deleteMeCtrl(w http.ResponseWriter, r *http.Request) { rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "can't make token") return } + link := fmt.Sprintf("%s/web/deleteme.html?token=%s", s.RemarkURL, tokenStr) render.JSON(w, r, JSON{"site": siteID, "user_id": user.ID, "token": tokenStr, "link": link}) } diff --git a/backend/app/rest/auth/auth.go b/backend/app/rest/auth/auth.go index a8eb2ae8..f9a9f202 100644 --- a/backend/app/rest/auth/auth.go +++ b/backend/app/rest/auth/auth.go @@ -73,6 +73,12 @@ func (a *Authenticator) Auth(reqAuth bool) func(http.Handler) http.Handler { return } + if claims.DeleteMe { + log.Printf("[DEBUG] invalid token flags for %s/%s", claims.User.Name, claims.User.ID) + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + if a.JWTService.IsExpired(claims) { if claims, err = a.refreshExpiredToken(w, claims); err != nil { log.Printf("[DEBUG] can't refresh jwt, %s", err) diff --git a/backend/app/rest/auth/auth_test.go b/backend/app/rest/auth/auth_test.go index 1799e174..39194f36 100644 --- a/backend/app/rest/auth/auth_test.go +++ b/backend/app/rest/auth/auth_test.go @@ -15,6 +15,8 @@ import ( var testJwtUserBlocked = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjI3ODkxOTE4MjIsImp0aSI6InJhbmRvbSBpZCIsImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZSI6IiIsImFkbWluIjpmYWxzZSwiYmxvY2siOnRydWV9LCJzdGF0ZSI6IjEyMzQ1NiIsImZyb20iOiJmcm9tIn0.6P_OwGf8CUJRtvNSlW20GmaMb5pFvCNemP94fHCqb5Q" +var testJwtDeleteMe = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjI3ODkxOTE4MjIsImp0aSI6InJhbmRvbSBpZCIsImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTI2ODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJuYW1lMSIsImlkIjoiaWQxIiwicGljdHVyZSI6IiIsImFkbWluIjpmYWxzZSwiYmxvY2siOmZhbHNlfSwiZGVsZXRlbWUiOnRydWV9.3wiT5fqDv_bzPky6-3IilU8ExfzCyvLpKDMPYOAFWEo" + func TestAuthJWTCookie(t *testing.T) { a := Authenticator{DevPasswd: "123456", JWTService: NewJWT("xyz 12345", false, time.Hour, time.Hour), PermissionChecker: &mockUserPermissions{}} @@ -100,6 +102,26 @@ func TestAuthJWtBlocked(t *testing.T) { assert.Equal(t, 401, resp.StatusCode, "blocked user") } +func TestAuthJWtFlags(t *testing.T) { + a := Authenticator{DevPasswd: "123456", JWTService: NewJWT("xyz 12345", false, time.Hour, time.Hour)} + router := chi.NewRouter() + router.With(a.Auth(true)).Get("/auth", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(201) + }) + server := httptest.NewServer(router) + defer server.Close() + + jar, err := cookiejar.New(nil) + require.Nil(t, err) + client := &http.Client{Jar: jar, Timeout: 5 * time.Second} + req, err := http.NewRequest("GET", server.URL+"/auth", nil) + require.Nil(t, err) + req.Header.Add("X-JWT", testJwtDeleteMe) + resp, err := client.Do(req) + require.NoError(t, err) + assert.Equal(t, 401, resp.StatusCode, "blocked user") +} + func TestAuthRequired(t *testing.T) { a := Authenticator{DevPasswd: "123456"} router := chi.NewRouter() diff --git a/backend/app/rest/auth/jwt.go b/backend/app/rest/auth/jwt.go index 68842ced..0b1673fe 100644 --- a/backend/app/rest/auth/jwt.go +++ b/backend/app/rest/auth/jwt.go @@ -29,6 +29,9 @@ type CustomClaims struct { From string `json:"from,omitempty"` SiteID string `json:"site_id,omitempty"` SessionOnly bool `json:"sess_only,omitempty"` + + // flags indicate different uses + DeleteMe bool `json:"deleteme,omitempty"` } const jwtCookieName = "JWT"