Skip to content

Conversation

@JorTurFer
Copy link
Contributor

@JorTurFer JorTurFer commented Oct 30, 2025

This PR implements JWT Authentication for Authorization Grant (RFC 7523 3.1) as alternative to static client credentials.

As usage example, this auth flow is used by STACKIT Service Accounts (Key Flow).

Note: This PR does not provide support for JWTs in Client Authentication Processing, but I can include it if you think that it's worth or I can add it as another PR following this

JorTurFer and others added 3 commits October 30, 2025 22:09
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Copy link
Member

@jkroepke jkroepke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question before we start the deeper review: What was the reason for using github.com/golang-jwt/jwt/v5 instead of golang.org/x/oauth2/jwt? Is it really necessary to define custom algorithms, or could we just rely on RS256 as golang.org/x/oauth2 does?

Background: I’m asking because the common library is shared across the whole Prometheus ecosystem including exporters, and we should be careful about which dependencies we pull in.

Similar to the UUID dependency used for the jti claim. As far as I know, jti is optional and not required when the JWT is short-lived.

@JorTurFer
Copy link
Contributor Author

JorTurFer commented Oct 31, 2025

Nice questions! I didn't think about the radius blast of adding new dependencies at core.

The problem that I see is that golang.org/x/oauth2/jwt 2 important features:

  • Multiple signing algorithms
  • jti

Although RS256 is the most common signing alg, there are auth providers which don't support it because they require RS512 as minimum (and eventually, there can be servers requesting other alg than RS but I guess that this is more overthinking because RS is accepted almost everywhere)

About jti, it's optional in the RFC but is also says

The authorization server MAY
ensure that JWTs are not replayed by maintaining the set of used
"jti" values for the length of time for which the JWT would be
considered valid based on the applicable "exp" instant.

So it's also on authorization server the option to enforce the value if it's verified. Said this, jti could provided as custom claim because it's used to ensure that not access tokens are signed at the same time for the same assertion, so golang.org/x/oauth2/jwt implementation should be fine using static jti.

Is it really necessary to define custom algorithms, or could we just rely on RS256 as golang.org/x/oauth2 does?

The biggest problem that I see is the signature methods hardcoded to RS256. RS384 and RS512 are the same method with different lengths and for enterprise authorization servers, increasing the length is mandatory.

About uuid dep, I used the same as prometheus is already using to avoid using multiple deps for same things

Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Contributor Author

I didn't notice it but prometheus is also using github.com/golang-jwt/jwt/v5 indirectly, so the dependency is already there (I know that this doesn't mean that it's already everywhere)

Signed-off-by: Jorge Turrado <[email protected]>
@jkroepke
Copy link
Member

jkroepke commented Nov 3, 2025

Tested locally against STACKIT API

package main

import (
	"fmt"
	"io"
	"net/http"
	"time"

	"github.com/prometheus/common/config"
)

func main() {
	httpConfig := config.HTTPClientConfig{
		OAuth2: &config.OAuth2{
			GrantType:                "urn:ietf:params:oauth:grant-type:jwt-bearer",
			ClientID:                 "f9355e72-eada-4ed7-9e16-d5b52e9cfdf4",
			ClientCertificateKeyFile: "/Users/jok/Documents/stackit.key",
			ClientCertificateKeyID:   "f9355e72-eada-4ed7-9e16-d5b52e9cfdf4",
			SignatureAltgorithm:      "RS512",
			Scopes:                   []string{},
			TokenURL:                 "https://service-account.api.stackit.cloud/token",
			Audience:                 "https://stackit-service-account-prod.apps.01.cf.eu01.stackit.cloud",
			Iss:                      "[email protected]",
			EndpointParams:           map[string]string{"hi": "hello"},
			Claims: map[string]interface{}{
				"sub": "3ba4a9e6-724a-437f-ae81-c311fca4e194",
			},
		},
	}

	if err := httpConfig.Validate(); err != nil {
		panic(err)
	}

	rt, err := config.NewRoundTripperFromConfig(httpConfig, "my-agent")
	if err != nil {
		panic(err)
	}

	client := http.Client{
		Transport: rt,
	}

	for {
		fmt.Printf("Making request... %s\n", time.Now().String())

		resp, err := client.Get("https://resource-manager.api.stackit.cloud/v2/projects?limit=50&[email protected]&offset=0")
		if err != nil {
			panic(err)
		}

		body, err := io.ReadAll(resp.Body)
		if err != nil {
			panic(err)
		}

		resp.Body.Close()

		fmt.Println("Response status:", resp.Status)
		fmt.Println("Body:", string(body))

		time.Sleep(time.Minute * 5)
	}
}

It works over a hour, refresh a token should work as well.

@JorTurFer Could you confirm that refreshing works on your side as well?

@jkroepke
Copy link
Member

jkroepke commented Nov 3, 2025

And it confirm that the upstream x/oauth2 implementations is not sufficient, since it only supports RS256, but STACKIT requires RS512 signature algo.

@bwplotka
Copy link
Member

bwplotka commented Nov 3, 2025

As per indirect dep spread, I think we are safe from client_golang perspective for this particular package (config) as per: https://cloud-native.slack.com/archives/C01AUBA4PFE/p1762179354393169?thread_ts=1762169496.875849&cid=C01AUBA4PFE

@JorTurFer
Copy link
Contributor Author

@JorTurFer Could you confirm that refreshing works on your side as well?

For sure, let me keep it running for a long time too.

As per indirect dep spread, I think we are safe from client_golang perspective for this particular package (config) as per: https://cloud-native.slack.com/archives/C01AUBA4PFE/p1762179354393169?thread_ts=1762169496.875849&cid=C01AUBA4PFE

I'm going to update client_golang with this specific branch and I'll check too

@JorTurFer
Copy link
Contributor Author

@JorTurFer Could you confirm that refreshing works on your side as well?

image

I can confirm it too, the refresh works as expected

Copy link
Member

@jkroepke jkroepke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkroepke
Copy link
Member

jkroepke commented Nov 7, 2025

https://cloud-native.slack.com/archives/C01AUBA4PFE/p1762272462094259?thread_ts=1762169496.875849&cid=C01AUBA4PFE

@bwplotka I talked with @David Ashpole and apparently they were doing similar thing in Otel: They were having tests in different module because deps were leaking. Now Go modules were improved and testing deps are still part of your go.mod, but not leaking as indirect to importers, which is why you see the when you tested importing client_golang. As a result they could unify the test back to a main module.
TL;DR in theory, assuming we accept burden of potentially security vulns of client_golang due to indirect test use, we don't even need prometheus/client_golang#1912 change (which is bigger than I thought)
I would say it's ok to proceed with common and don't change our examples :affe_sieht_nichts:

@jkroepke jkroepke merged commit f4c0aea into prometheus:main Nov 7, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants