Skip to content
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.

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

Passwordless-GDM: enable authentication method selection and EIdP login #9

Closed
wants to merge 12 commits into from

Conversation

ikerexxe
Copy link
Owner

@ikerexxe ikerexxe commented Feb 22, 2024

This PR enables the authentication method selection and the EIdP login from the GNOME login screen.

The design page is available at SSSD/sssd.io#79

The COPR repo for testing is available at https://copr.fedorainfracloud.org/coprs/ipedrosa/passwordles-gdm. You should update sssd, gnome-desktop3, mutter, gdm and gnome-shell packages.

In order to test it's necessary to set

[pam]
pam_json_services = gdm-switchable-auth
...

Constraints: if a user has enabled both password and EIdP authentication, they will only be able to authenticate using EIdP. There's a known problem with password authentication.

@ikerexxe ikerexxe force-pushed the gdm_eidp branch 6 times, most recently from 63bebaa to 42c4e62 Compare February 26, 2024 15:33
@ikerexxe ikerexxe force-pushed the gdm_eidp branch 4 times, most recently from 59bab73 to 253a7cf Compare March 6, 2024 16:32
@ikerexxe
Copy link
Owner Author

ikerexxe commented Mar 7, 2024

Remember to set

[pam]
pam_json_services = gdm-unified-auth
...

in sssd.conf in order to test this properly.

@ikerexxe
Copy link
Owner Author

@justin-stephenson and @sumit-bose this is the PR for passwordless-GDM integration

Copy link

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

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

Ack, thank you.

src/responder/pam/pamsrv_json.c Outdated Show resolved Hide resolved
src/responder/pam/pamsrv_json.c Outdated Show resolved Hide resolved
src/man/sssd.conf.5.xml Outdated Show resolved Hide resolved
src/responder/pam/pamsrv_cmd.c Show resolved Hide resolved
@sumit-bose
Copy link

Hi,

thanks for the patch, after realizing that Fedora 40 already has some newer package version than your test repo everything was working fine in my tests. I also rebased the patches on top of current master to see if the change in krb5_child about handling multiple authentication methods will cause issues, but currently they don't.

The comments I added can be handled when moving forward adding more methods.

bye,
Sumit

ikerexxe added 3 commits May 8, 2024 12:07
This API gets the selected response type data from the response_data
linked list. Includes unit tests.

Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Ray Strode <[email protected]>
Integration with GDM requests two prompts for EIdP so adding them to
prompt_config structure. In addition, implement all the functions needed
to manipulate the structure for these new prompts. Finally, add
unit-tests for the new functions.

Signed-off-by: Iker Pedrosa <[email protected]>
These new options are needed by the GDM integration, but they can be
reused for CLI prompting.

:config: New options to tune EIdP prompting: 'init_prompt' and
         'link_prompt'.

Signed-off-by: Iker Pedrosa <[email protected]>
@ikerexxe ikerexxe force-pushed the gdm_eidp branch 6 times, most recently from 46c7c56 to 0ab4821 Compare May 13, 2024 14:40
@ikerexxe
Copy link
Owner Author

Hi @sumit-bose ,

This is ready for review again. CI failures are unrelated and COPR repo is updated with latest code, including prompt tuning.

Return `prompt_config` structure  in `pam_eval_prompting_config` to tune
the prompts from the SSSD config in the GUI.

Signed-off-by: Iker Pedrosa <[email protected]>
ikerexxe added 8 commits May 23, 2024 10:40
Implement a set of functions to check the available authentication
mechanisms and their associated data, and generate a JSON message with
it. This JSON formatted message will be consumed by apps that provide
GUI login (i.e. GDM). Currently, the implementation only takes into
account password and OAUTH2 mechanisms.

Include unit tests to check the implemented functions.

Signed-off-by: Iker Pedrosa <[email protected]>
Implement a set of functions to unpack the JSON reply from the GUI.
Include unit tests to check the implemented functions.

Signed-off-by: Iker Pedrosa <[email protected]>
Implement a function to check whether the PAM service file in use is
enabled for the JSON procotol. This helps us filter which applications
are compatible with this protocol.

Signed-off-by: Iker Pedrosa <[email protected]>
This new option is used to enable the JSON protocol in the PAM responder
based on the PAM service file in use.

:config: Add pam_json_services option to enable JSON protocol to
         communicate the available authentication mechanisms.

Signed-off-by: Iker Pedrosa <[email protected]>
Call JSON message generation function and fill the data structure
containing the response_data linked list.

Signed-off-by: Iker Pedrosa <[email protected]>
Forward the available authentication mechanisms and their associated
data message to the GUI login using a PAM conversation. Then, obtain the
reply and forward it to the responder, so that it can parse it.

Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Ray Strode <[email protected]>
Parse GUI reply and set the appropriate data in `sss_auth_token`
structure.

Signed-off-by: Iker Pedrosa <[email protected]>
Include JSON message where applies.

Signed-off-by: Iker Pedrosa <[email protected]>
@sumit-bose
Copy link

Hi,

thanks for the update, code looks good. I will run some tests, does the build in your copr repo contain the latest changes or would it be better if I build SSSD from this PR?

bye,
Sumit

@ikerexxe
Copy link
Owner Author

You can test it using the COPR repository, as it has been updated.

@sumit-bose
Copy link

Hi,

thanks, test were working fine. I have no further comments.

bye,
Sumit

@ikerexxe
Copy link
Owner Author

@sumit-bose can you approve it?

@justin-stephenson I made some changes after Sumit's review. Do you want to take another look?

Copy link

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

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

Hi,

all is looking and working fine, ACK.

bye,
Sumit

@justin-stephenson
Copy link

@justin-stephenson I made some changes after Sumit's review. Do you want to take another look?

It's okay for me, Ack.

@ikerexxe
Copy link
Owner Author

ikerexxe commented Jun 3, 2024

Thanks for the reviews. I'm closing this PR and rebasing the development branch manually to add the Reviewed-by tags.

@ikerexxe ikerexxe closed this Jun 3, 2024
@ikerexxe ikerexxe deleted the gdm_eidp branch June 3, 2024 09:30
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.

3 participants