Handling 403 status codes with "anonymous" #2163
Conversation
k8s api server nowadays defaults to allowing anonymous requests, so that rather than returning a 401, a 403 is returned.
| // A 403 directly from the auth proxy requires reauthentication. | ||
| if (Auth.usingOIDCToken() && Auth.is403FromAuthProxy(response)) { | ||
| dispatchErrorAndLogout(message); | ||
| return Promise.reject(new UnauthorizedError(message)); | ||
| } | ||
| // If not using auth proxy, an anonymous response is sent when | ||
| // a serviceaccount token expires (eg., delete secret xxx) | ||
| // or the token is managed externally (eg., vsphere kubectl login) | ||
| // In this case, force reauthentication | ||
| if (!Auth.usingOIDCToken() && Auth.isAnonymous(response)) { | ||
| dispatchErrorAndLogout(message); | ||
| return Promise.reject(new UnauthorizedError(message)); |
There was a problem hiding this comment.
these two ifs can be handled as one if is403FromAuthProxy || isAnonymous (the other comparison is irrelevant)
Also, why are you returning early? In theory message can be a JSON message (not sure for this use case) and it's handled later.
There was a problem hiding this comment.
Yes, I simply added another in case we should take other actions rather than logout. But, yes, if we gonna do the same in both alternatives we can merge them.
Regarding the rejection, I just thought that the logout action would be the last action and, consequently, there's no need to try to parse the JSON message (particularly, when it is an anonymous 403 it will always fail). But ok, I'll remove the early rejections just in case.
dashboard/src/shared/Auth.ts
Outdated
| return ( | ||
| (response?.data?.message && response.data.message.includes("system:anonymous")) || | ||
| (response?.data && | ||
| response.data && | ||
| typeof response.data === "string" && | ||
| response.data.includes("system:anonymous")) | ||
| ); |
There was a problem hiding this comment.
this condition is getting a bit messy :) Here is an alternative:
const msg = get(response, "data.message") || get(response, "data");
return typeof msg === "string" && msg.includes("system:anonymous");
(being get lodash.get)
There was a problem hiding this comment.
Not aware of lodash.get. Much simpler now, thanks for the tip!
| } catch (error) { | ||
| expect(store.getActions()).toEqual(expectedActions); | ||
| } |
There was a problem hiding this comment.
nitpick, if the axios method doesn't throw the error you may get a false negative:
try {
await axios.get(testPath);
} catch (error) {
// compare here the error, if needed
}
expect(store.getActions()).toEqual(expectedActions);
k8s api server nowadays defaults to allowing anonymous requests, so that rather than returning a 401, a 403 is returned.
Currently, we capture some 403 errors for displaying useful information (e.g., "user x doesn't have access to namespace y").
However, some edge cases remained unhandled. For instance, a user was able to 'bypass' UI authentication by simply entering " " (whitespace). Internally, it resulted in an anonymous authentication and the API returned 403 instead of 401.
Description of the change
This PR is to add a new handled 403 case so that the user gets redirected to the login page instead of viewing a meaningless error message.
Benefits
Users using tokens with expiration (i.e., #2143) will leverage from a better UX.
Possible drawbacks
I'm not fully aware of what possibly be the behavior when using OIDC. That's the reason why the anonymous 403 is only being handled when is not OIDC login.
Applicable issues
Additional information
N/A