Format and filter session_required_* query parameters - #1430
Conversation
sirosen
left a comment
sirosen
left a comment
There was a problem hiding this comment.
This worked out to be about as clean as I could hope.
I have one optional change which I leave open to you as the author, but I would be happy to merge this as it stands. Therefore, approved!
| encoded_params = urllib.parse.urlencode(params) | ||
| return f"{authorize_base_url}?{encoded_params}" | ||
|
|
||
| return super()._get_authorize_url( |
There was a problem hiding this comment.
Since it's a distinct method name, we could also use self._get_authorize_url(), right? I haven't missed anything?
I ask because I wonder if it should have a more-different name, like self._encode_authorize_url. I'm curious to hear what you think.
There was a problem hiding this comment.
It's an internal function, so I think it could be renamed. However, doing so requires me to return to the code instead of merging it immediately with the existing approval, so I'm inclined to leave it as-is.
2 participants
Fixed
session_required_*query parameters when generating authorization URLs.Fixes #1425