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

RFC7592: set default values for grant_types and response_types when updating client #636

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

frankie567
Copy link
Contributor

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Other, please describe:

  • You consent that the copyright of your pull request source code belongs to Authlib's author.

While implementing RFC7592 endpoint in my code base, I noticed a crash could occur when updating the client if grant_types or response_types were not provided:

  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.12/site-packages/authlib/oauth2/rfc7592/endpoint.py", line 147, in _validate_grant_types
    return grant_types_supported.issuperset(set(value))
                                            ^^^^^^^^^^
TypeError: 'NoneType' object is not iterable

In RFC7591 endpoint, there is a fail-safe fallback to avoid this:

def _validate_response_types(claims, value):
# If omitted, the default is that the client will use only the "code"
# response type.
response_types = set(value) if value else {"code"}
return response_types_supported.issuperset(response_types)

def _validate_grant_types(claims, value):
# If omitted, the default behavior is that the client will use only
# the "authorization_code" Grant Type.
grant_types = set(value) if value else {"authorization_code"}
return grant_types_supported.issuperset(grant_types)

This PR just backports this behavior to RFC7592. The effect is that, if not provided, grant_types and response_types will be set to the default value. From my understanding, this behavior is compliant with the specification:

Omitted fields MUST be treated as null or empty values by the server,
indicating the client's request to delete them from the client's
registration. The authorization server MAY ignore any null or empty
value in the request just as any other value.

@azmeuk
Copy link
Collaborator

azmeuk commented Apr 5, 2024

Thank you @frankie567. For the record, this is a port of #512 for RFC7592.
Can you maybe add some unit tests?

@frankie567
Copy link
Contributor Author

Done! Note that I had to change the setup and an existing assertion to comply with the spec (or, to be fair, my understanding of it 😅)

@lepture lepture merged commit f18c816 into lepture:master Apr 8, 2024
10 of 11 checks passed
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.

3 participants