Skip to content

No error returned when credentials are incorrect #491

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

Closed
sraillard opened this issue Oct 5, 2020 · 12 comments
Closed

No error returned when credentials are incorrect #491

sraillard opened this issue Oct 5, 2020 · 12 comments

Comments

@sraillard
Copy link

When calling a method with basic authentication (login and password), if the credentials are incorrect, there is no exception. An example with the following will just produce a ns variable with a null value:

KubernetesClientConfiguration config = new KubernetesClientConfiguration
{
    Username = "xxx",
    Password = "yyy",
    Host = "https://zzz:6443",
    SkipTlsVerify = true
};
Kubernetes client = new Kubernetes(config);
V1Namespace ns = client.ReadNamespace("test");
@sraillard
Copy link
Author

For information, basic auth has been removed in kubernetes 1.19

@brendandburns
Copy link
Contributor

brendandburns commented Oct 5, 2020

I believe that this is intentional. Kubernetes returns 404 if you try to access a namespace that you are unauthorized to access (to prevent namespace probing attacks) so the client interprets that as null.

If you disable anonymous authentication in your Kubernetes cluster, I believe you should get a 403 and then an exception.

If the server is returning 403 and you're not seeing an exception, that is a bug.

Edit I was wrong about this, it should throw on 404.

@sraillard
Copy link
Author

sraillard commented Oct 6, 2020

How I can get the HTTP status code returned? When a 404 is returned, an exception is thrown. How to check for the 403? I don't have any exceptions thrown and I didn't find a method or a property that could tell me the result of the last request.

When the password is incorrect, all these methods are returning a null object without throwing any exception: ReadNamespace, ReadNamespacedSecret, ReadNamespacedServiceAccount.

@simonvane
Copy link

simonvane commented Oct 6, 2020

@sraillard That's something that took me a while to work out. I've got an example of handling something like that here - #489 (comment)

Basically, I catch HttpOperationException and look at the response status code within that.

In one case I had to use the lower-level methods (that end with ...WithHttpMessagesAsync).

@sraillard
Copy link
Author

@simonvane : I don't get any exception! That is my issue...

@brendandburns
Copy link
Contributor

Actually, I looked at the code. It returns null on unauthenticated HTTP 401. It should throw on unauthorized 403 (and on 404, I was wrong above)

This doesn't seem right to me, but it will need to be fixed in the generated code.

@brendandburns
Copy link
Contributor

I dug into this some more. This is occurring because the Kubernetes swagger has the following declaration in it:

          "401": {
            "description": "Unauthorized"
          }

Which basically means that 401 is a "expected" error and thus it doesn't throw.

I'm going to see if I can strip this from the swagger because it really doesn't make sense to me for 401 not to throw.

@sraillard
Copy link
Author

You mean it's in the original JSON provided by the Kubernetes project?

@brendandburns
Copy link
Contributor

Yes, the swagger.json that is provided by the Kubernetes project, that is then used to generate this library.

There's a pre-processing script that we can use to strip it, I'm making the PR.

@brendandburns
Copy link
Contributor

See kubernetes-client/gen#170

@simonvane
Copy link

@simonvane : I don't get any exception! That is my issue...

@sraillard Sorry, I was hoping using the ...WithHttpMessagesAsync might throw the exception for you.

@brendandburns
Copy link
Contributor

This was fixed by #505

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 a pull request may close this issue.

3 participants