diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts index 4bc013b21..8bde376ac 100755 --- a/server/auth/types/authentication_type.ts +++ b/server/auth/types/authentication_type.ts @@ -31,7 +31,7 @@ import { SecuritySessionCookie } from '../../session/security_cookie'; import { SecurityClient } from '../../backend/opensearch_security_client'; import { resolveTenant, isValidTenant } from '../../multitenancy/tenant_resolver'; import { UnauthenticatedError } from '../../errors'; -import { AuthType, GLOBAL_TENANT_SYMBOL } from '../../../common'; +import { GLOBAL_TENANT_SYMBOL } from '../../../common'; export interface IAuthenticationType { type: string; @@ -119,7 +119,7 @@ export abstract class AuthenticationType implements IAuthenticationType { try { const additionalAuthHeader = await this.getAdditionalAuthHeader(request); Object.assign(authHeaders, additionalAuthHeader); - authInfo = await this.securityClient.authinfo(request, '', additionalAuthHeader); + authInfo = await this.securityClient.authinfo(request, additionalAuthHeader); cookie = this.getCookie(request, authInfo); // set tenant from cookie if exist @@ -210,8 +210,7 @@ export abstract class AuthenticationType implements IAuthenticationType { } } if (!authInfo) { - const authRequestType = cookie.isAnonymousAuth ? AuthType.ANONYMOUS : cookie.authType; - authInfo = await this.securityClient.authinfo(request, authRequestType, authHeaders); + authInfo = await this.securityClient.authinfo(request, authHeaders); } authState.authInfo = authInfo; @@ -244,10 +243,9 @@ export abstract class AuthenticationType implements IAuthenticationType { authHeader: any, authInfo: any ): Promise { - const authType = cookie.isAnonymousAuth ? AuthType.ANONYMOUS : cookie.authType; if (!authInfo) { try { - authInfo = await this.securityClient.authinfo(request, authType, authHeader); + authInfo = await this.securityClient.authinfo(request, authHeader); } catch (error: any) { throw new UnauthenticatedError(error); } diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts index 2838047f5..113bbb9a7 100644 --- a/server/auth/types/basic/basic_auth.ts +++ b/server/auth/types/basic/basic_auth.ts @@ -30,7 +30,12 @@ import { BasicAuthRoutes } from './routes'; import { AuthenticationType } from '../authentication_type'; import { LOGIN_PAGE_URI } from '../../../../common'; import { composeNextUrlQueryParam } from '../../../utils/next_url'; -import { AUTH_HEADER_NAME, AuthType, OPENDISTRO_SECURITY_ANONYMOUS } from '../../../../common'; +import { + ANONYMOUS_AUTH_LOGIN, + AUTH_HEADER_NAME, + AuthType, + OPENDISTRO_SECURITY_ANONYMOUS, +} from '../../../../common'; export class BasicAuthentication extends AuthenticationType { public readonly type: string = AuthType.BASIC; @@ -111,13 +116,21 @@ export class BasicAuthentication extends AuthenticationType { request, this.coreSetup.http.basePath.serverBasePath ); - - const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`; - return response.redirected({ - headers: { - location: `${redirectLocation}`, - }, - }); + if (this.config.auth.anonymous_auth_enabled) { + const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${ANONYMOUS_AUTH_LOGIN}?${nextUrlParam}`; + return response.redirected({ + headers: { + location: `${redirectLocation}`, + }, + }); + } else { + const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`; + return response.redirected({ + headers: { + location: `${redirectLocation}`, + }, + }); + } } else { return response.unauthorized({ body: `Authentication required`, diff --git a/server/auth/types/basic/routes.ts b/server/auth/types/basic/routes.ts index 23e3d71c6..bae3e338c 100755 --- a/server/auth/types/basic/routes.ts +++ b/server/auth/types/basic/routes.ts @@ -186,11 +186,7 @@ export class BasicAuthRoutes { } context.security_plugin.logger.info('The Redirect Path is ' + redirectUrl); try { - user = await this.securityClient.authenticateWithHeaders( - request, - AuthType.ANONYMOUS, - {} - ); + user = await this.securityClient.authenticateWithHeaders(request, {}); } catch (error) { context.security_plugin.logger.error( `Failed authentication: ${error}. Redirecting to Login Page` diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts index 9b87e81ea..c23e26b1f 100644 --- a/server/auth/types/openid/routes.ts +++ b/server/auth/types/openid/routes.ts @@ -187,8 +187,7 @@ export class OpenIdAuthRoutes { const user = await this.securityClient.authenticateWithHeader( request, this.openIdAuthConfig.authHeaderName as string, - `Bearer ${tokenResponse.idToken}`, - AuthType.OPEN_ID + `Bearer ${tokenResponse.idToken}` ); // set to cookie diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts index 7e717cf0a..3017106d4 100644 --- a/server/auth/types/saml/routes.ts +++ b/server/auth/types/saml/routes.ts @@ -219,8 +219,7 @@ export class SamlAuthRoutes { const user = await this.securityClient.authenticateWithHeader( request, 'authorization', - credentials.authorization, - AuthType.SAML + credentials.authorization ); let expiryTime = Date.now() + this.config.session.ttl; diff --git a/server/backend/opensearch_security_client.ts b/server/backend/opensearch_security_client.ts index 1aa50004b..6134056db 100755 --- a/server/backend/opensearch_security_client.ts +++ b/server/backend/opensearch_security_client.ts @@ -53,7 +53,6 @@ export class SecurityClient { request: OpenSearchDashboardsRequest, headerName: string, headerValue: string, - authRequestType: string, whitelistedHeadersAndValues: any = {}, additionalAuthHeaders: any = {} ): Promise { @@ -72,7 +71,6 @@ export class SecurityClient { const esResponse = await this.esClient .asScoped(request) .callAsCurrentUser('opensearch_security.authinfo', { - [AUTH_TYPE_PARAM]: authRequestType, headers, }); return { @@ -90,14 +88,12 @@ export class SecurityClient { public async authenticateWithHeaders( request: OpenSearchDashboardsRequest, - authRequestType: string, additionalAuthHeaders: any = {} ): Promise { try { const esResponse = await this.esClient .asScoped(request) .callAsCurrentUser('opensearch_security.authinfo', { - [AUTH_TYPE_PARAM]: authRequestType, headers: additionalAuthHeaders, }); return { @@ -112,16 +108,11 @@ export class SecurityClient { } } - public async authinfo( - request: OpenSearchDashboardsRequest, - authRequestType: string = '', - headers: any = {} - ) { + public async authinfo(request: OpenSearchDashboardsRequest, headers: any = {}) { try { return await this.esClient .asScoped(request) .callAsCurrentUser('opensearch_security.authinfo', { - [AUTH_TYPE_PARAM]: authRequestType, headers, }); } catch (error: any) { diff --git a/server/backend/opensearch_security_plugin.ts b/server/backend/opensearch_security_plugin.ts index 9894a7efc..33b72cc0d 100644 --- a/server/backend/opensearch_security_plugin.ts +++ b/server/backend/opensearch_security_plugin.ts @@ -13,9 +13,6 @@ * permissions and limitations under the License. */ -import { AUTH_TYPE_PARAM } from '../../common'; -import { addQueryParamsToURLIfAny } from './utils'; - // eslint-disable-next-line import/no-default-export export default function (Client: any, config: any, components: any) { const ca = components.clientAction.factory; @@ -29,14 +26,7 @@ export default function (Client: any, config: any, components: any) { */ Client.prototype.opensearch_security.prototype.authinfo = ca({ url: { - fmt: `/_plugins/_security/authinfo`, - opt: { - [AUTH_TYPE_PARAM]: { - type: 'string', - required: false, - }, - }, - template: (params) => addQueryParamsToURLIfAny(params, '/_plugins/_security/authinfo'), + fmt: '/_plugins/_security/authinfo', }, }); diff --git a/server/backend/test/utils.test.ts b/server/backend/test/utils.test.ts deleted file mode 100644 index 47ad18e74..000000000 --- a/server/backend/test/utils.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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 { addQueryParamsToURLIfAny } from '../utils'; - -describe('Test backend utils', () => { - it('checks addQueryParamsToURLIfAny', () => { - const url = '/_plugins/_security/abc'; - - const requestObject = { foo: 'bar', por: 'que' }; - - const expected = url + '?foo=bar&por=que'; - const actual = addQueryParamsToURLIfAny(requestObject, url); - - expect(actual).toEqual(expected); - }); -}); diff --git a/server/backend/utils.ts b/server/backend/utils.ts deleted file mode 100644 index 8b0516bf0..000000000 --- a/server/backend/utils.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* - * 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. - */ - -export function addQueryParamsToURLIfAny(params: any = {}, url: string) { - // fetches all String components of query params and joins them by `&` - const queryParams = Object.entries(params) - .map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`) - .join('&'); - - if (queryParams) { - url += `?${queryParams}`; - } - - return url; -} diff --git a/server/readonly/readonly_service.ts b/server/readonly/readonly_service.ts index 901594812..6e690b5f7 100644 --- a/server/readonly/readonly_service.ts +++ b/server/readonly/readonly_service.ts @@ -24,7 +24,6 @@ import { isPrivateTenant, LOGIN_PAGE_URI, CUSTOM_ERROR_PAGE_URI, - AuthType, } from '../../common'; import { SecurityClient } from '../backend/opensearch_security_client'; import { IAuthenticationType, OpenSearchAuthInfo } from '../auth/types/authentication_type'; @@ -105,12 +104,7 @@ export class ReadonlyService extends BaseReadonlyService { return false; } - const authType = cookie - ? cookie.isAnonymousAuth - ? AuthType.ANONYMOUS - : cookie.authType - : ''; - const authInfo = await this.securityClient.authinfo(request, authType, headers); + const authInfo = await this.securityClient.authinfo(request, headers); if (!authInfo.user_requested_tenant && cookie) { authInfo.user_requested_tenant = cookie.tenant; diff --git a/test/jest_integration/basic_auth.test.ts b/test/jest_integration/basic_auth.test.ts index 421b76a83..cdb4a39d0 100644 --- a/test/jest_integration/basic_auth.test.ts +++ b/test/jest_integration/basic_auth.test.ts @@ -227,11 +227,16 @@ describe('start OpenSearch Dashboards server', () => { .unset(AUTHORIZATION_HEADER_NAME); expect(response.status).toEqual(302); - expect(response.header.location).toEqual('/app/login?nextUrl=%2Fapp%2Fhome'); + expect(response.header.location).toEqual('/auth/anonymous?nextUrl=%2Fapp%2Fhome'); const response2 = await osdTestServer.request.get(root, response.header.location); - expect(response2.status).toEqual(200); + expect(response2.status).toEqual(302); + expect(response2.header.location).toEqual('/app/login?nextUrl=%2Fapp%2Fhome'); + + const response3 = await osdTestServer.request.get(root, response2.header.location); + + expect(response3.status).toEqual(200); }); it('redirect for home follows login for anonymous auth disabled', async () => { @@ -260,10 +265,14 @@ describe('start OpenSearch Dashboards server', () => { .unset(AUTHORIZATION_HEADER_NAME); expect(response.status).toEqual(302); - expect(response.header.location).toEqual(expectedPath); const response2 = await osdTestServer.request.get(root, response.header.location); - expect(response2.status).toEqual(200); + expect(response2.status).toEqual(302); + expect(response2.header.location).toEqual(expectedPath); + + const response3 = await osdTestServer.request.get(root, response2.header.location); + + expect(response3.status).toEqual(200); }); });