IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Configuration: You can now see your expired API keys if you have no active ones (#42452)

Browse Source 
* Configuration: Always display expired API keys

* Use exclamation-triangle instead

* Reintroduce toggle, move logic into store and call both endpoints

* Handle apiKeys without TTL

* Remove backend changes and make checks in frontend instead
This commit is contained in:
Ashley Harrison 2021-12-16 11:46:09 +00:00 committed by GitHub
parent 609a1aa8ad
commit 80b55f09ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 280 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
public/app/features/api-keys/ApiKeysPage.test.tsx
View File

@ -14,6 +14,7 @@ const setup = (propOverrides: Partial<Props>) => {
const loadApiKeysMock = jest.fn();
const deleteApiKeyMock = jest.fn();
const addApiKeyMock = jest.fn();
const toggleIncludeExpiredMock = jest.fn();
const setSearchQueryMock = mockToolkitActionCreator(setSearchQuery);
const props: Props = {
navModel: {
@ -33,21 +34,31 @@ const setup = (propOverrides: Partial<Props>) => {
addApiKey: addApiKeyMock,
apiKeysCount: 0,
timeZone: 'utc',
includeExpired: false,
includeExpiredDisabled: false,
toggleIncludeExpired: toggleIncludeExpiredMock,
};
Object.assign(props, propOverrides);
const { rerender } = render(<ApiKeysPageUnconnected {...props} />);
return { rerender, props, loadApiKeysMock, setSearchQueryMock, deleteApiKeyMock, addApiKeyMock };
return {
rerender,
props,
loadApiKeysMock,
setSearchQueryMock,
deleteApiKeyMock,
addApiKeyMock,
toggleIncludeExpiredMock,
};
};
describe('ApiKeysPage', () => {
silenceConsoleOutput();
describe('when mounted', () => {
it('then it should call loadApiKeys without expired', () => {
it('then it should call loadApiKeys', () => {
const { loadApiKeysMock } = setup({});
expect(loadApiKeysMock).toHaveBeenCalledTimes(1);
expect(loadApiKeysMock).toHaveBeenCalledWith(false);
});
});
@ -82,19 +93,12 @@ describe('ApiKeysPage', () => {
});
describe('when a user toggles the Show expired toggle', () => {
it('then it should call loadApiKeys with correct parameters', async () => {
it('then it should dispatch toggleIncludeExpired', async () => {
const apiKeys = getMultipleMockKeys(3);
const { loadApiKeysMock } = setup({ apiKeys, apiKeysCount: apiKeys.length, hasFetched: true });
const { toggleIncludeExpiredMock } = setup({ apiKeys, apiKeysCount: apiKeys.length, hasFetched: true });
loadApiKeysMock.mockClear();
toggleShowExpired();
expect(loadApiKeysMock).toHaveBeenCalledTimes(1);
expect(loadApiKeysMock).toHaveBeenCalledWith(true);
loadApiKeysMock.mockClear();
toggleShowExpired();
expect(loadApiKeysMock).toHaveBeenCalledTimes(1);
expect(loadApiKeysMock).toHaveBeenCalledWith(false);
expect(toggleIncludeExpiredMock).toHaveBeenCalledTimes(1);
});
});
@ -128,7 +132,7 @@ describe('ApiKeysPage', () => {
expect(within(firstRow).getByRole('button', { name: /delete$/i })).toBeInTheDocument();
userEvent.click(within(firstRow).getByRole('button', { name: /delete$/i }));
expect(deleteApiKeyMock).toHaveBeenCalledTimes(1);
expect(deleteApiKeyMock).toHaveBeenCalledWith(1, false);
expect(deleteApiKeyMock).toHaveBeenCalledWith(1);
toggleShowExpired();
@ -140,7 +144,7 @@ describe('ApiKeysPage', () => {
skipPointerEventsCheck: true,
});
expect(deleteApiKeyMock).toHaveBeenCalledTimes(1);
expect(deleteApiKeyMock).toHaveBeenCalledWith(2, true);
expect(deleteApiKeyMock).toHaveBeenCalledWith(2);
});
});
@ -151,7 +155,7 @@ describe('ApiKeysPage', () => {
addApiKeyMock.mockClear();
userEvent.click(screen.getByTestId(selectors.components.CallToActionCard.buttonV2('New API key')));
await addAndVerifyApiKey(addApiKeyMock, false);
await addAndVerifyApiKey(addApiKeyMock);
});
});
@ -162,13 +166,13 @@ describe('ApiKeysPage', () => {
addApiKeyMock.mockClear();
userEvent.click(screen.getByRole('button', { name: /add api key/i }));
await addAndVerifyApiKey(addApiKeyMock, false);
await addAndVerifyApiKey(addApiKeyMock);
toggleShowExpired();
addApiKeyMock.mockClear();
userEvent.click(screen.getByRole('button', { name: /add api key/i }));
await addAndVerifyApiKey(addApiKeyMock, true);
await addAndVerifyApiKey(addApiKeyMock);
});
});
@ -190,11 +194,11 @@ describe('ApiKeysPage', () => {
});
function toggleShowExpired() {
expect(screen.queryByLabelText(/show expired/i)).toBeInTheDocument();
userEvent.click(screen.getByLabelText(/show expired/i));
expect(screen.queryByLabelText(/include expired keys/i)).toBeInTheDocument();
userEvent.click(screen.getByLabelText(/include expired keys/i));
}
async function addAndVerifyApiKey(addApiKeyMock: jest.Mock, includeExpired: boolean) {
async function addAndVerifyApiKey(addApiKeyMock: jest.Mock) {
expect(screen.getByRole('heading', { name: /add api key/i })).toBeInTheDocument();
expect(screen.getByPlaceholderText(/name/i)).toBeInTheDocument();
expect(screen.getByPlaceholderText(/1d/i)).toBeInTheDocument();
@ -204,9 +208,5 @@ async function addAndVerifyApiKey(addApiKeyMock: jest.Mock, includeExpired: bool
userEvent.type(screen.getByPlaceholderText(/1d/i), '60s');
userEvent.click(screen.getByRole('button', { name: /^add$/i }));
expect(addApiKeyMock).toHaveBeenCalledTimes(1);
expect(addApiKeyMock).toHaveBeenCalledWith(
{ name: 'Test', role: 'Viewer', secondsToLive: 60 },
expect.anything(),
includeExpired
);
expect(addApiKeyMock).toHaveBeenCalledWith({ name: 'Test', role: 'Viewer', secondsToLive: 60 }, expect.anything());
}

33
public/app/features/api-keys/ApiKeysPage.tsx
View File

@ -3,8 +3,8 @@ import { connect, ConnectedProps } from 'react-redux';
// Utils
import { ApiKey, NewApiKey, StoreState } from 'app/types';
import { getNavModel } from 'app/core/selectors/navModel';
import { getApiKeys, getApiKeysCount } from './state/selectors';
import { addApiKey, deleteApiKey, loadApiKeys } from './state/actions';
import { getApiKeys, getApiKeysCount, getIncludeExpired, getIncludeExpiredDisabled } from './state/selectors';
import { addApiKey, deleteApiKey, loadApiKeys, toggleIncludeExpired } from './state/actions';
import Page from 'app/core/components/Page/Page';
import { ApiKeysAddedModal } from './ApiKeysAddedModal';
import config from 'app/core/config';
@ -28,6 +28,8 @@ function mapStateToProps(state: StoreState) {
apiKeysCount: getApiKeysCount(state.apiKeys),
hasFetched: state.apiKeys.hasFetched,
timeZone: getTimeZone(state.user),
includeExpired: getIncludeExpired(state.apiKeys),
includeExpiredDisabled: getIncludeExpiredDisabled(state.apiKeys),
};
}
@ -35,6 +37,7 @@ const mapDispatchToProps = {
loadApiKeys,
deleteApiKey,
setSearchQuery,
toggleIncludeExpired,
addApiKey,
};
@ -45,14 +48,12 @@ interface OwnProps {}
export type Props = OwnProps & ConnectedProps<typeof connector>;
interface State {
includeExpired: boolean;
hasFetched: boolean;
isAdding: boolean;
}
export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
constructor(props: Props) {
super(props);
this.state = { includeExpired: false, hasFetched: false };
}
componentDidMount() {
@ -60,11 +61,11 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
}
async fetchApiKeys() {
await this.props.loadApiKeys(this.state.includeExpired);
await this.props.loadApiKeys();
}
onDeleteApiKey = (key: ApiKey) => {
this.props.deleteApiKey(key.id!, this.state.includeExpired);
this.props.deleteApiKey(key.id!);
};
onSearchQueryChange = (value: string) => {
@ -72,7 +73,7 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
};
onIncludeExpiredChange = (event: React.SyntheticEvent<HTMLInputElement>) => {
this.setState({ hasFetched: false, includeExpired: event.currentTarget.checked }, this.fetchApiKeys);
this.props.toggleIncludeExpired();
};
onAddApiKey = (newApiKey: NewApiKey) => {
@ -97,7 +98,7 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
...newApiKey,
secondsToLive: secondsToLiveAsNumber,
};
this.props.addApiKey(apiKey, openModal, this.state.includeExpired);
this.props.addApiKey(apiKey, openModal);
this.setState((prevState: State) => {
return {
...prevState,
@ -110,8 +111,16 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
};
render() {
const { hasFetched, navModel, apiKeysCount, apiKeys, searchQuery, timeZone } = this.props;
const { includeExpired } = this.state;
const {
hasFetched,
navModel,
apiKeysCount,
apiKeys,
searchQuery,
timeZone,
includeExpired,
includeExpiredDisabled,
} = this.props;
if (!hasFetched) {
return (
@ -150,7 +159,7 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
<ApiKeysForm show={isAdding} onClose={toggleIsAdding} onKeyAdded={this.onAddApiKey} />
{showTable ? (
<VerticalGroup>
<InlineField label="Show expired">
<InlineField disabled={includeExpiredDisabled} label="Include expired keys">
<InlineSwitch id="showExpired" value={includeExpired} onChange={this.onIncludeExpiredChange} />
</InlineField>
<ApiKeysTable apiKeys={apiKeys} timeZone={timeZone} onDelete={this.onDeleteApiKey} />

31
public/app/features/api-keys/ApiKeysTable.tsx
View File

@ -1,8 +1,9 @@
import React, { FC } from 'react';
import { DeleteButton } from '@grafana/ui';
import { dateTimeFormat, TimeZone } from '@grafana/data';
import { DeleteButton, Icon, IconName, Tooltip, useTheme2 } from '@grafana/ui';
import { dateTimeFormat, GrafanaTheme2, TimeZone } from '@grafana/data';
import { ApiKey } from '../../types';
import { css } from '@emotion/css';
interface Props {
apiKeys: ApiKey[];
@ -11,6 +12,9 @@ interface Props {
}
export const ApiKeysTable: FC<Props> = ({ apiKeys, timeZone, onDelete }) => {
const theme = useTheme2();
const styles = getStyles(theme);
return (
<table className="filter-table">
<thead>
@ -24,11 +28,21 @@ export const ApiKeysTable: FC<Props> = ({ apiKeys, timeZone, onDelete }) => {
{apiKeys.length > 0 ? (
<tbody>
{apiKeys.map((key) => {
const isExpired = Boolean(key.expiration && Date.now() > new Date(key.expiration).getTime());
return (
<tr key={key.id}>
<tr key={key.id} className={styles.tableRow(isExpired)}>
<td>{key.name}</td>
<td>{key.role}</td>
<td>{formatDate(key.expiration, timeZone)}</td>
<td>
{formatDate(key.expiration, timeZone)}
{isExpired && (
<span className={styles.tooltipContainer}>
<Tooltip content="This API key has expired.">
<Icon name={'exclamation-triangle' as IconName} />
</Tooltip>
</span>
)}
</td>
<td>
<DeleteButton aria-label="Delete API key" size="sm" onConfirm={() => onDelete(key)} />
</td>
@ -47,3 +61,12 @@ function formatDate(expiration: string | undefined, timeZone: TimeZone): string
}
return dateTimeFormat(expiration, { timeZone });
}
const getStyles = (theme: GrafanaTheme2) => ({
tableRow: (isExpired: boolean) => css`
color: ${isExpired ? theme.colors.text.secondary : theme.colors.text.primary};
`,
tooltipContainer: css`
margin-left: ${theme.spacing(1)};
`,
});

30
public/app/features/api-keys/state/actions.ts
View File

@ -1,31 +1,37 @@
import { getBackendSrv } from 'app/core/services/backend_srv';
import { ApiKey, ThunkResult } from 'app/types';
import { apiKeysLoaded, setSearchQuery } from './reducers';
import { apiKeysLoaded, includeExpiredToggled, isFetching, setSearchQuery } from './reducers';
export function addApiKey(
apiKey: ApiKey,
openModal: (key: string) => void,
includeExpired: boolean
): ThunkResult<void> {
export function addApiKey(apiKey: ApiKey, openModal: (key: string) => void): ThunkResult<void> {
return async (dispatch) => {
const result = await getBackendSrv().post('/api/auth/keys', apiKey);
dispatch(setSearchQuery(''));
dispatch(loadApiKeys(includeExpired));
dispatch(loadApiKeys());
openModal(result.key);
};
}
export function loadApiKeys(includeExpired: boolean): ThunkResult<void> {
export function loadApiKeys(): ThunkResult<void> {
return async (dispatch) => {
const response = await getBackendSrv().get('/api/auth/keys?includeExpired=' + includeExpired);
dispatch(apiKeysLoaded(response));
dispatch(isFetching());
const [keys, keysIncludingExpired] = await Promise.all([
getBackendSrv().get('/api/auth/keys?includeExpired=false'),
getBackendSrv().get('/api/auth/keys?includeExpired=true'),
]);
dispatch(apiKeysLoaded({ keys, keysIncludingExpired }));
};
}
export function deleteApiKey(id: number, includeExpired: boolean): ThunkResult<void> {
export function deleteApiKey(id: number): ThunkResult<void> {
return async (dispatch) => {
getBackendSrv()
.delete(`/api/auth/keys/${id}`)
.then(() => dispatch(loadApiKeys(includeExpired)));
.then(() => dispatch(loadApiKeys()));
};
}
export function toggleIncludeExpired(): ThunkResult<void> {
return (dispatch) => {
dispatch(includeExpiredToggled());
};
}

34
public/app/features/api-keys/state/reducers.test.ts
View File

@ -1,4 +1,11 @@
import { apiKeysLoaded, apiKeysReducer, initialApiKeysState, setSearchQuery } from './reducers';
import {
apiKeysLoaded,
apiKeysReducer,
includeExpiredToggled,
initialApiKeysState,
isFetching,
setSearchQuery,
} from './reducers';
import { getMultipleMockKeys } from '../__mocks__/apiKeysMock';
import { reducerTester } from '../../../../test/core/redux/reducerTester';
import { ApiKeysState } from '../../../types';
@ -7,10 +14,13 @@ describe('API Keys reducer', () => {
it('should set keys', () => {
reducerTester<ApiKeysState>()
.givenReducer(apiKeysReducer, { ...initialApiKeysState })
.whenActionIsDispatched(apiKeysLoaded(getMultipleMockKeys(4)))
.whenActionIsDispatched(
apiKeysLoaded({ keys: getMultipleMockKeys(4), keysIncludingExpired: getMultipleMockKeys(6) })
)
.thenStateShouldEqual({
...initialApiKeysState,
keys: getMultipleMockKeys(4),
keysIncludingExpired: getMultipleMockKeys(6),
hasFetched: true,
});
});
@ -24,4 +34,24 @@ describe('API Keys reducer', () => {
searchQuery: 'test query',
});
});
it('should toggle the includeExpired state', () => {
reducerTester<ApiKeysState>()
.givenReducer(apiKeysReducer, { ...initialApiKeysState })
.whenActionIsDispatched(includeExpiredToggled())
.thenStateShouldEqual({
...initialApiKeysState,
includeExpired: true,
});
});
it('should set state when fetching', () => {
reducerTester<ApiKeysState>()
.givenReducer(apiKeysReducer, { ...initialApiKeysState })
.whenActionIsDispatched(isFetching())
.thenStateShouldEqual({
...initialApiKeysState,
hasFetched: false,
});
});
});

21
public/app/features/api-keys/state/reducers.ts
View File

@ -3,9 +3,11 @@
import { ApiKeysState } from 'app/types';
export const initialApiKeysState: ApiKeysState = {
keys: [],
searchQuery: '',
hasFetched: false,
includeExpired: false,
keys: [],
keysIncludingExpired: [],
searchQuery: '',
};
const apiKeysSlice = createSlice({
@ -13,15 +15,26 @@ const apiKeysSlice = createSlice({
initialState: initialApiKeysState,
reducers: {
apiKeysLoaded: (state, action): ApiKeysState => {
return { ...state, hasFetched: true, keys: action.payload };
const { keys, keysIncludingExpired } = action.payload;
const includeExpired =
action.payload.keys.length === 0 && action.payload.keysIncludingExpired.length > 0
? true
: state.includeExpired;
return { ...state, hasFetched: true, keys, keysIncludingExpired, includeExpired };
},
setSearchQuery: (state, action): ApiKeysState => {
return { ...state, searchQuery: action.payload };
},
includeExpiredToggled: (state): ApiKeysState => {
return { ...state, includeExpired: !state.includeExpired };
},
isFetching: (state): ApiKeysState => {
return { ...state, hasFetched: false };
},
},
});
export const { setSearchQuery, apiKeysLoaded } = apiKeysSlice.actions;
export const { apiKeysLoaded, includeExpiredToggled, isFetching, setSearchQuery } = apiKeysSlice.actions;
export const apiKeysReducer = apiKeysSlice.reducer;

141
public/app/features/api-keys/state/selectors.test.ts
View File

@ -1,25 +1,140 @@
import { getApiKeys } from './selectors';
import { getApiKeys, getApiKeysCount, getIncludeExpired, getIncludeExpiredDisabled } from './selectors';
import { getMultipleMockKeys } from '../__mocks__/apiKeysMock';
import { ApiKeysState } from 'app/types';
describe('API Keys selectors', () => {
describe('Get API Keys', () => {
const mockKeys = getMultipleMockKeys(5);
const mockKeys = getMultipleMockKeys(5);
const mockKeysIncludingExpired = getMultipleMockKeys(8);
it('should return all keys if no search query', () => {
const mockState: ApiKeysState = { keys: mockKeys, searchQuery: '', hasFetched: false };
const keys = getApiKeys(mockState);
expect(keys).toEqual(mockKeys);
describe('getApiKeysCount', () => {
it('returns the correct count when includeExpired is false', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: false,
};
const keyCount = getApiKeysCount(mockState);
expect(keyCount).toBe(5);
});
it('should filter keys if search query exists', () => {
const mockState: ApiKeysState = { keys: mockKeys, searchQuery: '5', hasFetched: false };
it('returns the correct count when includeExpired is true', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: true,
};
const keyCount = getApiKeysCount(mockState);
expect(keyCount).toBe(8);
});
});
const keys = getApiKeys(mockState);
describe('getApiKeys', () => {
describe('when includeExpired is false', () => {
it('should return all keys if no search query', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: false,
};
const keys = getApiKeys(mockState);
expect(keys).toEqual(mockKeys);
});
expect(keys.length).toEqual(1);
it('should filter keys if search query exists', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '5',
hasFetched: true,
includeExpired: false,
};
const keys = getApiKeys(mockState);
expect(keys.length).toEqual(1);
});
});
describe('when includeExpired is true', () => {
it('should return all keys if no search query', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: true,
};
const keys = getApiKeys(mockState);
expect(keys).toEqual(mockKeysIncludingExpired);
});
it('should filter keys if search query exists', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '5',
hasFetched: true,
includeExpired: true,
};
const keys = getApiKeys(mockState);
expect(keys.length).toEqual(1);
});
});
});
describe('getIncludeExpired', () => {
it('returns true if includeExpired is true', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: true,
};
const includeExpired = getIncludeExpired(mockState);
expect(includeExpired).toBe(true);
});
it('returns false if includeExpired is false', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: false,
};
const includeExpired = getIncludeExpired(mockState);
expect(includeExpired).toBe(false);
});
});
describe('getIncludeExpiredDisabled', () => {
it('returns true if there are no active keys but there are expired keys', () => {
const mockState: ApiKeysState = {
keys: [],
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: true,
};
const includeExpiredDisabled = getIncludeExpiredDisabled(mockState);
expect(includeExpiredDisabled).toBe(true);
});
it('returns false otherwise', () => {
const mockState: ApiKeysState = {
keys: mockKeys,
keysIncludingExpired: mockKeysIncludingExpired,
searchQuery: '',
hasFetched: true,
includeExpired: false,
};
const includeExpiredDisabled = getIncludeExpired(mockState);
expect(includeExpiredDisabled).toBe(false);
});
});
});

11
public/app/features/api-keys/state/selectors.ts
View File

@ -1,11 +1,18 @@
import { ApiKeysState } from 'app/types';
export const getApiKeysCount = (state: ApiKeysState) => state.keys.length;
export const getApiKeysCount = (state: ApiKeysState) =>
state.includeExpired ? state.keysIncludingExpired.length : state.keys.length;
export const getApiKeys = (state: ApiKeysState) => {
const regex = RegExp(state.searchQuery, 'i');
const keysToFilter = state.includeExpired ? state.keysIncludingExpired : state.keys;
return state.keys.filter((key) => {
return keysToFilter.filter((key) => {
return regex.test(key.name) || regex.test(key.role);
});
};
export const getIncludeExpired = (state: ApiKeysState) => state.includeExpired;
export const getIncludeExpiredDisabled = (state: ApiKeysState) =>
state.keys.length === 0 && state.keysIncludingExpired.length > 0;

2
public/app/types/apiKeys.ts
View File

@ -15,7 +15,9 @@ export interface NewApiKey {
}
export interface ApiKeysState {
includeExpired: boolean;
keys: ApiKey[];
keysIncludingExpired: ApiKey[];
searchQuery: string;
hasFetched: boolean;
}