ARC-1355 add jwt cookie

[gxueatlassian]

Adding a new mechanism to protect user from connecting to other jira host.

https://developer.atlassian.com/cloud/confluence/cacheable-app-iframes-for-connect-apps/#retrieving-context-using-ap-context-gettoken-- - essentially we are doing what's recommended in the docs but passing the value in cookies, not in a payload/query parameter

[mboudreau]

This var will bleed between different requests

[bgvozdev]

That's what we want :) Once the FF is on, the value will eventually be updated (2nd call after the flip). Given the distributed nature of feature-flags, this is OK imho

[gxueatlassian]

Yup, understand that, and the full story is:

1. We want to have some feature flags to guard this code. But the middleware is NOT async at the moment, therefore can't use the feature flag (unless we make it async)
2. I understand that making the middleware async should not have problem. But I also think turning middleware async vs have a global var here, the latter one is less risky.
3. I understand the race condition of multi request, but I think they will all lead to expected result (which is "the flag will be used eventually" if turned on.

Unless you think there's still impact on this global var?

[mboudreau]

please use await instead

[bgvozdev]

We don't want to change the whole chain to be async @mboudreau cause it is pretty long.

[mboudreau]

We could probably do encodeURIComponent of the cookie values in case there's a semi-colon or something else in there.

[mboudreau]

This shouldn't be exported

[bgvozdev]

Well, we kinda need that...

[bgvozdev]

@mboudreau Thanks a lot for looking in the PR. We discussed your concerns regarding using one middleware in another one with @gxueatlassian and agreed on the follwing:

1. We cannot use directly verifySymmetricJwtTokenMiddleware because it requires secrets, and to have a secret, we need to fetch installation etc, which means essentially to duplicate the code from verifyJiraJwtMiddleware (the nested middleware we are calling).

2. Probably the perfect way to do that would be to refactor verifyJiraJwtMiddleware so it won't be a middleware, but that would require quite some work and not quite to the point of the ticket: we are fixing a major security problem. If we really want to do that refactoring, we can do this in another PR, because it will touch other parts of the code, too (which are not related to our current ticket).

[mboudreau]

I disagree. We're in this predicament for exactly the same reason: we had a security issue, so someone coded the verifyJwtMiddleware super quickly in a bad manner, but then never readdressed it and now we're still using that bad code to implement more bad code. In the time we are debating around this issue, creating a new verifyJwtToken function would have already been done with a lot less risk involved as it wouldn't affect any other system and the new code wouldn't rely on the old verify middleware. I personally think my version is less risky because there's less code in total, less files changed and less dependency on bad code.

But I won't stop you I guess.

[bgvozdev]

Hey @mboudreau , we actually made it better because now we force JWT verification for all jiraHost paths. If you check, before there was not any validation at all, now you cannot just add another jiraHost into a request and get it through not validated....