tuxis-ie / nsedit

DNS Editor working with PowerDNS's new API
GNU General Public License v2.0
199 stars 55 forks source link

Group support & permissions #137

Open richard-underwood opened 7 years ago

richard-underwood commented 7 years ago

Hi, this is a pretty big set of changes, so it would be worth getting some early feedback. It's designed to fix the following:

Issue 68 - group support Issue 17 - forbid users from editing administrative records

Unfortunately, there's no point in having groups without some sort of permissions system, so that's included as well.

As the DB schema also needed updating for this, I've put a framework in place for schema versioning.

tuxis-ie commented 7 years ago

Holy moly! I'll try to review this soon and await the rest of the patches. But good work!

richard-underwood commented 7 years ago

Thanks, one thing I should mention is that the interface is using j-query autocomplete rather than drop-downs for the user and group selection boxes as the number of users could be large. I hope this is OK.

tuxis-ie commented 7 years ago

Yeah, I'd think that's ok.

righter83 commented 7 years ago

Hi are there any news to commit this into master? We can use that group feature also. thanks

pasikarkkainen commented 6 years ago

Ping? Any plans to get this PR reviewed and merged?

tuxis-ie commented 6 years ago

@richard-underwood Was it ready?

richard-underwood commented 6 years ago

Apologies - other tasks got in the way and it stalled. It's not finished, and will need re-merging now. I'll try and get it up to date later this week and see what the state of play is.

tuxis-ie commented 6 years ago

Not a problem. Thanks.

richard-underwood commented 6 years ago

I've merged the upstream changes, which looked OK, but not fully tested everything yet.

This is a large change, I'll try and list the changes below:

Essentially, each domain still has an owner who is always admin, but other users or groups can be assigned to view, update or admin a zone. There's a config option to restrict editing (e.g. to stop SOA and NS records being updated).

I'm certain there's more to do, but it'll take me a while to remember exactly what. In particular, I have a feeling there are issues editing zones which have bene created, but not in the database. Hopefully I'll have a chance to work on this over the next week.

If anyone who is interested in this pull request could have a look ON A TEST SYSTEM and see if it works for you, if there's anything fundamentally missing, if there are any security issues, etc. then it'd be appreciated. Bear in mind that the database schema will be changed, so take a backup of this if it's important to you.

tuxis-ie commented 6 years ago

@righter83 @pasikarkkainen ? Are you able to test this ON A TEST SYSTEM ? I'm currently not able to test this extensively.

righter83 commented 6 years ago

Hi, thanks for investing more time into this feature!

I've pulled it into my test system and almost everything works. I've tested the main stuff and not everthing. The only bug I found: Zone Clone: if you clone a zone and set a specific owner user: owner is admin afterwards instead of the specifiic user.

just my 2c on this feature. We needed a superadmin, and medium admins for our reseller structure. That means a medium-admin is also granted to create new zones -> Everyone in his group will be granted to edit that zone. But the medium admin sees only the zones which are granted to them. the superadmin sees then all zones.

My programming is not that fine, therefore I haven't mad a pull request. But if others are interested in this model i could try to invest more time into it as soon as this pull will be merged into the master.

thanks

richard-underwood commented 6 years ago

righter83 - thanks for looking at it. I didn't think I'd made any changes to zone cloning, so I suspect that that had always happened (but will check). However, the permissions will not be copied on zone clone, which I'm guessing they should be - so will need to add that.

Thanks.

richard-underwood commented 6 years ago

I've raised issue #160 for the owner change on cloning - it looks like the "kind" field is also ignored.

richard-underwood commented 6 years ago

I'm happy for this to be merged, if everyone else is - although I think it could still do with some more review or testing. In particular, @tuxis-ie, are you happy with the database update scripts?

metrax commented 6 years ago

+1 for merging this feature soon :-)

ghost commented 6 years ago

@tuxis-ie any news about this? If you need some more testing, I'd be available too (have a test system up and running)

ruben-herold commented 6 years ago

+1 for merging that will lift nsedit to a new level