Skip to content

Prompt parameter customizable and default to None#59

Merged
rayluo merged 1 commit intodevfrom
customizable-prompt
Jul 5, 2025
Merged

Prompt parameter customizable and default to None#59
rayluo merged 1 commit intodevfrom
customizable-prompt

Conversation

@rayluo
Copy link
Copy Markdown
Owner

@rayluo rayluo commented Jun 30, 2025

This PR will resolve #58

To test this PR, you can install the package by pip install https://github.com/rayluo/identity/archive/refs/heads/customizable-prompt.zip.

CC: @cjknapp12epic

@cjknapp12epic
Copy link
Copy Markdown

This looks good to me. I tested this version with my Flask app and verified that the default behavior, and explicitly setting prompt=None or prompt='select_account' all function as expected.

The only concern I would have is whether more communication is needed about changing the default behavior, in case anyone was relying on the previous behavior but would now need to explicitly set prompt='select_account' to achieve it.

@rayluo
Copy link
Copy Markdown
Owner Author

rayluo commented Jun 30, 2025

This looks good to me. I tested this version with my Flask app and verified that the default behavior, and explicitly setting prompt=None or prompt='select_account' all function as expected.

The only concern I would have is whether more communication is needed about changing the default behavior, in case anyone was relying on the previous behavior but would now need to explicitly set prompt='select_account' to achieve it.

Yeah, I know. In your test with prompt=None, will you run into the issue I mentioned in issue #58?

@cjknapp12epic
Copy link
Copy Markdown

I added a logout link to my app and tested it while having prompt=None and with post_logout_view not set (i.e. defaulting to a view decorated with login_required()), and I did get immediately dropped back into the login flow, but it actually presented the select_account behavior (even though prompt=select_account does not appear in the auth URL).

This is probably fine, because if I wanted to log out and didn't have a logged-out view to land on, this prompt probably makes sense to land on.

@rayluo
Copy link
Copy Markdown
Owner Author

rayluo commented Jun 30, 2025

During your logout test, were you redirected to the upstream ID provider (i.e. https://login.microsoftonline.com) and did you actually complete the logout there?

If so, then you finished a complete logout including the logout from the ID provider. That's why the subsequent sign-in attempt will still land you on the account picker.

If you (as a developer) purposely chose to skip logging out from the ID provider, then the situation I described in #58 will kick in.

All in all, it seems that the new default prompt=None is fine for most normal end users. I'll see how to add more "communication" into docs and/or release notes.

@cjknapp12epic
Copy link
Copy Markdown

Ah, yes, I did get taken through a complete logout flow from the upstream provider, although I didn't have to click anything to do it.

I agree that this is probably fine if one would have to deliberately circumvent the upstream logout to get into the behavior your described.

@rayluo rayluo force-pushed the customizable-prompt branch from 937a7fe to 85614ed Compare July 4, 2025 23:44
@rayluo rayluo merged commit 85614ed into dev Jul 5, 2025
6 checks passed
@rayluo rayluo deleted the customizable-prompt branch July 5, 2025 05:06
@rayluo
Copy link
Copy Markdown
Owner Author

rayluo commented Jul 7, 2025

Shipped in Identity 0.11.0

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.

Allow prompt option to be configurable

2 participants