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

Version v2.36.16 removes logs for the --credential-process flag #1293

Open
seraco opened this issue Jun 12, 2024 · 2 comments
Open

Version v2.36.16 removes logs for the --credential-process flag #1293

seraco opened this issue Jun 12, 2024 · 2 comments

Comments

@seraco
Copy link

seraco commented Jun 12, 2024

What

Version v2.36.16 stopped producing logs when the --credential-process flag is passed to the login command. This stops showing the MFA number necessary for logging in.

Points to note:

  • We are using the ADFS provider.
  • We use MFA in conjunction with saml2aws in credential helper mode.
  • We use the MFA mode where the phone app prompts for a two-digit number (the user has to read that two digit number from the saml2aws logs).
  • The version v2.36.16 breaks the auth flow for us and maybe for other MFA users?
  • If the change is intentional, is there an alternative approach?
  • We are happy to raise a PR for any solution we agree on with you.

Why

In the following lines version v2.36.16 added a conditional check to exclude the logs:

    if *quiet || (command == cmdLogin.FullCommand() && loginFlags.CredentialProcess) {
        log.SetOutput(io.Discard)
        logrus.SetOutput(io.Discard)
    }
@tinaboyce
Copy link
Contributor

tinaboyce commented Jul 15, 2024

Hi @seraco , thank you for raising this issue. It had been under my radar for a while and it appears there is quite a number of support, so I made it a priority to take a investigate it. Sorry for not getting back to you sooner.

Found the actual PR that caused this issue and in conclusion, and would be happy if you just updated the line of code you have discovered to remove the 2nd half of the OR condition of the if statement, it shouldn't break the feature that introduced this undesired side-effect.

If the change is intentional, is there an alternative approach?

No, it wasn't intentional.

We are happy to raise a PR for any solution we agree on with you.

Yes please. We are due to cut a new release soon but more than happy to wait for this to go through.

tinaboyce added a commit to tinaboyce/saml2aws that referenced this issue Jul 19, 2024
tinaboyce added a commit to tinaboyce/saml2aws that referenced this issue Jul 19, 2024
mapkon added a commit that referenced this issue Jul 21, 2024
fix: credential-process flag remove logs [#1293]
@tinaboyce
Copy link
Contributor

tinaboyce commented Jul 21, 2024

@seraco, just letting you know I made changes to undo the quieting of the logs and pushed a new release so hopefully v2.36.17 fixed it 🤞

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

No branches or pull requests

2 participants