Support X-Access-Token for Grafana Cloud #107
Conversation
| @@ -67,6 +70,7 @@ func oncallClientFromContext(ctx context.Context) (*aapi.Client, error) { | |||
|
|
|||
| grafanaOnCallURL = strings.TrimRight(grafanaOnCallURL, "/") | |||
|
|
|||
| // TODO: Allow access to OnCall using an access token instead of an API key. | |||
There was a problem hiding this comment.
This TODO will wait for another PR as the OnCall client needs to be upgraded to take a transport. In the meantime we may just need to disable these tools when we use an access token.
There was a problem hiding this comment.
@sd2k mentioned there are additional complications apart from just the OnCall client upgrade (something about the way routing is set up in cloud IIRC) but yeah that's non-blocking for this PR.
In Grafana Cloud it is possible to have a plugin submit requests on behalf of the calling user by setting the X-Access-Token header instead of the standard Authorization header. This PR will preferentially look for the presence of an access token and use that before falling back to an API key like normal.
csmarchbanks
force-pushed
the
support-x-access-token
branch
from
fbfe711 to
d8ed43d
Compare
| @@ -67,6 +70,7 @@ func oncallClientFromContext(ctx context.Context) (*aapi.Client, error) { | |||
|
|
|||
| grafanaOnCallURL = strings.TrimRight(grafanaOnCallURL, "/") | |||
|
|
|||
| // TODO: Allow access to OnCall using an access token instead of an API key. | |||
There was a problem hiding this comment.
@sd2k mentioned there are additional complications apart from just the OnCall client upgrade (something about the way routing is set up in cloud IIRC) but yeah that's non-blocking for this PR.
| @@ -97,11 +97,12 @@ type siftClient struct { | |||
| url string | |||
| } | |||
|
|
|||
| func newSiftClient(url, apiKey string) *siftClient { | |||
| func newSiftClient(url, accessToken, apiKey string) *siftClient { | |||
| client := &http.Client{ | |||
| Transport: &authRoundTripper{ | |||
There was a problem hiding this comment.
we should move this authRoundTripper out into a common location in the future
mcpgrafana.go
Outdated
| @@ -103,6 +104,12 @@ func WithGrafanaAPIKey(ctx context.Context, apiKey string) context.Context { | |||
| return context.WithValue(ctx, grafanaAPIKeyKey{}, apiKey) | |||
| } | |||
|
|
|||
| // WithGrafanaAccessToken adds the Grafana access token to the context. An | |||
| // access token is used for on-behalf-of auth in Grafana Cloud. | |||
There was a problem hiding this comment.
This is true but only if complemented with an identity token, right? Out of an abundance of caution, it might be worth mentioning that in the doc comment for this (and GrafanaAccessTokenFromContext) to avoid implying that an access token is all that's required for on-behalf-of auth. Otherwise someone may use this thinking they're getting OBO auth automatically when actually they're just authenticating as the access policy associated with the access token, unless an ID token is also added.
There was a problem hiding this comment.
Good callout. We definitely want to change this to take the user token as well and not allow requests without the user token as a request with just an access token will still work but is unlikely to be what someone wants to be doing.
There was a problem hiding this comment.
New commit fixes this to require that an id token is always provided as well.
csmarchbanks
dismissed
annanay25’s stale review
Dismissing as I have changed the signature
In Grafana Cloud it is possible to have a plugin submit requests on behalf of the calling user by setting the X-Access-Token header instead of the standard Authorization header. This PR will preferentially look for the presence of an access token and use that before falling back to an API key like normal.