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

OAuth users are automatically assigned with user role #14454

Closed
magicbelette opened this issue May 10, 2019 · 17 comments · Fixed by #23588
Closed

OAuth users are automatically assigned with user role #14454

magicbelette opened this issue May 10, 2019 · 17 comments · Fixed by #23588
Assignees

Comments

@magicbelette
Copy link
Contributor

Description:

Users coming from OAuth service get "guest" role thanks to #5842 but seems that since version 1.0.3, "user" role is also added

Steps to reproduce:

  1. Configure OAuth service with role mapping
  2. Login with OAuth service and a new user
  3. The new user has role from OAuth mapping + "user" role

Expected behavior:

Only role from OAuth mapping should stay

Actual behavior:

User has role from OAuth mapping + "user" role

Server Setup Information:

  • Version of Rocket.Chat Server: 1.0.3
  • Operating System: Debian
  • Deployment Method: tar
  • Number of Running Instances: 7
  • DB Replicaset Oplog: true
  • NodeJS Version: 8.12.0
  • MongoDB Version: 4.0.7

Additional context

Maybe linked to #13823 ?

@magicbelette
Copy link
Contributor Author

For the 2nd connection with OAuth service, user looses the "user" role. So it seems to be related to the user creation process.

@MarcosSpessatto
Copy link
Member

@magicbelette I think this PR should solve the problem.

@magicbelette
Copy link
Contributor Author

@magicbelette I think this PR should solve the problem.

Unfortunately, new users still have "user" role after patching my instance.

  1. patch
  2. restart rocket.chat
  3. delete previous OAuth users
  4. login with OAuth, roles are "guest,user"

@MarcosSpessatto
Copy link
Member

@magicbelette What OAuth service are you using?

@magicbelette
Copy link
Contributor Author

A custom one

@MarcosSpessatto
Copy link
Member

Is the setting Merge Roles from SSO = true by chance? 🤔

@magicbelette
Copy link
Contributor Author

Is the setting Merge Roles from SSO = true by chance? thinking

Yes you're right. If I set Merge Roles from SSO = false, OAuth user just get the "user" role.

Maybe I'm not doing it the right way. I need a specific role ("guest") for users coming from OAtuh service. This role must not create channel, group or direct message.

Other users (LDAP auth) have the default role "user".

Thx for your help

@MarcosSpessatto
Copy link
Member

@magicbelette Can we close this?

@magicbelette
Copy link
Contributor Author

Sorry but it isn't clear for me...
Main users are coming from LDAP with role "user", can I automatically assign a different role to OAuth users ?

@MarcosSpessatto
Copy link
Member

@magicbelette yes, if you need assign roles to users coming from Oauth services, you can set in this fields.
Screenshot from 2019-05-14 12-02-49

@magicbelette
Copy link
Contributor Author

Ok, to sum up, "Default roles for authentication services" is set to "user". So, all my LDAP users inherit "user" role, that's what I want.

By adding an OAuth service with "roles/groups field name" set (OAuth send me "guest") :

  • "Merge role from SSO: false" : OAuth users have "user" role. Not good, I expect "guest" role
  • "Merge role from SSO: true" : OAuth users have "guest, user" roles for the first connection then only "guest" for the next connection. That's almost what I'm looking for excepted for the first connection, even with this PR

I don't figured out if there's some kind of bug or if I misunderstood the "Merge role from SSO" option. Maybe the solution will come from this PR to separate roles given for LDAP and OAuth ?

@MarcosSpessatto
Copy link
Member

Reading what you have detailed now, I think the PR that separate roles should solve your problem. What do you think?

@magicbelette
Copy link
Contributor Author

Unfortunately not, cause the PR that separate roles implies that all of my 90K users belongs to the same LDAP group.

I think that the problem is with this PR.

  • If you don't want to merge role from SSO = "Merge role from SSO: false", SSO users must only have the role defined by the SSO
  • If you do want to merge role from SSO = "Merge role from SSO: true", SSO users must have the role defined by the SSO and the default one set by "Default roles for authentication services"

@geekgonecrazy
Copy link
Contributor

Ok, to sum up, "Default roles for authentication services" is set to "user". So, all my LDAP users inherit "user" role, that's what I want.

I think this might be key... this user role is getting set from here for all services, not just ldap

@magicbelette
Copy link
Contributor Author

Sure, but the option "Merge role from SSO: false" must override the "Default roles for authentication services" or I don't understand the point to keep this option.

@magicbelette
Copy link
Contributor Author

I've just found that roles other than the ones coming from OAuth service are correctly removed for the second connection by this method updateRolesFromSSO() in oauth_helpers.js :

const toRemove = user.roles.filter((val) => !rolesFromSSO.includes(val));

	// loop through roles that user has that sso doesnt have and remove
	toRemove.forEach(function(role) {
		removeUserFromRoles(user._id, role);

});

The problem is that this method is called before the user's creation instead of being called after user's creation.

Any idea on how to (nicely) solve this ?

@magicbelette
Copy link
Contributor Author

I've just found that roles other than the ones coming from OAuth service are correctly removed for the second connection by this method updateRolesFromSSO() in oauth_helpers.js :

const toRemove = user.roles.filter((val) => !rolesFromSSO.includes(val));

	// loop through roles that user has that sso doesnt have and remove
	toRemove.forEach(function(role) {
		removeUserFromRoles(user._id, role);

});

The problem is that this method is called before the user's creation instead of being called after user's creation.

Any idea on how to (nicely) solve this ?

Any idea @rodrigok ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants