Addresses an unhandled exception path & nullability concerns

[StuFrankish]

What issue does this PR address?
This PR addresses an unhandled exception that gets raised whenever an expected collection of scopes is null.

• Includes nullability changes from community PR. (default! to required, and amended unit tests).
• Updates the DefaultScopeParser to return gracefully rather than throwing an exception (which was ultimately unhandled).
• Introduces a new unit test to cover AuthorizedScopes being null. A similar unit test to cover an empty collection for scopes is already present.

Important: Any code or remarks in your Pull Request are under the following terms:
If You provide us with any comments, bug reports, feedback, enhancements, or modifications proposed or suggested by You for the Software, such Feedback is provided on a non-confidential basis (notwithstanding any notice to the contrary You may include in any accompanying communication), and Licensor shall have the right to use such Feedback at its discretion, including, but not limited to the incorporation of such suggested changes into the Software. You hereby grant Licensor a perpetual, irrevocable, transferable, sublicensable, nonexclusive license under all rights necessary to incorporate and use your Feedback for any purpose, including to make and sell any products and services.

(see our license, section 7)

[Copilot]

Pull Request Overview

This PR addresses an unhandled exception path by gracefully handling null collections of scopes and updates related nullability annotations. Key changes include:

• Changing null checks in DefaultScopeParser to log an error and return errors rather than throwing.
• Updating the RefreshTokenCreationRequest model with required properties.
• Amending unit tests to account for scenarios when AuthorizedScopes is null or empty.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
identity-server/test/IdentityServer.UnitTests/Validation/TokenRequest Validation/TokenRequestValidation_Invalid.cs	Adds a test to verify behavior when AuthorizedScopes is null.
identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultRefreshTokenServiceTests.cs	Updates refresh token creation calls to explicitly include an empty AuthorizedScopes collection.
identity-server/src/IdentityServer/Validation/Default/DefaultScopeParser.cs	Modifies null checking to log an error and return a validation error instead of throwing an exception.
identity-server/src/IdentityServer/Models/RefreshTokenCreationRequest.cs	Updates property declarations to use required for nullability support.

Comments suppressed due to low confidence (1)

identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultRefreshTokenServiceTests.cs:103

• The empty array literal '[]' is not valid C# syntax; please use 'Array.Empty()' or 'new string[0]' to create an empty collection.

var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client, AuthorizedScopes = [] });

[josephdecock]

Please fix the minor xmldoc formatting nit that I commented on before merging. Otherwise looks great!

[stefannikolei]

Just for me. This Mr is flagged with non breaking. This should not break people who use it correct. But adding required to an property is breaking, or?

[StuFrankish]

Just for me. This Mr is flagged with non breaking. This should not break people who use it correct. But adding required to an property is breaking, or?

I've had a quick chat with Damian and you are quite right, switching from ... = default!; to .. required .. is considered a breaking API change which we can't pull into a minor build, it'll have to wait until version 8 at the earliest so I'll update the backlog accordingly.

The changes to the DefaultScopeParser should be fine however, so I'll revert the RefreshTokenCreationRequest changes and continue with the PR.

@josephdecock (for visibility)