Login endpoint at /api/traces/v1/{tenent}/login #274
Conversation
Signed-off-by: Ed Snible <[email protected]>
| @@ -434,3 +434,7 @@ func (a oidcAuthenticator) checkAuth(ctx context.Context, token string) (context | |||
|
|
|||
| return ctx, "", http.StatusOK, codes.OK | |||
| } | |||
|
|
|||
| func (a oidcAuthenticator) LoginPath(tenant string) string { | |||
| return strings.ReplaceAll("/oidc/{tenant}/login", "{tenant}", tenant) | |||
There was a problem hiding this comment.
Maybe path.Join("/oidc", tenant, "login") would be better here?
There was a problem hiding this comment.
I am happy to make this change.
We currently have handlerPrefix = "/oidc/{tenant}" and return "/oidc/{tenant}", a.handler. My thought was that it was nice to keep a similar appearance in related code. Eventually those two fragments and my new piece should be merged.
Similar code exists in authentication/openshift.go. Let me know what you think and I will make it happen.
There was a problem hiding this comment.
I'd still say path.Join is more common in our code base as well as easier on the eyes, it's a minor point though.
|
Hi, whats the status here? |
|
@matej-g I marked this ready for review six days ago. |
| @@ -434,3 +434,7 @@ func (a oidcAuthenticator) checkAuth(ctx context.Context, token string) (context | |||
|
|
|||
| return ctx, "", http.StatusOK, codes.OK | |||
| } | |||
|
|
|||
| func (a oidcAuthenticator) LoginPath(tenant string) string { | |||
| return strings.ReplaceAll("/oidc/{tenant}/login", "{tenant}", tenant) | |||
There was a problem hiding this comment.
I'd still say path.Join is more common in our code base as well as easier on the eyes, it's a minor point though.
Fair enough. I was trying to be helpful by offering a proposal for how a user might log out of one tenant and into another. How do you envision it working for logs and metrics? |
Resolves #266
This creates the endpoint /api/traces/v1/{tenent}/login which redirects to that tenant's provider-specific login endpoint.
This allows the trace UI to log out using a relative path, e.g.
./logout, rather than requiring knowledge of the authentication provider specified in tenants.yaml.Currently only exposed for tracing, but all the plumbing is there other UIs.
PR is created as a draft so please comment on the direction/idea. If the approach is good I will begin work on tests.