Add prompt option to creating authorization urls #383
Conversation
salbito-workos
force-pushed
the
allow-passing-prompt-parameter-for-um-authorization-url
branch
from
f8c66a2 to
ec0980a
Compare
| if prompt is not None: | ||
| params["prompt"] = prompt |
There was a problem hiding this comment.
Could you add a test for this new parameter? Looks like this PR is still in draft, so might be on the way.
mattgd
left a comment
mattgd
left a comment
There was a problem hiding this comment.
Left one optional suggestion, but looks good.
| prompt (str): Used to specify whether the upstream provider should prompt the user for credentials or other | ||
| consent. Valid values depend on the provider. Currently only applies to provider values of 'GoogleOAuth', | ||
| 'MicrosoftOAuth', or 'GitHubOAuth'. (Optional) |
There was a problem hiding this comment.
Looks like Google OAuth, Microsoft OAuth, and GitHub OAuth all have a pretty limited set of values for prompt. Could be nice to maintain a set of allowed values with a literal. Not required, though if we want to do it, would be good to do it now, otherwise it'd be a breaking change later.
There was a problem hiding this comment.
Since the values are meaningful to the downstream providers (not us), are not consistent across them (though somewhat overlap), and if any providers changed later would require us to update the SDK before new values could be passed, I'm leaning towards skipping validation in order to stay decoupled.
Not a bad idea! Just not sure it's worth it in this particular case.
mthadley
deleted the
allow-passing-prompt-parameter-for-um-authorization-url
branch
Description
This changset adds the prompt option to generating oauth authorization urls.
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.