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 AuthenticationProviderId and PasswordResetProviderId #10553

Merged
merged 7 commits into from
Nov 10, 2023

Conversation

Chris-Codes-It
Copy link
Contributor

Changes
POST /Users/{userId}/Policy returns BadRequest when PasswordResetProviderId is null or empty string

Issues
Issue 10417

@jellyfin-bot jellyfin-bot added the merge conflict Merge conflicts should be resolved before a merge label Nov 8, 2023
@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@jellyfin-bot jellyfin-bot removed the merge conflict Merge conflicts should be resolved before a merge label Nov 8, 2023
@crobibero
Copy link
Member

Would adding [Required(AllowEmptyStrings = false)] to both AuthenticationProviderId and PasswordResetProviderId in UserPolicy provide the same experience?

Copy link

github-actions bot commented Nov 9, 2023

Changes in OpenAPI specification found. Expand to see details.

What's Changed


POST /Users/{userId}/Policy
Request:

Changed content type : application/json

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId

Changed content type : text/json

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId

Changed content type : application/*+json

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
GET /Users
Return Type:

Changed response : 200 OK

Users returned.

  • Changed content type : application/json

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
GET /Users/{userId}
Return Type:

Changed response : 200 OK

User returned.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
POST /Users/{userId}
Request:

Changed content type : application/json

Updated UserDto :

  • Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId

Changed content type : text/json

Updated UserDto :

  • Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId

Changed content type : application/*+json

Updated UserDto :

  • Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
GET /Users/Me
Return Type:

Changed response : 200 OK

User returned.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
POST /Users/New
Return Type:

Changed response : 200 OK

User created.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
GET /Users/Public
Return Type:

Changed response : 200 OK

Public users returned.

  • Changed content type : application/json

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

Changed items (object):

Class UserDto.

openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
POST /Users/{userId}/Authenticate
Return Type:

Changed response : 200 OK

User authenticated.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
POST /Users/AuthenticateByName
Return Type:

Changed response : 200 OK

User authenticated.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
POST /Users/AuthenticateWithQuickConnect
Return Type:

Changed response : 200 OK

User authenticated.

  • Changed content type : application/json

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="CamelCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId
  • Changed content type : application/json; profile="PascalCase"

openapi-base openapi-changes.md openapi-head Changed property User (object)

Class UserDto.

Updated UserDto :
openapi-base openapi-changes.md openapi-head Changed property Policy (object)

Gets or sets the policy.

Updated UserPolicy :
New required properties:

  • AuthenticationProviderId
  • PasswordResetProviderId

@Chris-Codes-It
Copy link
Contributor Author

Chris-Codes-It commented Nov 9, 2023

Would adding [Required(AllowEmptyStrings = false)] to both AuthenticationProviderId and PasswordResetProviderId in UserPolicy provide the same experience?

@crobibero that's a fair comment; I did consider it however I didn't see any reference to ModelState.IsValid so I assumed no DataAnnotation binding had been wired-in. My bad. Have changed for both fields and updated the tests.

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

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

https://learn.microsoft.com/en-us/aspnet/core/web-api/?view=aspnetcore-7.0#automatic-http-400-responses

The controller automatically validates models so the manual check isn't needed

@crobibero crobibero changed the title Issue/10417 Validate AuthenticationProviderId and PasswordResetProviderId Nov 10, 2023
@crobibero crobibero merged commit 3fd505a into jellyfin:master Nov 10, 2023
18 checks passed
@Chris-Codes-It Chris-Codes-It deleted the issue/10417 branch November 11, 2023 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants