Check User status in LDAP Group w/out admin auth #162
Conversation
Should allow a user to check group status without requiring admin authentication. Not including AD at this point. Signed-off-by: "Jake Champlin" <jchamplin@jake.champlin.27@gmail.com> Testing Group List w/out admin auth Should allow LDAP user to search group membership without authenticating as admin LDAP user. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Check for group status w/out admin auth Need to dev better way of testing changes. Laziness ensues. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Forgot return statement. Silly Not too bright. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Testing still. Returning all known groups. Hopefully will return all LDAP groups with filter in place. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Basic Commits Commit 6. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Commit. Unsure of how to test gems locally without pulling into a ruby app Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Commit. gah. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Debugging statements Just work Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Complete shit commit Horrible coding practices. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Cleaning up commit. Commit Cleanup. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Cleanup Commit Debugging Still. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> It works Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Working Gem so far Gem works to find groups to authenticate by without neededing admin auth Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Works Still. Still works as a passable gem. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Debugging statements Debugs Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Adding Check Membership for Auth Adding membership check without admin authentication as a config option. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Fixed a spelling error Took 10 minutes. Wow Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com> Added logging statement for Auth Added logging statement for authentication tests Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com>
|
Thanks for the patch @jchampy. I can own this and power through merging it on Saturday. I like the interface and appreciate the contribution. I'll toss in an acceptance test with our LDAP mock server, but if you wanted a shot at that @jchampy or had something in the works, let me know, and I'll give you dibs on doing a pass at it. Also, sorry but a quick request: I'd love to bring the code into line with the existing code style (2 space indents rather than tab indents). I can do the formatting myself, but I'd rather have the lines blame directly to you so you get credit :). If you want to update this PR with a commit with the 2 space indents, that would be awesome; if not or if you don't care, that's cool too. Thanks again! |
|
Thanks for the kind words and help @cairo140! I'll push a commit to fix the formatting styles later today. I'll hack on it this weekend and attempt an acceptance test with the server, and I'll stay in touch with my progress. |
Fixed formatting issues to comply with repo's coding style. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com>
Quick RSPEC acceptance test for authorizing a user group without binding to admin user account. Signed-off-by: "Jake Champlin" <jake.champlin.27@gmail.com>
|
@jchampy sorry for the delay. I'm checking it out now and it appears there are a few dead ends in the code like calls to auth_user_groups. Also, the integration tests test a function that hasn't been added to the Devise model. I'll fill in these as best as I can. If you see something I missed let me know. |
|
@cairo140 no worries. I had planned to incorporate the auth_user_groups function to return a list of all groups that a member was in without binding to an admin account, but didn't get around to it. I don't know if it is essential or not, but it is not for my application of the gem. If you want to add the function to the Devise model I would appreciate that. |
|
@jchampy sounds good. I thought a bunch on the use of exposing two separate Devise model-level functions for the admin-powered group check and the non-admin-powered group check. I didn't see a use case that made sense to support that extra interface, so I stuck with the in_ldap_group? method on the Devise model and had it go through a flow that checked your new static config to determine if it should use the user's own LDAP binding or the admin binding. I left some semi-coherent remarks in the commit. If I missed some compelling use case you had in mind to support the dual |
|
Thanks for the fixes @cairo140. Your updates make sense to me. Less clutter. Thanks for the help! |
Should allow a user to check group status without requiring admin authentication.
Not including AD at this point.
Signed-off-by: "Jake Champlin" jake.champlin.27@gmail.com