Skip to content

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

Closed
ikerexxe wants to merge 12 commits intopasswordless_gdmfrom
gdm_eidp
Closed

Passwordless-GDM: enable authentication method selection and EIdP login#9
ikerexxe wants to merge 12 commits intopasswordless_gdmfrom
gdm_eidp

Conversation

@ikerexxe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner Author

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

Copy link
Copy Markdown

@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.

@sumit-bose
Copy link
Copy Markdown

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 <ipedrosa@redhat.com>
Signed-off-by: Ray Strode <halfline@redhat.com>
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 <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
@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
Copy Markdown
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 <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
Call JSON message generation function and fill the data structure
containing the response_data linked list.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
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 <ipedrosa@redhat.com>
Signed-off-by: Ray Strode <halfline@redhat.com>
Parse GUI reply and set the appropriate data in `sss_auth_token`
structure.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Include JSON message where applies.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
@sumit-bose
Copy link
Copy Markdown

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
Copy Markdown
Owner Author

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

@sumit-bose
Copy link
Copy Markdown

Hi,

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

bye,
Sumit

@ikerexxe
Copy link
Copy Markdown
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
Copy Markdown

@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
Copy Markdown

@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
Copy Markdown
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