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: include nonce in oidc request by default MONGOSH-1905 #2269

Merged
merged 3 commits into from
Nov 21, 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
15 changes: 5 additions & 10 deletions .github/workflows/cron-tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,24 @@ jobs:
name: Update automatically generated files
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
# don't checkout a detatched HEAD
ref: ${{ github.head_ref }}

# this is important so git log can pick up on
# the whole history to generate the list of AUTHORS
fetch-depth: '0'
fetch-depth: "0"

- name: Set up Git
run: |
git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com"
git config --local user.name "github-actions[bot]"

- uses: actions/setup-node@v2
- uses: actions/setup-node@v4
with:
node-version: ^16.x
cache: 'npm'
cache: "npm"

- name: Install npm@8
run: |
Expand All @@ -50,41 +50,36 @@ jobs:
run: |
npm run update-authors
git add AUTHORS \*/AUTHORS
git commit --no-allow-empty -m "chore: update AUTHORS" || true

- name: Generate Error Documentation
run: |
npm run generate-error-overview
mv error-overview.md error-overview.rst packages/errors/generated/
npm run reformat
git add packages/errors/generated
git commit --no-allow-empty -m "chore: update error documentation" || true

- name: Regenerate Evergreen Config
run: |
npm run update-evergreen-config
git add .evergreen.yml
git commit --no-allow-empty -m "chore: update evergreen config" || true

- name: Update Security Test Summary
run: |
npm run update-security-test-summary
git add docs/security-test-summary.md
git commit --no-allow-empty -m "chore: update security test summary" || true

- name: Regenerate CLI usage text in README files
run: |
npm run update-cli-usage-text packages/*/*.md *.md
git add packages/*/*.md *.md
git commit --no-allow-empty -m "chore: update CLI usage text" || true

- name: Create pull request
id: cpr
uses: peter-evans/create-pull-request@v6
with:
commit-message: Update auto-generated files
branch: ci/cron-tasks-update-files
title: 'chore: update auto-generated files'
title: "chore: update auto-generated files"
body: |
- Update auto-generated files

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ variable. For detailed instructions for each of our supported platforms, please
--oidcTrustedEndpoint Treat the cluster/database mongosh as a trusted endpoint
--oidcIdTokenAsAccessToken Use ID tokens in place of access tokens for auth
--oidcDumpTokens[=mode] Debug OIDC by printing tokens to mongosh's output [full|include-secrets]
--oidcNoNonce Don't send a nonce argument in the OIDC auth request

DB Address Examples:

Expand Down
114 changes: 62 additions & 52 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions packages/arg-parser/src/arg-mapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,22 @@ describe('arg-mapper.mapCliToDriver', function () {
});
});

context('when cli args have oidcNoNonce', function () {
const cliOptions: CliOptions = {
oidcNoNonce: true,
};

it('maps to oidc skipNonceInAuthCodeRequest', function () {
expect(optionsTest(cliOptions)).to.deep.equal({
driver: {
oidc: {
skipNonceInAuthCodeRequest: true,
},
},
});
});
});

context('when cli args have browser', function () {
it('maps to oidc command', function () {
expect(optionsTest({ browser: '/usr/bin/browser' })).to.deep.equal({
Expand Down
3 changes: 2 additions & 1 deletion packages/arg-parser/src/arg-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function setServerApi<Key extends keyof ServerApi>(
const serverApi =
typeof previousServerApi === 'string'
? { version: previousServerApi }
: { ...previousServerApi } ?? {};
: { ...previousServerApi };
serverApi[key] = value;
return setDriver(i, 'serverApi', serverApi as Required<ServerApi>);
}
Expand Down Expand Up @@ -237,6 +237,7 @@ const MAPPINGS: {
v.split(',').filter(Boolean) as OIDCOptions['allowedFlows']
),
oidcIdTokenAsAccessToken: (i, v) => setOIDC(i, 'passIdTokenAsAccessToken', v),
oidcNoNonce: (i, v) => setOIDC(i, 'skipNonceInAuthCodeRequest', v),
browser: (i, v) =>
setOIDC(i, 'openBrowser', typeof v === 'string' ? { command: v } : v),
};
Expand Down
1 change: 1 addition & 0 deletions packages/arg-parser/src/cli-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@ export interface CliOptions {
oidcTrustedEndpoint?: boolean;
oidcIdTokenAsAccessToken?: boolean;
oidcDumpTokens?: boolean | 'redacted' | 'include-secrets';
oidcNoNonce?: boolean;
browser?: string | false;
}
1 change: 1 addition & 0 deletions packages/cli-repl/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ of mongosh, visit https://www.mongodb.com/try/download/shell.
--oidcTrustedEndpoint Treat the cluster/database mongosh as a trusted endpoint
--oidcIdTokenAsAccessToken Use ID tokens in place of access tokens for auth
--oidcDumpTokens[=mode] Debug OIDC by printing tokens to mongosh's output [full|include-secrets]
--oidcNoNonce Don't send a nonce argument in the OIDC auth request

DB Address Examples:

Expand Down
1 change: 1 addition & 0 deletions packages/cli-repl/src/arg-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const OPTIONS = {
'norc',
'oidcTrustedEndpoint',
'oidcIdTokenAsAccessToken',
'oidcNoNonce',
'perfTests',
'quiet',
'retryWrites',
Expand Down
3 changes: 3 additions & 0 deletions packages/cli-repl/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ export const USAGE = `
--oidcDumpTokens[=mode] ${i18n.__(
'cli-repl.args.oidcDumpTokens'
)}
--oidcNoNonce ${i18n.__(
'cli-repl.args.oidcNoNonce'
)}
${clr(i18n.__('cli-repl.args.dbAddressOptions'), 'mongosh:section-header')}
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"devDependencies": {
"@mongodb-js/eslint-config-mongosh": "^1.0.0",
"@mongodb-js/oidc-mock-provider": "^0.10.0",
"@mongodb-js/oidc-mock-provider": "^0.10.2",
"@mongodb-js/prettier-config-devtools": "^1.0.1",
"@mongodb-js/tsconfig-mongosh": "^1.0.0",
"@types/chai-as-promised": "^7.1.3",
Expand Down
36 changes: 23 additions & 13 deletions packages/e2e-tests/test/e2e-oidc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,20 +180,30 @@ describe('OIDC auth e2e', function () {
);
}

it('can successfully authenticate using OIDC Auth Code Flow', async function () {
shell = this.startTestShell({
args: [
await testServer.connectionString(),
'--authenticationMechanism=MONGODB-OIDC',
'--oidcRedirectUri=http://localhost:0/',
`--browser=${fetchBrowserFixture}`,
],
for (const useNonce of [true, false]) {
describe(`with nonce=${useNonce}`, function () {
it('can successfully authenticate using OIDC Auth Code Flow', async function () {
const args = [
await testServer.connectionString(),
'--authenticationMechanism=MONGODB-OIDC',
'--oidcRedirectUri=http://localhost:0/',
`--browser=${fetchBrowserFixture}`,
];

if (!useNonce) {
args.push('--oidcNoNonce');
}

shell = this.startTestShell({
args,
});
await shell.waitForPrompt();

await verifyUser(shell, 'testuser', 'testServer-group');
shell.assertNoErrors();
});
});
await shell.waitForPrompt();

await verifyUser(shell, 'testuser', 'testServer-group');
shell.assertNoErrors();
});
}

it('can successfully authenticate using OIDC Auth Code Flow when a username is specified', async function () {
shell = this.startTestShell({
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/locales/en_US.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ const translations: Catalog = {
'Use ID tokens in place of access tokens for auth',
oidcDumpTokens:
"Debug OIDC by printing tokens to mongosh's output [full|include-secrets]",
oidcNoNonce: "Don't send a nonce argument in the OIDC auth request",
},
'arg-parser': {
'unknown-option': 'Error parsing command line: unrecognized option:',
Expand Down
1 change: 1 addition & 0 deletions packages/mongosh/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ of mongosh, visit https://www.mongodb.com/try/download/shell.
--oidcTrustedEndpoint Treat the cluster/database mongosh as a trusted endpoint
--oidcIdTokenAsAccessToken Use ID tokens in place of access tokens for auth
--oidcDumpTokens[=mode] Debug OIDC by printing tokens to mongosh's output [full|include-secrets]
--oidcNoNonce Don't send a nonce argument in the OIDC auth request

DB Address Examples:

Expand Down
2 changes: 1 addition & 1 deletion packages/service-provider-node-driver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
},
"dependencies": {
"@mongodb-js/devtools-connect": "^3.3.3",
"@mongodb-js/oidc-plugin": "^1.1.1",
"@mongodb-js/oidc-plugin": "^1.1.2",
"@mongosh/errors": "0.0.0-dev.0",
"@mongosh/service-provider-core": "0.0.0-dev.0",
"@mongosh/types": "0.0.0-dev.0",
Expand Down
Loading