Skip to content

krb5: fix OTP authentication#8296

Merged
justin-stephenson merged 2 commits intoSSSD:masterfrom
sumit-bose:krb5_fix_otp
Dec 11, 2025
Merged

krb5: fix OTP authentication#8296
justin-stephenson merged 2 commits intoSSSD:masterfrom
sumit-bose:krb5_fix_otp

Conversation

@sumit-bose
Copy link
Contributor

@sumit-bose sumit-bose commented Dec 11, 2025

Resolves: #8292

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Dec 11, 2025
gemini-code-assist[bot]
gemini-code-assist bot reviewed Dec 11, 2025
View reviewed changes
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes an issue with OTP authentication in the Kerberos provider. The change correctly sets the kr->otp flag in the request_otp function, which is called during pre-authentication when OTP is available. This fix is necessary to ensure the user is prompted for OTP. The logic is now consistent with the corresponding answer_otp function. The change is simple, targeted, and I find it to be correct.

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Dec 11, 2025

JFTR: instructions how to run FreeIPA tests for this patch.

@sumit-bose
Copy link
Contributor Author

sumit-bose commented Dec 11, 2025

JFTR: instructions how to run FreeIPA tests for this patch.

PR to run FreeIPA tests: freeipa/freeipa#8055

@ikerexxe
Copy link
Contributor

ikerexxe commented Dec 11, 2025

Added an additional commit to fix a related issue

@sumit-bose sumit-bose mentioned this pull request Dec 11, 2025
Closed
@alexey-tikhonov alexey-tikhonov requested review from justin-stephenson and removed request for ikerexxe December 11, 2025 14:05
justin-stephenson
justin-stephenson reviewed Dec 11, 2025
View reviewed changes
@justin-stephenson justin-stephenson added the coverity Trigger a coverity scan label Dec 11, 2025
@justin-stephenson justin-stephenson added Accepted and removed coverity Trigger a coverity scan labels Dec 11, 2025
@justin-stephenson
Copy link
Contributor

justin-stephenson commented Dec 11, 2025

Covscan is green.

sumit-bose and others added 2 commits December 11, 2025 16:19
@sumit-bose @sssd-bot
krb5: fix OTP authentication
6c754ce 
Resolves: SSSD#8292
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
@ikerexxe @sssd-bot
krb5_child: fix OTP authentication for PAM stacked tokens
11fcc28 
The `tokeninfo_matches()` function already handles PAM stacked tokens
correctly by processing them through the 2FA single path, so the
`answer_otp()` function should allow this token type to proceed.

Add SSS_AUTHTOK_TYPE_PAM_STACKED to the allowed authentication token
types in `answer_otp()` to restore previous functionality.

Fixes: 4cb99a2 ("krb5_child: advertise authentication methods").
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
@sssd-bot
Copy link
Contributor

sssd-bot commented Dec 11, 2025

The pull request was accepted by @justin-stephenson with the following PR CI status:

🟢 CodeFactor (success)
🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-41) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟡 ci / system (centos-10) (in_progress)
🟡 ci / system (fedora-41) (in_progress)
🟡 ci / system (fedora-42) (in_progress)
🟡 ci / system (fedora-43) (in_progress)
🟡 ci / system (fedora-44) (in_progress)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@justin-stephenson
Copy link
Contributor

justin-stephenson commented Dec 11, 2025 •
edited
Loading

@ikerexxe @sumit-bose This will need manual backport to sssd-2-9 after passworldess GDM patches are merged there, yes?

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Dec 11, 2025

@ikerexxe @sumit-bose This will need manual backport to sssd-2-9 after passworldess GDM patches are merged there, yes?

Imo, it can be included in open PR directly.

@alexey-tikhonov alexey-tikhonov mentioned this pull request Dec 11, 2025
Open
@justin-stephenson justin-stephenson merged commit df15165 into SSSD:master Dec 11, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justin-stephenson justin-stephenson justin-stephenson approved these changes

@ikerexxe ikerexxe Awaiting requested review from ikerexxe

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ikerexxe ikerexxe

@justin-stephenson justin-stephenson

Labels

Accepted no-backport This should go to target branch only.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test failure: ssh with OTP login in IPA environment

5 participants

@sumit-bose @alexey-tikhonov @ikerexxe @justin-stephenson @sssd-bot