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

Add 3.6+ user authenticationRestrictions #229

Merged
merged 20 commits into from
Aug 3, 2018
Merged

Add 3.6+ user authenticationRestrictions #229

merged 20 commits into from
Aug 3, 2018

Conversation

timvaillancourt
Copy link

@timvaillancourt timvaillancourt commented Jul 31, 2018

This PR adds support for MongoDB user authenticationRestrictions, added in 3.6:

The authentication restrictions the server enforces on the created user. Specifies a list of IP addresses and CIDR ranges from which the user is allowed to connect to the server or from which the server can accept users.

https://docs.mongodb.com/manual/reference/method/db.createUser/#authentication-restrictions

Example user with authenticationRestrictions, post-change:

> db.system.users.find({},{_id:0, credentials:0}).pretty()
{
	"user" : "tim",
	"db" : "admin",
	"roles" : [
		{
			"role" : "root",
			"db" : "admin"
		}
	],
	"authenticationRestrictions" : [
		{
			"clientSource" : [
				"127.0.0.1"
			],
			"serverAddress" : [
				"127.0.0.1"
			]
		}
	]
}

If it's valuable I can add checks to confirm a valid IP or CIDR was passed as 'clientSource' or 'serverAddress', please let me know.

@szank
Copy link

szank commented Aug 2, 2018

Hi @timvaillancourt,
This looks like a great addition, and thank you for taking the time to implement it. It is really appreciated!

I have one remark though. Could you please add some unit tests for using the AuthenticationRestriction? The existing UpsertUser unit tests should be a good template.
A success and failure case unit tests would be greatly appreciated.

It will be a nice addition when it is merged :)

Copy link

@szank szank left a comment

Choose a reason for hiding this comment

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

Empty comment, I have put a proper comment in the issue page. Github won't let me leave an empty one.

@timvaillancourt
Copy link
Author

Whoops, sounds good @szank. I will add unit tests shortly

@timvaillancourt
Copy link
Author

timvaillancourt commented Aug 2, 2018

@szank I've added a test for this. Please review, thanks

EDIT: I made a few more commits to clean up the way I wrote the tests. 100% done this time

@szank szank removed the needs tests label Aug 3, 2018
@domodwyer domodwyer merged commit 593df06 into globalsign:development Aug 3, 2018
@domodwyer
Copy link

Thanks @timvaillancourt !

@timvaillancourt timvaillancourt deleted the 3.6_authenticationRestrictions branch August 3, 2018 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants