[BUG] Error creating secure cookie, caused by: securecookie: the value is too long

[yubofredwang]

Describe the bug

When I use the AzureAD + OpenID for flyte authentication. The id token is too large to be stored in the cookie store. Thus, it will error out as:

{"json":{"src":"handlers.go:141"},"level":"debug","msg":"Running callback handler... for RequestURI /callback?code=0.AQEAv4j5cvGGr0GRqy180BHbRyHCmSqcLoZDjb392JCAaB8aAAA.AgABAAIAAAD--DLA3VO7QrddgJg7WevrAgDs_wUA9P8pIaLZ0SovwKJIJ5RTq6pg_bfpdtJyfIgzV-efLwbA9NvA7JCYOpdRodROyekph4oHqW3cBQldkYMJ6Fr1x-Qy8VxFRqrIi3bOILsw0YA9tMvvCZ22Dx2C-Olpp0le3ip1xTRwPsBNX7CfblaK58-MAFYlOiWBMaQ32pQLMbge_bokdvEXauWRijMYL2EtetUA-msuwxiHOI8VVWYvOD7QB9Llx2btcZbNjS78pNuhCMFPbFWWsqLHAn6GPsMUFhsW9TGPR193Iv0Jw2HEx-GXSB8yUwJEYlUkikC6JKUyJFkMDTWp_NcxGKQiH3S62E4m1tJol2xY-hEAO03iIUDFThXXJeY8MumjzlQfi4XOcur12CvAoNTqEnooCJ3ShkJ_iaWDDSEup1Gy1b8kEMnSOCY9vKCU2itzocIhAGDdoECB30tn5bBzOqlbi2sUqGE7ObZFBt92GJK8nhXNlk57lWWN3efYA0P4ojO5vbZhTU9ST8a0ImNwrz46AqGJSwF1ECYfPT8mmGG0yb7lObtsYkSaj8aoARDQhR2uwGA3A818Jq3Q-_L0YwhqZfAjDpg6K41JzGekT9x00i8TtNgv1-rLhIa4kCai7TbwRld2IMhhNMpq7TLlwtnM5FBmKFjmX4tMY84SqPfypN3xmMgAtsU6fwVuKJquWPMoZJz-zM-psBLZXvSn7Wf8xRY1EyZd5GliCHwT-ZSdQ5za5iWSeVuqDABZAEiGpvIZ6SDGqa-NIGlG32vJESHNowXZqF094QTTtD6xcs2QcRVC3nOfZzXMnmwrSjr-GcKI1ge9Ze5efRLjIpwI0-yi3GF_AVBONSelcXS-CxmpP5ipHbMR4inzQslCkmYrf57R62zQojRA0JtT-3nAkS_JUvYVnGs\u0026state=3e851cb5ebcde0b9ac7e583beaf02d628dfcebb299d9b899251a7cb83ae4c342\u0026session_state=9420d37f-ae08-448a-b3c2-6c464387ac10","ts":"2023-06-06T17:49:44Z"}
{"json":{"src":"cookie_manager.go:155"},"level":"error","msg":"Error generating encrypted id token cookie [SECURE_COOKIE_ERROR] Error creating secure cookie, caused by: securecookie: the value is too long","ts":"2023-06-06T17:49:45Z"}
{"json":{"src":"handlers.go:162"},"level":"error","msg":"Error setting encrypted JWT cookie [SECURE_COOKIE_ERROR] Error creating secure cookie, caused by: securecookie: the value is too long","ts":"2023-06-06T17:49:45Z"}

User's authentication will fail.

Expected behavior

User should be able to authenticate.

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[In0ut]

Same issue here.

[davidmirror-ops]

@yubofredwang Is this a consistent behavior across users?

[yubofredwang]

@In0ut you can remove some claims from you id token to reduce the size. For me, I removed groups from the id token.

@davidmirror-ops yes, this is consistent

[In0ut]

@yubofredwang Thanks I am going to try it

[AlexanderSarson]

I am experiencing the same problem and i've tried to emit groups from the token but it still says the token is too long.

[wild-endeavor]

played around with this a bit. I was wondering if not using encryption would shorten the cookie size at all and it does but not by much. You can change the cookie limit actually above the 4096 but that's actually the limit on a lot of browsers, so really shouldn't change that. I think the only thing to do unf is to try to look at the token that's coming back from your idp and seeing what can be shortened. You can also add an option to remove the secure cookie component entirely if you'd like, but not sure how i feel about that. definitely worth bringing up to a wider audience first. are azure tokens that much longer than okta's? Can you paste an example? (redacted ofc, but without changing length)

func TestCookieManager_Manual(t *testing.T) {
	hashKey := securecookie.GenerateRandomKey(64)
	//blockKey := securecookie.GenerateRandomKey(16)

	sc := securecookie.New(hashKey, nil)
	encoded, err := sc.Encode("test", strings.Repeat("#", 2457))
	assert.True(t, len(encoded) > 0)
	assert.NoError(t, err)
}

[wild-endeavor]

For background, the reason these are encrypted is because flyte admin might be hosted on the same domain as other apps. If those other apps share the same idp, then logging into flyte will unlock access to other applications as well. This is not secure behavior so it's protected on the user/browser side by encrypting it with something only flyte knows about.

Someone would have to check to make sure, but likely only the user id portion of the jwt is used. One possible solution to this is to just encrypt and store the user id, or whatever portion of the jwt that's used rather than the whole thing. this would shorten the cookie considerably.

[yubofredwang]

I am currently doing a fix in our internal fork but would like to see how @wild-endeavor thinks about the solution. Essentially, we are splitting up the access tokens into two and store them in two separate cookies. (flyte_at has the first half, flyte_at_1 has the second half). I can generalize this to handle token of arbitrary length by splitting into small cookies. Do you think that sounds like a valid solution to the community?

[EraYaN]

I think it's the only real solution (besides storing in LocalStorage or something instead). Azure AD tokens for example are always huge even without all the groups.

[davidmirror-ops]

@yubofredwang @wild-endeavor As other Azure users are hitting the same issue, I was wondering If we could resume the discussion and see if we can work out a solution?

[ingoke]

We get exact the same issue when trying to use Azure AD for OIDC. I wonder why the cookie limit is relevant because the length of the query string is less than 1200 chars. Debug output of the RequestURI shows UTF-representation of the ampersand \0026 (instead of &) which looks strange.

[yubofredwang]

hey guys, for this issue, the cause is "github.com/gorilla/securecookie"'s hash function will generate a hashed token that exceeds the limit of 4096 of cookie size limit of browsers, which means the original query string might not exceed the limit but the encoded string does. Our internal solution is: split the hashed the access token into 2 cookies and combine them when fetching. It worked fine for us so we didn't really investigate on a more formal solution.

[gdabisias]

Hi @yubofredwang ! We are encountering the same issue. Is your solution something that you could upstream as contribution or share the implemented solution with me?
Many thanks! 🙇

PS: Do you know a way to unblock the situation when the issue happens?

[yubofredwang]

sure, let me see if I can create a PR in a day or two. you can copy the code from the PR and apply locally. Should not be too complicated. @wild-endeavor @pingsutw what do you guys think?

[gdabisias]

That would be amazing @yubofredwang :)
Even an initial unpolished PR would be enough for me just to understand the required changes and then this can be contributed back in due time 🙇

[yubofredwang]

@gdabisias sorry responding a little late due to work. Please refer to #4863 for a sample implmentation

[gdabisias]

nice, thanks. @davidmirror-ops can we get this in so that we can then just have this in the next release?