If orgID already present, rename instead of reject#63
Conversation
|
Why rename it? What is looking at the new "orig" headers introduced in this PR? |
|
True, I don't plan to look at it. I was mostly thinking it might be useful to have it around for debugging weird issues. I also considered logging a warning or debug message. Happy to change it, what do people prefer? |
rndstr
left a comment
rndstr
left a comment
There was a problem hiding this comment.
Code implements what the description says 👍 noted one minor cleanup suggestion
| newCtx = metadata.NewOutgoingContext(ctx, md) | ||
| } | ||
| md[lowerOrgIDHeaderName] = []string{orgID} | ||
| newCtx = metadata.NewOutgoingContext(ctx, md) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
jml
left a comment
jml
left a comment
There was a problem hiding this comment.
I don't really understand the motivation for this change.
I'm a little worried that by making this succeed rather than fail, we open up opportunities for accidental impersonation.
I think if we do proceed, logging is probably better than passing along the old header.
|
Going to decline this, in favor of adding a middleware to authfe which drops any offending incoming headers. |
route: Allow contextFn to return an error
4 participants
As discussed in a related private change, I am trying a certain request from an internal service which results in this header already being present.
If we simply rename the offending header, rather than reject the request, then things should work.