fastapi-sso
fastapi-sso copied to clipboard
Add documentation about redirect_uri
It was unclear to us that we needed it in verify_and_process and led to some debugging.
Hi @mluogh and thanks for you PR, however, I believe it's now quite correct - verify_and_process
does not accept redirect_uri
. Also, you are passing it the callback uri from within the callback itself.
I believe it should be:
@api.get("/login")
async def login(request: Request):
return sso.get_login_redirect(request.url_for("callback"))
@api.get("/callback")
async def callback(request: Request):
...
Or something similar. Would you be up to updating the PR to reflect the actual usage or should I take care of it?
Either way, thanks again for bringing this up, my understanding is that fastapi-sso's documentation is not very well written nor extensive.
verify_and_process
takes redirect_uri here, doesn't it?
https://github.com/tomasvotava/fastapi-sso/blob/master/fastapi_sso/sso/base.py#L195
I believe putting verify_and_process in the callback itself is the correct thing since putting it here means it will eventually be propagated to prepare_token_request
here.
https://github.com/tomasvotava/fastapi-sso/blob/master/fastapi_sso/sso/base.py#L271
Well, would you believe how little I know my own package?
You are right, but this only applies after you had already passed the same redirect_uri
to the get_login_redirect
method. Wouldn't it make more sense to remember the redirect uri from get_login_redirect
and not require it again in verify_and_process
? I could do that.
That would be wonderful and makes the most sense to me.
If you're busy, I could also send you a PR for that since I have a little bit of downtime.
It would be awesome if you could do this ❤️ Could you please also wait for #60? I'll merge it in a minute, I'd like the new PR to be already using codecov.