Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Short URL redirection for SAML login #1744

Merged
merged 11 commits into from
Feb 5, 2024
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 @@ -102,7 +102,7 @@
samlLogin();

cy.get('#private').should('be.enabled');
cy.get('#private').click({ force: true });

Check warning on line 105 in test/cypress/e2e/saml/saml_auth_test.spec.js

View workflow job for this annotation

GitHub Actions / Run unit tests (ubuntu-latest)

Do not use force on click and type calls

Check warning on line 105 in test/cypress/e2e/saml/saml_auth_test.spec.js

View workflow job for this annotation

GitHub Actions / Run unit tests (windows-latest)

Do not use force on click and type calls

Check warning on line 105 in test/cypress/e2e/saml/saml_auth_test.spec.js

View workflow job for this annotation

GitHub Actions / Run unit tests (macos-latest)

Do not use force on click and type calls

cy.get('button[data-test-subj="confirm"]').click();

Expand All @@ -121,4 +121,18 @@

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>;
}
}
Loading