Skip to content

Commit d362be6

Browse files
Remove multi-tenant path check in auth handler (#1151) (#1168)
Signed-off-by: Yan Zeng <zengyan@amazon.com> Co-authored-by: Chang Liu <lc12251109@gmail.com> (cherry picked from commit 6236fe2) Co-authored-by: Yan Zeng <46499415+zengyan-amazon@users.noreply.github.com>
1 parent a616b0a commit d362be6

3 files changed

Lines changed: 111 additions & 25 deletions

File tree

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
import { SecurityPluginConfigType } from '../..';
17+
import { AuthenticationType } from './authentication_type';
18+
import { httpServerMock } from '../../../../../src/core/server/mocks';
19+
20+
class DummyAuthType extends AuthenticationType {
21+
buildAuthHeaderFromCookie() {}
22+
getAdditionalAuthHeader() {}
23+
handleUnauthedRequest() {}
24+
getCookie() {
25+
return {};
26+
}
27+
isValidCookie() {
28+
return Promise.resolve(true);
29+
}
30+
requestIncludesAuthInfo() {
31+
return false;
32+
}
33+
resolveTenant(): Promise<string | undefined> {
34+
return Promise.resolve('dummy_tenant');
35+
}
36+
}
37+
38+
describe('test tenant header', () => {
39+
const config = {
40+
multitenancy: {
41+
enabled: true,
42+
},
43+
auth: {
44+
unauthenticated_routes: [] as string[],
45+
},
46+
session: {
47+
keepalive: false,
48+
},
49+
} as SecurityPluginConfigType;
50+
const sessionStorageFactory = {
51+
asScoped: jest.fn(() => {
52+
return {
53+
clear: jest.fn(),
54+
get: () => {
55+
return {
56+
tenant: 'dummy_tenant',
57+
};
58+
},
59+
};
60+
}),
61+
};
62+
const router = jest.fn();
63+
const esClient = {
64+
asScoped: jest.fn().mockImplementation(() => {
65+
return {
66+
callAsCurrentUser: jest.fn().mockImplementation(() => {
67+
return { username: 'dummy-username' };
68+
}),
69+
};
70+
}),
71+
};
72+
const coreSetup = jest.fn();
73+
const logger = {
74+
error: jest.fn(),
75+
};
76+
77+
const dummyAuthType = new DummyAuthType(
78+
config,
79+
sessionStorageFactory,
80+
router,
81+
esClient,
82+
coreSetup,
83+
logger
84+
);
85+
86+
it(`common API includes tenant info`, async () => {
87+
const request = httpServerMock.createOpenSearchDashboardsRequest({
88+
path: '/api/v1',
89+
});
90+
const response = jest.fn();
91+
const toolkit = {
92+
authenticated: jest.fn((value) => value),
93+
};
94+
const result = await dummyAuthType.authHandler(request, response, toolkit);
95+
expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant');
96+
});
97+
98+
it(`internal API includes tenant info`, async () => {
99+
const request = httpServerMock.createOpenSearchDashboardsRequest({
100+
path: '/internal/v1',
101+
});
102+
const response = jest.fn();
103+
const toolkit = {
104+
authenticated: jest.fn((value) => value),
105+
};
106+
const result = await dummyAuthType.authHandler(request, response, toolkit);
107+
expect(result.requestHeaders.securitytenant).toEqual('dummy_tenant');
108+
});
109+
});

server/auth/types/authentication_type.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,7 @@ import {
2929
import { SecurityPluginConfigType } from '../..';
3030
import { SecuritySessionCookie } from '../../session/security_cookie';
3131
import { SecurityClient } from '../../backend/opensearch_security_client';
32-
import {
33-
isMultitenantPath,
34-
resolveTenant,
35-
isValidTenant,
36-
} from '../../multitenancy/tenant_resolver';
32+
import { resolveTenant, isValidTenant } from '../../multitenancy/tenant_resolver';
3733
import { UnauthenticatedError } from '../../errors';
3834

3935
export interface IAuthenticationType {
@@ -147,7 +143,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
147143
}
148144

149145
// resolve tenant if necessary
150-
if (this.config.multitenancy?.enabled && isMultitenantPath(request)) {
146+
if (this.config.multitenancy?.enabled) {
151147
try {
152148
const tenant = await this.resolveTenant(request, cookie!, authHeaders, authInfo);
153149
// return 401 if no tenant available

server/multitenancy/tenant_resolver.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -81,25 +81,6 @@ export function resolveTenant(
8181
);
8282
}
8383

84-
/**
85-
* Determines whether the request requires tenant info.
86-
* @param request opensearch-dashboards request.
87-
*
88-
* @returns true if the request requires tenant info, otherwise false.
89-
*/
90-
export function isMultitenantPath(request: OpenSearchDashboardsRequest): boolean {
91-
return (
92-
request.url.pathname?.startsWith('/opensearch') ||
93-
request.url.pathname?.startsWith('/api') ||
94-
request.url.pathname?.startsWith('/app') ||
95-
// short url path
96-
request.url.pathname?.startsWith('/goto') ||
97-
// bootstrap.js depends on tenant info to fetch opensearch-dashboards configs in tenant index
98-
(request.url.pathname?.indexOf('bootstrap.js') || -1) > -1 ||
99-
request.url.pathname === '/'
100-
);
101-
}
102-
10384
function resolve(
10485
username: string,
10586
requestedTenant: string | undefined,

0 commit comments

Comments
 (0)