Skip to content

Add prompt option to creating authorization urls #383

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.

Sign up for GitHub

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

Merged
mthadley merged 3 commits into from
Nov 22, 2024
Merged

Add prompt option to creating authorization urls #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec0980a
Add prompt option to creating authorization urls
salbito-workos Nov 20, 2024
ef181c2
Add test
mthadley Nov 22, 2024
4ced01a
Update doc string to be more specific about usage
mthadley Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions tests/test_user_management.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,26 @@ def test_authorization_url_has_expected_query_params_with_provider(self):
"response_type": RESPONSE_TYPE_CODE,
}

def test_authorization_url_has_expected_query_params_with_prompt(self):
provider = "GoogleOAuth"
redirect_uri = "https://localhost/auth/callback"
prompt = "consent"
authorization_url = self.user_management.get_authorization_url(
provider=provider,
redirect_uri=redirect_uri,
prompt=prompt,
)

parsed_url = urlparse(authorization_url)
assert parsed_url.path == "/user_management/authorize"
assert dict(parse_qsl(str(parsed_url.query))) == {
"client_id": self.http_client.client_id,
"redirect_uri": redirect_uri,
"response_type": RESPONSE_TYPE_CODE,
"provider": provider,
"prompt": prompt,
}

def test_authorization_url_has_expected_query_params_with_domain_hint(self):
connection_id = "connection_123"
redirect_uri = "https://localhost/auth/callback"
Expand Down
6 changes: 6 additions & 0 deletions workos/user_management.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ def get_authorization_url(
connection_id: Optional[str] = None,
organization_id: Optional[str] = None,
code_challenge: Optional[str] = None,
prompt: Optional[str] = None,
) -> str:
"""Generate an OAuth 2.0 authorization URL.

Expand All @@ -349,6 +350,9 @@ def get_authorization_url(
state (str): An encoded string passed to WorkOS that'd be preserved through the authentication workflow, passed
back as a query parameter. (Optional)
code_challenge (str): Code challenge is derived from the code verifier used for the PKCE flow. (Optional)
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)
Comment on lines +353 to +355
Copy link
Contributor

@mattgd mattgd Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@mthadley mthadley Nov 22, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@mattgd mattgd Nov 22, 2024

Choose a reason for hiding this comment

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

Yep, that makes sense!


Returns:
str: URL to redirect a User to to begin the OAuth workflow with WorkOS
Expand Down Expand Up @@ -379,6 +383,8 @@ def get_authorization_url(
if code_challenge:
params["code_challenge"] = code_challenge
params["code_challenge_method"] = "S256"
if prompt is not None:
params["prompt"] = prompt
Comment on lines +386 to +387
Copy link
Contributor

@mattgd mattgd Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could you add a test for this new parameter? Looks like this PR is still in draft, so might be on the way.


return RequestHelper.build_url_with_query_params(
base_url=self._client_configuration.base_url,
Expand Down
Loading