Skip to content

Commit

Permalink
enhancement: enhance Security by Allowing Same-Site Cookie Value Modi…
Browse files Browse the repository at this point in the history
…fication

This commit introduces a significant enhancement to the security of our application by allowing the modification of the 'SameSite' attribute of cookies from the consuming application.
The 'SameSite' attribute is a security measure that browsers use to restrict how cookies are sent with cross-site requests. By default, the Identity Provider (IDP) should be reachable from multiple domains; hence the 'SameSite' attribute is set to 'None'.
This allows cookies to be sent in all requests, irrespective of the site that the requests are being made from.

However, there are scenarios where the IDP should only be reachable from the same domain. In such cases, the 'SameSite' attribute needs to be set to 'Strict'.

This restricts the browser from sending cookies with any cross-site requests, thereby limiting the exposure of the user's session and mitigating the risk of Cross-Site Request Forgery (CSRF) attacks.
By allowing the 'SameSite' attribute to be modifiable, we provide the flexibility to tighten security measures based on the specific requirements and threat models of the consuming application.
This change does not impact existing functionality but provides an additional layer of security where needed.
  • Loading branch information
fschade authored and longsleep committed Mar 22, 2024
1 parent 1e9a82b commit 72cf922
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 46 deletions.
18 changes: 12 additions & 6 deletions bootstrap/backends/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,18 @@ func NewIdentityManager(bs bootstrap.Bootstrap) (identity.Manager, error) {
activeIdentifier, err := identifier.NewIdentifier(&identifier.Config{
Config: config.Config,

BaseURI: config.IssuerIdentifierURI,
PathPrefix: bs.MakeURIPath(bootstrap.APITypeSignin, ""),
StaticFolder: config.IdentifierClientPath,
LogonCookieName: "__Secure-KKT", // Kopano-Konnect-Token
ScopesConf: config.IdentifierScopesConf,
WebAppDisabled: config.IdentifierClientDisabled,
BaseURI: config.IssuerIdentifierURI,
PathPrefix: bs.MakeURIPath(bootstrap.APITypeSignin, ""),
StaticFolder: config.IdentifierClientPath,
ScopesConf: config.IdentifierScopesConf,
WebAppDisabled: config.IdentifierClientDisabled,

LogonCookieName: "__Secure-KKT", // Kopano-Konnect-Token
LogonCookieSameSite: config.CookieSameSite,

ConsentCookieSameSite: config.CookieSameSite,

StateCookieSameSite: config.CookieSameSite,

AuthorizationEndpointURI: fullAuthorizationEndpointURL,
SignedOutEndpointURI: fullSignedOutEndpointURL,
Expand Down
18 changes: 12 additions & 6 deletions bootstrap/backends/libregraph/libregraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,18 @@ func NewIdentityManager(bs bootstrap.Bootstrap) (identity.Manager, error) {
activeIdentifier, err := identifier.NewIdentifier(&identifier.Config{
Config: config.Config,

BaseURI: config.IssuerIdentifierURI,
PathPrefix: bs.MakeURIPath(bootstrap.APITypeSignin, ""),
StaticFolder: config.IdentifierClientPath,
LogonCookieName: "__Secure-KKT", // Kopano-Konnect-Token
ScopesConf: config.IdentifierScopesConf,
WebAppDisabled: config.IdentifierClientDisabled,
BaseURI: config.IssuerIdentifierURI,
PathPrefix: bs.MakeURIPath(bootstrap.APITypeSignin, ""),
StaticFolder: config.IdentifierClientPath,
ScopesConf: config.IdentifierScopesConf,
WebAppDisabled: config.IdentifierClientDisabled,

LogonCookieName: "__Secure-KKT", // Kopano-Konnect-Token
LogonCookieSameSite: config.CookieSameSite,

ConsentCookieSameSite: config.CookieSameSite,

StateCookieSameSite: config.CookieSameSite,

AuthorizationEndpointURI: fullAuthorizationEndpointURL,
SignedOutEndpointURI: fullSignedOutEndpointURL,
Expand Down
18 changes: 14 additions & 4 deletions bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"fmt"
"io/ioutil"
"net"
"net/http"
"net/url"
"os"
"path/filepath"
Expand Down Expand Up @@ -58,6 +59,7 @@ const (
DefaultSigningKeyBits = 2048

DefaultGuestIdentityManagerName = "guest"
DefaultCookieSameSite = http.SameSiteNoneMode
)

// Bootstrap is a data structure to hold configuration required to start
Expand Down Expand Up @@ -332,6 +334,12 @@ func (bs *bootstrap) initialize(settings *Settings) error {
}
bs.config.DyamicClientSecretDurationSeconds = settings.DyamicClientSecretDurationSeconds

// add setting to allow setting the same site attribute of the cookies
bs.config.CookieSameSite = settings.CookieSameSite
if bs.config.CookieSameSite == 0 {
bs.config.CookieSameSite = DefaultCookieSameSite
}

return nil
}

Expand Down Expand Up @@ -472,11 +480,13 @@ func (bs *bootstrap) setupOIDCProvider(ctx context.Context) (*oidcProvider.Provi
CheckSessionIframePath: bs.MakeURIPath(APITypeKonnect, "/session/check-session.html"),
RegistrationPath: registrationPath,

BrowserStateCookiePath: bs.MakeURIPath(APITypeKonnect, "/session/"),
BrowserStateCookieName: "__Secure-KKBS", // Kopano-Konnect-Browser-State
BrowserStateCookiePath: bs.MakeURIPath(APITypeKonnect, "/session/"),
BrowserStateCookieName: "__Secure-KKBS", // Kopano-Konnect-Browser-State
BrowserStateCookieSameSite: bs.config.CookieSameSite,

SessionCookiePath: sessionCookiePath,
SessionCookieName: "__Secure-KKCS", // Kopano-Konnect-Client-Session
SessionCookiePath: sessionCookiePath,
SessionCookieName: "__Secure-KKCS", // Kopano-Konnect-Client-Session
SessionCookieSameSite: bs.config.CookieSameSite,

AccessTokenDuration: time.Duration(bs.config.AccessTokenDurationSeconds) * time.Second,
IDTokenDuration: time.Duration(bs.config.IDTokenDurationSeconds) * time.Second,
Expand Down
3 changes: 3 additions & 0 deletions bootstrap/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto"
"crypto/tls"
"crypto/x509"
"net/http"
"net/url"

"github.com/golang-jwt/jwt/v4"
Expand Down Expand Up @@ -64,4 +65,6 @@ type Config struct {
IDTokenDurationSeconds uint64
RefreshTokenDurationSeconds uint64
DyamicClientSecretDurationSeconds uint64

CookieSameSite http.SameSite
}
5 changes: 5 additions & 0 deletions bootstrap/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

package bootstrap

import (
"net/http"
)

// Settings is a typed application config which represents the user accessible
// boostrap settings params.
type Settings struct {
Expand Down Expand Up @@ -48,6 +52,7 @@ type Settings struct {
ValidationKeysPath string
CookieBackendURI string
CookieNames []string
CookieSameSite http.SameSite
AccessTokenDurationSeconds uint64
IDTokenDurationSeconds uint64
RefreshTokenDurationSeconds uint64
Expand Down
13 changes: 10 additions & 3 deletions identifier/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package identifier

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

"github.com/libregraph/lico/config"
Expand All @@ -28,9 +29,15 @@ import (
type Config struct {
Config *config.Config

BaseURI *url.URL
LogonCookieName string
ScopesConf string
BaseURI *url.URL
ScopesConf string

LogonCookieName string
LogonCookieSameSite http.SameSite

ConsentCookieSameSite http.SameSite

StateCookieSameSite http.SameSite

PathPrefix string
StaticFolder string
Expand Down
12 changes: 6 additions & 6 deletions identifier/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (i *Identifier) setLogonCookie(rw http.ResponseWriter, value string) error
Path: i.pathPrefix + "/identifier/_/",
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.logonCookieSameSite,
}
http.SetCookie(rw, &cookie)

Expand All @@ -55,7 +55,7 @@ func (i *Identifier) removeLogonCookie(rw http.ResponseWriter) error {
Path: i.pathPrefix + "/identifier/_/",
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.logonCookieSameSite,

Expires: farPastExpiryTime,
}
Expand All @@ -78,7 +78,7 @@ func (i *Identifier) setConsentCookie(rw http.ResponseWriter, cr *ConsentRequest
Path: i.pathPrefix + "/identifier/_/",
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.consentCookieSameSite,
}
http.SetCookie(rw, &cookie)

Expand Down Expand Up @@ -106,7 +106,7 @@ func (i *Identifier) removeConsentCookie(rw http.ResponseWriter, req *http.Reque
Path: i.pathPrefix + "/identifier/_/",
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.consentCookieSameSite,

Expires: farPastExpiryTime,
}
Expand Down Expand Up @@ -150,7 +150,7 @@ func (i *Identifier) setStateCookie(rw http.ResponseWriter, scope string, state
Path: i.pathPrefix + "/identifier/" + scope,
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.stateCookieSameSite,
}
http.SetCookie(rw, &cookie)

Expand Down Expand Up @@ -178,7 +178,7 @@ func (i *Identifier) removeStateCookie(rw http.ResponseWriter, req *http.Request
Path: i.pathPrefix + "/identifier/" + scope,
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: i.stateCookieSameSite,

Expires: farPastExpiryTime,
}
Expand Down
23 changes: 18 additions & 5 deletions identifier/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ import (
"github.com/gorilla/mux"
"github.com/longsleep/rndm"
"github.com/sirupsen/logrus"
jose "gopkg.in/square/go-jose.v2"
jwt "gopkg.in/square/go-jose.v2/jwt"
"gopkg.in/square/go-jose.v2"
"gopkg.in/square/go-jose.v2/jwt"

"github.com/libregraph/oidc-go"

konnect "github.com/libregraph/lico"
"github.com/libregraph/lico/identifier/backends"
Expand All @@ -44,7 +46,6 @@ import (
"github.com/libregraph/lico/identity/clients"
"github.com/libregraph/lico/managers"
"github.com/libregraph/lico/utils"
"github.com/libregraph/oidc-go"
)

// audienceMarker defines the value which gets included in logon cookies. Valid
Expand All @@ -62,10 +63,16 @@ type Identifier struct {
baseURI *url.URL
pathPrefix string
staticFolder string
logonCookieName string
scopesConf string
webappIndexHTML []byte

logonCookieName string
logonCookieSameSite http.SameSite

consentCookieSameSite http.SameSite

stateCookieSameSite http.SameSite

authorizationEndpointURI *url.URL
signedOutEndpointURI *url.URL
oauth2CbEndpointURI *url.URL
Expand Down Expand Up @@ -114,10 +121,16 @@ func NewIdentifier(c *Config) (*Identifier, error) {
baseURI: c.BaseURI,
pathPrefix: c.PathPrefix,
staticFolder: staticFolder,
logonCookieName: c.LogonCookieName,
scopesConf: c.ScopesConf,
webappIndexHTML: webappIndexHTML,

logonCookieName: c.LogonCookieName,
logonCookieSameSite: c.LogonCookieSameSite,

consentCookieSameSite: c.ConsentCookieSameSite,

stateCookieSameSite: c.StateCookieSameSite,

authorizationEndpointURI: c.AuthorizationEndpointURI,
signedOutEndpointURI: c.SignedOutEndpointURI,
oauth2CbEndpointURI: oauth2CbEndpointURI,
Expand Down
11 changes: 7 additions & 4 deletions oidc/provider/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package provider

import (
"net/http"
"time"

"github.com/libregraph/lico/config"
Expand All @@ -37,11 +38,13 @@ type Config struct {
CheckSessionIframePath string
RegistrationPath string

BrowserStateCookiePath string
BrowserStateCookieName string
BrowserStateCookiePath string
BrowserStateCookieName string
BrowserStateCookieSameSite http.SameSite

SessionCookiePath string
SessionCookieName string
SessionCookiePath string
SessionCookieName string
SessionCookieSameSite http.SameSite

AccessTokenDuration time.Duration
IDTokenDuration time.Duration
Expand Down
8 changes: 4 additions & 4 deletions oidc/provider/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (p *Provider) setBrowserStateCookie(rw http.ResponseWriter, value string) e
Path: p.browserStateCookiePath,
Secure: true,
HttpOnly: false, // This Cookie is intended to be read by Javascript.
SameSite: http.SameSiteNoneMode,
SameSite: p.browserStateCookieSameSite,
}
http.SetCookie(rw, &cookie)

Expand All @@ -43,7 +43,7 @@ func (p *Provider) removeBrowserStateCookie(rw http.ResponseWriter) error {
Path: p.browserStateCookiePath,
Secure: true,
HttpOnly: false, // This Cookie is intended to be read by Javascript.
SameSite: http.SameSiteNoneMode,
SameSite: p.browserStateCookieSameSite,

Expires: farPastExpiryTime,
}
Expand All @@ -60,7 +60,7 @@ func (p *Provider) setSessionCookie(rw http.ResponseWriter, value string) error
Path: p.sessionCookiePath,
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: p.sessionCookieSameSite,
}
http.SetCookie(rw, &cookie)

Expand All @@ -83,7 +83,7 @@ func (p *Provider) removeSessionCookie(rw http.ResponseWriter) error {
Path: p.sessionCookiePath,
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
SameSite: p.sessionCookieSameSite,

Expires: farPastExpiryTime,
}
Expand Down
20 changes: 12 additions & 8 deletions oidc/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@ type Provider struct {
validationKeys map[string]crypto.PublicKey
certificates map[string][]*x509.Certificate

browserStateCookiePath string
browserStateCookieName string
browserStateCookiePath string
browserStateCookieName string
browserStateCookieSameSite http.SameSite

sessionCookiePath string
sessionCookieName string
sessionCookiePath string
sessionCookieName string
sessionCookieSameSite http.SameSite

accessTokenDuration time.Duration
idTokenDuration time.Duration
Expand Down Expand Up @@ -105,11 +107,13 @@ func NewProvider(c *Config) (*Provider, error) {
validationKeys: make(map[string]crypto.PublicKey),
certificates: make(map[string][]*x509.Certificate),

browserStateCookiePath: c.BrowserStateCookiePath,
browserStateCookieName: c.BrowserStateCookieName,
browserStateCookiePath: c.BrowserStateCookiePath,
browserStateCookieName: c.BrowserStateCookieName,
browserStateCookieSameSite: c.BrowserStateCookieSameSite,

sessionCookiePath: c.SessionCookiePath,
sessionCookieName: c.SessionCookieName,
sessionCookiePath: c.SessionCookiePath,
sessionCookieName: c.SessionCookieName,
sessionCookieSameSite: c.SessionCookieSameSite,

accessTokenDuration: c.AccessTokenDuration,
idTokenDuration: c.IDTokenDuration,
Expand Down

0 comments on commit 72cf922

Please sign in to comment.