claims: add SessionID to known claims #205
Conversation
app/tokens/identities/identity.go
Outdated
| @@ -43,6 +43,7 @@ func New(cfg *app.Config, session *sessions.Claims, accountID int, audience stri | |||
| Audience: jwt.Audience{audience}, | |||
| Expiry: jwt.NewNumericDate(time.Now().Add(cfg.AccessTokenTTL)), | |||
| IssuedAt: jwt.NewNumericDate(time.Now()), | |||
| ID: session.ID, | |||
There was a problem hiding this comment.
I looked through https://www.iana.org/assignments/jwt/jwt.xhtml#claims to see if any well-known claims would be a good fit. What do you think of sid?
There was a problem hiding this comment.
I like it kind of surprised ID in the jwt package doesn't end up there but that was mostly just my prior ignorance of this document :)
Followed same pattern as AuthTime in this commit: 219ca73
I will open PR in the go client with same - and put a comment about the embed with a link to the file here 😄
app/tokens/sessions/session.go
Outdated
| @@ -70,6 +71,7 @@ func New(store data.RefreshTokenStore, cfg *app.Config, accountID int, authorize | |||
| Subject: string(refreshToken), | |||
| Audience: jwt.Audience{cfg.AuthNURL.String()}, | |||
| IssuedAt: jwt.NewNumericDate(time.Now()), | |||
| ID: uuid.NewString(), | |||
|
moving original description here to make room for something merge worthy:
|
AlexCuse
changed the title
AlexCuse
changed the title
|
I added a test for session creation / parsing at least. Let me know if you have any tips on setup for Also squashed down to a single commit to prep for merge. |
| @@ -41,6 +42,8 @@ func TestSessionRefresher(t *testing.T) { | |||
| assert.NoError(t, err) | |||
| assert.NotEmpty(t, identityToken) | |||
|
|
|||
| // TODO: parse token and validate SessionID still set - what setup is needed? | |||
There was a problem hiding this comment.
have a look in server/test/asserts.go at AssertIDTokenResponse. i doubt it will be useful in whole, but it should provide an example of parsing and asserting on the JWT.
There was a problem hiding this comment.
Ah that's nice I was trying to think of how to use the key setup for key store, didn't think to go at it through JWT library directly. Will give a go hopefully one more force push.
Need to spend a little time tomorrow to build a docker image locally but the change itself feels very straightforward don't anticipate any problems as long as we deploy the client change with our MFA stuff.
There was a problem hiding this comment.
Yup that worked perfectly thanks - just force pushed the last test update.
|
Released in v1.16.0 |
marshaled to 'sid' JWT claim https://www.iana.org/assignments/jwt/jwt.xhtml#claims