Merge pull request #36 from mattmoyer/fix-expiration-handling

Fix expiration timestamp handling

internal/registry/loginrequest/rest.go

@@ -138,10 +138,11 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 		return failureResponse(), nil
 	}
 
+	expires := metav1.NewTime(time.Now().UTC().Add(clientCertificateTTL))
 	return &placeholderapi.LoginRequest{
 		Status: placeholderapi.LoginRequestStatus{
 			Credential: &placeholderapi.LoginRequestCredential{
-				ExpirationTimestamp:   nil,
+				ExpirationTimestamp:   &expires,
 				ClientCertificateData: string(certPEM),
 				ClientKeyData:         string(keyPEM),
 			},

internal/registry/loginrequest/rest_test.go

@@ -153,6 +153,13 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test
 	response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken))
 
 	require.NoError(t, err)
+	require.IsType(t, &placeholderapi.LoginRequest{}, response)
+
+	expires := response.(*placeholderapi.LoginRequest).Status.Credential.ExpirationTimestamp
+	require.NotNil(t, expires)
+	require.InDelta(t, time.Now().Add(1*time.Hour).Unix(), expires.Unix(), 5)
+	response.(*placeholderapi.LoginRequest).Status.Credential.ExpirationTimestamp = nil
+
 	require.Equal(t, response, &placeholderapi.LoginRequest{
 		Status: placeholderapi.LoginRequestStatus{
 			User: &placeholderapi.User{

pkg/client/client.go

@@ -132,6 +132,8 @@ func ExchangeToken(ctx context.Context, token, caBundle, apiEndpoint string) (*C
 		Kind       string `json:"kind"`
 		Status     struct {
 			Credential *struct {
+				ExpirationTimestamp   string `json:"expirationTimestamp"`
+				Token                 string `json:"token"`
 				ClientCertificateData string `json:"clientCertificateData"`
 				ClientKeyData         string `json:"clientKeyData"`
 			}
@@ -146,8 +148,18 @@ func ExchangeToken(ctx context.Context, token, caBundle, apiEndpoint string) (*C
 		return nil, fmt.Errorf("%w: %s", ErrLoginFailed, respBody.Status.Message)
 	}
 
-	return &Credential{
+	result := Credential{
+		Token:                 respBody.Status.Credential.Token,
 		ClientCertificateData: respBody.Status.Credential.ClientCertificateData,
 		ClientKeyData:         respBody.Status.Credential.ClientKeyData,
-	}, nil
+	}
+	if str := respBody.Status.Credential.ExpirationTimestamp; str != "" {
+		expiration, err := time.Parse(time.RFC3339, str)
+		if err != nil {
+			return nil, fmt.Errorf("invalid login response: %w", err)
+		}
+		result.ExpirationTimestamp = &expiration
+	}
+
+	return &result, nil
 }

pkg/client/client_test.go

@@ -151,6 +151,33 @@ func TestExchangeToken(t *testing.T) {
 		require.Nil(t, got)
 	})
 
+	t.Run("invalid timestamp failure", func(t *testing.T) {
+		t.Parallel()
+		// Start a test server that returns success but with an error message
+		caBundle, endpoint := startTestServer(t, func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("content-type", "application/json")
+			w.WriteHeader(http.StatusCreated)
+			_, _ = w.Write([]byte(`
+				{
+				  "kind": "LoginRequest",
+				  "apiVersion": "placeholder.suzerain-io.github.io/v1alpha1",
+				  "metadata": {
+					"creationTimestamp": null
+				  },
+				  "spec": {},
+				  "status": {
+					"credential": {
+					  "expirationTimestamp": "invalid"
+					}
+				  }
+				}`))
+		})
+
+		got, err := ExchangeToken(ctx, "", caBundle, endpoint)
+		require.EqualError(t, err, `invalid login response: parsing time "invalid" as "2006-01-02T15:04:05Z07:00": cannot parse "invalid" as "2006"`)
+		require.Nil(t, got)
+	})
+
 	t.Run("success", func(t *testing.T) {
 		t.Parallel()
 
@@ -172,8 +199,8 @@ func TestExchangeToken(t *testing.T) {
 				  "spec": {
 					"type": "token",
 					"token": {
-                      "value": "test-token"
-                    }
+					  "value": "test-token"
+					}
 				  },
 				  "status": {}
 				}`,
@@ -192,6 +219,8 @@ func TestExchangeToken(t *testing.T) {
 				  "spec": {},
 				  "status": {
 					"credential": {
+					  "expirationTimestamp": "2020-07-30T15:52:01Z",
+					  "token": "test-token",
 					  "clientCertificateData": "test-certificate",
 					  "clientKeyData": "test-key"
 					}
@@ -201,7 +230,10 @@ func TestExchangeToken(t *testing.T) {
 
 		got, err := ExchangeToken(ctx, "test-token", caBundle, endpoint)
 		require.NoError(t, err)
+		expires := time.Date(2020, 07, 30, 15, 52, 1, 0, time.UTC)
 		require.Equal(t, &Credential{
+			ExpirationTimestamp:   &expires,
+			Token:                 "test-token",
 			ClientCertificateData: "test-certificate",
 			ClientKeyData:         "test-key",
 		}, got)

test/integration/client_test.go

@@ -67,6 +67,8 @@ func TestClient(t *testing.T) {
 	clientConfig := library.NewClientConfig(t)
 	resp, err := client.ExchangeToken(ctx, tmcClusterToken, string(clientConfig.CAData), clientConfig.Host)
 	require.NoError(t, err)
+	require.NotNil(t, resp.ExpirationTimestamp)
+	require.InDelta(t, time.Until(*resp.ExpirationTimestamp), 1*time.Hour, float64(5*time.Second))
 
 	// Create a client using the certificate and key returned by the token exchange.
 	validClient := library.NewClientsetWithConfig(t, library.NewClientConfigWithCertAndKey(t, resp.ClientCertificateData, resp.ClientKeyData))

test/integration/loginrequest_test.go

@@ -55,7 +55,8 @@ func TestSuccessfulLoginRequest(t *testing.T) {
 	require.Empty(t, response.Status.Credential.Token)
 	require.NotEmpty(t, response.Status.Credential.ClientCertificateData)
 	require.NotEmpty(t, response.Status.Credential.ClientKeyData)
-	require.Nil(t, response.Status.Credential.ExpirationTimestamp)
+	require.NotNil(t, response.Status.Credential.ExpirationTimestamp)
+	require.InDelta(t, time.Until(response.Status.Credential.ExpirationTimestamp.Time), 1*time.Hour, float64(5*time.Second))
 	require.NotNil(t, response.Status.User)
 	require.NotEmpty(t, response.Status.User.Name)
 	require.Contains(t, response.Status.User.Groups, "tmc:member")