armada
armada copied to clipboard
Improve no perms error message
Currently, the error returned to users looks something like:
{"error":"Request in not authenticated with any of the supported schemes.","code":16,"message":"Request in not authenticated with any of the supported schemes."}
We should improve this to give more info. For example
- Info on which schemes were tried (type, principal name, etc.) and, for each scheme, if it resulted in "permission denied" or an error.
- What action the user was trying to make.
The goal is to help users understand if they're using the wrong creds, forgot to supply creds, if they're trying to perform some action other than what they intended, or if they just don't have perms. Currently, these events would all result in the same error message.
This is the part of the code that should be updated: https://github.com/G-Research/armada/blob/master/internal/common/auth/authorization/common.go#L139
Hey team! Please add your planning poker estimate with ZenHub @dejanzele @jayofdoom @kannon92 @richscott
Going to take a look at this.
@severinson Right now it looks like the behavior is the following:
Pseudocode:
for each auth service:
try to auth with service
if auth fails due to missing creds, try the next one
if auth fails because creds are invalid, error immediately
otherwise auth succeeded and we're good
endfor
if we get here we couldn't auth with any of the configured services so return an unauthenticated error
Do you want me to change the behavior of this function to continue trying services even if one errors on credentials?
Don't we log the request taking place in other gRPC middleware?
I think it's best we return an error immediately on this if auth fails because creds are invalid, error immediately
.
(If I've said differently previously, I've changed my mind :) )
This doesn't relate to logging. It relates to the error returned to the user. Since users never see the logs, it'd be helpful for them if we returned an error that explains what they were trying to do and why it failed.