-
Notifications
You must be signed in to change notification settings - Fork 70
access: prevent login with username-password when using wrong keys #174
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
base: main
Are you sure you want to change the base?
Conversation
Fixes apache#168 Signed-off-by: Abhishek Kumar <[email protected]>
@shwstppr |
@weizhouapache no. I think that is the issue @ingox was mentioning. |
Yes, it is the issue that @ingox reported. It seems like @DaanHoogland and @ingox have agreed it is a bash issue.
my understanding is a bit different. |
Thanks @weizhouapache. I don't have a strong opinion either way, so I'm happy to close this if we have agreement, it should work as it is. |
✅ Build complete for PR #174. 🔗 Download the cmk binaries (expires on August 22, 2025) |
@shwstppr work kind of like expected. I have one functional concern though:
as you can see above, setting only a “wrong” APIkey does not stop me from logging in. Is that what we want? I think this does not address all of @ingox concern. In this way we can still fool ourselfves.
when not using a profile as the basis it works as expected btw:
What do you think about my comment here ? In short, I would expect any configured credentials to be ignored once the CLI contains any credentials. |
@DaanHoogland I think this needs a bit more discussion. I agree that if valid credentials are available, any invalid ones should be ignored. However, the use case @ingox raised is also valid. One option is to add a config flag—say, allowfallback—to toggle this behaviour. Alternatively, to keep cmk simple, we could avoid a new setting and address Ingo’s scenario with a few preparatory steps before setting keys (e.g., clear stale credentials, explicitly select the target profile, and validate with a quick API call). I'm converting this to draft for now |
Fixes #168
Tested by setting wrong keys,