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

Validate client IDs before applying them #17426

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Nov 5, 2024

Prerequisites

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

Description

OpenId Connect Client IDs should only be made up of "unreserved characters" - that is, [a-z], [A-Z], [0-9] and [-._~] (see https://en.wikipedia.org/wiki/Percent-encoding).

The back-office client already limits the Client ID input to alphanumerical chars and dash, but of course the API also needs a validation - and that's what this PR adds 😄

Testing this PR

You'll need to use Swagger to test this, due to the above-mentioned restrictions in the back-office.

  1. It should be possible to create Client IDs for API Users which consists of valid chars.
  2. It should not be possible to create Client IDs for API Users containing one or more invalid chars.

@kjac kjac marked this pull request as ready for review November 6, 2024 06:58
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

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

Took the liberty of adding a reponse for the operation result 😁
Looks good, tests good 🚀

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 suggestion.

Files not reviewed (1)
  • src/Umbraco.Infrastructure/Security/BackOfficeUserClientCredentialsManager.cs: Evaluated as low risk

…als/ClientCredentialsUserControllerBase.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Zeegaan Zeegaan merged commit 669c585 into release/15.0 Nov 7, 2024
13 of 15 checks passed
@Zeegaan Zeegaan deleted the v15/fix/validate-client-ids branch November 7, 2024 09:42
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.

3 participants