cschiewek / devise_ldap_authenticatable

Devise Module for LDAP
MIT License
593 stars 359 forks source link

Proposal: Remove group and attribute based authorization #137

Open cschiewek opened 11 years ago

cschiewek commented 11 years ago

Cross post from Google Group

Hello All,

I'm considering removing the functionality that checks users for specific groups and/or attributes before authenticating them.

In my mind, authorization is out of scope for devise, as it is just authentication gem. We really should only be verifying that the user is who they are by authenticating against LDAP.

It makes much more sense to me that you would have your authorization logic somewhere else entirely, which is typically how I've handled those use cases.

Thoughts?

danspencer commented 11 years ago

I have to agree. It makes sense to have the logic separate. I've used devise for authentication and cancan for any authorization.

ebinmore commented 11 years ago

I don't fully understand why you would want to check attributes of the user before authentication - but it feels out of place here. Especially since the intent of this gem is for authentication - i.e. determining if the user is who they claim to be.

Whether or not that user has been authorized to access or use whatever service should be determined after the user has been authenticated, not before, and seems to be outside of the scope of this gem. Besides, by configuring the LDAP base appropriately, you should be able to restrict the authentication to certain groups.

My 2 cents.