Revisit checkAPIToken and similar functions

[bbockelm]

(I think this one is important to get done before 7.3.0 is out!)

There's a few issues in checkAPIToken and friends I spotted when working on #404:

1. We should give it a more precise name or make the issuer sources configurable. There's very few cases where we accept any of the 4 different sources of authorization. A precise name would be like checkAnyAuthorization; making it configurable would be providing a bitmask for the different sources of authorization as an input variable.
2. It's ambiguous what scopes are allowed from what issuer. There are three different scopes listed (pelican.directorScrape, pelican.promMetric, and prometheus.read) within promMetricAuthHandler; I'm pretty sure we shouldn't be accepting pelican.directorScrape from the origin's issuer.
3. Speaking of scopes, it's not immediately clear which scope does what. For example, I don't think there's a distinction between pelican.directorScrape and prometheus.read: it's about the permission being granted, not who is doing the action.
   • If there's a reason to keep multiple scopes around (e.g., backward compatibility), then we should add a comment as to why those scopes are present.
4. I've always assumed that the federation issuer is the director issuer: that is, there's no reason to have both checked for and the director would set its iss claim to be the federation URL. Is there a strong reason why we'd need to separate them?
5. When we do check the tokens, we always look up the JWKS from the remote service (this would become a DoS in cases where someone pings the /metrics endpoint repeatedly with a fake token). Instead, we should follow what's done in the director and use the jwk library's Cache object to maintain the key.
   • As part of the consolidation of the key handling we need to do, perhaps we should centralize this logic to make it more reusable?

[haoming29]

I agree that checkAPIToken is a bit messy and unnecessarily cumbersome with what is checks. I think the original logic is purely for checking the token for Prometheus query engine (/api/v1.0/prometheus) and I adapted/refactored most of the logic for the /metrics endpoint assuming the two should have similar if not the same audience.

I think it's a bit unclear to me at the start why we would need to check all three places (authz param, Authorization header, and cookie) for the /api/v1.0/prometheus endpoint as this should only be accessed by web user (i.e. via cookie auth) and it's not for director/federation to grab data from an origin. If this is true, then we would only do IssuerCheck with the token from login cookie.

Regarding the other questions:

2&3 are in tight couple and I'm not sure how I should proceed in solving both. For 2, Yes, we shouldn't accept pelican.directorScrape from origin's issuer but at the same time, if the scope is the permission granted, which I assume should be entity.action and in this case, prometheus_metrics.read, it's hard to distinguish the scope requested in different issuer check functions.

I suggest that we abort using a single place (checkAPIToken) to check tokens for both /metrics and /api/v1.0/prometheus. Instead, we should use two functions but keep each *Check function (DirectorCheck, etc). This way, we can have max control of what scopes each issuer should issue.

For 3, I think at the end of the day there should be two Prometheus scopes exist, one should be something like prometheus_metrics.read and the other is prometheus_promql.read. The former is for reading /metrics and the later is for/api/v1.0/prometheus

4. The director has its own issuer and iss url because we need private key to issue a token under iss behalf and if it's the federation, we need the private key from whichever federation server we put in iss but we can't do it, so we set up director to be an issuer which we can access the private key from to sign the token. I don't think we ever used Federation check as no token was currently issued under federation URL.

If we do want to merge federation/director into a single issuer, we need to somehow add director's public key to federation's jwks_uri, and we need to figure out a way for the local development environment to work with a generated director's private key (the public key from which is not in the federation's metadata discovery endpoint)

[haoming29]

Related: #159

[haoming29]

Based on the discussion with @bbockelm, this issue can be detailed as follows:

Keep checkAPIToken function with renaming to checkAnyAuthorization, with a parameter called authOptions like []AuthList where AuthList has source issuer and scopes.

The function will call corresponding issuer check function (DirectorCheck FederationCheck etc) with token get from source (cookie/header/authz, etc) based on the authOptions.

The scopes for Prometheus will be renamed to monitoring.scrape and monitoring.query, for /metrics endpoint and PromQL endpoint respectively.

We keep checking Authorization header and cookie for PromQL given that we might want third-party system to access PromQL such as Grafana