-
Notifications
You must be signed in to change notification settings - Fork 17
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 the ability to add aditional data into sessions #5
base: master
Are you sure you want to change the base?
Conversation
…ame, so that additional data can be added to the auth response such as group membership, permissions etc
Conflicts: src/main/java/org/vertx/mods/AuthManager.java
Isn't it the responsibility of the auth manager to accept/deny access based on the group and other data rather than sending that data back to the client? Or perhaps I am missing something?| |
Currently the auth manager simply checks for a valid username and password pair and does not do any checking of any other criteria such as user permissions, groups etc. If the result of the auth request is simply an accept/deny this makes it difficult for downstream modules to perform actions specific to a user. For instance if a user logs into a web UI with a username and password and now wants to know what permissions that user has. With a simple accept/deny we would have to make separate db calls to discover what that users permissions are, however the auth manager could simply return what the users permissions are as part of the accept response. Also if a simple accept/deny was used, relatively complex permissions would also require multiple auth requests for testing different aspects of the permissions. This would add complexity to auth manager and the downstream modules, as well as adding the overhead of multiple requests. An alternative to returning users full mongo document is adding a "payload" field to the users data so it would look as follows:
The payload data would then be returned by the auth manager. This would address the issue of leaking any data from existing user databases. I am happy to make this modification if the general concept of returning more than accept/deny from the auth manager is ok. |
maybe its a good Idea to keep the auth manager the way it is, but rename it into authentification manager. Then an additional and replaceable authorisation manager could do right things, because there are many different approaches of implementing a right engine. Groups, Tokens, Trees.... |
Are you proposing having a different verticle that listens on a different address, which then implements some authorisation scheme (such as the one I have proposed with returning groups/permissions or tress..), and presumably backs onto the authentication manager for the actual password check? Sounds like a good idea. Not quite sure where that vertical would live, sounds like it would require creating a new module, not sure it would work well for the mod-auth-mgr to bring up other verticals that implement various authorisation schemes. One thing I want to avoid is having to always make a call to some auth manager downstream for each individual request, I would rather that it returns some group/permissions so downstream verticals can make decisions of their own. My use case is that I have added additional auth to the EventBusBridge which prevents certain users sending messages based on a group. I.e if the are in group 'normal-user' they can send messages to 'normal-user.my.address' but not to 'admin.my.address'. Obviously if every request to the EventBusBridge required a hit to the authorisation manager to check the users permissions this becomes a reasonable bottleneck on performance. However if the authorisation manager returns some permissions info this can be cached and checked at the site of the call, basically exactly how the current session is returned and cached/checked, just extending it further. |
Why an additional auth to the EventBusBridge (Maybe I got you wrong)? You can define the address where you authenticate against: https://github.com/vert-x/mod-auth-mgr#configuration So It's possible to have a GroupGuardVerticle listening on (i.e.) vertx.groupguard. Than he's asking the Auth-mgr for authorising the sessionid and gets a username.
AFAIK this happens if you activate So I think it should be doable that way? |
Currently the auth manager does not allow for adding any additional data to the login such as group names, permissions etc. Making it difficult to manage more sophisticated permissions than simply authed/not authed. This commit allows for applications to add this by adding the data returned from the request to the db into the result of the login request. This meta data can then be simply added to the db entry for a given user and it will be returned inside the result of a successful auth request. So all a person needs to do is for instance add a 'group': 'foo' to the users db record and this will be returned as part of the successful auth.
There is a risk to consider with the current implementation is if anyone has added private data to their users record, then this successful auth response will contain that private data. I am already explicitly removing the password field to avoid sending around the password unnecessarily. If it is felt that this scenario is probable enough that the behaviour of this pull request is too risky then I would suggest adding a whitelist of fields from the db that the user can configure to be returned. I am happy to do this work if you feel this is necessary.