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(e2e) Port OIDC tests to multiple connections COMPASS-8105 #6063

Merged
merged 11 commits into from
Aug 1, 2024

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Jul 26, 2024

Starting mongodb server like this:

(using https://github.com/lerouxb/start-oidc)

~/mongo/start-oidc % docker run -e EXPERIMENTAL_DOCKER_DESKTOP_FORCE_QEMU=1 --network host --platform linux/amd64 --rm -v .:/start-oidc -w /start-oidc node:20 bash -c 'npm i && npx ts-node start-oidc.ts "http://localhost:38231"'

This requires the experimental host networking feature enabled for mac and windows which requires signin to docker.

Running with:

~/mongo/compass/packages/compass-e2e-tests % OIDC_BIND_IP_ALL=true OIDC_CONNECTION_STRING="mongodb://127.0.0.1:27017/" OIDC_MOCK_HOSTNAME=localhost OIDC_MOCK_PORT=38231 npm run test-noserver-nocompile -- --test-multiple-connections

Obviously with or without -- --test-multiple-connections and also tweak test-noserver-nocompile to be either test-noserver or test. You probably want a .only on the describe too.

What this does is it starts the OIDC mock server listening on all ips so that the database server in docker can get to it. It starts it with a known hostname and port so that we can get a consistent, known issuer url to use in the mongodb server config. (38231 was just the last port picked automatically while I was working on this.). It then knows to use that mongodb server rather than start its own.

@lerouxb lerouxb changed the title WIP fix OIDC tests for multiple connections fix(e2e) Port OIDC tests to multiple connections COMPASS-8105 Jul 29, 2024
@github-actions github-actions bot added the fix label Jul 29, 2024
@lerouxb lerouxb added feature flagged PRs labeled with this label will not be included in the release notes of the next release no release notes Fix or feature not for release notes labels Jul 29, 2024
authMethod: 'MONGODB-OIDC',
connectionName: this.test?.fullTitle(),
connectionName,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was wrong ever since I added it but it is not used for single connections and the tests never executed for multiple connections so we never realised.

// we have to browse somewhere that will fire off commands that require
// authentication so that those commands get rejected due to the expired
// auth and then that will trigger the confirmation modal we expect.
await browser.selectConnectionMenuItem(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to point out again: I still don't know what in the single connection world kicked off a command that requires authentication after 10 seconds which would trigger the re-authorization modal. Somehow it just worked.

@lerouxb lerouxb marked this pull request as ready for review July 31, 2024 16:17
@@ -22,7 +22,7 @@ export async function hideIndex(
await browser.screenshot(screenshotName);
}

await browser.clickVisible(Selectors.ConfirmationModalConfirmButton());
await browser.clickVisible(Selectors.confirmationModalConfirmButton());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by: I cleaned up the naming. It is TitleCase for constants and camelCase for functions.

const modalHeader = await browser.$(`${modal} h1`);
return (await modalHeader.getText()).includes('Authentication expired');
});
await browser.pause(10_000);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment here that this should aligned with the token expiration or something

@lerouxb lerouxb merged commit ccd037c into main Aug 1, 2024
4 of 5 checks passed
@lerouxb lerouxb deleted the fix-oidc-tests branch August 1, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature flagged PRs labeled with this label will not be included in the release notes of the next release fix no release notes Fix or feature not for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants