Skip to content

Commit

Permalink
fix(session): properly declare session secrets
Browse files Browse the repository at this point in the history
Previously, a misconfiguration of Gorilla's session store caused incorrect handling of the configured secrets. From now on, cookies will also be properly encrypted at all times.

BREAKING CHANGE: If you are using two or more secrets for the `secrets.session`, this patch might break existing Ory Session Cookies. This has the effect that users will need to re-authenticate when visiting your app.

Closes #2272
  • Loading branch information
aeneasr committed Mar 7, 2022
1 parent 731b3c7 commit 6312afd
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
9 changes: 8 additions & 1 deletion driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package driver

import (
"context"
"crypto/sha256"
"net/http"
"strings"
"sync"
Expand Down Expand Up @@ -426,7 +427,13 @@ func (m *RegistryDefault) SelfServiceErrorHandler() *errorx.Handler {
}

func (m *RegistryDefault) CookieManager(ctx context.Context) sessions.StoreExact {
cs := sessions.NewCookieStore(m.Config(ctx).SecretsSession()...)
var keys [][]byte
for _, k := range m.Config(ctx).SecretsSession() {
encrypt := sha256.Sum256(k)
keys = append(keys, k, encrypt[:])
}

cs := sessions.NewCookieStore(keys...)
cs.Options.Secure = !m.Config(ctx).IsInsecureDevMode()
cs.Options.HttpOnly = true

Expand Down
25 changes: 25 additions & 0 deletions session/manager_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,31 @@ func TestManagerHTTP(t *testing.T) {
assert.EqualValues(t, http.StatusOK, res.StatusCode)
})

t.Run("case=key rotation", func(t *testing.T) {
original := conf.Source().Strings(config.ViperKeySecretsCookie)
t.Cleanup(func() {
conf.MustSet(config.ViperKeySecretsCookie, original)
})
conf.MustSet(config.ViperKeySessionLifespan, "1m")
conf.MustSet(config.ViperKeySecretsCookie, []string{"foo"})

i := identity.Identity{Traits: []byte("{}")}
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i))
s, _ = session.NewActiveSession(&i, conf, time.Now(), identity.CredentialsTypePassword)

c := testhelpers.NewClientWithCookies(t)
testhelpers.MockHydrateCookieClient(t, c, pts.URL+"/session/set")

res, err := c.Get(pts.URL + "/session/get")
require.NoError(t, err)
assert.EqualValues(t, http.StatusOK, res.StatusCode)

conf.MustSet(config.ViperKeySecretsCookie, []string{"bar", "foo"})
res, err = c.Get(pts.URL + "/session/get")
require.NoError(t, err)
assert.EqualValues(t, http.StatusOK, res.StatusCode)
})

t.Run("case=no panic on invalid cookie name", func(t *testing.T) {
conf.MustSet(config.ViperKeySessionLifespan, "1m")
conf.MustSet(config.ViperKeySessionName, "$%˜\"")
Expand Down

0 comments on commit 6312afd

Please sign in to comment.