Skip to content

Commit

Permalink
Remove multi-tenant path check in auth handler (opensearch-project#1151)
Browse files Browse the repository at this point in the history
Signed-off-by: Yan Zeng <[email protected]>

Co-authored-by: Chang Liu <[email protected]>
  • Loading branch information
zengyan-amazon and cliu123 authored Oct 28, 2022
1 parent a21c7be commit 6236fe2
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 25 deletions.
109 changes: 109 additions & 0 deletions server/auth/types/authentication_type.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright OpenSearch Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

import { SecurityPluginConfigType } from '../..';
import { AuthenticationType } from './authentication_type';
import { httpServerMock } from '../../../../../src/core/server/mocks';

class DummyAuthType extends AuthenticationType {
buildAuthHeaderFromCookie() {}
getAdditionalAuthHeader() {}
handleUnauthedRequest() {}
getCookie() {
return {};
}
isValidCookie() {
return Promise.resolve(true);
}
requestIncludesAuthInfo() {
return false;
}
resolveTenant(): Promise<string | undefined> {
return Promise.resolve('dummy_tenant');
}
}

describe('test tenant header', () => {
const config = {
multitenancy: {
enabled: true,
},
auth: {
unauthenticated_routes: [] as string[],
},
session: {
keepalive: false,
},
} as SecurityPluginConfigType;
const sessionStorageFactory = {
asScoped: jest.fn(() => {
return {
clear: jest.fn(),
get: () => {
return {
tenant: 'dummy_tenant',
};
},
};
}),
};
const router = jest.fn();
const esClient = {
asScoped: jest.fn().mockImplementation(() => {
return {
callAsCurrentUser: jest.fn().mockImplementation(() => {
return { username: 'dummy-username' };
}),
};
}),
};
const coreSetup = jest.fn();
const logger = {
error: jest.fn(),
};

const dummyAuthType = new DummyAuthType(
config,
sessionStorageFactory,
router,
esClient,
coreSetup,
logger
);

it(`common API includes tenant info`, async () => {
const request = httpServerMock.createOpenSearchDashboardsRequest({
path: '/api/v1',
});
const response = jest.fn();
const toolkit = {
authenticated: jest.fn((value) => value),
};
const result = await dummyAuthType.authHandler(request, response, toolkit);
expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant');
});

it(`internal API includes tenant info`, async () => {
const request = httpServerMock.createOpenSearchDashboardsRequest({
path: '/internal/v1',
});
const response = jest.fn();
const toolkit = {
authenticated: jest.fn((value) => value),
};
const result = await dummyAuthType.authHandler(request, response, toolkit);
expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant');
});
});
8 changes: 2 additions & 6 deletions server/auth/types/authentication_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ import {
import { SecurityPluginConfigType } from '../..';
import { SecuritySessionCookie } from '../../session/security_cookie';
import { SecurityClient } from '../../backend/opensearch_security_client';
import {
isMultitenantPath,
resolveTenant,
isValidTenant,
} from '../../multitenancy/tenant_resolver';
import { resolveTenant, isValidTenant } from '../../multitenancy/tenant_resolver';
import { UnauthenticatedError } from '../../errors';
import { GLOBAL_TENANT_SYMBOL } from '../../../public/apps/configuration/utils/tenant-utils';

Expand Down Expand Up @@ -170,7 +166,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
}

// resolve tenant if necessary
if (this.config.multitenancy?.enabled && isMultitenantPath(request)) {
if (this.config.multitenancy?.enabled) {
try {
const tenant = await this.resolveTenant(request, cookie!, authHeaders, authInfo);
// return 401 if no tenant available
Expand Down
19 changes: 0 additions & 19 deletions server/multitenancy/tenant_resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,25 +82,6 @@ export function resolveTenant(
);
}

/**
* Determines whether the request requires tenant info.
* @param request opensearch-dashboards request.
*
* @returns true if the request requires tenant info, otherwise false.
*/
export function isMultitenantPath(request: OpenSearchDashboardsRequest): boolean {
return (
request.url.pathname?.startsWith('/opensearch') ||
request.url.pathname?.startsWith('/api') ||
request.url.pathname?.startsWith('/app') ||
// short url path
request.url.pathname?.startsWith('/goto') ||
// bootstrap.js depends on tenant info to fetch opensearch-dashboards configs in tenant index
(request.url.pathname?.indexOf('bootstrap.js') || -1) > -1 ||
request.url.pathname === '/'
);
}

function resolve(
username: string,
requestedTenant: string | undefined,
Expand Down

0 comments on commit 6236fe2

Please sign in to comment.