Skip to content

Conversation

@eisenmann-b1
Copy link

This splits the OAuth2 prompt Authenticate at <url> and press ENTER. into an info message Authenticate at "<url>" and a prompt Press ENTER to continue.
The latter can be skipped by setting the PAM option skip_enter_prompt.

@eisenmann-b1 eisenmann-b1 marked this pull request as ready for review December 1, 2025 11:19
@sumit-bose
Copy link
Contributor

Hi,

thank you for your patch. Can you explain a bit more why you do not like "Press Enter" message?

In general I would prefer to not handle this with PAM module options but with prompting options in sssd.conf, see
"PROMPTING CONFIGURATION SECTION" in man sssd.conf for details. Currently there is nothing for "oauth2" and adding new options here would give a much greater flexibility in configuring the prompts for OAuth2.

bye,
Sumit

@eisenmann-b1
Copy link
Author

I think prompting the user to press ENTER, while at the same time asking them to use another device to log in to some URL is actually harmful to the user experience.
If pressed too early, the request will most likely time out before the user can log in on the other device, and if pressed too late the request might already be expired.
IMO, setting the idp_request_timeout to a higher number and disabling the ENTER prompt allows for a better experience for the user.

Regarding the prompting configuration, I agree that it makes sense to have the option there. Then I will move the option to the sssd.conf. Maybe something along the lines of:

[prompting/oauth2]
interactive: show prompt before continuing. (default: true)
interactive_prompt: the message for the interactive prompt. (default: "Press ENTER")

@eisenmann-b1
Copy link
Author

As suggested, I removed the PAM option, and added a [prompting/oauth2] section, with options interactive (default true) and interactive_prompt (default Press ENTER to continue.).

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Dec 2, 2025
@sumit-bose
Copy link
Contributor

Hi,

thank you, the new approach looks much better now. Can you squash your initial two patches into the new one so that the changes from the first two does not have to be removed by the third? Please remove the changes to the *.pot file as well, this will be done at certain times before a new release.

Additionally, I would like to ask you for a bit of patience and wait until #8212 is merged and then rebase your patch on top of it. And @ikerexxe, the author of #8212, might also have some comments if your new options fits into the general plan for configurable prompts.

bye,
Sumit

@eisenmann-b1 eisenmann-b1 force-pushed the oauth2-prompt-changes branch 2 times, most recently from 2375d07 to 481d9fb Compare December 2, 2025 16:13
@eisenmann-b1 eisenmann-b1 changed the title Make OAuth2 prompt for ENTER optional Add OAuth2 prompting config Dec 3, 2025
@eisenmann-b1
Copy link
Author

I squashed the commits and dropped the changes to the *.pot file.
Also changed the title to better reflect the changes.

Looking forward to seeing the mentioned PR merged.

@eisenmann-b1 eisenmann-b1 force-pushed the oauth2-prompt-changes branch from 481d9fb to 7db70c9 Compare December 3, 2025 13:59
:config: New options to customize the OAuth2 prompting behavior:
         `interactive` and `interactive_prompt`.
@eisenmann-b1 eisenmann-b1 force-pushed the oauth2-prompt-changes branch from 7db70c9 to 87d2d5f Compare December 8, 2025 09:42
@eisenmann-b1
Copy link
Author

Rebased, since there were some conflicts from the passwordless-gdm PR that was merged.

Also, CodeFactor complained about complexity of pc_list_from_response, so I moved the string copying blocks there into a separate function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants