Repository: oauth2-proxy/oauth2-proxy
Pull request #715 (2020-08-07 09:27:54)

Ensure session times are not nil before printing them
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
This fixes a panic when printing a string that did not have a creation or expiry

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Fix a panic

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
N/A, no unit tests for these functions, will follow up with some later

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My change requires a change to the documentation or CHANGELOG.
- [ ] I have updated the documentation/CHANGELOG accordingly.
- [x] I have created a feature (non-master) branch for my PR.


JoelSpeed: @NickMeves Added some tests for the string function, PTAL

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 03e5de7e..2f16ce2f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
 
 ## Changes since v6.0.0
 
+- [#715](https://github.com/oauth2-proxy/oauth2-proxy/pull/715) Ensure session times are not nil before printing them (@JoelSpeed)
 - [#714](https://github.com/oauth2-proxy/oauth2-proxy/pull/714) Support passwords with Redis session stores (@NickMeves)
 - [#719](https://github.com/oauth2-proxy/oauth2-proxy/pull/719) Add Gosec fixes to areas that are intermittently flagged on PRs (@NickMeves)
 - [#718](https://github.com/oauth2-proxy/oauth2-proxy/pull/718) Allow Logging to stdout with separate Error Log Channel
diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go
index 2015df8c..e69c4db4 100644
--- a/pkg/apis/sessions/session_state.go
+++ b/pkg/apis/sessions/session_state.go
@@ -52,10 +52,10 @@ func (s *SessionState) String() string {
 	if s.IDToken != "" {
 		o += " id_token:true"
 	}
-	if !s.CreatedAt.IsZero() {
+	if s.CreatedAt != nil && !s.CreatedAt.IsZero() {
 		o += fmt.Sprintf(" created:%s", s.CreatedAt)
 	}
-	if !s.ExpiresOn.IsZero() {
+	if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() {
 		o += fmt.Sprintf(" expires:%s", s.ExpiresOn)
 	}
 	if s.RefreshToken != "" {


Test patch:
diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go
index ac554c60..08216b26 100644
--- a/pkg/apis/sessions/session_state_test.go
+++ b/pkg/apis/sessions/session_state_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
+	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -16,6 +17,101 @@ func timePtr(t time.Time) *time.Time {
 	return &t
 }
 
+func TestString(t *testing.T) {
+	g := NewWithT(t)
+	created, err := time.Parse(time.RFC3339, "2000-01-01T00:00:00Z")
+	g.Expect(err).ToNot(HaveOccurred())
+	expires, err := time.Parse(time.RFC3339, "2000-01-01T01:00:00Z")
+	g.Expect(err).ToNot(HaveOccurred())
+
+	testCases := []struct {
+		name         string
+		sessionState *SessionState
+		expected     string
+	}{
+		{
+			name: "Minimal Session",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user}",
+		},
+		{
+			name: "Full Session",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				CreatedAt:         &created,
+				ExpiresOn:         &expires,
+				AccessToken:       "access.token",
+				IDToken:           "id.token",
+				RefreshToken:      "refresh.token",
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user token:true id_token:true created:2000-01-01 00:00:00 +0000 UTC expires:2000-01-01 01:00:00 +0000 UTC refresh_token:true}",
+		},
+		{
+			name: "With a CreatedAt",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				CreatedAt:         &created,
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user created:2000-01-01 00:00:00 +0000 UTC}",
+		},
+		{
+			name: "With an ExpiresOn",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				ExpiresOn:         &expires,
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user expires:2000-01-01 01:00:00 +0000 UTC}",
+		},
+		{
+			name: "With an AccessToken",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				AccessToken:       "access.token",
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user token:true}",
+		},
+		{
+			name: "With an IDToken",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				IDToken:           "id.token",
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user id_token:true}",
+		},
+		{
+			name: "With a RefreshToken",
+			sessionState: &SessionState{
+				Email:             "email@email.email",
+				User:              "some.user",
+				PreferredUsername: "preferred.user",
+				RefreshToken:      "refresh.token",
+			},
+			expected: "Session{email:email@email.email user:some.user PreferredUsername:preferred.user refresh_token:true}",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gs := NewWithT(t)
+			gs.Expect(tc.sessionState.String()).To(Equal(tc.expected))
+		})
+	}
+}
+
 func TestIsExpired(t *testing.T) {
 	s := &SessionState{ExpiresOn: timePtr(time.Now().Add(time.Duration(-1) * time.Minute))}
 	assert.Equal(t, true, s.IsExpired())