Skip to content

Commit

Permalink
Renames query param and removes unused variables
Browse files Browse the repository at this point in the history
Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura committed Apr 8, 2024
1 parent 932fd5b commit 1f1ecfa
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 30 deletions.
4 changes: 1 addition & 3 deletions common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ export const OPENID_AUTH_LOGIN_WITH_FRAGMENT = '/auth/openid/captureUrlFragment'
export const SAML_AUTH_LOGIN = '/auth/saml/login';
export const SAML_AUTH_LOGIN_WITH_FRAGMENT = '/auth/saml/captureUrlFragment';
export const ANONYMOUS_AUTH_LOGIN = '/auth/anonymous';
export const AUTH_REQUEST_TYPE_HEADER = 'auth_request_type';
export const SAML_AUTH_REQUEST_TYPE = 'saml';
export const OPEN_ID_AUTH_REQUEST_TYPE = 'openid';
export const AUTH_TYPE_PARAM = 'auth_type';

export const OPENID_AUTH_LOGOUT = '/auth/openid/logout';
export const SAML_AUTH_LOGOUT = '/auth/saml/logout';
Expand Down
8 changes: 5 additions & 3 deletions server/auth/types/authentication_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -210,7 +210,8 @@ export abstract class AuthenticationType implements IAuthenticationType {
}
}
if (!authInfo) {
authInfo = await this.securityClient.authinfo(request, authHeaders);
const authRequestType = cookie.isAnonymousAuth ? 'anonymous' : cookie.authType;
authInfo = await this.securityClient.authinfo(request, authRequestType, authHeaders);
}
authState.authInfo = authInfo;

Expand Down Expand Up @@ -243,9 +244,10 @@ export abstract class AuthenticationType implements IAuthenticationType {
authHeader: any,
authInfo: any
): Promise<string | undefined> {
const authType = cookie.isAnonymousAuth ? 'anonymous' : cookie.authType;
if (!authInfo) {
try {
authInfo = await this.securityClient.authinfo(request, authHeader);
authInfo = await this.securityClient.authinfo(request, authType, authHeader);
} catch (error: any) {
throw new UnauthenticatedError(error);
}
Expand Down
2 changes: 1 addition & 1 deletion server/auth/types/basic/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class BasicAuthRoutes {
}
context.security_plugin.logger.info('The Redirect Path is ' + redirectUrl);
try {
user = await this.securityClient.authenticateWithHeaders(request, {});
user = await this.securityClient.authenticateWithHeaders(request, 'anonymous', {});
} catch (error) {
context.security_plugin.logger.error(
`Failed authentication: ${error}. Redirecting to Login Page`
Expand Down
3 changes: 1 addition & 2 deletions server/auth/types/openid/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {
AUTH_GRANT_TYPE,
AUTH_RESPONSE_TYPE,
OPENID_AUTH_LOGOUT,
OPEN_ID_AUTH_REQUEST_TYPE,
} from '../../../../common';

import {
Expand Down Expand Up @@ -189,7 +188,7 @@ export class OpenIdAuthRoutes {
request,
this.openIdAuthConfig.authHeaderName as string,
`Bearer ${tokenResponse.idToken}`,
OPEN_ID_AUTH_REQUEST_TYPE
AuthType.OPEN_ID
);

// set to cookie
Expand Down
17 changes: 6 additions & 11 deletions server/auth/types/saml/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ import { SecurityPluginConfigType } from '../../..';
import { SecurityClient } from '../../../backend/opensearch_security_client';
import { CoreSetup } from '../../../../../../src/core/server';
import { validateNextUrl } from '../../../utils/next_url';
import {
AuthType,
SAML_AUTH_LOGIN,
SAML_AUTH_LOGOUT,
SAML_AUTH_REQUEST_TYPE,
} from '../../../../common';
import { AuthType, SAML_AUTH_LOGIN, SAML_AUTH_LOGOUT } from '../../../../common';

import {
clearSplitCookies,
Expand Down Expand Up @@ -140,13 +135,13 @@ export class SamlAuthRoutes {
requestId,
samlResponse: request.body.SAMLResponse,
acsEndpoint: undefined,
authRequestType: SAML_AUTH_REQUEST_TYPE,
authRequestType: AuthType.SAML,
});
const user = await this.securityClient.authenticateWithHeader(
request,
'authorization',
credentials.authorization,
SAML_AUTH_REQUEST_TYPE
AuthType.SAML
);

let expiryTime = Date.now() + this.config.session.ttl;
Expand Down Expand Up @@ -219,13 +214,13 @@ export class SamlAuthRoutes {
requestId: undefined,
samlResponse: request.body.SAMLResponse,
acsEndpoint,
authRequestType: SAML_AUTH_REQUEST_TYPE,
authRequestType: AuthType.SAML,
});
const user = await this.securityClient.authenticateWithHeader(
request,
'authorization',
credentials.authorization,
SAML_AUTH_REQUEST_TYPE
AuthType.SAML
);

let expiryTime = Date.now() + this.config.session.ttl;
Expand Down Expand Up @@ -387,7 +382,7 @@ export class SamlAuthRoutes {
},
async (context, request, response) => {
try {
const authInfo = await this.securityClient.authinfo(request);
const authInfo = await this.securityClient.authinfo(request, AuthType.SAML);
await clearSplitCookies(
request,
this.getExtraAuthStorageOptions(context.security_plugin.logger)
Expand Down
21 changes: 14 additions & 7 deletions server/backend/opensearch_security_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { ILegacyClusterClient, OpenSearchDashboardsRequest } from '../../../../src/core/server';
import { User } from '../auth/user';
import { TenancyConfigSettings } from '../../public/apps/configuration/panels/tenancy-config/types';
import { AUTH_REQUEST_TYPE_HEADER, AuthType, SAML_AUTH_REQUEST_TYPE } from '../../common';
import { AUTH_TYPE_PARAM, AuthType } from '../../common';

export class SecurityClient {
constructor(private readonly esClient: ILegacyClusterClient) {}
Expand All @@ -32,7 +32,7 @@ export class SecurityClient {
headers: {
authorization: `Basic ${authHeader}`,
},
AUTH_REQUEST_TYPE_HEADER: AuthType.BASIC,
[AUTH_TYPE_PARAM]: AuthType.BASIC,
});
return {
username: credentials.username,
Expand All @@ -53,7 +53,7 @@ export class SecurityClient {
request: OpenSearchDashboardsRequest,
headerName: string,
headerValue: string,
authRequestType: string | undefined,
authRequestType: string,
whitelistedHeadersAndValues: any = {},
additionalAuthHeaders: any = {}
): Promise<User> {
Expand All @@ -66,13 +66,13 @@ export class SecurityClient {
if (headerValue) {
headers[headerName] = headerValue;
}
headers.AUTH_REQUEST_TYPE_HEADER = authRequestType;

// cannot get config elasticsearch.requestHeadersWhitelist from kibana.yml file in new platfrom
// meanwhile, do we really need to save all headers in cookie?
const esResponse = await this.esClient
.asScoped(request)
.callAsCurrentUser('opensearch_security.authinfo', {
[AUTH_TYPE_PARAM]: authRequestType,
headers,
});
return {
Expand All @@ -90,12 +90,14 @@ export class SecurityClient {

public async authenticateWithHeaders(
request: OpenSearchDashboardsRequest,
authRequestType: string,
additionalAuthHeaders: any = {}
): Promise<User> {
try {
const esResponse = await this.esClient
.asScoped(request)
.callAsCurrentUser('opensearch_security.authinfo', {
[AUTH_TYPE_PARAM]: authRequestType,
headers: additionalAuthHeaders,
});
return {
Expand All @@ -110,11 +112,16 @@ export class SecurityClient {
}
}

public async authinfo(request: OpenSearchDashboardsRequest, headers: any = {}) {
public async authinfo(
request: OpenSearchDashboardsRequest,
authRequestType: string = '',
headers: any = {}
) {
try {
return await this.esClient
.asScoped(request)
.callAsCurrentUser('opensearch_security.authinfo', {
[AUTH_TYPE_PARAM]: authRequestType,
headers,
});
} catch (error: any) {
Expand Down Expand Up @@ -187,7 +194,7 @@ export class SecurityClient {
try {
// response is expected to be an error
await this.esClient.asScoped(request).callAsCurrentUser('opensearch_security.authinfo', {
[AUTH_REQUEST_TYPE_HEADER]: SAML_AUTH_REQUEST_TYPE,
[AUTH_TYPE_PARAM]: AuthType.SAML,
});
} catch (error: any) {
// the error looks like
Expand Down Expand Up @@ -237,7 +244,7 @@ export class SecurityClient {
try {
return await this.esClient.asScoped().callAsCurrentUser('opensearch_security.authtoken', {
body,
[AUTH_REQUEST_TYPE_HEADER]: authRequestType,
[AUTH_TYPE_PARAM]: authRequestType,
});
} catch (error: any) {
console.log(error);
Expand Down
31 changes: 30 additions & 1 deletion server/backend/opensearch_security_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* permissions and limitations under the License.
*/

import { AUTH_TYPE_PARAM } from '../../common';

// eslint-disable-next-line import/no-default-export
export default function (Client: any, config: any, components: any) {
const ca = components.clientAction.factory;
Expand All @@ -26,7 +28,34 @@ export default function (Client: any, config: any, components: any) {
*/
Client.prototype.opensearch_security.prototype.authinfo = ca({
url: {
fmt: '/_plugins/_security/authinfo',
fmt: `/_plugins/_security/authinfo`,
opt: {
[AUTH_TYPE_PARAM]: {
type: 'string',
required: false,
},
},
template: (requestObj) => {
const obj = requestObj || (requestObj = {});
let __p = '/_plugins/_security/authinfo';
const __q = []; // Array to hold query string components

// Iterate over the object properties to construct the query string
for (const key in obj) {
if (obj.hasOwnProperty(key)) {
// Ensure the value is a string to avoid URL encoding issues
const value = String(obj[key]);
__q.push(encodeURIComponent(key) + '=' + encodeURIComponent(value));
}
}

// If there are query parameters, append them to the URL
if (__q.length > 0) {
__p += '?' + __q.join('&');
}

return __p;
},
},
});

Expand Down
3 changes: 2 additions & 1 deletion server/readonly/readonly_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ export class ReadonlyService extends BaseReadonlyService {
return false;
}

const authInfo = await this.securityClient.authinfo(request, headers);
const authType = cookie ? (cookie.isAnonymousAuth ? 'anonymous' : cookie.authType) : '';
const authInfo = await this.securityClient.authinfo(request, authType, headers);

if (!authInfo.user_requested_tenant && cookie) {
authInfo.user_requested_tenant = cookie.tenant;
Expand Down
4 changes: 3 additions & 1 deletion test/jest_integration/basic_auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ describe('start OpenSearch Dashboards server', () => {
});

it('enforce authentication on api/status route', async () => {
const response = await osdTestServer.request.get(root, '/api/status');
const response = await osdTestServer.request
.get(root, '/api/status')
.unset(AUTHORIZATION_HEADER_NAME);
expect(response.status).toEqual(401);
});

Expand Down

0 comments on commit 1f1ecfa

Please sign in to comment.