Skip to content

Commit

Permalink
fix privileges and last check issue
Browse files Browse the repository at this point in the history
  • Loading branch information
semd committed Jan 30, 2024
1 parent d189669 commit e0be9c9
Show file tree
Hide file tree
Showing 13 changed files with 241 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ const IndexPropertiesComponent: React.FC<Props> = ({
})
: EMPTY_MARKDOWN_COMMENTS;

const checkedAt = partitionedFieldMetadata ? Date.now() : undefined;

const updatedRollup = {
...patternRollup,
results: {
Expand All @@ -262,7 +264,7 @@ const IndexPropertiesComponent: React.FC<Props> = ({
markdownComments,
pattern,
sameFamily: indexSameFamily,
checkedAt: Date.now(),
checkedAt,
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export const getSummaryTableColumns = ({
name: i18n.LAST_CHECK,
render: (_, { checkedAt }) => (
<EuiText size="xs">
{moment(checkedAt).isValid() ? moment(checkedAt).fromNow() : EMPTY_STAT}
{checkedAt && moment(checkedAt).isValid() ? moment(checkedAt).fromNow() : EMPTY_STAT}
</EuiText>
),
sortable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ export const formatStorageResult = ({
batchId: report.batchId,
indexName: result.indexName,
isCheckAll: report.isCheckAll,
checkedAt: result.checkedAt,
checkedAt: result.checkedAt ?? Date.now(),
docsCount: result.docsCount ?? 0,
totalFieldCount: partitionedFieldMetadata.all.length,
ecsFieldCount: partitionedFieldMetadata.ecsCompliant.length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export interface DataQualityCheckResult {
markdownComments: string[];
sameFamily: number | undefined;
pattern: string;
checkedAt: number;
checkedAt: number | undefined;
}

export interface PatternRollup {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ describe('helpers', () => {
indexName: '.ds-packetbeat-8.6.1-2023.02.04-000001',
markdownComments: [],
pattern: 'packetbeat-*',
checkedAt: expect.any(Number),
checkedAt: undefined,
},
},
sizeInBytes: 1464758182,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export const updateResultOnCheckCompleted = ({

const incompatible = partitionedFieldMetadata?.incompatible.length;
const sameFamily = partitionedFieldMetadata?.sameFamily.length;
const checkedAt = partitionedFieldMetadata ? Date.now() : undefined;

return {
...patternRollups,
Expand All @@ -156,7 +157,7 @@ export const updateResultOnCheckCompleted = ({
markdownComments,
pattern,
sameFamily,
checkedAt: Date.now(),
checkedAt,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,19 @@ export const useResultsRollup = ({ ilmPhases, patterns }: Props): UseResultsRoll

useEffect(() => {
if (!isEmpty(storedPatternsResults)) {
setPatternRollups((current) => {
const updatedRollups = { ...current };
storedPatternsResults.forEach((storedPatternResult) => {
const { pattern, results } = storedPatternResult;
updatedRollups[pattern] = {
...current[pattern],
pattern,
results,
};
});
return updatedRollups;
});
setPatternRollups((current) =>
storedPatternsResults.reduce(
(acc, { pattern, results }) => ({
...acc,
[pattern]: {
...current[pattern],
pattern,
results,
},
}),
current
)
);
}
}, [storedPatternsResults]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ const searchResponse = {
Record<string, { buckets: LatestAggResponseBucket[] }>
>;

const mockCheckIndicesPrivileges = jest
.fn()
.mockResolvedValue({ [resultDocument.indexName]: true });
jest.mock('./privileges', () => ({
checkIndicesPrivileges: (params: unknown) => mockCheckIndicesPrivileges(params),
}));

describe('getResultsRoute route', () => {
describe('querying', () => {
let server: ReturnType<typeof serverMock.create>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type { ResultDocument } from '../../schemas/result';
import { API_DEFAULT_ERROR_MESSAGE } from '../../translations';
import type { DataQualityDashboardRequestHandlerContext } from '../../types';
import { API_RESULTS_INDEX_NOT_AVAILABLE } from './translations';
import { checkIndicesPrivileges } from './privileges';

export const getQuery = (indexName: string[]) => ({
size: 0,
Expand Down Expand Up @@ -66,18 +67,19 @@ export const getResultsRoute = (
const { pattern } = request.query;

// Get authorized indices
const userEsClient = services.core.elasticsearch.client.asCurrentUser;
const indices = await userEsClient.indices.get({ index: pattern });
const authorizedIndexNames = Object.keys(indices);
const { client } = services.core.elasticsearch;
const indicesResponse = await client.asCurrentUser.indices.get({ index: pattern });
const indices = Object.keys(indicesResponse);

const hadIndexPrivileges = await checkIndicesPrivileges({ client, indices });
const authorizedIndexNames = indices.filter((indexName) => hadIndexPrivileges[indexName]);
if (authorizedIndexNames.length === 0) {
return response.ok({ body: [] });
}

// Get the latest result of each pattern
const query = { index, ...getQuery(authorizedIndexNames) };
const internalEsClient = services.core.elasticsearch.client.asInternalUser;

const { aggregations } = await internalEsClient.search<
const { aggregations } = await client.asInternalUser.search<
ResultDocument,
Record<string, { buckets: LatestAggResponseBucket[] }>
>(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ import type {
} from '@elastic/elasticsearch/lib/api/types';
import { resultDocument } from './results.mock';

const mockCheckIndicesPrivileges = jest
.fn()
.mockResolvedValue({ [resultDocument.indexName]: true });
jest.mock('./privileges', () => ({
checkIndicesPrivileges: (params: unknown) => mockCheckIndicesPrivileges(params),
}));

describe('postResultsRoute route', () => {
describe('indexation', () => {
let server: ReturnType<typeof serverMock.create>;
Expand Down Expand Up @@ -110,41 +117,34 @@ describe('postResultsRoute route', () => {
});

it('should authorize pattern', async () => {
const mockHasPrivileges =
context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
mockHasPrivileges.mockResolvedValueOnce({
has_all_requested: true,
} as unknown as SecurityHasPrivilegesResponse);
// const mockHasPrivileges =
// context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
// mockHasPrivileges.mockResolvedValueOnce({
// has_all_requested: true,
// } as unknown as SecurityHasPrivilegesResponse);

const response = await server.inject(req, requestContextMock.convertContext(context));
expect(mockHasPrivileges).toHaveBeenCalledWith({
index: [
{
names: [resultDocument.indexName],
privileges: ['all', 'read', 'view_index_metadata'],
},
],
expect(mockCheckIndicesPrivileges).toHaveBeenCalledWith({
client: context.core.elasticsearch.client,
indices: [resultDocument.indexName],
});
expect(context.core.elasticsearch.client.asInternalUser.index).toHaveBeenCalled();
expect(response.status).toEqual(200);
expect(response.body).toEqual({ result: 'created' });
});

it('should not index unauthorized pattern', async () => {
const mockHasPrivileges =
context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
mockHasPrivileges.mockResolvedValueOnce({
has_all_requested: false,
} as unknown as SecurityHasPrivilegesResponse);
// const mockHasPrivileges =
// context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
// mockHasPrivileges.mockResolvedValueOnce({
// has_all_requested: false,
// } as unknown as SecurityHasPrivilegesResponse);
mockCheckIndicesPrivileges.mockResolvedValueOnce({ [resultDocument.indexName]: false });

const response = await server.inject(req, requestContextMock.convertContext(context));
expect(mockHasPrivileges).toHaveBeenCalledWith({
index: [
{
names: [resultDocument.indexName],
privileges: ['all', 'read', 'view_index_metadata'],
},
],
expect(mockCheckIndicesPrivileges).toHaveBeenCalledWith({
client: context.core.elasticsearch.client,
indices: [resultDocument.indexName],
});
expect(context.core.elasticsearch.client.asInternalUser.index).not.toHaveBeenCalled();

Expand All @@ -154,9 +154,11 @@ describe('postResultsRoute route', () => {

it('handles pattern authorization error', async () => {
const errorMessage = 'Error!';
const mockHasPrivileges =
context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
mockHasPrivileges.mockRejectedValueOnce({ message: errorMessage });
// const mockHasPrivileges =
// context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges;
// mockHasPrivileges.mockRejectedValueOnce({ message: errorMessage });

mockCheckIndicesPrivileges.mockRejectedValueOnce(Error(errorMessage));

const response = await server.inject(req, requestContextMock.convertContext(context));
expect(response.status).toEqual(500);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { buildRouteValidation } from '../../schemas/common';
import { PostResultBody } from '../../schemas/result';
import { API_DEFAULT_ERROR_MESSAGE } from '../../translations';
import type { DataQualityDashboardRequestHandlerContext } from '../../types';
import { checkIndicesPrivileges } from './privileges';
import { API_RESULTS_INDEX_NOT_AVAILABLE } from './translations';

export const postResultsRoute = (
Expand Down Expand Up @@ -46,20 +47,18 @@ export const postResultsRoute = (
}

try {
const { client } = services.core.elasticsearch;

// Confirm user has authorization for the indexName payload
const { indexName } = request.body;
const userEsClient = services.core.elasticsearch.client.asCurrentUser;
const privileges = await userEsClient.security.hasPrivileges({
index: [{ names: [indexName], privileges: ['all', 'read', 'view_index_metadata'] }],
});
if (!privileges.has_all_requested) {
const hadIndexPrivileges = await checkIndicesPrivileges({ client, indices: [indexName] });
if (!hadIndexPrivileges[indexName]) {
return response.ok({ body: { result: 'noop' } });
}

// Index the result
const body = { '@timestamp': Date.now(), ...request.body };
const esClient = services.core.elasticsearch.client.asInternalUser;
const outcome = await esClient.index({ index, body });
const outcome = await client.asInternalUser.index({ index, body });

return response.ok({ body: { result: outcome.result } });
} catch (err) {
Expand Down
Loading

0 comments on commit e0be9c9

Please sign in to comment.