Skip to content

Commit

Permalink
address code review comments (#101764) (#101887)
Browse files Browse the repository at this point in the history
Co-authored-by: Stacey Gammon <[email protected]>
  • Loading branch information
kibanamachine and stacey-gammon authored Jun 10, 2021
1 parent bc6bc5a commit dd7076b
Show file tree
Hide file tree
Showing 14 changed files with 25 additions and 26 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/security/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
* 2.0.
*/

export { SecurityLicense } from './licensing';
export { AuthenticatedUser } from './model';
export type { SecurityLicense } from './licensing';
export type { AuthenticatedUser } from './model';
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';

import { apiKeysMock } from './api_keys/api_keys.mock';
import type { AuthenticationServiceStartInternal } from './authentication_service';
import type { InternalAuthenticationServiceStart } from './authentication_service';

export const authenticationServiceMock = {
createStart: (): DeeplyMockedKeys<AuthenticationServiceStartInternal> => ({
createStart: (): DeeplyMockedKeys<InternalAuthenticationServiceStart> => ({
apiKeys: apiKeysMock.create(),
login: jest.fn(),
logout: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ interface AuthenticationServiceStartParams {
loggers: LoggerFactory;
}

export interface AuthenticationServiceStartInternal extends AuthenticationServiceStart {
export interface InternalAuthenticationServiceStart extends AuthenticationServiceStart {
apiKeys: Pick<
APIKeys,
| 'areAPIKeysEnabled'
Expand Down Expand Up @@ -227,7 +227,7 @@ export class AuthenticationService {
legacyAuditLogger,
loggers,
session,
}: AuthenticationServiceStartParams): AuthenticationServiceStartInternal {
}: AuthenticationServiceStartParams): InternalAuthenticationServiceStart {
const apiKeys = new APIKeys({
clusterClient,
logger: this.logger.get('api-key'),
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export { canRedirectRequest } from './can_redirect_request';
export {
AuthenticationService,
AuthenticationServiceStart,
AuthenticationServiceStartInternal,
InternalAuthenticationServiceStart,
} from './authentication_service';
export { AuthenticationResult } from './authentication_result';
export { DeauthenticationResult } from './deauthentication_result';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { UIActions } from './ui';

/** Actions are used to create the "actions" that are associated with Elasticsearch's
* application privileges, and are used to perform the authorization checks implemented
* by the various `checkPrivilegesWithRequest` derivatives
* by the various `checkPrivilegesWithRequest` derivatives.
*/
export class Actions {
public readonly api = new ApiActions(this.versionNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export interface AuthorizationServiceSetup {
/**
* Actions are used to create the "actions" that are associated with Elasticsearch's
* application privileges, and are used to perform the authorization checks implemented
* by the various `checkPrivilegesWithRequest` derivatives
* by the various `checkPrivilegesWithRequest` derivatives.
*/
actions: Actions;
checkPrivilegesWithRequest: CheckPrivilegesWithRequest;
Expand Down
7 changes: 3 additions & 4 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,11 @@ import type { AuditServiceSetup } from './audit';
import { AuditService, SecurityAuditLogger } from './audit';
import type {
AuthenticationServiceStart,
AuthenticationServiceStartInternal,
InternalAuthenticationServiceStart,
} from './authentication';
import { AuthenticationService } from './authentication';
import type { AuthorizationServiceSetup } from './authorization';
import type { AuthorizationServiceSetup, AuthorizationServiceSetupInternal } from './authorization';
import { AuthorizationService } from './authorization';
import type { AuthorizationServiceSetupInternal } from './authorization/authorization_service';
import type { ConfigSchema, ConfigType } from './config';
import { createConfig } from './config';
import { ElasticsearchService } from './elasticsearch';
Expand Down Expand Up @@ -156,7 +155,7 @@ export class SecurityPlugin
private readonly authenticationService = new AuthenticationService(
this.initializerContext.logger.get('authentication')
);
private authenticationStart?: AuthenticationServiceStartInternal;
private authenticationStart?: InternalAuthenticationServiceStart;
private readonly getAuthentication = () => {
if (!this.authenticationStart) {
throw new Error(`authenticationStart is not registered!`);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/security/server/routes/api_keys/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { RequestHandler } from 'src/core/server';
import { kibanaResponseFactory } from 'src/core/server';
import { httpServerMock } from 'src/core/server/mocks';

import type { AuthenticationServiceStartInternal } from '../../authentication';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import { authenticationServiceMock } from '../../authentication/authentication_service.mock';
import type { SecurityRequestHandlerContext } from '../../types';
import { routeDefinitionParamsMock } from '../index.mock';
Expand All @@ -28,7 +28,7 @@ describe('Create API Key route', () => {
}

let routeHandler: RequestHandler<any, any, any, any>;
let authc: DeeplyMockedKeys<AuthenticationServiceStartInternal>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
beforeEach(() => {
authc = authenticationServiceMock.createStart();
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { RequestHandler } from 'src/core/server';
import { kibanaResponseFactory } from 'src/core/server';
import { httpServerMock } from 'src/core/server/mocks';

import type { AuthenticationServiceStartInternal } from '../../authentication';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import { authenticationServiceMock } from '../../authentication/authentication_service.mock';
import type { SecurityRequestHandlerContext } from '../../types';
import { routeDefinitionParamsMock } from '../index.mock';
Expand All @@ -28,7 +28,7 @@ describe('API keys enabled', () => {
}

let routeHandler: RequestHandler<any, any, any, any>;
let authc: DeeplyMockedKeys<AuthenticationServiceStartInternal>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
beforeEach(() => {
authc = authenticationServiceMock.createStart();
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { httpServerMock } from 'src/core/server/mocks';

import type { SecurityLicense, SecurityLicenseFeatures } from '../../../common/licensing';
import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock';
import type { AuthenticationServiceStartInternal } from '../../authentication';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import {
AuthenticationResult,
DeauthenticationResult,
Expand All @@ -28,7 +28,7 @@ import { defineCommonRoutes } from './common';

describe('Common authentication routes', () => {
let router: jest.Mocked<SecurityRouter>;
let authc: DeeplyMockedKeys<AuthenticationServiceStartInternal>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
let license: jest.Mocked<SecurityLicense>;
let mockContext: SecurityRequestHandlerContext;
beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { RequestHandler, RouteConfig } from 'src/core/server';
import { httpServerMock } from 'src/core/server/mocks';

import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock';
import type { AuthenticationServiceStartInternal } from '../../authentication';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import { AuthenticationResult, SAMLLogin } from '../../authentication';
import { authenticationServiceMock } from '../../authentication/authentication_service.mock';
import type { SecurityRouter } from '../../types';
Expand All @@ -21,7 +21,7 @@ import { defineSAMLRoutes } from './saml';

describe('SAML authentication routes', () => {
let router: jest.Mocked<SecurityRouter>;
let authc: DeeplyMockedKeys<AuthenticationServiceStartInternal>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
beforeEach(() => {
const routeParamsMock = routeDefinitionParamsMock.create();
router = routeParamsMock.router;
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/security/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { HttpResources, IBasePath, Logger } from 'src/core/server';

import type { KibanaFeature } from '../../../features/server';
import type { SecurityLicense } from '../../common/licensing';
import type { AuthenticationServiceStartInternal } from '../authentication';
import type { InternalAuthenticationServiceStart } from '../authentication';
import type { AuthorizationServiceSetupInternal } from '../authorization';
import type { ConfigType } from '../config';
import type { SecurityFeatureUsageServiceStart } from '../feature_usage';
Expand Down Expand Up @@ -39,7 +39,7 @@ export interface RouteDefinitionParams {
license: SecurityLicense;
getFeatures: () => Promise<KibanaFeature[]>;
getFeatureUsageService: () => SecurityFeatureUsageServiceStart;
getAuthenticationService: () => AuthenticationServiceStartInternal;
getAuthenticationService: () => InternalAuthenticationServiceStart;
}

export function defineRoutes(params: RouteDefinitionParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { coreMock, httpServerMock } from 'src/core/server/mocks';

import { mockAuthenticatedUser } from '../../../common/model/authenticated_user.mock';
import { AuthenticationResult } from '../../authentication';
import type { AuthenticationServiceStartInternal } from '../../authentication/authentication_service';
import type { InternalAuthenticationServiceStart } from '../../authentication';
import { authenticationServiceMock } from '../../authentication/authentication_service.mock';
import type { Session } from '../../session_management';
import { sessionMock } from '../../session_management/session.mock';
Expand All @@ -26,7 +26,7 @@ import { defineChangeUserPasswordRoutes } from './change_password';

describe('Change password', () => {
let router: jest.Mocked<SecurityRouter>;
let authc: DeeplyMockedKeys<AuthenticationServiceStartInternal>;
let authc: DeeplyMockedKeys<InternalAuthenticationServiceStart>;
let session: jest.Mocked<PublicMethodsOf<Session>>;
let routeHandler: RequestHandler<any, any, any, SecurityRequestHandlerContext>;
let routeConfig: RouteConfig<any, any, any, any>;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { CoreSetup, LegacyRequest } from 'src/core/server';

import { KibanaRequest, SavedObjectsClient } from '../../../../../src/core/server';
import type { AuditServiceSetup, SecurityAuditLogger } from '../audit';
import type { AuthorizationServiceSetupInternal } from '../authorization/authorization_service';
import type { AuthorizationServiceSetupInternal } from '../authorization';
import type { SpacesService } from '../plugin';
import { SecureSavedObjectsClientWrapper } from './secure_saved_objects_client_wrapper';

Expand Down

0 comments on commit dd7076b

Please sign in to comment.