Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NumericDate and StringList types exported from internal package #1214

Closed
arxeiss opened this issue Oct 10, 2024 · 7 comments
Closed

Make NumericDate and StringList types exported from internal package #1214

arxeiss opened this issue Oct 10, 2024 · 7 comments
Assignees

Comments

@arxeiss
Copy link

arxeiss commented Oct 10, 2024

Abstract

I'm using jwt.Parse to get token claims. And then iterating over claims to store them into DB etc.
Issue comes if incoming token in OIDC and contains extra claims like address claim or updated_at. If I use simple t.Get() it returns raw data.

For AddressClaim I can do this:

addr := &openid.AddressClaim{}
if err := addr.Accept(rawClaim); err != nil {
	return err
}

so I can use your prepared code for that. But not for updated_at which is inside jwt/internal/types package. And even standard says updated_at should timestamp as integer, not everyone follows that (like Auth0)

And because JWX API doesn't allow me easily to get OIDC token out from jwt.Parse, it would be nice to expose those functions, so I can use then in my code.

Describe the proposed solution/change

Just export at least NumericDate type outside from internal package.

Analysis
Other possibility would be to modify jwt.Parse to produce openid.stdToken and not jwt.stdToken. But I guess it would be more complex.

@lestrrat
Copy link
Collaborator

Can you show me code for update_at that you are trying to make work but doesn't?

@arxeiss
Copy link
Author

arxeiss commented Oct 10, 2024

Simplified a bit

rawClaim, hasClaim := claims.Get(c.GetSelector())
if !hasClaim {
	continue
}
switch c.GetSelector() {
case "updated_at":
	var floatVal float64
	floatVal, isValidClaimType = rawClaim.(float64)
	// updated_at should be Integer, but JSON use always float64
	propToAdd.Value = objectspb.Int64Value(int64(floatVal))

If it is standard Token, it will be float64 (covered with tests) but if they send string (like Auth0 does, see the link above) jwt/internal/types/date.go@NumericDate.Accept will handle that. But not my code.

@lestrrat
Copy link
Collaborator

Off the top of my head (sorry, you caught me right as I was about to go run an errand), I think the correct fix would be to allow NumericDate to handle both timestamp values and integer values.

I'll take a look at it prob. tomorrow

@lestrrat
Copy link
Collaborator

lestrrat commented Oct 10, 2024

@arxeiss Well, I came back and started poking around a bit...

I'm a bit confused. Doesn't this do what you want? You just want the integer epoch time, right? (note, I haven't committed it yet)

func TestGH1214(t *testing.T) {
	const src = `{"updated_at": 123456789}`

	expected := time.Unix(123456789, 0).UTC()
	tok := openid.New()
	_, err := jwt.Parse(
		[]byte(src),
		jwt.WithToken(tok),
		jwt.WithVerify(false),
		jwt.WithValidate(false),
	)
	require.NoError(t, err, `jwt.Parse should succeed`)
	require.Equal(t, expected, tok.UpdatedAt(), `updated_at should match`)

	require.Equal(t, int64(123456789), tok.UpdatedAt().Unix(), `updated_at should match (epoch)`)
}

This passes for me, no problem

$ go test -run=GH1214 ./jwt/openid/ -v
=== RUN   TestGH1214
--- PASS: TestGH1214 (0.00s)
PASS
ok      github.com/lestrrat-go/jwx/v2/jwt/openid

@lestrrat
Copy link
Collaborator

@arxeiss This works too

func TestGH1214(t *testing.T) {
	const src = `{"updated_at": 123456789}`

	expected := time.Unix(123456789, 0).UTC()
	tok := openid.New()
	_, err := jwt.Parse(
		[]byte(src),
		jwt.WithToken(tok),
		jwt.WithVerify(false),
		jwt.WithValidate(false),
	)
	require.NoError(t, err, `jwt.Parse should succeed`)

	updatedAt, ok := tok.Get(openid.UpdatedAtKey)
	require.True(t, ok, `updated_at should be present`)
	require.Equal(t, expected, updatedAt, `updated_at should match`)

	require.Equal(t, int64(123456789), updatedAt.(time.Time).Unix(), `updated_at should match (epoch)`)
}

@arxeiss
Copy link
Author

arxeiss commented Oct 10, 2024

Ah, I missed that jwt.WithToken() option. That could do the trick, thank you. Will need to check, but I believe it should work.

@arxeiss arxeiss closed this as completed Oct 10, 2024
@lestrrat
Copy link
Collaborator

cool

Just FYI, other than jwat.Parse with jwt.WithToken, once you've verified the token, you could also go a bunch of other ways to handle your token, if you don't like what jwt.Parse does:

payload, _ := jws.Verify( encodedJWT, .... values ...)

// unmarshal into jwt.Token
tok := jwt.New()
_ := json.Unmarshal([]byte(payload), tok) // note: no validation

// unmarshal into openid.Token
tok := openid.New()
_ := json.Unmarshal([]byte(payload), tok) // note: no validation

// And obviously, unmarshal it into any data structure
var m map[string]interface{}
_ := json.Unmarshal([]byte(payload), &m)

etc.

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

No branches or pull requests

2 participants