Skip to content

Commit

Permalink
add cors middleware
Browse files Browse the repository at this point in the history
- fix bug with cookies differing when calling login and whoami endpoints
  • Loading branch information
kian99 committed Sep 4, 2024
1 parent 7734e22 commit 14f063e
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 28 deletions.
19 changes: 11 additions & 8 deletions cmd/jimmsrv/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ func start(ctx context.Context, s *service.Service) error {
return errors.E("jimm session store secret must be at least 64 characters")
}

corsAllowedOrigins := strings.Split(os.Getenv("CORS_ALLOWED_ORIGINS"), " ")

jimmsvc, err := jimmsvc.NewService(ctx, jimmsvc.Params{
ControllerUUID: os.Getenv("JIMM_UUID"),
DSN: os.Getenv("JIMM_DSN"),
Expand All @@ -167,17 +169,18 @@ func start(ctx context.Context, s *service.Service) error {
JWTExpiryDuration: jwtExpiryDuration,
InsecureSecretStorage: insecureSecretStorage,
OAuthAuthenticatorParams: jimmsvc.OAuthAuthenticatorParams{
IssuerURL: issuerURL,
ClientID: clientID,
ClientSecret: clientSecret,
Scopes: scopesParsed,
SessionTokenExpiry: sessionTokenExpiryDuration,
SessionCookieMaxAge: sessionCookieMaxAgeInt,
JWTSessionKey: sessionSecretKey,
IssuerURL: issuerURL,
ClientID: clientID,
ClientSecret: clientSecret,
Scopes: scopesParsed,
SessionTokenExpiry: sessionTokenExpiryDuration,
SessionCookieMaxAge: sessionCookieMaxAgeInt,
JWTSessionKey: sessionSecretKey,
SecureSessionCookies: secureSessionCookies,
},
DashboardFinalRedirectURL: os.Getenv("JIMM_DASHBOARD_FINAL_REDIRECT_URL"),
SecureSessionCookies: secureSessionCookies,
CookieSessionKey: []byte(sessionSecretKey),
CorsAllowedOrigins: corsAllowedOrigins,
})
if err != nil {
return err
Expand Down
23 changes: 18 additions & 5 deletions cmd/jimmsrv/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/juju/names/v5"
"github.com/juju/zaputil/zapctx"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/rs/cors"
"go.uber.org/zap"
"gorm.io/driver/postgres"
"gorm.io/gorm"
Expand Down Expand Up @@ -82,6 +83,10 @@ type OAuthAuthenticatorParams struct {
// SessionCookieMaxAge holds the max age for session cookies in seconds.
SessionCookieMaxAge int

// SecureSessionCookies determines if HTTPS must be enabled in order for JIMM
// to set cookies when creating browser based sessions.
SecureSessionCookies bool

// JWTSessionKey holds the secret key used for signing/verifying JWT tokens.
// See internal/auth/oauth2.go AuthenticationService.SessionSecretkey for more details.
JWTSessionKey string
Expand Down Expand Up @@ -175,14 +180,14 @@ type Params struct {
// the /callback in an authorisation code OAuth2.0 flow to finish the flow.
DashboardFinalRedirectURL string

// SecureSessionCookies determines if HTTPS must be enabled in order for JIMM
// to set cookies when creating browser based sessions.
SecureSessionCookies bool

// CookieSessionKey is a randomly generated secret passed via config used for signing
// cookie data. The recommended length is 32/64 characters from the Gorilla securecookie lib.
// https://github.com/gorilla/securecookie/blob/main/securecookie.go#L124
CookieSessionKey []byte

// CorsAllowedOrigins represents all addresses that are valid for cross-origin
// requests. A wildcard '*' is accepted to allow all cross-origin requests.
CorsAllowedOrigins []string
}

// A Service is the implementation of a JIMM server.
Expand Down Expand Up @@ -347,6 +352,7 @@ func NewService(ctx context.Context, p Params) (*Service, error) {
SessionTokenExpiry: p.OAuthAuthenticatorParams.SessionTokenExpiry,
SessionCookieMaxAge: p.OAuthAuthenticatorParams.SessionCookieMaxAge,
JWTSessionKey: p.OAuthAuthenticatorParams.JWTSessionKey,
SecureCookies: p.OAuthAuthenticatorParams.SecureSessionCookies,
Store: &s.jimm.Database,
SessionStore: sessionStore,
RedirectURL: redirectUrl,
Expand Down Expand Up @@ -380,6 +386,14 @@ func NewService(ctx context.Context, p Params) (*Service, error) {
return nil, errors.E(op, err, "failed to parse final redirect url for the dashboard")
}

// Setup CORS middleware
corsOpts := cors.New(cors.Options{
AllowedOrigins: p.CorsAllowedOrigins,
AllowedMethods: []string{"GET"},
AllowCredentials: true,
})
s.mux.Use(corsOpts.Handler)

// Setup all HTTP handlers.
mountHandler := func(path string, h jimmhttp.JIMMHttpHandler) {
s.mux.Mount(path, h.Routes())
Expand All @@ -406,7 +420,6 @@ func NewService(ctx context.Context, p Params) (*Service, error) {
oauthHandler, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{
Authenticator: authSvc,
DashboardFinalRedirectURL: p.DashboardFinalRedirectURL,
SecureCookies: p.SecureSessionCookies,
})
if err != nil {
zapctx.Error(ctx, "failed to setup authentication handler", zap.Error(err))
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ require (
github.com/lestrrat-go/iter v1.0.2
github.com/lestrrat-go/jwx/v2 v2.0.21
github.com/oklog/ulid/v2 v2.1.0
github.com/rs/cors v1.11.1
github.com/stretchr/testify v1.9.0
golang.org/x/oauth2 v0.16.0
gopkg.in/errgo.v1 v1.0.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,8 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/rs/cors v1.10.1 h1:L0uuZVXIKlI1SShY2nhFfo44TYvDPQ1w4oFkUJNfhyo=
github.com/rs/cors v1.10.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU=
github.com/rs/cors v1.11.1 h1:eU3gRzXLRK57F5rKMGMZURNdIG4EoAmX8k94r9wXWHA=
github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU=
github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc=
github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
Expand Down
22 changes: 19 additions & 3 deletions internal/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ type AuthenticationService struct {
sessionTokenExpiry time.Duration
// sessionCookieMaxAge holds the max age for session cookies in seconds.
sessionCookieMaxAge int
// secureCookies decides whether to set the secure flag on cookies.
secureCookies bool
// jwtSessionKey holds the secret key used for signing/verifying JWT tokens.
// According to https://datatracker.ietf.org/doc/html/rfc7518 minimum key lengths are
// HSXXX e.g. HS256 - 256 bits, RSA - at least 2048 bits.
Expand Down Expand Up @@ -119,6 +121,9 @@ type AuthenticationServiceParams struct {
// SessionCookieMaxAge holds the max age for session cookies in seconds.
SessionCookieMaxAge int

// SecureCookies decides whether to set the secure flag on cookies.
SecureCookies bool

// JWTSessionKey holds the secret key used for signing/verifying JWT tokens.
// See AuthenticationService.JWTSessionKey for more details.
JWTSessionKey string
Expand Down Expand Up @@ -163,6 +168,7 @@ func NewAuthenticationService(ctx context.Context, params AuthenticationServiceP
db: params.Store,
sessionStore: params.SessionStore,
sessionCookieMaxAge: params.SessionCookieMaxAge,
secureCookies: params.SecureCookies,
}, nil
}

Expand Down Expand Up @@ -420,13 +426,23 @@ func (as *AuthenticationService) VerifyClientCredentials(ctx context.Context, cl
return nil
}

// sessionCrossOriginSafe sets parameters on the session that allow its use in cross-origin requests.
// Options are not saved to the database so this must be called whenever a session cookie will be returned to a client.
//
// Note browsers require cookies with the same-site policy as 'none' to additionally have the secure flag set.
func sessionCrossOriginSafe(session *sessions.Session, secure bool) *sessions.Session {
session.Options.Secure = secure // Ensures only sent with HTTPS
session.Options.HttpOnly = false // Allow Javascript to read it
session.Options.SameSite = http.SameSiteNoneMode // Allow cross-origin requests via Javascript
return session
}

// CreateBrowserSession creates a session and updates the cookie for a browser
// login callback.
func (as *AuthenticationService) CreateBrowserSession(
ctx context.Context,
w http.ResponseWriter,
r *http.Request,
secureCookies bool,
email string,
) error {
const op = errors.Op("auth.AuthenticationService.CreateBrowserSession")
Expand All @@ -438,8 +454,7 @@ func (as *AuthenticationService) CreateBrowserSession(

session.IsNew = true // Sets cookie to a fresh new cookie
session.Options.MaxAge = as.sessionCookieMaxAge // Expiry in seconds
session.Options.Secure = secureCookies // Ensures only sent with HTTPS
session.Options.HttpOnly = false // Allow Javascript to read it
session = sessionCrossOriginSafe(session, as.secureCookies)

session.Values[SessionIdentityKey] = email
if err = session.Save(r, w); err != nil {
Expand All @@ -465,6 +480,7 @@ func (as *AuthenticationService) AuthenticateBrowserSession(ctx context.Context,
if err != nil {
return ctx, errors.E(op, err, "failed to retrieve session")
}
session = sessionCrossOriginSafe(session, as.secureCookies)

identityId, ok := session.Values[SessionIdentityKey]
if !ok {
Expand Down
3 changes: 2 additions & 1 deletion internal/auth/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func setupTestAuthSvc(ctx context.Context, c *qt.C, expiry time.Duration) (*auth
SessionStore: sessionStore,
SessionCookieMaxAge: 60,
JWTSessionKey: "secret-key",
SecureCookies: false,
})
c.Assert(err, qt.IsNil)
cleanup := func() {
Expand Down Expand Up @@ -295,7 +296,7 @@ func TestCreateBrowserSession(t *testing.T) {
req, err := http.NewRequest("GET", "", nil)
c.Assert(err, qt.IsNil)

err = authSvc.CreateBrowserSession(ctx, rec, req, false, "[email protected]")
err = authSvc.CreateBrowserSession(ctx, rec, req, "[email protected]")
c.Assert(err, qt.IsNil)

cookies := rec.Header().Get("Set-Cookie")
Expand Down
8 changes: 0 additions & 8 deletions internal/jimmhttp/auth_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ type OAuthHandler struct {
Router *chi.Mux
authenticator BrowserOAuthAuthenticator
dashboardFinalRedirectURL string
secureCookies bool
}

// OAuthHandlerParams holds the parameters to configure the OAuthHandler.
Expand All @@ -45,10 +44,6 @@ type OAuthHandlerParams struct {
// DashboardFinalRedirectURL is the final redirection URL to send users to
// upon completing the authorisation code flow.
DashboardFinalRedirectURL string

// SessionCookies determines if HTTPS must be enabled in order for JIMM
// to set cookies when creating browser based sessions.
SecureCookies bool
}

// BrowserOAuthAuthenticator handles authorisation code authentication within JIMM
Expand All @@ -63,7 +58,6 @@ type BrowserOAuthAuthenticator interface {
ctx context.Context,
w http.ResponseWriter,
r *http.Request,
secureCookies bool,
email string,
) error
Logout(ctx context.Context, w http.ResponseWriter, req *http.Request) error
Expand All @@ -83,7 +77,6 @@ func NewOAuthHandler(p OAuthHandlerParams) (*OAuthHandler, error) {
Router: chi.NewRouter(),
authenticator: p.Authenticator,
dashboardFinalRedirectURL: p.DashboardFinalRedirectURL,
secureCookies: p.SecureCookies,
}, nil
}

Expand Down Expand Up @@ -173,7 +166,6 @@ func (oah *OAuthHandler) Callback(w http.ResponseWriter, r *http.Request) {
ctx,
w,
r,
oah.secureCookies,
email,
); err != nil {
writeError(ctx, w, http.StatusInternalServerError, err, "failed to setup session")
Expand Down
2 changes: 1 addition & 1 deletion internal/jimmtest/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func SetupTestDashboardCallbackHandler(browserURL string, db *db.Database, sessi
SessionStore: sessionStore,
SessionCookieMaxAge: 60,
JWTSessionKey: "test-secret",
SecureCookies: false,
})
if err != nil {
return nil, err
Expand All @@ -247,7 +248,6 @@ func SetupTestDashboardCallbackHandler(browserURL string, db *db.Database, sessi
h, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{
Authenticator: authSvc,
DashboardFinalRedirectURL: browserURL,
SecureCookies: false,
})
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions internal/jujuapi/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (s *adminSuite) SetUpTest(c *gc.C) {
SessionStore: sessionStore,
SessionCookieMaxAge: 60,
JWTSessionKey: "test-secret",
SecureCookies: false,
})
c.Assert(err, gc.Equals, nil)
s.JIMM.OAuthAuthenticator = authSvc
Expand Down

0 comments on commit 14f063e

Please sign in to comment.