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

Fix issue when using OpenID Authentication with serverBasePath #1899

Merged
merged 48 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
98b1013
Test OIDC with basePath
cwperks Apr 18, 2024
6cc0d8f
Run OIDC tests with basePath
cwperks Apr 18, 2024
c41fc38
Run all tests with basePath
cwperks Apr 18, 2024
03c3030
Fix kibana url
cwperks Apr 18, 2024
937fa28
Remove requirement on nextUrl
cwperks Apr 18, 2024
f4ced49
Remove duplicate
cwperks Apr 18, 2024
34969d2
Add basePath to acs endpoint
cwperks Apr 18, 2024
7f61259
Update snapshot
cwperks Apr 18, 2024
124ab4c
Start IdP where acs contains the basePath
cwperks Apr 18, 2024
d6f5089
es:lint --fix
cwperks Apr 18, 2024
c95ded8
Generalize SAML tests to include basePath
cwperks Apr 19, 2024
30ae872
Add default
cwperks Apr 19, 2024
23ee814
Use -z
cwperks Apr 19, 2024
f9ae8b8
Cypress env
cwperks Apr 19, 2024
8c912e1
Cypress env
cwperks Apr 19, 2024
80d753c
Handle undefined
cwperks Apr 19, 2024
8557219
Always pass basePath
cwperks Apr 20, 2024
f3c0733
Remove ampersand
cwperks Apr 20, 2024
940391d
Pass basePath in yarn_command
cwperks Apr 20, 2024
abe86e0
re-add osd_base_path
cwperks Apr 20, 2024
1600362
Try ternary
cwperks Apr 20, 2024
61e07a0
Start with basePath
cwperks Apr 20, 2024
48b8f5c
Use separate steps
cwperks Apr 20, 2024
12d7855
Update shortenUrl
cwperks Apr 20, 2024
37b75b6
Remove basePath in acs endpoint and fix shorten URL
cwperks Apr 20, 2024
beb2dd6
Comma-separated env
cwperks Apr 20, 2024
d4e2fa6
update oidc_auth_test.spec.js
cwperks Apr 20, 2024
e899fb7
Check cookie after login
cwperks Apr 20, 2024
155497d
failOnStatusCode false
cwperks Apr 20, 2024
4481f88
mimic SAML
cwperks Apr 21, 2024
5ea601f
mimic SAML
cwperks Apr 21, 2024
f41a715
Remove cy.origin
cwperks Apr 21, 2024
37fa79d
Add cy.origin back
cwperks Apr 21, 2024
9aa2e58
Try wrapping in cy.url
cwperks Apr 21, 2024
c5db062
Visit directly
cwperks Apr 21, 2024
2a44af9
Try localhost
cwperks Apr 21, 2024
017c1a7
Comment out test for now
cwperks Apr 21, 2024
6e6ef37
Add another request after logout
cwperks Apr 22, 2024
1be2ba2
Wait for logout
cwperks Apr 22, 2024
2f88fbf
Small update
cwperks Apr 22, 2024
ab3c641
Add base_redirect_url
cwperks Apr 22, 2024
410c7f6
Merge branch 'main' into oidc-basepath-fix
cwperks Apr 26, 2024
d1a7be0
Explicitly add minimist as dev dependency
cwperks Apr 26, 2024
1eb2209
Merge branch 'main' into oidc-basepath-fix
DarshitChanpura Apr 29, 2024
a071f94
Merge branch 'main' into oidc-basepath-fix
DarshitChanpura Apr 30, 2024
a1ed129
Merge branch 'main' into oidc-basepath-fix
cwperks May 3, 2024
7c9718f
Merge branch 'main' into oidc-basepath-fix
DarshitChanpura May 8, 2024
bd82997
Merge branch 'main' into oidc-basepath-fix
RyanL1997 May 9, 2024
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.6.0",
"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 (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