Skip to content

Commit

Permalink
[Backport 1.3] Preserve URL Hash for SAML based login (#1226)
Browse files Browse the repository at this point in the history
* SAML Integration Tests (#1088)

* Preserve URL Hash for SAML based login (#1039)

* Preserve URL HASH after user logs via SAML IDP

Signed-off-by: Deepak Devarakonda <devardee@amazon.com>
Signed-off-by: Aniketh Jain <anijainc@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: anijain-Amazon <110471048+anijain-Amazon@users.noreply.github.com>
Co-authored-by: Deepak Devarakonda <devardee@amazon.com>
Co-authored-by: Deepak Devarakonda <80896069+devardee@users.noreply.github.com>
Co-authored-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com>
  • Loading branch information
5 people authored Dec 1, 2022
1 parent d948c78 commit c34dc55
Show file tree
Hide file tree
Showing 15 changed files with 4,039 additions and 19 deletions.
9 changes: 8 additions & 1 deletion .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ jobs:
name: Run integration tests
runs-on: ubuntu-latest
steps:
- uses: browser-actions/setup-geckodriver@latest
- run: geckodriver --version

- uses: browser-actions/setup-firefox@latest
- run: firefox --version

- name: Download OpenSearch Core
run: |
wget https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/1.3.7/latest/linux/x64/tar/builds/opensearch/dist/opensearch-min-1.3.7-linux-x64.tar.gz
Expand Down Expand Up @@ -37,7 +43,7 @@ jobs:
WORKDIR /opensearch/
ENTRYPOINT /docker-host/os-ep.sh
EOF
docker run -d -p 9200:9200 -p 9600:9600 -i opensearch-test:latest
docker run -d --network=host -i opensearch-test:latest
- name: Checkout OpenSearch Dashboard
uses: actions/checkout@v2
Expand Down Expand Up @@ -98,6 +104,7 @@ jobs:
run: |
cd ./OpenSearch-Dashboards
yarn osd bootstrap
node scripts/build_opensearch_dashboards_platform_plugins.js
- name: Run integration tests
run: |
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Unit Tests
on: [push, pull_request]

jobs:
tests:
unit-tests:
name: Run unit tests
runs-on: ubuntu-latest

Expand Down Expand Up @@ -48,8 +48,8 @@ jobs:
echo "Installing yarn ${{ steps.versions_step.outputs.yarn_version }}"
npm i -g yarn@${{ steps.versions.outputs.yarn_version }}
- name: Bootstrap OpenSearch Dashboards
working-directory: ./OpenSearch-Dashboards
run: |
cd ./OpenSearch-Dashboards
yarn osd bootstrap --oss
- name: Run lint
run: |
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ node_modules
/build/
target
.eslintcache
yarn.lock
/coverage/
.es/
yarn-error.log
Expand Down
3 changes: 3 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ plugins.security.allow_default_init_securityindex: true
plugins.security.authcz.admin_dn:
- CN=kirk,OU=client,O=client,L=test, C=de

plugins.security.unsupported.restapi.allow_securityconfig_modification: true
plugins.security.audit.type: internal_opensearch
plugins.security.enable_snapshot_restore_privilege: true
plugins.security.check_snapshot_restore_write_privileges: true
Expand Down Expand Up @@ -71,6 +72,8 @@ yarn build

We should be able to run Dashboards now changing back to its base directory and running `yarn start`. Navigating to the URL given as console output (something like `http://localhost:5601/omf`) you should now be able to log in with user `admin` and password `admin`.

To run selenium based integration tests, download and export the firefox web-driver to your PATH. Also, run `node scripts/build_opensearch_dashboards_platform_plugins.js` or `yarn start` before running the tests. This is essential to generate the bundles.

## Submitting Changes

See [CONTRIBUTING](CONTRIBUTING.md).
Expand Down
9 changes: 8 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,22 @@
"lint:es": "node ../../scripts/eslint",
"lint:sass": "node ../../scripts/sasslint",
"lint": "yarn run lint:es && yarn run lint:sass",
"pretest:jest_server": "node ./test/jest_integration/runIdpServer.js &",
"test:jest_server": "node ./test/run_jest_tests.js --config ./test/jest.config.server.js",
"test:jest_ui": "node ./test/run_jest_tests.js --config ./test/jest.config.ui.js"
},
"resolutions": {
"samlp": "6.0.2"
},
"devDependencies": {
"@elastic/eslint-import-resolver-kibana": "link:../../packages/osd-eslint-import-resolver-opensearch-dashboards",
"typescript": "4.0.2",
"gulp-rename": "2.0.0",
"@testing-library/react-hooks": "^3.4.1",
"@types/hapi__wreck": "^15.0.1"
"@types/hapi__wreck": "^15.0.1",
"saml-idp": "^1.2.1",
"samlp": "6.0.2",
"selfsigned": "^2.0.1"
},
"dependencies": {
"@hapi/wreck": "^15.0.2",
Expand Down
8 changes: 6 additions & 2 deletions public/apps/account/account-nav-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ export function AccountNavButton(props: {
<EuiListGroupItem
color="subdued"
key="tenant"
label={<EuiText size="xs">{resolveTenantName(props.tenant || '', username)}</EuiText>}
label={
<EuiText size="xs" id="tenantName">
{resolveTenantName(props.tenant || '', username)}
</EuiText>
}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down Expand Up @@ -140,7 +144,7 @@ export function AccountNavButton(props: {
</div>
);
return (
<EuiHeaderSectionItemButton>
<EuiHeaderSectionItemButton id="user-icon-btn">
<EuiPopover
data-test-subj="account-popover"
id="actionsMenu"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Account navigation button renders 1`] = `
<EuiHeaderSectionItemButton>
<EuiHeaderSectionItemButton
id="user-icon-btn"
>
<EuiPopover
anchorPosition="downCenter"
button={
Expand Down Expand Up @@ -63,6 +65,7 @@ exports[`Account navigation button renders 1`] = `
key="tenant"
label={
<EuiText
id="tenantName"
size="xs"
>
tenant1
Expand Down
137 changes: 132 additions & 5 deletions server/auth/types/saml/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class SamlAuthRoutes {
validate: validateNextUrl,
})
),
redirectHash: schema.string(),
}),
},
options: {
Expand All @@ -67,6 +68,7 @@ export class SamlAuthRoutes {
saml: {
nextUrl: request.query.nextUrl,
requestId: samlHeader.requestId,
redirectHash: request.query.redirectHash === 'true',
},
};
this.sessionStorageFactory.asScoped(request).set(cookie);
Expand Down Expand Up @@ -95,13 +97,15 @@ export class SamlAuthRoutes {
async (context, request, response) => {
let requestId: string = '';
let nextUrl: string = '/';
let redirectHash: boolean = false;
try {
const cookie = await this.sessionStorageFactory.asScoped(request).get();
if (cookie) {
requestId = cookie.saml?.requestId || '';
nextUrl =
cookie.saml?.nextUrl ||
`${this.coreSetup.http.basePath.serverBasePath}/app/opensearch-dashboards`;
redirectHash = cookie.saml?.redirectHash || false;
}
if (!requestId) {
return response.badRequest({
Expand Down Expand Up @@ -143,11 +147,21 @@ export class SamlAuthRoutes {
expiryTime,
};
this.sessionStorageFactory.asScoped(request).set(cookie);
return response.redirected({
headers: {
location: nextUrl,
},
});
if (redirectHash) {
return response.redirected({
headers: {
location: `${
this.coreSetup.http.basePath.serverBasePath
}/auth/saml/redirectUrlFragment?nextUrl=${escape(nextUrl)}`,
},
});
} else {
return response.redirected({
headers: {
location: nextUrl,
},
});
}
} catch (error) {
context.security_plugin.logger.error(
`SAML SP initiated authentication workflow failed: ${error}`
Expand Down Expand Up @@ -215,6 +229,119 @@ export class SamlAuthRoutes {
}
);

// captureUrlFragment is the first route that will be invoked in the SP initiated login.
// This route will execute the captureUrlFragment.js script.
this.coreSetup.http.resources.register(
{
path: '/auth/saml/captureUrlFragment',
validate: {
query: schema.object({
nextUrl: schema.maybe(
schema.string({
validate: validateNextUrl,
})
),
}),
},
options: {
authRequired: false,
},
},
async (context, request, response) => {
this.sessionStorageFactory.asScoped(request).clear();
const serverBasePath = this.coreSetup.http.basePath.serverBasePath;
return response.renderHtml({
body: `
<!DOCTYPE html>
<title>OSD SAML Capture</title>
<link rel="icon" href="data:,">
<script src="${serverBasePath}/auth/saml/captureUrlFragment.js"></script>
`,
});
}
);

// This script will store the URL Hash in browser's local storage.
this.coreSetup.http.resources.register(
{
path: '/auth/saml/captureUrlFragment.js',
validate: false,
options: {
authRequired: false,
},
},
async (context, request, response) => {
this.sessionStorageFactory.asScoped(request).clear();
return response.renderJs({
body: `let samlHash=window.location.hash.toString();
let redirectHash = false;
if (samlHash !== "") {
window.localStorage.removeItem('samlHash');
window.localStorage.setItem('samlHash', samlHash);
redirectHash = true;
}
let params = new URLSearchParams(window.location.search);
let nextUrl = params.get("nextUrl");
finalUrl = "login?nextUrl=" + encodeURIComponent(nextUrl);
finalUrl += "&redirectHash=" + encodeURIComponent(redirectHash);
window.location.replace(finalUrl);
`,
});
}
);

// Once the User is authenticated via the '_opendistro/_security/saml/acs' route,
// the browser will be redirected to '/auth/saml/redirectUrlFragment' route,
// which will execute the redirectUrlFragment.js.
this.coreSetup.http.resources.register(
{
path: '/auth/saml/redirectUrlFragment',
validate: {
query: schema.object({
nextUrl: schema.any(),
}),
},
options: {
authRequired: true,
},
},
async (context, request, response) => {
const serverBasePath = this.coreSetup.http.basePath.serverBasePath;
return response.renderHtml({
body: `
<!DOCTYPE html>
<title>OSD SAML Success</title>
<link rel="icon" href="data:,">
<script src="${serverBasePath}/auth/saml/redirectUrlFragment.js"></script>
`,
});
}
);

// This script will pop the Hash from local storage if it exists.
// And forward the browser to the next url.
this.coreSetup.http.resources.register(
{
path: '/auth/saml/redirectUrlFragment.js',
validate: false,
options: {
authRequired: true,
},
},
async (context, request, response) => {
return response.renderJs({
body: `let samlHash=window.localStorage.getItem('samlHash');
window.localStorage.removeItem('samlHash');
let params = new URLSearchParams(window.location.search);
let nextUrl = params.get("nextUrl");
finalUrl = nextUrl + samlHash;
window.location.replace(finalUrl);
`,
});
}
);

this.router.get(
{
path: `/auth/logout`,
Expand Down
11 changes: 6 additions & 5 deletions server/auth/types/saml/saml_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,18 @@ export class SamlAuthentication extends AuthenticationType {
private generateNextUrl(request: OpenSearchDashboardsRequest): string {
const path =
this.coreSetup.http.basePath.serverBasePath +
(request.url.path || '/app/opensearch-dashboards');
(request.url.pathname || '/app/opensearch-dashboards');
return escape(path);
}

private redirectToLoginUri(request: OpenSearchDashboardsRequest, toolkit: AuthToolkit) {
private redirectSAMlCapture = (request: OpenSearchDashboardsRequest, toolkit: AuthToolkit) => {
const nextUrl = this.generateNextUrl(request);
const clearOldVersionCookie = clearOldVersionCookieValue(this.config);
return toolkit.redirected({
location: `${this.coreSetup.http.basePath.serverBasePath}/auth/saml/login?nextUrl=${nextUrl}`,
location: `${this.coreSetup.http.basePath.serverBasePath}/auth/saml/captureUrlFragment?nextUrl=${nextUrl}`,
'set-cookie': clearOldVersionCookie,
});
}
};

private setupRoutes(): void {
const samlAuthRoutes = new SamlAuthRoutes(
Expand Down Expand Up @@ -97,6 +97,7 @@ export class SamlAuthentication extends AuthenticationType {
};
}

// Can be improved to check if the token is expiring.
async isValidCookie(cookie: SecuritySessionCookie): Promise<boolean> {
return (
cookie.authType === this.type &&
Expand All @@ -112,7 +113,7 @@ export class SamlAuthentication extends AuthenticationType {
toolkit: AuthToolkit
): IOpenSearchDashboardsResponse | AuthResult {
if (this.isPageRequest(request)) {
return this.redirectToLoginUri(request, toolkit);
return this.redirectSAMlCapture(request, toolkit);
} else {
return response.unauthorized();
}
Expand Down
1 change: 1 addition & 0 deletions server/backend/opensearch_security_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export class SecurityClient {
// location="https://<your-auth-domain.com>/api/saml2/v1/sso?SAMLRequest=<some-encoded-string>"
// requestId="<request_id>"
// '

if (!error.wwwAuthenticateDirective) {
throw error;
}
Expand Down
1 change: 1 addition & 0 deletions server/session/security_cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface SecuritySessionCookie {
saml?: {
requestId?: string;
nextUrl?: string;
redirectHash?: boolean;
};
}

Expand Down
2 changes: 1 addition & 1 deletion test/helper/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { AUTHORIZATION_HEADER_NAME } from '../constant';

export function extractAuthCookie(response: Response) {
const setCookieHeaders = response.header['set-cookie'] as string[];
let securityAuthCookie: string;
let securityAuthCookie: string | null = null;
for (const setCookie of setCookieHeaders) {
if (setCookie.startsWith('security_authentication=')) {
securityAuthCookie = setCookie.split(';')[0];
Expand Down
Loading

0 comments on commit c34dc55

Please sign in to comment.