4.x Branch Planning

[daggerhart]

Some notes for next major version of the plugin.

• leave you logged in without much fuss most of the time, but force you to have authenticated within some time window to do higher-privilege operations
• could have sessions that don't bother with the refresh tokens or possibly even access tokens
• support backchannel logout and revocation
• API for controlling the flow

  There's no way for an arbitrary piece of code in another plugin to say, "give me a login URL with these parameters", for example, let alone, "check if the user authenticated recently enough and if not force re-login", or "try and see if this anon user has logged on into the IdP via another app in this browser, and log them in here if so".

• backward compatibility
• Arbitrary headers/.parameters in the auth code request
• wp-json endpoints for authentication

Structure

Lazy load various services into a simple plugin. The main file registers hooks that either point to the other classes by name, or small wrapper functions that decide whether to invoke the other classes.

Plenty of WP Hooks that allow for in-code alterations for data and services.

Features

Reatures requested in other issues or comments.

• [ ] Domain authorization
• [ ] Autoconfiguration
• [ ] Session refresh / reconnexion
• [ ] Use a dedicated library OpenID Client Library

API

• ServiceProviderFactory - singleton/static, loads IdP-specific config, and returns a ServiceProvider
• ServiceProvider - handles remote calls to the IdP, mapping user claims, and logout redirection via the IdP (Ideally, this would be the only class that directly works with the WP settings, though it might create other objects with some of the settings.)
• AuthorizationNegotiatorFactory - singleton/static, loads and IdP-specific and Endpoint-specific configs and returns an AuthoriationNegotiator
• AuthorizationNegotiator - interacts with the IdP to obtain a User (that knows about identity stuff, handles userinfo fetching/mapping) and starts an IdpSession
• IdpUser - internal object that contains methods and data related to an IDP & a WP_User (such as token responses and scope data).
• IdpSession - handle anIdpUser's session between WP & the IDP
• EndpointController - manage routes (admin-ajax, rest api, custom url), manages an AuthorizationNegotiator, and returns the appropriate response. based on settings for an endpoint.

Each of these objects should be lazy-loaded, they can be overwritten with WP hooks so other plugin developers can completely replace them if desired.

Data Structures

• IDP - various settings for connecting to a single IDP
  • (most existing settings the plugin provides... i'll provide these shortly)
  • Associated endpoints.
• Endpoints - various settings for accepting responses as the SP.
  • URL
  • Headers
  • Additional parameters

UI

• Manage IDP connections. Could be a private post type (easily to dev the UI), but also fine as a option sub-pages.
• Manage Endpoints
• Import & export IDP connections & endpoints

[pjeby]

One thing I'd like to suggest is reifying things that should be objects, and turning the singleton objects into either statics or true singletons. Right now, the main objects in the program only ever have one instance, but that instance isn't accessible except from code that's been passed the object.

In contrast, most of the problem domain objects (sessions, tokens, flows, claims) are just passed around as data without any methods attached... which means there's a lot of code passing around bits and pieces of data that go together. Everything would be easier to follow if data and behavior were bound together for those things.

For example, I had to work pretty hard to graft session management into the current flows, going so far as having to create a function wrapper taking a callback. That could've been avoided by having a Session object encapsulating both the token refresh/fetch logic and the data management. But because the core API is based in instances rather than singletons or static methods, sessions would have to get a copy of the configuration, and so on... a bigger refactoring than I was willing to do for a relatively-constrained bug fix.

But if I could have had my way, I'd have had an IdentityProvider singleton whose job is to talk to the IdP, whose configuration knows about endpoints, http timeouts, and claim mappings. Sessions ask the IdP for tokens in order to refresh. Authorization and User objects ask the IdP to get tokens and claims and userinfo.

Such a design would also allow the plugin to be lazily loaded, too, since only requests that actually interact with the IdP would need to load the IdP class. And the Session class would only be loaded if the user was logged in. (The main plugin file would just register a hook function that triggers Session::current($user_id)->maybe_refresh() if there is a logged-in user, and that method in turn wouldn't even load the IdP unless the user was signed in via idp and its access token had timed out.)

Anyway, that's my current rough idea of how to organize it:

• IdentityProvider singleton loads IdP-specific config, handles remote calls to the IdP, mapping user claims, and logout redirection via the IdP (Ideally, this would be the only class that directly works with the WP settings, though it might create other objects with some of the settings.)
• The callback is handled by an Authorization instance that interacts with the IdP to obtain a User (that knows about identity stuff, handles userinfo fetching/mapping), and starts a Session
• The main file just registers hooks that either point to the other classes by name, or small wrapper functions that decide whether to invoke the other classes, or else registers them (e.g. the rest_api_init hook would register the login/out/callback endpoints, the admin_init hook would register the admin functions, and so on). It might also define some useful constants and functions within the plugin namespace. (And it really should have its own namespace: that requires PHP 5.3, but the plugin previously required 5.4 anyway so...)

These organizational thoughts are less important than functionality, but AFAICT some of the things I want to do become a heck of a lot easier to spec and implement if the code organization is more problem-domain focused.

[daggerhart]

This all makes perfect sense to me. I'll work on a very simple draft over the next few days. Lots of blank spaces and code comments, we can the structure more before diving in.

Question: I've been willing to bump the plugin requirements to 5.6 for a while now in order to use an existing client solution. Ex, phpleague's client What do you think about something like that? I'm open to other existing-client ideas.

Some other thoughts, feedback welcome as always:

• Expand the settings for multiple generic IDP connections- each with their own settings. "Add a Service Provider" would send the user to a new SP settings page.
• Endpoint(s) should be very configurable. Enable/Disable, change the urls, change the requests and responses within reason, etc.
• Import & export settings & endpoints.

I've updated the issue with our ideas. Feel free to edit if possible, Negotiator isn't a real pattern, just the first word that come to mind. Issue notes don't have to be perfect, more to help us remain on the same wavelength.

Factories may be too much. Open to discussion. My thought is that if this plugin is generalized enough, then another entire plugin could be written atop it. Maybe that is the "UI" aspect of the plugin. Essentially it's own plugin structure that provides a UI for the generalized configuration options.

[pjeby]

FYI, I've checked in a copy of my draft here. It's a working proof of concept, as in it logs people in and out, manages their sessions/refresh/expiration, does OIDC state properly (no expiring transients), and has an API (not documented). It allows more sophisticated user data mapping strings, and changes WP's login/logout URLs to pass through the plugin instead of wp-login.php. It uses the existing plugin's configuration and subject-id fields, but doesn't implement the same hooks. (There's no "login button" shortcode and no need for it, because you can get a login url via the standard WP login_url() API.)

I don't plan to support multiple endpoints, since my goal is a plugin that simply replaces WP's login system with a single, trusted IdP shared across multiple WP instances. (e.g. imagine a college or a company with centralized user management and lots of Wordpress-based apps and sites).

I did look at the link for the League client, but TBH I don't see where it adds any value. It's a framework for writing different clients for Oauth, and Oauth is different from one IdP to the next. OIDC is simpler as it doesn't have so many varieties. (Also, the big value in this plugin is the user creation... and simplicity.) Honestly I personally want it simpler, with fewer options, because for multiple IdP's there's Keycloak, which can federate them. For my own sites I expect to provide social login w/the big three (G+, FB, and Twitter) using Keycloak, then use this plugin to hook Keycloak to WP.

A few other thoughts regarding the 3.x plugin in this repo (that I've already done in mine or don't apply there):

• It would be good to hook refreshing into determine_current_user instead of allowing the user to be logged in and then logged out. That way, everything in Wordpress sees an expired SSO session as identical to an expired cookie-based session. (I took a look at doing this with the current code, but ran into the snag that the necessary objects aren't created until init, and the current user is defined before that point.) My plugin does it this way, and other plugins are MUCH happier that way.

• The 3.x plugin currently filters HTTP request timeouts for everything Wordpress does, not just its own requests. The calls to wp_remote_post() should just explicitly pass in the timeout: there's no need for a filter.

• There's some unused code in the client wrapper -- an $error var and get_error() method, that are never set or called.

• The autoloader should check for a prefix instead of the existence of a file; PHP does cache stat calls but only for a few seconds. (Autoloader functions get called for every class use, not just the classes in the same plugin!)

[pjeby]

Just an update: my draft now has its own option and settings page, working independently of this plugin. I need to do some more work on error handling before I can call it done, but it should now serve as an example of where the plugin could go. I ended up switching back to using wp-login.php, but using the login_init hook to disable the regular functionality and to also use wp-login.php as the IdP's redirect URI. This ended up simplifying a bunch of things (like not needing the REST API, or to filter the standard WP login/out URLs).

[daggerhart]

Thanks for the update. I've been watching your repo to see how it progresses. Unfortunately I got sick last week and everything is falling behind. I hope to work on this over the weekend at the latest. Sorry for the delay on being involved.

[pjeby]

No problem, it's pretty much done now for my purposes at least. The error handling is purely filter-based at this point, but I only just finished the last place where errors can be issued from (IdP-redirected error codes). But everything else is done now, I think.

[pjeby]

FYI, the new plugin now supports silent login and recent login checks.

Specifically:

• There's an option you can set that will redirect logged-out users through the IdP every so many minutes, to automatically log them in if they've already logged into another site via the same idP. (To avoid unnecessary redirects, it only happens if some user has previously attempted to log into the site using that particular browser.) This makes single-signon a lot more seamless.

• There's an API to say, "if the user isn't logged in or hasn't logged in recently, redirect them to the login page or return false if it's not ok to redirect right now." This lets you do Github-style "sudo mode", where someone can remain logged in for a long time for browsing purposes, but has to re-validate when doing higher-security operations like account management.

[drzraf]

• [ ] Autoconfiguration
• [ ] Session refresh / reconnexion

[drzraf]

@daggerhart here is a plugin relying upon openid-connect-generic to configure (at a low-level) domain authorization when people connect via OpenId.

I'm really happy openid-connect-generic now provides a powerful enough low-level API to hook in in order to create custom authorization schemes. But I wondered whether you'd find the above plugin features (authorize a set of domains), sufficiently useful to upstream some of its bits ?

[drzraf]

I would also like to mention "consider using a dedicated library" #88 for 4.x :)