SSO Plugins

[kiancross]

It would be nice to be able to write plugins for SSO providers. We have some non-generic SSO providers, which nobody else would want to use (so it wouldn't make a good PR), but would be a good fit for a plugin.

I think, unlike for the backend, it isn't possible to write an SSO plugin for customer accounts at the moment?

[raphaelm]

Correct, it isn't possible at the Moment. When we created the SSO capabilities, we decided against it for the time being to lower the complexity, mostly of the configuration pages in the backend. For arbitrary SSO methods, it becomes arbitrarily complex to build these. Generally, I'm not at all opposed to the idea of pluggable SSO backends for customers, though, and most of the core logic shouldn't be hard to extend – but the backend UI will be hard.

[kiancross]

I suppose the first task would be to make the SSO backend pluggable, and possibly move the current OpenID SSO to a built-in plugin?

but the backend UI will be hard

Do you mean the backend UI of any plugins which attempt to provide generic SSO functionality?

[raphaelm]

I suppose the first task would be to make the SSO backend pluggable, and possibly move the current OpenID SSO to a built-in plugin?

Yes.

Do you mean the backend UI of any plugins which attempt to provide generic SSO functionality?

Well, yes, because that UI would need to be integrated with SSOProviderForm in some way for it to make sense. So my concern is less about the plugin itself, but more about the UI that allows a plugin to integrate itself there. Form extension through plugins always ends up nasty :(

[kiancross]

@raphaelm I am outlining my current plan for this feature. I would appreciate any feedback you might have.

The high-level overview is to add a hook to get a list of (label, url) pairs to render on the login UI. From this point, the plugin is completely responsible for routing the user, creating their account, checking for duplicate users etc.

However, to assist the plugin developer, much of the current customer creation logic (e.g., duplication checking) will be refactored into a separate module, so it can be reused between implementations.

The final step is to move the current OIDC SSO provider into a plugin.

Phase 1

Create pluggable infrastructure.

• Add a pretix.presale.signals.sso_providers webhook. Responses should be dictionaries with label and url attributes. These will be rendered as buttons on the login page, where the current SSO providers are displayed. This could support an optional priority attribute to help with sorting.
• Refactor lots of the customer creation code (e.g., checking for duplicate customers) into a separate module, which can be reused between plugins.

Phase 2

Move the current SSO implementation to a separate OIDC plugin.

• Create an OIDC plugin.
  • Ensure this new plugin is enabled by default to maintain backwards compatibility.
• Move the current SSO logic to this new plugin, using the pluggable infrastructure from Phase 1.
  • Remove the routes from src/pretix/presale/urls.py.
• Use the nav_organizer hook with a parent value to ensure that the configuration menu is in the same place on the organisers page.
• Change references from the generic 'SSO' text to 'OIDC'. For example, change the menu text from SSO providers to OIDC providers.
• Migrate existing data to the plugin's database.

These can be done in two separate PRs to keep the changes slightly more manageable.

[raphaelm]

Hi @kiancross,

thanks for writing the outline! That's very helpful to talk about it in this stage! I agree with you that it would be nice to have this feature and I agree with you on the fact that the SSO provider should be treated no differently than all others in the end.

However, I disagree about the level of abstraction you propose. In simple terms, you are saying that the interface between core and the plugin should only be a (label, url) pair and every plugin should deal with everything else separately. I don't think that is the best approach, for multiple reasons:

• Having a very "small" interface looks like it would be easier to maintain from a core perspective, but it actually isn't. It means that basically the entire project becomes the interface and if e.g. each plugin creates Customer objects in a different way, that makes it very hard to iterate on the Customer model without breaking plugins. It also puts additional pressure on plugin authors, as we can't just build in new best practices from core, as we learned for backend SSO in Security: Add a mandatory external unique id field to pluggable auth API #2465.

  • A more extensive interface would also allow us to gradually add more functionality, such as logout integrations or links to a password change form in the customer profile, etc., without introducing lots of low-level signals

• I don't like it from a backend UX point of view. Suppose after this was successful, we end up with lots of SSO plugins. OIDC, SAML, LDAP, ShinySocialThing, …. Why should every one of them have their own entry in the main menu, with completely different behaviour and UI behind them? Why would I care as an organizer that "Login with Facebook" is OIDC behind the scenes and "Login with ShinySocialThing" is not, why can't I just have all of them in one list?

I propose a slightly different approach that is a little closer to our pluggable auth backends for backend users:

• We define a base class BaseSSOProviderMethod that provides specific attributes and methods such as

  • identifier
  • verbose_name
  • login_generate_url(return_url, next_url) to be called in SSOLoginView.get
  • login_handle_return(request, …) to be called in SSOLoginReturnView.get/post
    – this will probably the hardest API to get right, and for many SSO methods it might be superficially easier to provide a separate view, but SSOLoginReturnView contains so much special logic to handle things like "event has a different domain than organizer" or "shop is running in a iframe but SSO method does not allow iframe and must therefore be opened in a popup". I really want to keep that logic in one place as much as we can in case we need to change it in the future.
  • get_configuration_form(data, …) to return a subclass of SSOProviderForm that will be called used as the form class in SSOProviderCreateView and SSOProviderCreateView and provides provider-specific form fields and validation logic.
  • Potentially in the future: methods for handling logout, generating a "change my login details" URL, …

• We define a signal register_sso_provider_method to register these classes.

• We split SSOProviderCreateView into a two-step process that asks you to first choose the desired method, then displays the correct form for it.

What do you think?

(PS, I'm out of office the next week, so my next reply could take a while longer than usual.)

[kiancross]

Thanks for the detailed feedback, @raphaelm. I'm really glad I posted my proposal before starting!

Your proposal makes more sense, despite it being a bit more work to implement initially.

login_generate_url(return_url, next_url) to be called in SSOLoginView.get

What are return_url and next_url? Wouldn't this function only return a URL to use on the link?

It might be nice for SSOLoginView.get to call something like get_button_html (rather than login_generate_url), which has a default implementation in the base BaseSSOProviderMethod. The default implementation can call login_generate_url. This means some providers (e.g., Facebook) can nicely style their login button.

login_handle_return(request, …)

What does this return? Presumably, it needs to return information used for the creation of the Customer? It's tempting for it to just return a Customer?

[raphaelm]

What are return_url and next_url? Wouldn't this function only return a URL to use on the link?

Sorry, this was just a scribble on a long train ride, not a well-thought-out function signature 😉

I modeled this loosely after the function signature of the current oidc_authorize_url:

pretix/src/pretix/presale/views/customer.py

Lines 631 to 632 in 1424ae7

	if self.provider.method == "oidc":
	return redirect_to_url(oidc_authorize_url(self.provider, f'{nonce}%{next_url}', redirect_uri))

In my idea, return_url is the URL the identity provider should redirect you back to, which again will than call login_handle_return. next_url is the URL the user should be taken to after successful login.

It might be nice for SSOLoginView.get to call something like get_button_html (rather than login_generate_url), which has a default implementation in the base BaseSSOProviderMethod. The default implementation can call login_generate_url. This means some providers (e.g., Facebook) can nicely style their login button.

Yes, but maybe that's even something that could be added later. I see how it could be nice, but I also see lots of complications. Facebook would very much like us to pull in their JavaScript SDK full of tracking stuff and full of their own implementations of popup handling etc. Not sure if we want that?

What does this return? Presumably, it needs to return information used for the creation of the Customer? It's tempting for it to just return a Customer?

I think it should only return information, not a customer. I think it should basically replace the if block here:

pretix/src/pretix/presale/views/customer.py

Lines 682 to 716 in 1424ae7

	if self.provider.method == "oidc":
	if not request.GET.get('state'):
	return self._fail(
	_('Login was not successful. Error message: "{error}".').format(
	error='state parameter missing',
	),
	popup_origin,
	)
	nonce, redirect_to = re.split("[%#§]", request.GET['state'], 1) # Allow § and # for backwards-compatibility for a while
	if nonce != request.session.get(f'pretix_customerauth_{self.provider.pk}_nonce'):
	return self._fail(
	_('Login was not successful. Error message: "{error}".').format(
	error='invalid one-time token',
	),
	popup_origin,
	)
	redirect_uri = build_absolute_uri(
	self.request.organizer, 'presale:organizer.customer.login.return',
	kwargs={
	'provider': self.provider.pk
	}
	)
	try:
	profile = oidc_validate_authorization(
	self.provider,
	request.GET.get('code'),
	redirect_uri,
	)
	except ValidationError as e:
	for msg in e:
	return self._fail(msg, popup_origin)
	else:
	raise Http404("Unknown SSO method.")

I believe finding (or creating) the correct Customer object from that information should be then handled by core. But if there are strong reasons of returning a Customer directly (and providing something like UserManager.get_or_create_for_backend for a shared logic) I'm fine with that as well.

[kiancross]

In my idea, return_url is the URL the identity provider should redirect you back to, which again will than call login_handle_return. next_url is the URL the user should be taken to after successful login.

Ah ok, this makes sense! I misunderstood what return_url and next_url were for.

Yes, but maybe that's even something that could be added later. I see how it could be nice, but I also see lots of complications. Facebook would very much like us to pull in their JavaScript SDK full of tracking stuff and full of their own implementations of popup handling etc. Not sure if we want that?

Sure, let's leave this for later!

I think it should only return information, not a customer.

What information should be returned? I suppose email, phone, name, and external identifier? Essentially a subset of a customer (e.g., CustomerInfo)? Should all of these fields be required, or just some? I suppose any future fields in a Customer will need to be optional to prevent breaking existing SSO plugins? Do we want to allow arbitrary data to be stored (or possibly leave this to be considered in a future enhancement)?

[raphaelm]

What information should be returned? I suppose email, phone, name, and external identifier? Essentially a subset of a customer (e.g., CustomerInfo)?

I guess, yes.

Should all of these fields be required, or just some?

I think only identifier and email should be required.

I suppose any future fields in a Customer will need to be optional to prevent breaking existing SSO plugins?

Yes, I think so.

Do we want to allow arbitrary data to be stored (or possibly leave this to be considered in a future enhancement)?

Sounds like it could be useful, but can be easily added later, so I'd leave it for later to keep things manageable.

[kiancross]

I've been trying to implement this. Current difficulty is that SSOProviderForm already has an option to select the method. I guess this needs moving to SSOProviderCreateView? SSOProviderForm for displaying the correct form given the method (although only one method is implemented at the moment). I guess this will need refactoring?

[raphaelm]

Yes, this would need to be refactored into a two-step process