Merge pull request #268 from vmware-tanzu/secret-generation-prefactor

Secret generation prefactor

internal/oidc/auth/auth_handler.go

@@ -136,7 +136,7 @@ func NewHandler(
 	})
 }
 
-func readCSRFCookie(r *http.Request, codec oidc.Codec) csrftoken.CSRFToken {
+func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {
 	receivedCSRFCookie, err := r.Cookie(oidc.CSRFCookieName)
 	if err != nil {
 		// Error means that the cookie was not found
@@ -214,7 +214,7 @@ func upstreamStateParam(
 	return encodedStateParamValue, nil
 }
 
-func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken, codec oidc.Codec) error {
+func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken, codec oidc.Encoder) error {
 	encodedCSRFValue, err := codec.Encode(oidc.CSRFCookieEncodingName, csrfValue)
 	if err != nil {
 		return httperr.Wrap(http.StatusInternalServerError, "error encoding CSRF cookie", err)

internal/oidc/provider/manager/manager.go

@@ -88,10 +88,14 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
 		//  1. we would like to state to have an embedded expiration date while the cookie does not need that
 		//  2. we would like each downstream provider to use different secrets for signing/encrypting the upstream state, not share secrets
 		//  3. we would like *all* downstream providers to use the *same* signing key for the CSRF cookie (which doesn't need to be encrypted) because cookies are sent per-domain and our issuers can share a domain name (but have different paths)
-		var encoderHashKey = []byte("fake-hash-secret")  // TODO replace this secret
-		var encoderBlockKey = []byte("16-bytes-aaaaaaa") // TODO replace this secret
-		var encoder = securecookie.New(encoderHashKey, encoderBlockKey)
-		encoder.SetSerializer(securecookie.JSONEncoder{})
+		var upstreamStateEncoderHashKey = []byte("fake-state-hash-secret") // TODO replace this secret
+		var upstreamStateEncoderBlockKey = []byte("16-bytes-STATE01")      // TODO replace this secret
+		var upstreamStateEncoder = securecookie.New(upstreamStateEncoderHashKey, upstreamStateEncoderBlockKey)
+		upstreamStateEncoder.SetSerializer(securecookie.JSONEncoder{})
+
+		var csrfCookieEncoderHashKey = []byte("fake-csrf-hash-secret") // TODO replace this secret
+		var csrfCookieEncoder = securecookie.New(csrfCookieEncoderHashKey, nil)
+		csrfCookieEncoder.SetSerializer(securecookie.JSONEncoder{})
 
 		m.providerHandlers[(issuerHostWithPath + oidc.WellKnownEndpointPath)] = discovery.NewHandler(issuer)
 
@@ -104,15 +108,15 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
 			csrftoken.Generate,
 			pkce.Generate,
 			nonce.Generate,
-			encoder,
-			encoder,
+			upstreamStateEncoder,
+			csrfCookieEncoder,
 		)
 
 		m.providerHandlers[(issuerHostWithPath + oidc.CallbackEndpointPath)] = callback.NewHandler(
 			m.idpListGetter,
 			oauthHelperWithKubeStorage,
-			encoder,
-			encoder,
+			upstreamStateEncoder,
+			csrfCookieEncoder,
 			issuer+oidc.CallbackEndpointPath,
 		)
 

internal/oidc/provider/manager/manager_test.go

@@ -104,17 +104,14 @@ func TestManager(t *testing.T) {
 			redirectStateParam := parsedLocation.Query().Get("state")
 			r.NotEmpty(redirectStateParam)
 
-			cookieValueAndDirectivesSplit := strings.SplitN(recorder.Header().Get("Set-Cookie"), ";", 2)
-			r.Len(cookieValueAndDirectivesSplit, 2)
-			cookieKeyValueSplit := strings.Split(cookieValueAndDirectivesSplit[0], "=")
-			r.Len(cookieKeyValueSplit, 2)
-			csrfCookieName := cookieKeyValueSplit[0]
-			r.Equal("__Host-pinniped-csrf", csrfCookieName)
-			csrfCookieValue := cookieKeyValueSplit[1]
-			r.NotEmpty(csrfCookieValue)
+			cookies := recorder.Result().Cookies() //nolint:bodyclose
+			r.Len(cookies, 1)
+			csrfCookie := cookies[0]
+			r.Equal("__Host-pinniped-csrf", csrfCookie.Name)
+			r.NotEmpty(csrfCookie.Value)
 
 			// Return the important parts of the response so we can use them in our next request to the callback endpoint
-			return csrfCookieValue, redirectStateParam
+			return csrfCookie.Value, redirectStateParam
 		}
 
 		requireCallbackRequestToBeHandled := func(requestIssuer, requestURLSuffix, csrfCookieValue string) string {