policyfile: support SSH check period 'always'

This also renames the 'Duration' type to 'SSHCheckPeriod' to clarify that
it is used exclusively for the 'CheckPeriod' on 'ACLSSH'.

Updates tailscale/tailscale-client-go#128

Signed-off-by: Percy Wegmann <[email protected]>

Author: oxtoacart

client.go

@@ -369,31 +369,6 @@ func ErrorData(err error) []APIErrorData {
 	return nil
 }
 
-// Duration wraps a [time.Duration], allowing it to be JSON marshalled as a string like "20h" rather than
-// a numeric value.
-type Duration time.Duration
-
-func (d Duration) String() string {
-	return time.Duration(d).String()
-}
-
-func (d Duration) MarshalText() ([]byte, error) {
-	return []byte(d.String()), nil
-}
-
-func (d *Duration) UnmarshalText(b []byte) error {
-	text := string(b)
-	if text == "" {
-		text = "0s"
-	}
-	pd, err := time.ParseDuration(text)
-	if err != nil {
-		return err
-	}
-	*d = Duration(pd)
-	return nil
-}
-
 // PointerTo returns a pointer to the given value.
 // Pointers are used in PATCH requests to distinguish between specified and unspecified values.
 func PointerTo[T any](value T) *T {

policyfile.go

@@ -7,8 +7,48 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"time"
 )
 
+// CheckPeriodAlways is a magic value corresponding to the [SSHCheckPeriod]
+// "always". It indicates that re-authorization is required on every login.
+const CheckPeriodAlways SSHCheckPeriod = -1
+
+const checkPeriodAlways = "always"
+
+// SSHCheckPeriod wraps a [time.Duration], allowing it to be JSON marshalled as
+// a string like "20h" rather than a numeric value. It also supports the
+// special value "always", which forces a check on every connection.
+type SSHCheckPeriod time.Duration
+
+func (d SSHCheckPeriod) String() string {
+	return time.Duration(d).String()
+}
+
+func (d SSHCheckPeriod) MarshalText() ([]byte, error) {
+	if d == CheckPeriodAlways {
+		return []byte(checkPeriodAlways), nil
+	}
+	return []byte(d.String()), nil
+}
+
+func (d *SSHCheckPeriod) UnmarshalText(b []byte) error {
+	text := string(b)
+	if text == checkPeriodAlways {
+		*d = SSHCheckPeriod(CheckPeriodAlways)
+		return nil
+	}
+	if text == "" {
+		text = "0s"
+	}
+	pd, err := time.ParseDuration(text)
+	if err != nil {
+		return err
+	}
+	*d = SSHCheckPeriod(pd)
+	return nil
+}
+
 // PolicyFileResource provides access to https://tailscale.com/api#tag/policyfile.
 type PolicyFileResource struct {
 	*Client
@@ -98,13 +138,13 @@ type ACLDERPNode struct {
 }
 
 type ACLSSH struct {
-	Action          string   `json:"action,omitempty" hujson:"Action,omitempty"`
-	Users           []string `json:"users,omitempty" hujson:"Users,omitempty"`
-	Source          []string `json:"src,omitempty" hujson:"Src,omitempty"`
-	Destination     []string `json:"dst,omitempty" hujson:"Dst,omitempty"`
-	CheckPeriod     Duration `json:"checkPeriod,omitempty" hujson:"CheckPeriod,omitempty"`
-	Recorder        []string `json:"recorder,omitempty" hujson:"Recorder,omitempty"`
-	EnforceRecorder bool     `json:"enforceRecorder,omitempty" hujson:"EnforceRecorder,omitempty"`
+	Action          string         `json:"action,omitempty" hujson:"Action,omitempty"`
+	Users           []string       `json:"users,omitempty" hujson:"Users,omitempty"`
+	Source          []string       `json:"src,omitempty" hujson:"Src,omitempty"`
+	Destination     []string       `json:"dst,omitempty" hujson:"Dst,omitempty"`
+	CheckPeriod     SSHCheckPeriod `json:"checkPeriod,omitempty" hujson:"CheckPeriod,omitempty"`
+	Recorder        []string       `json:"recorder,omitempty" hujson:"Recorder,omitempty"`
+	EnforceRecorder bool           `json:"enforceRecorder,omitempty" hujson:"EnforceRecorder,omitempty"`
 }
 
 type NodeAttrGrant struct {

policyfile_test.go

@@ -114,7 +114,7 @@ func TestACL_Unmarshal(t *testing.T) {
 						Source:      []string{"tag:logging"},
 						Destination: []string{"tag:prod"},
 						Users:       []string{"root", "autogroup:nonroot"},
-						CheckPeriod: Duration(time.Hour * 20),
+						CheckPeriod: SSHCheckPeriod(time.Hour * 20),
 					},
 				},
 			},
@@ -194,7 +194,14 @@ func TestACL_Unmarshal(t *testing.T) {
 						Source:      []string{"tag:logging"},
 						Destination: []string{"tag:prod"},
 						Users:       []string{"root", "autogroup:nonroot"},
-						CheckPeriod: Duration(time.Hour * 20),
+						CheckPeriod: SSHCheckPeriod(time.Hour * 20),
+					},
+					{
+						Action:      "accept",
+						Source:      []string{"tag:logging2"},
+						Destination: []string{"tag:prod2"},
+						Users:       []string{"root", "autogroup:nonroot"},
+						CheckPeriod: CheckPeriodAlways,
 					},
 				},
 				Tests: []ACLTest{
@@ -275,6 +282,7 @@ func TestClient_SetACL(t *testing.T) {
 	assert.NoError(t, json.Unmarshal(server.Body.Bytes(), &actualACL))
 	assert.EqualValues(t, expectedACL, actualACL)
 }
+
 func TestClient_SetACL_HuJSON(t *testing.T) {
 	t.Parallel()
 
@@ -384,3 +392,35 @@ func TestClient_RawACL(t *testing.T) {
 	assert.EqualValues(t, "application/hujson", server.Header.Get("Accept"))
 	assert.EqualValues(t, "/api/v2/tailnet/example.com/acl", server.Path)
 }
+
+func TestSSHCheckPeriod(t *testing.T) {
+	testCases := []struct {
+		inStr  string
+		period SSHCheckPeriod
+		outStr string
+	}{
+		{"1h", SSHCheckPeriod(1 * time.Hour), "1h0m0s"},
+		{"", 0, "0s"},
+		{checkPeriodAlways, CheckPeriodAlways, checkPeriodAlways},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.inStr, func(t *testing.T) {
+			var got SSHCheckPeriod
+			if err := got.UnmarshalText([]byte(tc.inStr)); err != nil {
+				t.Fatalf("failed to marshal: %s", err)
+			}
+			if got != tc.period {
+				t.Fatalf("want period %s, got period %s", tc.period, got)
+			}
+			gotBytes, err := got.MarshalText()
+			if err != nil {
+				t.Fatalf("failed to marshal: %s", err)
+			}
+			gotStr := string(gotBytes)
+			if gotStr != tc.outStr {
+				t.Fatalf("want string %q, got string %q", tc.outStr, gotStr)
+			}
+		})
+	}
+}

testdata/acl.hujson

@@ -58,5 +58,12 @@
       "users": ["root", "autogroup:nonroot"],
       "checkPeriod": "20h"
     },
+    {
+      "action": "accept",
+      "src": ["tag:logging2"],
+      "dst": ["tag:prod2"],
+      "users": ["root", "autogroup:nonroot"],
+      "checkPeriod": "always"
+    },
   ]
 }