lestrrat-go / jwx

Implementation of various JWx (Javascript Object Signing and Encryption/JOSE) technologies
MIT License
1.93k stars 160 forks source link

Make `NumericDate` and `StringList` types exported from internal package #1214

Closed arxeiss closed 1 minute ago

arxeiss commented 2 hours ago

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 commented 1 hour ago

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

arxeiss commented 1 hour ago

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 commented 1 hour ago

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 commented 18 minutes ago

@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 commented 17 minutes ago

@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 commented 1 minute ago

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