[Multi User] Make user identity header configurable

[Bobgy]

/cc @yanniszark @jlewi
as discussed in #3645 (comment)

Part of #1223

• UI Server [UI Server] Refactor for configurable auth header #3753
• Api Server [Backend] Make user identity header configurable #3772 and [Backend] Fix user identity extraction logic in getUserIdentity() #3842

[yanniszark]

@Bobgy thanks a lot!
After seeing the changes, I have a small question.
I may be wrong, but is authorization happening in two places? (apiserver and ui-server)
If yes, is there any plan to consolidate authorization logic in one place?

[Bobgy]

@Bobgy thanks a lot!
After seeing the changes, I have a small question.
I may be wrong, but is authorization happening in two places? (apiserver and ui-server)
If yes, is there any plan to consolidate authorization logic in one place?

@yanniszark For clarification, authorization logic happens in only one place, the api server. Api server exposes an endpoint that ui-server queries. The usage of the header in ui-server was because ui-server needs to know what header to proxy to api-server, so api-server can authenticate correctly

[yanniszark]

@Bobgy awesome, thanks a lot for the explanation!
Is the endpoint you're talking about a special endpoint only for the UI-server to use?
Or is the UI-server making requests to the API-Server's usual endpoints on behalf of the user and includes the userid header in them (so requests happen to the usual endpoints)?

[Bobgy]

@yanniszark It's a special endpoint only for UI-server, because some features (e.g. tensorboard CRUD) is implemented in UI server. We'd want to keep current structure that UI server can implement some UI only apis, because we don't want to rewrite them in api server.

[yanniszark]

@Bobgy I see. Is this the endpoint you are talking about?
https://github.com/kubeflow/pipelines/blob/1dcea49472e725ab681db3d244d51837e291f5ba/backend/api/auth.proto

It seems like you're implementing a SubjectAccessReview-like endpoint but for the Pipelines API-Server.

[Bobgy]

@yanniszark yes, that's right. I made a thin wrapper following the same schema, to be compatible with future changes.

[elikatsis]

@Bobgy hi!
I found a bug on user id header parsing and opened this PR: #3842
It would be great if you included it in the main list of this issue, and it would be even greater if you took a look at it 😃

[Bobgy]

Regarding #4091, the prefix env var cannot be set as empty string.

[Bobgy]

#4091 fixed