Skip to content

Commit

Permalink
Fixes Short URL redirection for SAML login (#1744)
Browse files Browse the repository at this point in the history
Signed-off-by: Deepak Devarakonda <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Darshit Chanpura <[email protected]>
(cherry picked from commit 4d7e5f3)
  • Loading branch information
devardee authored and github-actions[bot] committed Feb 5, 2024
1 parent 5c2e015 commit 78e44f8
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 9 deletions.
5 changes: 4 additions & 1 deletion server/auth/types/saml/saml_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,12 @@ export class SamlAuthentication extends AuthenticationType {
}

private generateNextUrl(request: OpenSearchDashboardsRequest): string {
const path =
let path =
this.coreSetup.http.basePath.serverBasePath +
(request.url.pathname || '/app/opensearch-dashboards');
if (request.url.search) {
path += request.url.search;
}
return escape(path);
}

Expand Down
16 changes: 10 additions & 6 deletions server/multitenancy/tenant_resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
* permissions and limitations under the License.
*/

import { isEmpty, findKey, cloneDeep } from 'lodash';
import { cloneDeep, findKey, isEmpty } from 'lodash';
import { OpenSearchDashboardsRequest } from 'opensearch-dashboards/server';
import { ResponseObject } from '@hapi/hapi';
import { modifyUrl } from '@osd/std';
import { SecuritySessionCookie } from '../session/security_cookie';
import { SecurityPluginConfigType } from '..';
import { GLOBAL_TENANT_SYMBOL, PRIVATE_TENANT_SYMBOL, globalTenantName } from '../../common';
import { GLOBAL_TENANT_SYMBOL, globalTenantName, PRIVATE_TENANT_SYMBOL } from '../../common';
import { ensureRawRequest } from '../../../../src/core/server/http/router';
import { GOTO_PREFIX } from '../../../../src/plugins/share/common/short_url_routes';

Expand Down Expand Up @@ -210,16 +210,20 @@ export function addTenantParameterToResolvedShortLink(request: OpenSearchDashboa
if (request.url.pathname.startsWith(`${GOTO_PREFIX}/`)) {
const rawRequest = ensureRawRequest(request);
const rawResponse = rawRequest.response as ResponseObject;
const responsePath = rawResponse.headers.location as string;

// Make sure the request really should redirect
if (rawResponse.headers.location) {
const modifiedUrl = modifyUrl(rawResponse.headers.location as string, (parts) => {
if (
rawResponse.headers.location &&
!responsePath.includes('security_tenant') &&
request.headers.securitytenant
) {
// Mutating the headers toolkit.next({headers: ...}) logs a warning about headers being overwritten
rawResponse.headers.location = modifyUrl(responsePath, (parts) => {
if (parts.query.security_tenant === undefined) {
parts.query.security_tenant = request.headers.securitytenant as string;
}
// Mutating the headers toolkit.next({headers: ...}) logs a warning about headers being overwritten
});
rawResponse.headers.location = modifiedUrl;
}
}

Expand Down
16 changes: 15 additions & 1 deletion test/cypress/e2e/saml/saml_auth_test.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { ALL_ACCESS_ROLE } from '../../support/constants';
import { ALL_ACCESS_ROLE, SHORTEN_URL_DATA } from '../../support/constants';

import samlUserRoleMapping from '../../fixtures/saml/samlUserRoleMappiing.json';

Expand Down Expand Up @@ -121,4 +121,18 @@ describe('Log in via SAML', () => {

cy.get('#tenantName').should('have.text', 'Private');
});

it('Login to Dashboard with Goto URL', () => {
localStorage.setItem('home:newThemeModal:show', 'false');
cy.shortenUrl(SHORTEN_URL_DATA, 'global').then((response) => {
// We need to explicitly clear cookies,
// since the Shorten URL api is return's set-cookie header for admin user.
cy.clearCookies().then(() => {
const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`;
cy.visit(gotoUrl);
samlLogin();
cy.getCookie('security_authentication').should('exist');
});
});
});
});
14 changes: 13 additions & 1 deletion test/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { SEC_API, ADMIN_AUTH } from './constants';
import { SEC_API, ADMIN_AUTH, DASHBOARDS_API } from './constants';

/**
* Overwrite request command to support authentication similar to visit.
Expand Down Expand Up @@ -90,3 +90,15 @@ Cypress.Commands.add('loginWithSamlMultiauth', () => {
cy.get('input[id=userName]').should('be.visible');
cy.get('button[id=btn-sign-in]').should('be.visible').click();
});

Cypress.Commands.add('shortenUrl', (data, tenant) => {
cy.request({
url: `http://localhost:5601${DASHBOARDS_API.SHORTEN_URL}`,
method: 'POST',
body: data,
headers: { securitytenant: tenant, 'osd-xsrf': 'osd-fetch' },
}).then((response) => {
expect(response.status).to.eq(200);
return response.body;
});
});
4 changes: 4 additions & 0 deletions test/cypress/support/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ export const SEC_API = {
ROLE_BASE: `${SEC_API_PREFIX}/roles`,
ROLE_MAPPING_BASE: `${SEC_API_PREFIX}/rolesmapping`,
};
export const DASHBOARDS_API = {
SHORTEN_URL: '/api/shorten_url',
};
export const SHORTEN_URL_DATA = { url: '/app/home#/tutorial_directory' };
12 changes: 12 additions & 0 deletions test/cypress/support/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,16 @@ declare namespace Cypress {
*/
createRoleMapping<S = any>(roleID: string, rolemappingJson: string): Chainable<S>;
}

interface Chainable<Subject> {
/**
* Generate a UUID for the passed URL and store it in the tenant index.
* @example :
* cy.shortenUrl({url: "/app/home#/tutorial_directory"}, 'global')
*
* @param data - The Object which contains the url.
* @param tenant - The tenant index which will store the UUID
*/
shortenUrl<S = any>(data: object, tenant: string): Chainable<S>;
}
}

0 comments on commit 78e44f8

Please sign in to comment.