Admin: token expiration colors and calculations (#45231)

* token expiration colorign and calculations

* Update public/app/features/serviceaccounts/ServiceAccountTokensTable.tsx

* removed unused calculation for expiry

* optional attribute

* fix: typo

* implement failing test :thumpsup:

* tests

* refactor: tests to use assertify

* tiem

* refactor: remote porntf

* refactor: make test NOT sleep 1 sec for all builds :D

Co-authored-by: J Guerreiro <joao.guerreiro@grafana.com>

pkg/services/serviceaccounts/api/api_test.go

@@ -17,14 +17,13 @@ import (
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 )
 
 var (
@@ -63,7 +62,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
 		}
 		serviceAccountRequestScenario(t, http.MethodDelete, serviceaccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
 			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
-			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, database.NewServiceAccountsStore(store))
 			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.Id))).Code
 			require.Equal(t, testcase.expectedCode, actual)
 		})
@@ -87,7 +86,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
 		}
 		serviceAccountRequestScenario(t, http.MethodDelete, serviceaccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
 			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
-			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, database.NewServiceAccountsStore(store))
 			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.Id)).Code
 			require.Equal(t, testcase.expectedCode, actual)
 		})
@@ -99,8 +98,11 @@ func serviceAccountRequestScenario(t *testing.T, httpMethod string, endpoint str
 	fn(httpMethod, endpoint, user)
 }
 
-func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, routerRegister routing.RouteRegister, acmock *accesscontrolmock.Mock, sqlStore *sqlstore.SQLStore) *web.Mux {
+func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
-	a := NewServiceAccountsAPI(setting.NewCfg(), svc, acmock, routerRegister, database.NewServiceAccountsStore(sqlStore), sqlStore)
+	routerRegister routing.RouteRegister,
+	acmock *accesscontrolmock.Mock,
+	sqlStore *sqlstore.SQLStore, saStore serviceaccounts.Store) *web.Mux {
+	a := NewServiceAccountsAPI(setting.NewCfg(), svc, acmock, routerRegister, saStore, sqlStore)
 	a.RegisterAPIEndpoints(featuremgmt.WithFeatures(featuremgmt.FlagServiceAccounts))
 
 	a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration
@@ -190,7 +192,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
 					createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
 					scopeID = int(createdUser.Id)
 				}
-				server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store)
+				server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store))
 
 				actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, scopeID))
 
@@ -300,7 +302,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
 					createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
 					scopeID = int(createdUser.Id)
 				}
-				server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store)
+				server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store))
 
 				var rawBody io.Reader = http.NoBody
 				if tc.body != nil {

pkg/services/serviceaccounts/api/token.go

@@ -16,6 +16,26 @@ import (
 
 const failedToDeleteMsg = "Failed to delete API key"
 
+type TokenDTO struct {
+	Id                     int64           `json:"id"`
+	Name                   string          `json:"name"`
+	Role                   models.RoleType `json:"role"`
+	Created                *time.Time      `json:"created"`
+	Expiration             *time.Time      `json:"expiration"`
+	SecondsUntilExpiration *float64        `json:"secondsUntilExpiration"`
+	HasExpired             bool            `json:"hasExpired"`
+}
+
+func hasExpired(expiration *int64) bool {
+	if expiration == nil {
+		return false
+	}
+	v := time.Unix(*expiration, 0)
+	return (v).Before(time.Now())
+}
+
+const sevenDaysAhead = 7 * 24 * time.Hour
+
 func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response {
 	saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64)
 	if err != nil {
@@ -23,18 +43,28 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo
 	}
 
 	if saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgId, saID); err == nil {
-		result := make([]*models.ApiKeyDTO, len(saTokens))
+		result := make([]*TokenDTO, len(saTokens))
 		for i, t := range saTokens {
 			var expiration *time.Time = nil
+			var secondsUntilExpiration float64 = 0
+
+			isExpired := hasExpired(t.Expires)
 			if t.Expires != nil {
 				v := time.Unix(*t.Expires, 0)
 				expiration = &v
+				if !isExpired && (*expiration).Before(time.Now().Add(sevenDaysAhead)) {
+					secondsUntilExpiration = time.Until(*expiration).Seconds()
+				}
 			}
-			result[i] = &models.ApiKeyDTO{
+
-				Id:         t.Id,
+			result[i] = &TokenDTO{
-				Name:       t.Name,
+				Id:                     t.Id,
-				Role:       t.Role,
+				Name:                   t.Name,
-				Expiration: expiration,
+				Role:                   t.Role,
+				Created:                &t.Created,
+				Expiration:             expiration,
+				SecondsUntilExpiration: &secondsUntilExpiration,
+				HasExpired:             isExpired,
 			}
 		}
 

pkg/services/serviceaccounts/api/token_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/bus"
@@ -17,6 +18,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/web"
@@ -29,7 +31,7 @@ const (
 	serviceaccountIDTokensDetailPath = "/api/serviceaccounts/%v/tokens/%v" // #nosec G101
 )
 
-func createTokenforSA(t *testing.T, keyName string, orgID int64, saID int64) *models.ApiKey {
+func createTokenforSA(t *testing.T, keyName string, orgID int64, saID int64, secondsToLive int64) *models.ApiKey {
 	key, err := apikeygen.New(orgID, keyName)
 	require.NoError(t, err)
 	cmd := models.AddApiKeyCommand{
@@ -37,13 +39,12 @@ func createTokenforSA(t *testing.T, keyName string, orgID int64, saID int64) *mo
 		Role:             "Viewer",
 		OrgId:            orgID,
 		Key:              key.HashedKey,
-		SecondsToLive:    0,
+		SecondsToLive:    secondsToLive,
 		ServiceAccountId: &saID,
 		Result:           &models.ApiKey{},
 	}
 	err = bus.Dispatch(context.Background(), &cmd)
 	require.NoError(t, err)
-
 	return cmd.Result
 }
 
@@ -129,7 +130,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
 				bodyString = string(b)
 			}
 
-			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store))
 			actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString))
 
 			actualCode := actual.Code
@@ -215,11 +216,11 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			token := createTokenforSA(t, tc.keyName, sa.OrgId, sa.Id)
+			token := createTokenforSA(t, tc.keyName, sa.OrgId, sa.Id, 1)
 
 			endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.Id, token.Id)
 			bodyString := ""
-			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, database.NewServiceAccountsStore(store))
 			actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString))
 
 			actualCode := actual.Code
@@ -238,3 +239,122 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
 		})
 	}
 }
+
+type saStoreMockTokens struct {
+	serviceaccounts.Store
+	saAPIKeys []*models.ApiKey
+}
+
+func (s *saStoreMockTokens) ListTokens(ctx context.Context, orgID, saID int64) ([]*models.ApiKey, error) {
+	return s.saAPIKeys, nil
+}
+
+func TestServiceAccountsAPI_ListTokens(t *testing.T) {
+	store := sqlstore.InitTestDB(t)
+	svcmock := tests.ServiceAccountMock{}
+	sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
+
+	type testCreateSAToken struct {
+		desc                      string
+		tokens                    []*models.ApiKey
+		expectedHasExpired        bool
+		expectedResponseBodyField string
+		expectedCode              int
+		acmock                    *accesscontrolmock.Mock
+	}
+
+	var saId int64 = 1
+	var timeInFuture = time.Now().Add(time.Second * 100).Unix()
+	var timeInPast = time.Now().Add(-time.Second * 100).Unix()
+
+	testCases := []testCreateSAToken{
+		{
+			desc: "should be able to list serviceaccount with no expiration date",
+			tokens: []*models.ApiKey{{
+				Id:               1,
+				OrgId:            1,
+				ServiceAccountId: &saId,
+				Expires:          nil,
+				Name:             "Test1",
+			}},
+			acmock: tests.SetupMockAccesscontrol(
+				t,
+				func(c context.Context, siu *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
+					return []*accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
+				},
+				false,
+			),
+			expectedHasExpired:        false,
+			expectedResponseBodyField: "hasExpired",
+			expectedCode:              http.StatusOK,
+		},
+		{
+			desc: "should be able to list serviceaccount with secondsUntilExpiration",
+			tokens: []*models.ApiKey{{
+				Id:               1,
+				OrgId:            1,
+				ServiceAccountId: &saId,
+				Expires:          &timeInFuture,
+				Name:             "Test2",
+			}},
+			acmock: tests.SetupMockAccesscontrol(
+				t,
+				func(c context.Context, siu *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
+					return []*accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
+				},
+				false,
+			),
+			expectedHasExpired:        false,
+			expectedResponseBodyField: "secondsUntilExpiration",
+			expectedCode:              http.StatusOK,
+		},
+		{
+			desc: "should be able to list serviceaccount with expired token",
+			tokens: []*models.ApiKey{{
+				Id:               1,
+				OrgId:            1,
+				ServiceAccountId: &saId,
+				Expires:          &timeInPast,
+				Name:             "Test3",
+			}},
+			acmock: tests.SetupMockAccesscontrol(
+				t,
+				func(c context.Context, siu *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
+					return []*accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
+				},
+				false,
+			),
+			expectedHasExpired:        true,
+			expectedResponseBodyField: "secondsUntilExpiration",
+			expectedCode:              http.StatusOK,
+		},
+	}
+
+	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder {
+		req, err := http.NewRequest(httpMethod, requestpath, requestBody)
+		require.NoError(t, err)
+		req.Header.Add("Content-Type", "application/json")
+		recorder := httptest.NewRecorder()
+		server.ServeHTTP(recorder, req)
+		return recorder
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			endpoint := fmt.Sprintf(serviceaccountIDPath+"/tokens", sa.Id)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, &saStoreMockTokens{saAPIKeys: tc.tokens})
+			actual := requestResponse(server, http.MethodGet, endpoint, http.NoBody)
+
+			actualCode := actual.Code
+			actualBody := []map[string]interface{}{}
+
+			_ = json.Unmarshal(actual.Body.Bytes(), &actualBody)
+			require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
+
+			require.Equal(t, tc.expectedCode, actualCode)
+			require.Equal(t, tc.expectedHasExpired, actualBody[0]["hasExpired"])
+			_, exists := actualBody[0][tc.expectedResponseBodyField]
+			require.Equal(t, exists, true)
+		})
+	}
+}

pkg/services/serviceaccounts/database/database.go

@@ -128,9 +128,9 @@ func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgID int64,
 	err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
 		var sess *xorm.Session
 
-		sess = dbSession.Limit(100, 0).
+		sess = dbSession.
 			Join("inner", "user", "user.id = api_key.service_account_id").
-			Where("user.org_id=? AND user.id=? AND ( expires IS NULL or expires >= ?)", orgID, serviceAccountID, time.Now().Unix()).
+			Where("user.org_id=? AND user.id=?", orgID, serviceAccountID).
 			Asc("name")
 
 		return sess.Find(&result)

public/app/features/serviceaccounts/ServiceAccountTokensTable.tsx

@@ -1,5 +1,5 @@
 import React, { FC } from 'react';
-import { DeleteButton, Icon, Tooltip, useTheme2 } from '@grafana/ui';
+import { DeleteButton, Icon, Tooltip, useStyles2, useTheme2 } from '@grafana/ui';
 import { dateTimeFormat, GrafanaTheme2, TimeZone } from '@grafana/data';
 
 import { ApiKey } from '../../types';
@@ -22,25 +22,19 @@ export const ServiceAccountTokensTable: FC<Props> = ({ tokens, timeZone, onDelet
           <tr>
             <th>Name</th>
             <th>Expires</th>
+            <th>Created</th>
             <th style={{ width: '34px' }} />
           </tr>
         </thead>
         <tbody>
           {tokens.map((key) => {
-            const isExpired = !!(key.expiration && Date.now() > new Date(key.expiration).getTime());
             return (
-              <tr key={key.id} className={styles.tableRow(isExpired)}>
+              <tr key={key.id} className={styles.tableRow(key.hasExpired)}>
                 <td>{key.name}</td>
                 <td>
-                  {formatDate(timeZone, key.expiration)}
+                  <TokenExpiration timeZone={timeZone} token={key} />
-                  {isExpired && (
-                    <span className={styles.tooltipContainer}>
-                      <Tooltip content="This API key has expired.">
-                        <Icon name="exclamation-triangle" />
-                      </Tooltip>
-                    </span>
-                  )}
                 </td>
+                <td>{formatDate(timeZone, key.created)}</td>
                 <td>
                   <DeleteButton aria-label="Delete API key" size="sm" onConfirm={() => onDelete(key)} />
                 </td>
@@ -60,11 +54,61 @@ function formatDate(timeZone: TimeZone, expiration?: string): string {
   return dateTimeFormat(expiration, { timeZone });
 }
 
+function formatSecondsLeftUntilExpiration(secondsUntilExpiration: number): string {
+  const days = Math.floor(secondsUntilExpiration / (3600 * 24));
+  const daysFormat = days > 1 ? `${days} days` : `${days} day`;
+  return `Expires in ${daysFormat}`;
+}
+
+interface TokenExpirationProps {
+  timeZone: TimeZone;
+  token: ApiKey;
+}
+
+const TokenExpiration = ({ timeZone, token }: TokenExpirationProps) => {
+  const styles = useStyles2(getStyles);
+  if (!token.expiration) {
+    return <span className={styles.neverExpire}>Never</span>;
+  }
+  if (token.secondsUntilExpiration) {
+    return (
+      <span className={styles.secondsUntilExpiration}>
+        {formatSecondsLeftUntilExpiration(token.secondsUntilExpiration)}
+      </span>
+    );
+  }
+  if (token.hasExpired) {
+    return (
+      <span className={styles.hasExpired}>
+        Expired
+        <span className={styles.tooltipContainer}>
+          <Tooltip content="This API key has expired.">
+            <Icon name="exclamation-triangle" className={styles.toolTipIcon} />
+          </Tooltip>
+        </span>
+      </span>
+    );
+  }
+  return <span>{formatDate(timeZone, token.expiration)}</span>;
+};
+
 const getStyles = (theme: GrafanaTheme2) => ({
-  tableRow: (isExpired: boolean) => css`
+  tableRow: (hasExpired: boolean | undefined) => css`
-    color: ${isExpired ? theme.colors.text.secondary : theme.colors.text.primary};
+    color: ${hasExpired ? theme.colors.text.secondary : theme.colors.text.primary};
   `,
   tooltipContainer: css`
     margin-left: ${theme.spacing(1)};
   `,
+  toolTipIcon: css`
+    color: ${theme.colors.error.text};
+  `,
+  secondsUntilExpiration: css`
+    color: ${theme.colors.warning.text};
+  `,
+  hasExpired: css`
+    color: ${theme.colors.error.text};
+  `,
+  neverExpire: css`
+    color: ${theme.colors.text.secondary};
+  `,
 });

public/app/types/apiKeys.ts

@@ -6,6 +6,9 @@ export interface ApiKey {
   role: OrgRole;
   secondsToLive: number | null;
   expiration?: string;
+  secondsUntilExpiration?: number;
+  hasExpired?: boolean;
+  created?: string;
 }
 
 export interface NewApiKey {