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

chore(oidc): start mongodb with auth enabled to ensure we're testing it correctly #26

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

Anemy
Copy link
Member

@Anemy Anemy commented Dec 18, 2023

Previously we would keep default authentication enabled. This made it so that command would successfully run against the server when auth was failing, so testing with this config could give some false positives on behavior. This pr makes it so that the server requires auth for commands, and creates roles for the OIDC user to use.

It's currently in draft as I can't figure out what the correct db.createRole command should be for the mock oidc auth user.

db.createRole({
  role: 'dev/groups',
  privileges: [ ],
  roles: [ 'dbOwner' ]
})

done

# Creates the OIDC user role in the database.
mongosh "mongodb://localhost:27017/admin" --eval "JSON.stringify(db.createRole({ role: \"dev/groups\", privileges: [ ], roles: [ \"dbOwner\" ] }));"
Copy link
Contributor

Choose a reason for hiding this comment

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

The role(s) would need to match the entries inside the groups claim, so that would be whatever OIDC_TOKEN_PAYLOAD_GROUPS specifies (and we probably want to use that because it is supposed to be a parameter for this image, right?)

Suggested change
mongosh "mongodb://localhost:27017/admin" --eval "JSON.stringify(db.createRole({ role: \"dev/groups\", privileges: [ ], roles: [ \"dbOwner\" ] }));"
mongosh "mongodb://localhost:27017/admin" --eval "JSON.stringify((process.env.OIDC_TOKEN_PAYLOAD_GROUPS ?? 'testgroup').split(',').map(group => db.createRole({ role: 'dev/' + group, privileges: [ ], roles: [ \"dbOwner\" ] })));"

or, if we’re sticking with a recent mongosh anyway,

Suggested change
mongosh "mongodb://localhost:27017/admin" --eval "JSON.stringify(db.createRole({ role: \"dev/groups\", privileges: [ ], roles: [ \"dbOwner\" ] }));"
mongosh "mongodb://localhost:27017/admin" --json --eval "(process.env.OIDC_TOKEN_PAYLOAD_GROUPS ?? 'testgroup').split(',').map(group => db.createRole({ role: 'dev/' + group, privileges: [ ], roles: [ \"dbOwner\" ] }));"

(otherwise, the default is testgroup)

Copy link
Contributor

Choose a reason for hiding this comment

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

Tbh maybe I overkilled it by making them configurable, wouldn't mind if we don't want to do that, just felt like it wouldn't hurt 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t think it’s a bad idea at all, but yeah, if we really want to leverage this, we’d probably also want to give the user the ability to apply a specific role to each group (i.e. make this a map group → role rather than a list of groups) so that behavior actually differs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated - thanks for the help on this.

@Anemy Anemy marked this pull request as ready for review December 19, 2023 16:34
@Anemy Anemy merged commit c16e1ac into main Dec 19, 2023
1 check passed
@Anemy Anemy deleted the update-mock-oidc-to-require-auth branch December 19, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants