Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kim 163890622 update cookie attributes #1852

Merged
merged 11 commits into from Mar 14, 2019
Copy path View file
@@ -788,16 +788,20 @@ func main() {
if err != nil {
logger.Fatal("Registering login provider", zap.Error(err))
}

useSecureCookie := true
if isDevOrTest {
useSecureCookie = false
}

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 13, 2019

Contributor

Could you just write useSecureCookie := !isDevOrTest?

// Session management and authentication middleware
noSessionTimeout := v.GetBool("no-session-timeout")
sessionCookieMiddleware := auth.SessionCookieMiddleware(zapLogger, clientAuthSecretKey, noSessionTimeout, myHostname, officeHostname, tspHostname)
maskedCSRFMiddleware := auth.MaskedCSRFMiddleware(zapLogger, noSessionTimeout)
sessionCookieMiddleware := auth.SessionCookieMiddleware(zapLogger, clientAuthSecretKey, noSessionTimeout, myHostname, officeHostname, tspHostname, useSecureCookie)
maskedCSRFMiddleware := auth.MaskedCSRFMiddleware(zapLogger, noSessionTimeout, useSecureCookie)
userAuthMiddleware := authentication.UserAuthMiddleware(zapLogger)
clientCertMiddleware := authentication.ClientCertMiddleware(zapLogger, dbConnection)

handlerContext := handlers.NewHandlerContext(dbConnection, zapLogger)
handlerContext.SetCookieSecret(clientAuthSecretKey)
handlerContext.SetUseSecureCookie(useSecureCookie)
if noSessionTimeout {
handlerContext.SetNoSessionTimeout()
}
@@ -1137,16 +1141,16 @@ func main() {
authMux := goji.SubMux()
root.Handle(pat.New("/auth/*"), authMux)
authMux.Handle(pat.Get("/login-gov"), authentication.RedirectHandler{Context: authContext})
authMux.Handle(pat.Get("/login-gov/callback"), authentication.NewCallbackHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout))
authMux.Handle(pat.Get("/logout"), authentication.NewLogoutHandler(authContext, clientAuthSecretKey, noSessionTimeout))
authMux.Handle(pat.Get("/login-gov/callback"), authentication.NewCallbackHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout, useSecureCookie))
authMux.Handle(pat.Get("/logout"), authentication.NewLogoutHandler(authContext, clientAuthSecretKey, noSessionTimeout, useSecureCookie))

if isDevOrTest {
logger.Info("Enabling devlocal auth")
localAuthMux := goji.SubMux()
root.Handle(pat.New("/devlocal-auth/*"), localAuthMux)
localAuthMux.Handle(pat.Get("/login"), authentication.NewUserListHandler(authContext, dbConnection))
localAuthMux.Handle(pat.Post("/login"), authentication.NewAssignUserHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout))
localAuthMux.Handle(pat.Post("/new"), authentication.NewCreateAndLoginUserHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout))
localAuthMux.Handle(pat.Post("/new"), authentication.NewCreateAndLoginUserHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout, useSecureCookie))
localAuthMux.Handle(pat.Post("/create"), authentication.NewCreateUserHandler(authContext, dbConnection, clientAuthSecretKey, noSessionTimeout))

devlocalCa, err := ioutil.ReadFile(v.GetString("devlocal-ca")) // #nosec
Copy path View file
@@ -88,14 +88,16 @@ type LogoutHandler struct {
Context
clientAuthSecretKey string
noSessionTimeout bool
useSecureCookie bool
}

// NewLogoutHandler creates a new LogoutHandler
func NewLogoutHandler(ac Context, clientAuthSecretKey string, noSessionTimeout bool) LogoutHandler {
func NewLogoutHandler(ac Context, clientAuthSecretKey string, noSessionTimeout bool, useSecureCookie bool) LogoutHandler {
handler := LogoutHandler{
Context: ac,
clientAuthSecretKey: clientAuthSecretKey,
noSessionTimeout: noSessionTimeout,
useSecureCookie: useSecureCookie,
}
return handler
}
@@ -116,7 +118,7 @@ func (h LogoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
session.IDToken = ""
session.UserID = uuid.Nil
auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger)
auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger, h.useSecureCookie)
http.Redirect(w, r, logoutURL, http.StatusTemporaryRedirect)
} else {
// Can't log out of login.gov without a token, redirect and let them re-auth
@@ -154,15 +156,17 @@ type CallbackHandler struct {
db *pop.Connection
clientAuthSecretKey string
noSessionTimeout bool
useSecureCookie bool
}

// NewCallbackHandler creates a new CallbackHandler
func NewCallbackHandler(ac Context, db *pop.Connection, clientAuthSecretKey string, noSessionTimeout bool) CallbackHandler {
func NewCallbackHandler(ac Context, db *pop.Connection, clientAuthSecretKey string, noSessionTimeout bool, useSecureCookie bool) CallbackHandler {
handler := CallbackHandler{
Context: ac,
db: db,
clientAuthSecretKey: clientAuthSecretKey,
noSessionTimeout: noSessionTimeout,
useSecureCookie: useSecureCookie,
}
return handler
}
@@ -311,7 +315,7 @@ func authorizeKnownUser(userIdentity *models.UserIdentity, h CallbackHandler, se

h.logger.Info("logged in", zap.Any("session", session))

auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger)
auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger, h.useSecureCookie)
http.Redirect(w, r, lURL, http.StatusTemporaryRedirect)
}

@@ -369,7 +373,7 @@ func authorizeUnknownUser(openIDUser goth.User, h CallbackHandler, session *auth

h.logger.Info("logged in", zap.Any("session", session))

auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger)
auth.WriteSessionCookie(w, session, h.clientAuthSecretKey, h.noSessionTimeout, h.logger, h.useSecureCookie)
http.Redirect(w, r, lURL, http.StatusTemporaryRedirect)
}

@@ -100,6 +100,7 @@ type devlocalAuthHandler struct {
db *pop.Connection
clientAuthSecretKey string
noSessionTimeout bool
useSecureCookie bool
}

// AssignUserHandler logs a user in directly
@@ -186,12 +187,13 @@ func createUser(h devlocalAuthHandler, w http.ResponseWriter, r *http.Request) m
type CreateAndLoginUserHandler devlocalAuthHandler

// NewCreateAndLoginUserHandler creates a new CreateAndLoginUserHandler
func NewCreateAndLoginUserHandler(ac Context, db *pop.Connection, clientAuthSecretKey string, noSessionTimeout bool) CreateAndLoginUserHandler {
func NewCreateAndLoginUserHandler(ac Context, db *pop.Connection, clientAuthSecretKey string, noSessionTimeout bool, useSecureCookie bool) CreateAndLoginUserHandler {
handler := CreateAndLoginUserHandler{
Context: ac,
db: db,
clientAuthSecretKey: clientAuthSecretKey,
noSessionTimeout: noSessionTimeout,
useSecureCookie: useSecureCookie,
}
return handler
}
@@ -257,7 +259,7 @@ func loginUser(handler devlocalAuthHandler, user *models.User, w http.ResponseWr
}

handler.logger.Info("logged in", zap.Any("session", session))
auth.WriteSessionCookie(w, session, handler.clientAuthSecretKey, handler.noSessionTimeout, handler.logger)
auth.WriteSessionCookie(w, session, handler.clientAuthSecretKey, handler.noSessionTimeout, handler.logger, handler.useSecureCookie)

lURL := handler.landingURL(session)
http.Redirect(w, r, lURL, http.StatusSeeOther)
Copy path View file
@@ -101,7 +101,7 @@ func sessionClaimsFromRequest(logger *zap.Logger, secret string, appName Applica
}

// WriteMaskedCSRFCookie update the masked_gorilla_csrf cookie value
func WriteMaskedCSRFCookie(w http.ResponseWriter, csrfToken string, noSessionTimeout bool, logger *zap.Logger) {
func WriteMaskedCSRFCookie(w http.ResponseWriter, csrfToken string, noSessionTimeout bool, logger *zap.Logger, useSecureCookie bool) {

expiry := GetExpiryTimeFromMinutes(SessionExpiryInMinutes)
maxAge := sessionExpiryInSeconds
@@ -113,38 +113,44 @@ func WriteMaskedCSRFCookie(w http.ResponseWriter, csrfToken string, noSessionTim

// New cookie
cookie := http.Cookie{
Name: MaskedGorillaCSRFToken,
Value: csrfToken,
Path: "/",
Expires: expiry,
MaxAge: maxAge,
Name: MaskedGorillaCSRFToken,
Value: csrfToken,
Path: "/",
Expires: expiry,
MaxAge: maxAge,
HttpOnly: true,

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Mar 11, 2019

Contributor

For this one we want HttpOnly: false per our discussion. Let's add a note that the CSRF cookie needs to be able to be read by the JS so it can be added in the X-CSRF-Token header.

SameSite: http.SameSiteStrictMode,
Secure: useSecureCookie,
}

http.SetCookie(w, &cookie)
}

// MaskedCSRFMiddleware handles setting the CSRF Token cookie
func MaskedCSRFMiddleware(logger *zap.Logger, noSessionTimeout bool) func(next http.Handler) http.Handler {
func MaskedCSRFMiddleware(logger *zap.Logger, noSessionTimeout bool, useSecureCookie bool) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
mw := func(w http.ResponseWriter, r *http.Request) {
WriteMaskedCSRFCookie(w, csrf.Token(r), noSessionTimeout, logger)
WriteMaskedCSRFCookie(w, csrf.Token(r), noSessionTimeout, logger, useSecureCookie)
next.ServeHTTP(w, r)
}
return http.HandlerFunc(mw)
}
}

// WriteSessionCookie update the cookie for the session
func WriteSessionCookie(w http.ResponseWriter, session *Session, secret string, noSessionTimeout bool, logger *zap.Logger) {
func WriteSessionCookie(w http.ResponseWriter, session *Session, secret string, noSessionTimeout bool, logger *zap.Logger, useSecureCookie bool) {

// Delete the cookie
cookieName := fmt.Sprintf("%s_%s", string(session.ApplicationName), UserSessionCookieName)
cookie := http.Cookie{
Name: cookieName,
Value: "blank",
Path: "/",
Expires: time.Unix(0, 0),
MaxAge: -1,
Name: cookieName,
Value: "blank",
Path: "/",
Expires: time.Unix(0, 0),
MaxAge: -1,
HttpOnly: true,
SameSite: http.SameSiteStrictMode,
Secure: useSecureCookie,
}

// unless we have a valid session
@@ -193,7 +199,7 @@ func ApplicationName(hostname, milHostname, officeHostname, tspHostname string)
}

// SessionCookieMiddleware handle serializing and de-serializing the session between the user_session cookie and the request context
func SessionCookieMiddleware(logger *zap.Logger, secret string, noSessionTimeout bool, milHostname, officeHostname, tspHostname string) func(next http.Handler) http.Handler {
func SessionCookieMiddleware(logger *zap.Logger, secret string, noSessionTimeout bool, milHostname, officeHostname, tspHostname string, useSecureCookie bool) func(next http.Handler) http.Handler {
logger.Info("Creating session", zap.String("milHost", milHostname), zap.String("officeHost", officeHostname), zap.String("tspHost", tspHostname))
return func(next http.Handler) http.Handler {
mw := func(w http.ResponseWriter, r *http.Request) {
@@ -224,7 +230,7 @@ func SessionCookieMiddleware(logger *zap.Logger, secret string, noSessionTimeout
ctx = SetSessionInRequestContext(r.WithContext(ctx), &session)

// And update the cookie. May get over-ridden later
WriteSessionCookie(w, &session, secret, noSessionTimeout, logger)
WriteSessionCookie(w, &session, secret, noSessionTimeout, logger, useSecureCookie)

span.AddTraceField("auth.application_name", session.ApplicationName)
span.AddTraceField("auth.hostname", session.Hostname)
Copy path View file
@@ -62,7 +62,7 @@ func (suite *authSuite) TestSessionCookieMiddlewareWithBadToken() {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resultingSession = SessionFromRequestContext(r)
})
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost)(handler)
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost, false)(handler)

expiry := GetExpiryTimeFromMinutes(SessionExpiryInMinutes)
rr, req := getHandlerParamsWithToken(fakeToken, expiry)
@@ -104,7 +104,7 @@ func (suite *authSuite) TestSessionCookieMiddlewareWithValidToken() {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resultingSession = SessionFromRequestContext(r)
})
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost)(handler)
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost, false)(handler)

middleware.ServeHTTP(rr, req)

@@ -146,7 +146,7 @@ func (suite *authSuite) TestSessionCookieMiddlewareWithExpiredToken() {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resultingSession = SessionFromRequestContext(r)
})
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost)(handler)
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost, false)(handler)

rr, req := getHandlerParamsWithToken(ss, expiry)

@@ -192,9 +192,9 @@ func (suite *authSuite) TestSessionCookiePR161162731() {
var resultingSession *Session
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
resultingSession = SessionFromRequestContext(r)
WriteSessionCookie(w, resultingSession, "freddy", false, suite.logger)
WriteSessionCookie(w, resultingSession, "freddy", false, suite.logger, false)
})
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost)(handler)
middleware := SessionCookieMiddleware(suite.logger, pem, false, MilTestHost, OfficeTestHost, TspTestHost, false)(handler)

middleware.ServeHTTP(rr, req)

@@ -238,7 +238,7 @@ func (suite *authSuite) TestMaskedCSRFMiddleware() {
}

func (suite *authSuite) TestMiddlewareConstructor() {
adm := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost)
adm := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost, false)
suite.NotNil(adm)
}

@@ -252,7 +252,7 @@ func (suite *authSuite) TestMiddlewareMilApp() {
suite.False(session.IsTspApp(), "first should not be tsp app")
suite.Equal(MilTestHost, session.Hostname)
})
milMoveMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost)(milMoveTestHandler)
milMoveMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost, false)(milMoveTestHandler)

req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/some_url", MilTestHost), nil)
milMoveMiddleware.ServeHTTP(rr, req)
@@ -274,7 +274,7 @@ func (suite *authSuite) TestMiddlwareOfficeApp() {
suite.False(session.IsTspApp(), "should not be tsp app")
suite.Equal(OfficeTestHost, session.Hostname)
})
officeMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost)(officeTestHandler)
officeMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost, false)(officeTestHandler)

req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/some_url", OfficeTestHost), nil)
officeMiddleware.ServeHTTP(rr, req)
@@ -296,7 +296,7 @@ func (suite *authSuite) TestMiddlwareTspApp() {
suite.True(session.IsTspApp(), "should be tsp app")
suite.Equal(TspTestHost, session.Hostname)
})
tspMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost)(tspTestHandler)
tspMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost, false)(tspTestHandler)

req := httptest.NewRequest("GET", fmt.Sprintf("http://%s/some_url", TspTestHost), nil)
tspMiddleware.ServeHTTP(rr, req)
@@ -314,7 +314,7 @@ func (suite *authSuite) TestMiddlewareBadApp() {
noAppTestHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
suite.Fail("Should not be called")
})
noAppMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost)(noAppTestHandler)
noAppMiddleware := SessionCookieMiddleware(suite.logger, "secret", false, MilTestHost, OfficeTestHost, TspTestHost, false)(noAppTestHandler)

req := httptest.NewRequest("GET", "http://totally.bogus.hostname/some_url", nil)
noAppMiddleware.ServeHTTP(rr, req)
Copy path View file
@@ -37,6 +37,8 @@ type HandlerContext interface {
SetIWSPersonLookup(rbs iws.PersonLookup)
SendProductionInvoice() bool
SetSendProductionInvoice(sendProductionInvoice bool)
UseSecureCookie() bool
SetUseSecureCookie(useSecureCookie bool)

GexSender() services.GexSender
SetGexSender(gexSender services.GexSender)
@@ -62,6 +64,7 @@ type handlerContext struct {
dpsAuthParams dpsauth.Params
senderToGex services.GexSender
icnSequencer sequence.Sequencer
useSecureCookie bool
}

// NewHandlerContext returns a new handlerContext with its required private fields set.
@@ -190,3 +193,13 @@ func (hctx *handlerContext) DPSAuthParams() dpsauth.Params {
func (hctx *handlerContext) SetDPSAuthParams(params dpsauth.Params) {
hctx.dpsAuthParams = params
}

// UseSecureCookie determines if the field "Secure" is set to true or false upon cookie creation
func (hctx *handlerContext) UseSecureCookie() bool {
return hctx.useSecureCookie
}

// Sets flag for using Secure cookie
func (hctx *handlerContext) SetUseSecureCookie(useSecureCookie bool) {
hctx.useSecureCookie = useSecureCookie
}
@@ -148,7 +148,7 @@ func (h CreateServiceMemberHandler) Handle(params servicememberop.CreateServiceM
// And return
serviceMemberPayload := payloadForServiceMemberModel(h.FileStorer(), newServiceMember)
responder := servicememberop.NewCreateServiceMemberCreated().WithPayload(serviceMemberPayload)
return handlers.NewCookieUpdateResponder(params.HTTPRequest, h.CookieSecret(), h.NoSessionTimeout(), h.Logger(), responder)
return handlers.NewCookieUpdateResponder(params.HTTPRequest, h.CookieSecret(), h.NoSessionTimeout(), h.Logger(), responder, h.UseSecureCookie())
}

// ShowServiceMemberHandler returns a serviceMember for a user and service member ID
Copy path View file
@@ -19,21 +19,23 @@ type CookieUpdateResponder struct {
noSessionTimeout bool
logger *zap.Logger
Responder middleware.Responder
useSecureCookie bool
}

// NewCookieUpdateResponder constructs a wrapper for the responder which will update cookies
func NewCookieUpdateResponder(request *http.Request, secret string, noSessionTimeout bool, logger *zap.Logger, responder middleware.Responder) middleware.Responder {
func NewCookieUpdateResponder(request *http.Request, secret string, noSessionTimeout bool, logger *zap.Logger, responder middleware.Responder, useSecureCookie bool) middleware.Responder {
return &CookieUpdateResponder{
session: auth.SessionFromRequestContext(request),
cookieSecret: secret,
noSessionTimeout: noSessionTimeout,
logger: logger,
Responder: responder,
useSecureCookie: useSecureCookie,
}
}

// WriteResponse updates the session cookie before writing out the details of the response
func (cur *CookieUpdateResponder) WriteResponse(rw http.ResponseWriter, p runtime.Producer) {
auth.WriteSessionCookie(rw, cur.session, cur.cookieSecret, cur.noSessionTimeout, cur.logger)
auth.WriteSessionCookie(rw, cur.session, cur.cookieSecret, cur.noSessionTimeout, cur.logger, cur.useSecureCookie)
cur.Responder.WriteResponse(rw, p)
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.