-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
API access with client credentials (core functionality) #16817
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elit0451
approved these changes
Jul 29, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests out great 💪 🙌
1 task
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
Description
This PR introduces API access via client credentials (client ID + secret) to both the Management and the Delivery API. Client credentials is a necessary access mechanism for machine-to-machine integrations with these APIs.
The feature works by impersonating a concrete User (Management API) or Member (Delivery API). Thus we can reuse the entire permissions setup for both APIs.
Please note that this PR actively ensures a prefix for all created client IDs. This is to avoid collisions with the core client IDs:
umbraco-back-office-
.umbraco-member-
.Also note that this PR does not include any UI support for managing client secrets. Only the core functionality (including Management APIs for client secrets) is included here.
The Management API
With this PR it becomes possible to define pairs of client IDs and secrets for a specific User. In other words, one User can be associated with multiple client credentials. There are multiple applications for this, including rotation of client credentials.
Managing access to the Management API is quite critical, so we want to ensure that it's possible to distinguish between regular Users and integration Users in the UI. To that end, a new concept of "User type" is introduced:
Default
: A regular back-office User.Api
: A back-office User meant to be used with client credentials for API access.Client credentials can only be defined for
Api
Users. Likewise,Api
Users are not allowed to define passwords, making it impossible to sign into the back-office with anApi
User.We only ever store the client IDs in the Umbraco database (the
umbracoUser2ClientId
table). We use them figure out which User to impersonate for a given client ID, and to avoid collisions between client IDs. The client secrets however are stored and handled exclusively by OpenIddict. This means that:At this point there are no complexity requirements for client secrets. We might consider enforcing a certain level of client secret complexity in the future.
The Delivery API
Member authorization is defined by configuration (see the docs for details).
When introducing Member authorization (with Authorization Code Flow + PKCE) in the Delivery API, we prepared the
MemberAuthorization
configuration for an eventual future with more authorization flows. That future is now 😄 and the configuration has been extended with aClientCredentialsFlow
option:"Enabled": true
).AssociatedMembers
, which is an array that maps client credentials and Members. This makes it possible to have multiple client credentials mapped to different (or the same) Member access setups.Testing this PR
Management API, Client Credentials Flow
Using Swagger:
Api
User (use"type": "Api"
in the payload).Download this Node app and tweak the
.env
variables to match your setup:UMBRACO_CLIENT_ID
andUMBRACO_CLIENT_SECRET
must match the your client credentials (client ID and secret, respectively).UMBRACO_DOCUMENT_ID
must be an ID (key) of an existing document in your site.Run the Node app from a terminal with
npm run start
. The app should print out the resulting JSON from the/umbraco/management/api/v1/document/{id}
Management API endpoint:Using Swagger, delete the client credentials from the
Api
User. Re-run the Node app and verify that it is no longer allowed to access the Management API.Management API, Authorization Code Flow + PKCE
Verify that you can still log into the back-office 😆
Delivery Api, Client Credentials Flow
In Umbraco:
article
(it doesn't need to have any properties).By configuration:
ClientCredentialsFlow
as described above.Download this Node app and tweak the
.env
variables, soUMBRACO_CLIENT_ID
andUMBRACO_CLIENT_SECRET
match the your client credentials (client ID and secret, respectively).Run the Node app from a terminal with
npm run start
. The app should print out all thearticle
documents available to the configured client credentials.For good measure, verify that
/umbraco/delivery/api/v2/content/?fetch=children:/&filter=contentType:article
only outputs the publicly availablearticle
documents when requested in a browser (i.e. anonymous access).Delivery API, Authorization Code Flow + PKCE
These tests are quite cumbersome. It might be easier to get in touch with @kjac who's got a test setup ready to go.
Create an application as described in this docs artice and verify that Authorization Code Flow + PKCE is not broken by this PR.
Also, ensure that refresh tokens still work (it might be simplest to do with Postman, once the application above has obtained a valid token).
Upgrading Umbraco
The new "User type" means that the
User
class is expanded with a newType
property. Historically this has sometimes been a challenge when performing upgrades.Test that it's possible to perform an upgrade to this PR:
Breaking changes
The changes in this PR are binary breaking, due to the new methods in
IBackOfficeApplicationManager
,IUserService
andIUserRepository
. However, the PR targets a new major (V15.0), so binary breakage is probably OK 😄