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

Role mapping is only done once on user creation, so changes in AD never apply #94

Open
roborourke opened this issue Jan 15, 2025 · 4 comments

Comments

@roborourke
Copy link
Contributor

Somewhat related to #58 in terms of AD values only being used when creating a user for the first time, the user roles and mappings are only processed once when a user first logs in and a WP user entry is created.

If there are subsequent changes in AD to a users roles etc... then these will not be updated in WP on subsequent login attempts so there is a sync issue.

We could make this behaviour default, with a return true/false filter to opt out of it, or vice versa, allowing sites to opt in to this behaviour instead. Given you already have to opt in to user role mapping via AD using a filter, I would vote for making this the default behaviour when using AD for managing roles.

@rmccue
Copy link
Member

rmccue commented Jan 15, 2025

The biggest issue here is deciding whether AD is authoritative; i.e. if a user has a role and they don't have it in AD, should we remove it? Those could be roles they used to have, or they could be roles manually assigned in the admin.

Right now, we only have "unauthoritative mode" where it's applied once at user creation and not re-synched. Introducing an "authoritative mode" would probably be the best way to do this, by saying that the role in AD will always override WP's manually assigned roles, in which case maybe it also makes sense to disable parts of WP's UI?

Do we know what other solutions do here?

@roborourke
Copy link
Contributor Author

I would say this filter indicates whether AD is authoritative or not, or we could use it as such:

https://github.com/humanmade/wp-simple-saml/blob/master/inc/namespace.php#L537-L539

It makes sense if you want to use programmatic role settings IMO, and those roles are derived from AD. Users can't change their own role unless they're admin or super anyway so I don't think we need to worry about the UI necessarily. I doubt that's even possible to change.

@rmccue
Copy link
Member

rmccue commented Jan 16, 2025

I think that overloads the one filter with both modes? To enable "unauthoritative mode" you still need to activate that filter from my understanding, so it doesn't seem to distinguish between the two from my reading.

What I'm thinking is if you have SSO on a multisite and grant network-level roles, but could perhaps have more specific roles that are granted on each site or something along those lines.

(Realistically, multiple roles are rare, but more frequent amongst enterprise users who are using SSO than the baseline.)

@roborourke
Copy link
Contributor Author

Maybe so then. To me that filter is enabling the follow up processing via the map role filter, which specifically gives you the SAML attributes to look at. That's why I think you're effectively saying I want to manage roles via SSO.

But you're right, there could potentially be use cases in between, so we need to document combinations of the following:

  • Automatically adding users to sites with default role on auth
  • Managing roles using SAML attributes
  • Using SSO in authoritative mode (e.g. updating roles & user data per login, rather than once on initial login)

Keeps us back compatible that way I suppose.

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

No branches or pull requests

2 participants