Skip to content

Commit 2c23d17

Browse files
committed
Fix issue with cookie deletion
1 parent 653f09c commit 2c23d17

File tree

3 files changed

+20
-14
lines changed

3 files changed

+20
-14
lines changed

pkg/api/keystone/keystone.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ func getNewToken(c *middleware.Context) (string, error) {
6666

6767
var keystonePasswordObj interface{}
6868
if setting.KeystoneCookieCredentials {
69-
keystonePasswordObj = c.GetCookie(middleware.SESS_KEY_PASSWORD)
70-
if keystonePasswordObj == nil {
69+
if keystonePasswordObj = c.GetCookie(middleware.SESS_KEY_PASSWORD); keystonePasswordObj == nil {
7170
return "", errors.New("Couldn't find cookie containing keystone password")
7271
} else {
7372
log.Debug("Got password from cookie")
@@ -94,7 +93,7 @@ func getNewToken(c *middleware.Context) (string, error) {
9493
}
9594
if err := AuthenticateScoped(&auth); err != nil {
9695
if setting.KeystoneCookieCredentials {
97-
c.SetCookie(middleware.SESS_KEY_PASSWORD, "", 0)
96+
c.SetCookie(middleware.SESS_KEY_PASSWORD, "", -1, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
9897
} else {
9998
c.Session.Set(middleware.SESS_KEY_PASSWORD, nil)
10099
}
@@ -182,7 +181,15 @@ func decryptPassword(base64ciphertext string) string {
182181
log.Error(3, "Error: NewCipher(%d bytes) = %s", len(setting.KeystoneCredentialAesKey), err)
183182
}
184183
ciphertext, err := base64.StdEncoding.DecodeString(base64ciphertext)
184+
if err != nil {
185+
log.Error(3, "Error: %s", err)
186+
return ""
187+
}
185188
iv := ciphertext[:aes.BlockSize]
189+
if aes.BlockSize > len(ciphertext) {
190+
log.Error(3, "Error: ciphertext %s is shorter than AES blocksize %d", ciphertext, aes.BlockSize)
191+
return ""
192+
}
186193
password := make([]byte, len(ciphertext)-aes.BlockSize)
187194
stream := cipher.NewOFB(block, iv)
188195
stream.XORKeyStream(password, ciphertext[aes.BlockSize:])

pkg/api/login.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
m "github.com/grafana/grafana/pkg/models"
1414
"github.com/grafana/grafana/pkg/setting"
1515
"github.com/grafana/grafana/pkg/util"
16-
"gopkg.in/macaron.v1"
1716
)
1817

1918
const (
@@ -125,7 +124,7 @@ func LoginPost(c *middleware.Context, cmd dtos.LoginCommand) Response {
125124
} else {
126125
days = 86400 * setting.LogInRememberDays
127126
}
128-
c.SetCookie(middleware.SESS_KEY_PASSWORD, cmd.Password, days, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
127+
c.SetCookie(middleware.SESS_KEY_PASSWORD, cmd.Password, days, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
129128
} else {
130129
c.Session.Set(middleware.SESS_KEY_PASSWORD, cmd.Password)
131130
}
@@ -152,22 +151,18 @@ func loginUserWithUser(user *m.User, c *middleware.Context) {
152151

153152
days := 86400 * setting.LogInRememberDays
154153
if days > 0 {
155-
c.SetCookie(setting.CookieUserName, user.Login, days, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
154+
c.SetCookie(setting.CookieUserName, user.Login, days, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
156155
c.SetSuperSecureCookie(util.EncodeMd5(user.Rands+user.Password),
157-
setting.CookieRememberName, user.Login, days, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
156+
setting.CookieRememberName, user.Login, days, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
158157
}
159158

160159
c.Session.Set(middleware.SESS_KEY_USERID, user.Id)
161160
}
162161

163162
func Logout(c *middleware.Context) {
164-
c.SetCookie(setting.CookieUserName, "", -1, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
165-
c.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
166-
c.SetCookie(middleware.SESS_KEY_PASSWORD, "", -1, setting.AppSubUrl+"/", nil, isSecure(&c.Req), true)
163+
c.SetCookie(setting.CookieUserName, "", -1, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
164+
c.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
165+
c.SetCookie(middleware.SESS_KEY_PASSWORD, "", -1, setting.AppSubUrl+"/", nil, middleware.IsSecure(c), true)
167166
c.Session.Destory(c)
168167
c.Redirect(setting.AppSubUrl + "/login")
169168
}
170-
171-
func isSecure(r *macaron.Request) bool {
172-
return (r.TLS != nil) || (r.Header.Get("X-Forwarded-Proto") == "https")
173-
}

pkg/middleware/middleware.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,3 +232,7 @@ func (ctx *Context) HasUserRole(role m.RoleType) bool {
232232
func (ctx *Context) TimeRequest(timer metrics.Timer) {
233233
ctx.Data["perfmon.timer"] = timer
234234
}
235+
236+
func IsSecure(ctx *Context) bool {
237+
return (ctx.Req.TLS != nil) || (ctx.Req.Header.Get("X-Forwarded-Proto") == "https")
238+
}

0 commit comments

Comments
 (0)