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

nomad login errors before determining the default auth method type #16501

Closed
xkisu opened this issue Mar 15, 2023 · 4 comments · Fixed by #16504
Closed

nomad login errors before determining the default auth method type #16501

xkisu opened this issue Mar 15, 2023 · 4 comments · Fixed by #16504
Assignees
Labels
Milestone

Comments

@xkisu
Copy link

xkisu commented Mar 15, 2023

Nomad version

v1.5.0

Operating system and Environment details

Ubuntu 22.04.2 LTS

Issue

The documentation for the nomad login command and the integration guide here state that the type flag is optional if an admin has configured a default, however this is not the case.

Line 117 in login.go explicitly rejects any calls to nomad login that don't have OIDC set as the type:

switch l.authMethodType {
case api.ACLAuthMethodTypeOIDC:
default:
	l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", l.authMethodType))
	return 1
}

There is a check further down for a default auth method, at line 143 that will set the type to be the type of the default auth method, but that section of the code is never reached because the switch statement above returns an error before it can try to determine the default type.

Reproduction steps

  1. Register an auth method with Nomad (i.e. following this guide)
  2. Make sure the auth method is set as default (nomad acl auth-method info vault)
  3. Try to run nomad login without specifying the type, you'll receive an error Unsupported authentication type ""

Expected Result

It should be selecting the default auth method and using the type from that. These is code defined in the command to do so, but the switch statement further up cancels it out before it can try to determine the default.

Actual Result

Unsupported authentication type ""
@xkisu xkisu added the type/bug label Mar 15, 2023
@pkazmierczak pkazmierczak self-assigned this Mar 15, 2023
@pkazmierczak
Copy link
Contributor

pkazmierczak commented Mar 15, 2023

Hi @xkisu, thanks for pointing out the bug. You are right that the documented and expected behavior don't match the implementation, we'll be fixing it promptly.

@pkazmierczak
Copy link
Contributor

Hey @xkisu, we just merged a fix and it will be included in the next release. While fixing it, we noticed that nomad login doesn't actually need a -type flag, since auth method names are unique. Thus the fix removes the flag (although it will not error if the flag is submitted, it will just print a deprecation warning until 1.6.0), and it now correctly respects the default auth method if set.

Once again thanks for catching this!

@xkisu
Copy link
Author

xkisu commented Mar 17, 2023

Hey @xkisu, we just merged a fix and it will be included in the next release. While fixing it, we noticed that nomad login doesn't actually need a -type flag, since auth method names are unique. Thus the fix removes the flag (although it will not error if the flag is submitted, it will just print a deprecation warning until 1.6.0), and it now correctly respects the default auth method if set.

Once again thanks for catching this!

Thank you for merging a fix so quickly!

Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
3 participants