Skip to content

Commit

Permalink
feat: New setting to specify SameSite cookie mode (#1718)
Browse files Browse the repository at this point in the history
Recent changes to Chrome require setting of SameSite cookie policy if it is acceptable for cookies to be used in a third party setting: https://blog.chromium.org/2020/02/samesite-cookie-changes-in-february.html

Some discussion on this in the community board https://community.ory.sh/t/does-hydra-support-samesite-none-for-cookies/1491

Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com>
  • Loading branch information
mattfawcett and aeneasr authored Feb 6, 2020
1 parent a86c8e6 commit 715522a
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 3 deletions.
3 changes: 2 additions & 1 deletion consent/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,14 @@ func matchScopes(scopeStrategy fosite.ScopeStrategy, previousConsent []HandledCo
return nil
}

func createCsrfSession(w http.ResponseWriter, r *http.Request, store sessions.Store, name, csrf string, secure bool) error {
func createCsrfSession(w http.ResponseWriter, r *http.Request, store sessions.Store, name, csrf string, secure bool, sameSiteMode http.SameSite) error {
// Errors can be ignored here, because we always get a session session back. Error typically means that the
// session doesn't exist yet.
session, _ := store.Get(r, name)
session.Values["csrf"] = csrf
session.Options.HttpOnly = true
session.Options.Secure = secure
session.Options.SameSite = sameSiteMode
if err := session.Save(r, w); err != nil {
return errors.WithStack(err)
}
Expand Down
5 changes: 3 additions & 2 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func (s *DefaultStrategy) forwardAuthenticationRequest(w http.ResponseWriter, r
return errors.WithStack(err)
}

if err := createCsrfSession(w, r, s.r.CookieStore(), cookieAuthenticationCSRFName, csrf, s.c.ServesHTTPS()); err != nil {
if err := createCsrfSession(w, r, s.r.CookieStore(), cookieAuthenticationCSRFName, csrf, s.c.ServesHTTPS(), s.c.CookieSameSiteMode()); err != nil {
return errors.WithStack(err)
}

Expand Down Expand Up @@ -444,6 +444,7 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
cookie.Options.MaxAge = session.RememberFor
}
cookie.Options.HttpOnly = true
cookie.Options.SameSite = s.c.CookieSameSiteMode()

if s.c.ServesHTTPS() {
cookie.Options.Secure = true
Expand Down Expand Up @@ -548,7 +549,7 @@ func (s *DefaultStrategy) forwardConsentRequest(w http.ResponseWriter, r *http.R
return errors.WithStack(err)
}

if err := createCsrfSession(w, r, s.r.CookieStore(), cookieConsentCSRFName, csrf, s.c.ServesHTTPS()); err != nil {
if err := createCsrfSession(w, r, s.r.CookieStore(), cookieConsentCSRFName, csrf, s.c.ServesHTTPS(), s.c.CookieSameSiteMode()); err != nil {
return errors.WithStack(err)
}

Expand Down
3 changes: 3 additions & 0 deletions docs/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ serve:
# For more information head over to: https://www.ory.sh/docs/hydra/production#tls-termination
allow_termination_from:
- 127.0.0.1/32
cookies:
# specify the SameSite mode that cookies should be sent with
same_site_mode: Lax

# dsn sets the data source name. This configures the backend where ORY Hydra persists data.
#
Expand Down
2 changes: 2 additions & 0 deletions driver/configuration/provider.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package configuration

import (
"net/http"
"net/url"
"time"

Expand Down Expand Up @@ -43,6 +44,7 @@ type Provider interface {
AdminDisableHealthAccessLog() bool
PublicListenOn() string
PublicDisableHealthAccessLog() bool
CookieSameSiteMode() http.SameSite
ConsentRequestMaxAge() time.Duration
AccessTokenLifespan() time.Duration
RefreshTokenLifespan() time.Duration
Expand Down
16 changes: 16 additions & 0 deletions driver/configuration/provider_viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configuration

import (
"fmt"
"net/http"
"net/url"
"strings"
"time"
Expand Down Expand Up @@ -48,6 +49,7 @@ const (
ViperKeyPublicListenOnHost = "serve.public.host"
ViperKeyPublicListenOnPort = "serve.public.port"
ViperKeyPublicDisableHealthAccessLog = "serve.public.access_log.disable_for_health"
ViperKeyCookieSameSiteMode = "serve.cookies.same_site_mode"
ViperKeyConsentRequestMaxAge = "ttl.login_consent_request"
ViperKeyAccessTokenLifespan = "ttl.access_token" // #nosec G101
ViperKeyRefreshTokenLifespan = "ttl.refresh_token" // #nosec G101
Expand Down Expand Up @@ -217,6 +219,20 @@ func (v *ViperProvider) adminPort() int {
return viperx.GetInt(v.l, ViperKeyAdminListenOnPort, 4445, "ADMIN_PORT")
}

func (v *ViperProvider) CookieSameSiteMode() http.SameSite {
sameSiteModeStr := viperx.GetString(v.l, ViperKeyCookieSameSiteMode, "default", "COOKIE_SAME_SITE_MODE")
switch strings.ToLower(sameSiteModeStr) {
case "lax":
return http.SameSiteLaxMode
case "strict":
return http.SameSiteStrictMode
case "none":
return http.SameSiteNoneMode
default:
return http.SameSiteDefaultMode
}
}

func (v *ViperProvider) ConsentRequestMaxAge() time.Duration {
return viperx.GetDuration(v.l, ViperKeyConsentRequestMaxAge, time.Minute*30, "LOGIN_CONSENT_REQUEST_LIFESPAN")
}
Expand Down
12 changes: 12 additions & 0 deletions driver/configuration/provider_viper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package configuration
import (
"fmt"
"io/ioutil"
"net/http"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -119,3 +120,14 @@ func TestViperProvider_IssuerURL(t *testing.T) {
p2 := NewViperProvider(l, false, nil)
assert.Equal(t, "http://hydra.localhost/", p2.IssuerURL().String())
}

func TestViperProvider_CookieSameSiteMode(t *testing.T) {
l := logrusx.New()
l.SetOutput(ioutil.Discard)

p := NewViperProvider(l, false, nil)
assert.Equal(t, http.SameSiteDefaultMode, p.CookieSameSiteMode())

os.Setenv("COOKIE_SAME_SITE_MODE", "none")
assert.Equal(t, http.SameSiteNoneMode, p.CookieSameSiteMode())
}

0 comments on commit 715522a

Please sign in to comment.