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

[Backport 2.x] Fix issue when using OpenID Authentication with serverBasePath #1946

Merged
merged 1 commit into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .github/actions/run-cypress-tests/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ inputs:
yarn_command:
description: 'The yarn command to start running cypress tests'
required: true
osd_base_path:
description: 'The base path for OpenSearch Dashboards'
required: false
default: ''

runs:
using: "composite"
Expand Down Expand Up @@ -56,14 +60,22 @@ runs:
if: ${{ runner.os == 'Linux' }}
run: |
cd ./OpenSearch-Dashboards/plugins/security-dashboards-plugin
yarn runIdp
if [ -z "${{ inputs.osd_base_path }}" ]; then
yarn runIdp &
else
yarn runIdp --basePath ${{ inputs.osd_base_path }} &
fi
shell: bash

- name: Run OpenSearch Dashboards with provided configuration
if: ${{ runner.os == 'Linux' }}
run: |
cd ./OpenSearch-Dashboards
nohup yarn start --no-base-path --no-watch --csp.warnLegacyBrowsers=false | tee dashboard.log &
if [ -z "${{ inputs.osd_base_path }}" ]; then
nohup yarn start --no-base-path --no-watch --csp.warnLegacyBrowsers=false | tee dashboard.log &
else
nohup yarn start --no-watch --csp.warnLegacyBrowsers=false | tee dashboard.log &
fi
shell: bash

# Check if OSD is ready with a max timeout of 600 seconds
Expand Down
21 changes: 19 additions & 2 deletions .github/workflows/cypress-test-multiauth-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu-latest ]
basePath: [ "", "/osd" ]
runs-on: ${{ matrix.os }}

steps:
Expand Down Expand Up @@ -61,7 +62,7 @@ jobs:
metadata_url: http://localhost:7000/metadata
sp:
entity_id: https://localhost:9200
kibana_url: http://localhost:5601
kibana_url: http://localhost:5601${{ matrix.basePath }}
exchange_key: 6aff3042-1327-4f3d-82f0-40a157ac4464
authentication_backend:
type: noop
Expand All @@ -82,14 +83,30 @@ jobs:
opensearch_security.multitenancy.tenants.preferred: ["Private", "Global"]
opensearch_security.readonly_mode.roles: ["kibana_read_only"]
opensearch_security.cookie.secure: false
server.xsrf.allowlist: ["/_plugins/_security/api/authtoken", "/_opendistro/_security/api/authtoken", "/_opendistro/_security/saml/acs", "/_opendistro/_security/saml/acs/idpinitiated", "/_opendistro/_security/saml/logout"]
server.xsrf.allowlist: ["/_opendistro/_security/saml/acs", "/_opendistro/_security/saml/acs/idpinitiated", "/_opendistro/_security/saml/logout"]
opensearch_security.auth.type: ["basicauth","saml"]
opensearch_security.auth.multiple_auth_enabled: true
opensearch_security.auth.anonymous_auth_enabled: false
home.disableWelcomeScreen: true
EOT

- name: Run OSD with basePath
if: ${{ matrix.basePath != '' }}
run: |
echo "server.basePath: \"${{ matrix.basePath }}\"" >> opensearch_dashboards_multiauth.yml
echo "server.rewriteBasePath: true" >> opensearch_dashboards_multiauth.yml

- name: Run Cypress Tests with basePath
if: ${{ matrix.basePath != '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_multiauth.yml
dashboards_config_file: opensearch_dashboards_multiauth.yml
yarn_command: 'yarn cypress:run --browser chrome --headless --env loginMethod=saml_multiauth,basePath=${{ matrix.basePath }} --spec "test/cypress/e2e/saml/*.js"'
osd_base_path: ${{ matrix.basePath }}

- name: Run Cypress Tests
if: ${{ matrix.basePath == '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_multiauth.yml
Expand Down
24 changes: 20 additions & 4 deletions .github/workflows/cypress-test-oidc-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu-latest ]
basePath: [ "", "/osd" ]
runs-on: ${{ matrix.os }}

steps:
Expand Down Expand Up @@ -62,7 +63,7 @@ jobs:
chmod +x kcadm.sh
echo "Creating client"
./kcadm.sh config credentials --server http://localhost:8080/auth --realm master --user admin --password admin
CID=$(./kcadm.sh create clients -r master -s clientId=opensearch -s secret="${{ env.TEST_KEYCLOAK_CLIENT_SECRET }}" -s 'attributes."access.token.lifespan"=60' -s 'redirectUris=["http://localhost:5603/auth/openid/login", "http://localhost:5601", "http://localhost:5601/auth/openid/login"]' -i)
CID=$(./kcadm.sh create clients -r master -s clientId=opensearch -s secret="${{ env.TEST_KEYCLOAK_CLIENT_SECRET }}" -s 'attributes."access.token.lifespan"=60' -s 'redirectUris=["http://localhost:5603${{ matrix.basePath }}/auth/openid/login", "http://localhost:5601${{ matrix.basePath }}", "http://localhost:5601${{ matrix.basePath }}/auth/openid/login"]' -i)
./kcadm.sh get clients/$CID/installation/providers/keycloak-oidc-keycloak-json > tmp
echo "Getting client secret for dashboards configuration purpose"
CLIENT_SECRET=$(grep -o '"secret" : "[^"]*' tmp | grep -o '[^"]*$')
Expand Down Expand Up @@ -129,13 +130,28 @@ jobs:
opensearch_security.openid.connect_url: "http://127.0.0.1:8080/auth/realms/master/.well-known/openid-configuration"
opensearch_security.openid.client_id: "opensearch"
opensearch_security.openid.client_secret: "${{ env.TEST_KEYCLOAK_CLIENT_SECRET }}"
opensearch_security.auth.type: ["openid"]
opensearch_security.auth.multiple_auth_enabled: true
opensearch_security.ui.openid.login.buttonname: "OIDC"
opensearch_security.openid.base_redirect_url: http://localhost:5601${{ matrix.basePath }}
opensearch_security.auth.type: "openid"
home.disableWelcomeScreen: true
EOT

- name: Run OSD with basePath
if: ${{ matrix.basePath != '' }}
run: |
echo "server.basePath: \"${{ matrix.basePath }}\"" >> opensearch_dashboards_openid.yml
echo "server.rewriteBasePath: true" >> opensearch_dashboards_openid.yml

- name: Run Cypress Tests with basePath
if: ${{ matrix.basePath != '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_openid.yml
dashboards_config_file: opensearch_dashboards_openid.yml
yarn_command: 'yarn cypress:run --browser chrome --headless --spec "test/cypress/e2e/oidc/*.js" --env basePath=${{ matrix.basePath }}'
osd_base_path: ${{ matrix.basePath }}

- name: Run Cypress Tests
if: ${{ matrix.basePath == '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_openid.yml
Expand Down
21 changes: 19 additions & 2 deletions .github/workflows/cypress-test-saml-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu-latest ]
basePath: [ "", "/osd" ]
runs-on: ${{ matrix.os }}

steps:
Expand Down Expand Up @@ -61,7 +62,7 @@ jobs:
metadata_url: http://localhost:7000/metadata
sp:
entity_id: https://localhost:9200
kibana_url: http://localhost:5601
kibana_url: http://localhost:5601${{ matrix.basePath }}
exchange_key: 6aff3042-1327-4f3d-82f0-40a157ac4464
authentication_backend:
type: noop
Expand All @@ -82,14 +83,30 @@ jobs:
opensearch_security.multitenancy.tenants.preferred: ["Private", "Global"]
opensearch_security.readonly_mode.roles: ["kibana_read_only"]
opensearch_security.cookie.secure: false
server.xsrf.allowlist: ["/_plugins/_security/api/authtoken", "/_opendistro/_security/api/authtoken", "/_opendistro/_security/saml/acs", "/_opendistro/_security/saml/acs/idpinitiated", "/_opendistro/_security/saml/logout"]
server.xsrf.allowlist: ["/_opendistro/_security/saml/acs", "/_opendistro/_security/saml/acs/idpinitiated", "/_opendistro/_security/saml/logout"]
opensearch_security.auth.type: ["saml"]
opensearch_security.auth.multiple_auth_enabled: true
opensearch_security.auth.anonymous_auth_enabled: false
home.disableWelcomeScreen: true
EOT

- name: Run OSD with basePath
if: ${{ matrix.basePath != '' }}
run: |
echo "server.basePath: \"${{ matrix.basePath }}\"" >> opensearch_dashboards_saml.yml
echo "server.rewriteBasePath: true" >> opensearch_dashboards_saml.yml

- name: Run Cypress Tests with basePath
if: ${{ matrix.basePath != '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_saml.yml
dashboards_config_file: opensearch_dashboards_saml.yml
yarn_command: 'yarn cypress:run --browser chrome --headless --spec "test/cypress/e2e/saml/*.js" --env basePath=${{ matrix.basePath }}'
osd_base_path: ${{ matrix.basePath }}

- name: Run Cypress Tests
if: ${{ matrix.basePath == '' }}
uses: ./.github/actions/run-cypress-tests
with:
security_config_file: config_saml.yml
Expand Down
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"lint:es": "node ../../scripts/eslint",
"lint:style": "node ../../scripts/stylelint",
"lint": "yarn run lint:es && yarn run lint:style",
"runIdp": "node ./test/jest_integration/runIdpServer.js &",
"runIdp": "node ./test/jest_integration/runIdpServer.js",
"test:jest_server": "ADMIN_PASSWORD=$ADMIN_PASSWORD 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",
"prepare": "husky install"
Expand All @@ -30,14 +30,15 @@
"@types/hapi__wreck": "^15.0.1",
"cypress": "^13.5.1",
"cypress-mochawesome-reporter": "^3.3.0",
"eslint-plugin-cypress": "^2.8.1",
"eslint-plugin-unused-imports": "3.1.0",
"gulp-rename": "2.0.0",
"husky": "^8.0.0",
"jose": "^5.2.4",
"minimist": "^1.2.8",
"saml-idp": "^1.2.1",
"selfsigned": "^2.0.1",
"typescript": "4.0.2",
"eslint-plugin-cypress": "^2.8.1",
"eslint-plugin-unused-imports": "3.1.0",
"husky": "^8.0.0"
"typescript": "4.0.2"
},
"dependencies": {
"@hapi/cryptiles": "5.0.0",
Expand Down
2 changes: 1 addition & 1 deletion public/apps/login/login-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function extractNextUrlFromWindowLocation(): string {
const urlParams = new URLSearchParams(window.location.search);
let nextUrl = urlParams.get('nextUrl');
if (!nextUrl || nextUrl.toLowerCase().includes('//')) {
nextUrl = encodeURIComponent('/');
return '';
} else {
nextUrl = encodeURIComponent(nextUrl);
const hash = window.location.hash || '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ exports[`Login page renders renders with config value for multiauth 1`] = `
aria-label="openid_login_button"
className="test-btn-style"
data-test-subj="submit"
href="/app/opensearch-dashboards/auth/openid/captureUrlFragment?nextUrl=%2F"
href="/app/opensearch-dashboards/auth/openid/captureUrlFragment"
iconType="http://localhost:5601/images/test.png"
size="s"
type="prime"
Expand All @@ -141,7 +141,7 @@ exports[`Login page renders renders with config value for multiauth 1`] = `
aria-label="saml_login_button"
className="test-btn-style"
data-test-subj="submit"
href="/app/opensearch-dashboards/auth/saml/captureUrlFragment?nextUrl=%2F"
href="/app/opensearch-dashboards/auth/saml/captureUrlFragment"
iconType="http://localhost:5601/images/test.png"
size="s"
type="prime"
Expand Down Expand Up @@ -292,7 +292,7 @@ exports[`Login page renders renders with config value for multiauth with anonymo
aria-label="openid_login_button"
className="test-btn-style"
data-test-subj="submit"
href="/app/opensearch-dashboards/auth/openid/captureUrlFragment?nextUrl=%2F"
href="/app/opensearch-dashboards/auth/openid/captureUrlFragment"
iconType="http://localhost:5601/images/test.png"
size="s"
type="prime"
Expand All @@ -312,7 +312,7 @@ exports[`Login page renders renders with config value for multiauth with anonymo
aria-label="saml_login_button"
className="test-btn-style"
data-test-subj="submit"
href="/app/opensearch-dashboards/auth/saml/captureUrlFragment?nextUrl=%2F"
href="/app/opensearch-dashboards/auth/saml/captureUrlFragment"
iconType="http://localhost:5601/images/test.png"
size="s"
type="prime"
Expand Down
2 changes: 1 addition & 1 deletion public/apps/login/test/login-page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('test extractNextUrlFromWindowLocation', () => {
const originalLocation = window.location;
delete window.location;
window.location = new URL('http://localhost:5601/app/home');
expect(extractNextUrlFromWindowLocation()).toEqual('?nextUrl=%2F');
expect(extractNextUrlFromWindowLocation()).toEqual('');
});
});

Expand Down
6 changes: 4 additions & 2 deletions server/auth/types/openid/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,10 @@ export class OpenIdAuthRoutes {
}
let params = new URLSearchParams(window.location.search);
let nextUrl = params.get("nextUrl");
finalUrl = "login?nextUrl=" + encodeURIComponent(nextUrl);
finalUrl += "&redirectHash=" + encodeURIComponent(redirectHash);
finalUrl = "login?redirectHash=" + encodeURIComponent(redirectHash);
if (!!nextUrl) {
finalUrl += "&nextUrl=" + encodeURIComponent(nextUrl);
}
window.location.replace(finalUrl);
`,
});
Expand Down
6 changes: 4 additions & 2 deletions server/auth/types/saml/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,10 @@ export class SamlAuthRoutes {
}
let params = new URLSearchParams(window.location.search);
let nextUrl = params.get("nextUrl");
finalUrl = "login?nextUrl=" + encodeURIComponent(nextUrl);
finalUrl += "&redirectHash=" + encodeURIComponent(redirectHash);
finalUrl = "login?redirectHash=" + encodeURIComponent(redirectHash);
if (!!nextUrl) {
finalUrl += "&nextUrl=" + encodeURIComponent(nextUrl);
}
window.location.replace(finalUrl);
`,
});
Expand Down
2 changes: 1 addition & 1 deletion server/session/security_cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function getSecurityCookieOptions(
// TODO: with setting redirect attributes to support OIDC and SAML,
// we need to do additional cookie validation in AuthenticationHandlers.
// if SAML fields present
if (sessionStorage.saml && sessionStorage.saml.requestId && sessionStorage.saml.nextUrl) {
if (sessionStorage.saml && sessionStorage.saml.requestId) {
return { isValid: true, path: '/' };
}

Expand Down
20 changes: 12 additions & 8 deletions test/cypress/e2e/oidc/oidc_auth_test.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

const basePath = Cypress.env('basePath') || '';

describe('Log in via OIDC', () => {
afterEach(() => {
cy.clearCookies();
Expand All @@ -37,7 +39,7 @@
};

it('Login to app/opensearch_dashboards_overview#/ when OIDC is enabled', () => {
cy.visit('http://localhost:5601/app/opensearch_dashboards_overview', {
cy.visit(`http://localhost:5601${basePath}/app/opensearch_dashboards_overview`, {
failOnStatusCode: false,
});

Expand All @@ -52,24 +54,22 @@
});

it('Login to app/dev_tools#/console when OIDC is enabled', () => {
cy.visit('http://localhost:5601/app/opensearch_dashboards_overview', {
cy.visit(`http://localhost:5601${basePath}/app/dev_tools#/console`, {
failOnStatusCode: false,
});

kcLogin();

cy.getCookie('security_authentication').should('exist');

localStorage.setItem('opendistro::security::tenant::saved', '""');
localStorage.setItem('home:newThemeModal:show', 'false');

cy.visit('http://localhost:5601/app/dev_tools#/console');

cy.get('a[data-test-subj="breadcrumb first last"]').contains('Dev Tools').should('be.visible');

cy.getCookie('security_authentication').should('exist');
});

it('Login to Dashboard with Hash', () => {
const urlWithHash = `http://localhost:5601/app/security-dashboards-plugin#/getstarted`;
const urlWithHash = `http://localhost:5601${basePath}/app/security-dashboards-plugin#/getstarted`;

cy.visit(urlWithHash, {
failOnStatusCode: false,
Expand All @@ -92,7 +92,7 @@
});

it('Tenancy persisted after logout in OIDC', () => {
cy.visit('http://localhost:5601/app/opensearch_dashboards_overview#/', {
cy.visit(`http://localhost:5601${basePath}/app/opensearch_dashboards_overview#/`, {
failOnStatusCode: false,
});

Expand All @@ -107,7 +107,7 @@
localStorage.setItem('home:newThemeModal:show', 'false');

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

Check warning on line 110 in test/cypress/e2e/oidc/oidc_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

Check warning on line 110 in test/cypress/e2e/oidc/oidc_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 110 in test/cypress/e2e/oidc/oidc_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

Check warning on line 110 in test/cypress/e2e/oidc/oidc_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 110 in test/cypress/e2e/oidc/oidc_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 110 in test/cypress/e2e/oidc/oidc_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

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

Expand All @@ -115,8 +115,12 @@

cy.get('button[id="user-icon-btn"]').click();

cy.intercept('GET', `${basePath}/auth/openid/logout`).as('openidLogout');

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

cy.wait('@openidLogout').then(() => {});

kcLogin();

cy.get('#user-icon-btn').should('be.visible');
Expand Down
Loading
Loading