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

Add member auth to the Delivery API #14730

Merged
merged 30 commits into from Sep 26, 2023

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Aug 27, 2023

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This PR adds member auth to the Delivery API. The PR is targeted v14 on purpose, because we need to ensure that the solution can co-exist with users auth in the Management API. A separate task exists to copy all relevant parts of this PR to v12 once this PR is merged.

This "backwards" kind of developing means I've tried to target a v12 PR in a v14 PR. It gets a little weird in some places. For example:

  • Some obsoletion messages in this PR target v14, because they're going to be copied over to the v12 branch.
  • Some changes appear to be terribly breaking, but they affect only code that is currently part of v14 (like the removed Paths and the changed OAuthClientIds), so they're OK to merge in.

At this time, member auth is not expected to be widely adopted by consumers of the Delivery API. Therefore this PR has been constructed to have zero negative impact at query time, if member auth is not used in the Delivery API. This results in a few somewhat wonky code execution paths, but as the alternative would be to inflict a performance penalty on every last consumer of the Delivery API, the wonky paths are a fair trade-off.

Testing this PR

This PR should be tested alongside the docs article that is being finalized now. The article contains a lot of code samples that are required to test the PR.

It's not going to make a lot of sense to outline every last test scenario here, as there are a LOT and most all of them require additional test projects on the side to perform auth against the Delivery API. Thus, testing will be performed as peer testing with @kjac

A little checklist of things to absolutely remember (mostly for my own sake):

  • custom cookie options login path
  • member auth can be enabled and disabled by config
  • protected content is not indexed if member auth is disabled
  • external auth is possible (i.e. GitHub)
  • hybrid/combined auth also works
  • multiple return URLs can be used, also with external auth
  • user auth cannot authenticate members and vice versa
  • member tokens are revoked when member role membership changes
  • locked out, unapproved and deleted members cannot use previously issued tokens
  • refresh tokens work
  • can perform token revoke
  • can perform sign-out from the server
  • can query protected content when authorized
  • cannot query protected content as anonymous
  • can fetch protected content (single items) when authorized
  • cannot fetch protected content (single items) as anonymous
  • must use redirect URLs that are on the allow-list when logging in
  • must use redirect URLs that are on the allow-list when logging out
  • can enable Swagger auth for members

bergmania and others added 6 commits August 28, 2023 10:20
# Conflicts:
#	src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs
#	src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs
#	src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs
#	src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs
Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 💪 🙌 , I've left a few comments for you to consider 🙂

Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! 🚀🔥

kjac and others added 5 commits September 13, 2023 11:27
…endpoints (as of now they do not exist, a separate task will fix that)
…in screen for new back-office (will be swapped back again to ensure correct .well-known endpoints, see FIXME comment)
@elit0451 elit0451 merged commit 83321a8 into v14/dev Sep 26, 2023
13 of 14 checks passed
@elit0451 elit0451 deleted the v14/feature/delivery-api-member-auth branch September 26, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants